Closed Bug 1043705 Opened 8 years ago Closed 7 years ago

Selecting same resolution twice will make the preview completely blank


(Firefox OS Graveyard :: Gaia::Camera, defect)

Gonk (Firefox OS)
Not set


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

2.1 S1 (1aug)
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed


(Reporter: bhargavg1, Assigned: justindarc)




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


(2 files)

46 bytes, text/x-github-pull-request
: review+
Details | Review
46 bytes, text/x-github-pull-request
: review+
Details | Review
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.
[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)
(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]

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.
Attached file pull-request (master)

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.


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)
Assignee: nobody → wilsonpage
blocking-b2g: 2.0? → 2.0+
Waiting for bug 1038172 to land on 'v2.0' before uplifting
Depends on: 1038172
Target Milestone: --- → 2.1 S1 (1aug)
Whiteboard: [caf priority: p2][CR 698541] → [caf priority: p2][CR 698541][patch ready]
Taking from Wilson -- May need to make a v2.0-only patch.
Assignee: wilsonpage → jdarcangelo
Attached file pull-request (v2.0)
Patch rebased for v2.0 -- Carrying R+ forward
Attachment #8467999 - Flags: review+
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)
Closed: 7 years ago
Flags: needinfo?(ryanvm)
Resolution: --- → FIXED
Whiteboard: [caf priority: p2][CR 698541][patch ready] → [caf priority: p2][CR 698541]
No longer depends on: 1038172
covered by this test case:
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:
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.