Selecting same resolution twice will make the preview completely blank

RESOLVED FIXED in 2.1 S1 (1aug)

Status

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bhargavg1, Assigned: justindarc)

Tracking

unspecified
2.1 S1 (1aug)
ARM
Gonk (Firefox OS)
Bug Flags:
in-moztrap +

Firefox Tracking Flags

(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 fixed)

Details

(Whiteboard: [caf priority: p2][CR 698541], URL)

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
STR
1. Launch Camera and switch to camcorder
2. Select any resolution for example cif
3. Click on the resolutions options and select cif resolution again
4. Observe that preview becomes completely blank.
(Reporter)

Comment 1

5 years ago
[Blocking Requested - why for this release]:
blocking-b2g: --- → 2.0?
QA Wanted - Can we reproduce this on Flame.
Keywords: qawanted
Whiteboard: [CR 698541] → [caf priority: p2][CR 698541]
Since when can we select the recording resolution from the Camera app? I don't have this option on my Flame.
Flags: needinfo?(wilsonpage)
(Reporter)

Comment 4

5 years ago
(In reply to Alexandre LISSY :gerard-majax from comment #3)
> Since when can we select the recording resolution from the Camera app? I
> don't have this option on my Flame.

the build was made after enabling the recorder profiles option [1]

[1] https://github.com/mozilla-b2g/gaia/blob/v2.0/apps/camera/js/config/config.js#L363
I'll take a look at this.
Assignee: nobody → mhabicher
Diego, it looks like changing the video resolution the first time generates a call to setConfiguration() (and the subsequent callback), but selecting the same resolution a second time doesn't call setConfiguration(). I'm guessing the camera app is waiting for the onConfigurationChange callback that never comes.
Assignee: mhabicher → nobody
Flags: needinfo?(dmarcos)
(In reply to Jason Smith [:jsmith] from comment #2)
> QA Wanted - Can we reproduce this on Flame.

No. As mentioned on comment 3, we don't have an option to change recording resolution on Flame.
Preview looks as expected on Flame 2.0.

Tested on:
Device: Flame
Build ID: 20140724190006
Gaia: 9b6d7357031f2412b18a2fb140d5c974842d4393
Gecko: fbb3b8be8f6c
Version: 32.0 (2.0)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: qawanted
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review+]
By uncommenting also the pictureSizes as explained in comment 4, this gets reproduced on picture preview also.
Created attachment 8464037 [details] [review]
pull-request (master)

REASON

We were always fading out the viewfinder even if the picture-size or recorder-profile didn't change, this meant that the preview stream didn't fire the necessary 'started' event required to fade the viewfinder back in.

FIX

We now check if the picture-size changed before fading-out the viewfinder. I also noticed that the notification wasn't showing the changed value, only 'Picture Resolution', so I fixed this at the same time.
Attachment #8464037 - Flags: review?(dmarcos)
Flags: needinfo?(wilsonpage)
Attachment #8464037 - Flags: review?(dmarcos) → review+
Flags: needinfo?(dmarcos)

Updated

5 years ago
Assignee: nobody → wilsonpage
blocking-b2g: 2.0? → 2.0+
Waiting for bug 1038172 to land on 'v2.0' before uplifting
Depends on: 1038172

Updated

5 years ago
Target Milestone: --- → 2.1 S1 (1aug)

Updated

5 years ago
Whiteboard: [caf priority: p2][CR 698541] → [caf priority: p2][CR 698541][patch ready]
(Assignee)

Comment 12

5 years ago
Taking from Wilson -- May need to make a v2.0-only patch.
Assignee: wilsonpage → jdarcangelo
(Assignee)

Comment 13

5 years ago
Created attachment 8467999 [details] [review]
pull-request (v2.0)

Patch rebased for v2.0 -- Carrying R+ forward
Attachment #8467999 - Flags: review+
(Assignee)

Comment 14

5 years ago
Hema, if Bug 1038172 doesn't get uplifted to v2.0, this patch is ready to land.
Flags: needinfo?(hkoka)
(In reply to Justin D'Arcangelo [:justindarc] from comment #14)
> Hema, if Bug 1038172 doesn't get uplifted to v2.0, this patch is ready to
> land.

Justin: Looks like the videos are going to take longer per Tapas comment in bug 1038172. And from a previous comment, fxos 2.0 numbers look better than android on JB, so that bug may be removed as a 2.0 blocker. So lets get this fix uplifted and not depend on bug 1038172. Thanks for creating a 2.0 PR

NI Ryan to help with 2.0 uplift.
Flags: needinfo?(hkoka) → needinfo?(ryanvm)
v2.0: https://github.com/mozilla-b2g/gaia/commit/7e71fb4d7399e6ca502f252ab259b5462ef41a1d
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-b2g-v2.0: --- → fixed
status-b2g-v2.1: --- → fixed
Flags: needinfo?(ryanvm)
Resolution: --- → FIXED
Whiteboard: [caf priority: p2][CR 698541][patch ready] → [caf priority: p2][CR 698541]

Updated

5 years ago
No longer depends on: 1038172

Comment 17

5 years ago
covered by this test case: https://moztrap.mozilla.org/manage/case/12572/
QA Whiteboard: [QAnalyst-Triage+][lead-review+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Flags: in-moztrap?(srapanan)
No test case added to moztrap since we have no way of changing the resolution for camera on the Flame.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Flags: in-moztrap?(srapanan)
Flags: in-moztrap-
Ignore comment 18. I will be adding a test case to moztrap.
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: in-moztrap- → in-moztrap?
Flags: needinfo?(ktucker)
Test case added to moztrap:

https://moztrap.mozilla.org/manage/case/14397/
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Flags: in-moztrap?
Flags: in-moztrap+
You need to log in before you can comment on or make changes to this bug.