1043705 - Selecting same resolution twice will make the preview completely blank

Status: RESOLVED FIXED

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

Description

[bhargavg1]

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.

Comment 1

[bhargavg1]

[Blocking Requested - why for this release]:

Comment 2

[Jason Smith [:jsmith]]

QA Wanted - Can we reproduce this on Flame.

Comment 3

[:gerard-majax]

Since when can we select the recording resolution from the Camera app? I don't have this option on my Flame.

Comment 4

[bhargavg1]

(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

Comment 5

[Mike Habicher [:mikeh] (high bugzilla latency)]

I'll take a look at this.

Comment 6

[Mike Habicher [:mikeh] (high bugzilla latency)]

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.

Comment 7

[Pi Wei Cheng [:piwei] (inactive)]

(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

Comment 8

[:gerard-majax]

By uncommenting also the pictureSizes as explained in comment 4, this gets reproduced on picture preview also.

Comment 9

[Wilson Page [:wilsonpage]]

Attachment: 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.

Comment 10

[Wilson Page [:wilsonpage]]

Landed on 'master'

https://github.com/mozilla-b2g/gaia/commit/2472ea81e551781c2f6253116989b4e09c29709a

Comment 11

[Wilson Page [:wilsonpage]]

Waiting for bug 1038172 to land on 'v2.0' before uplifting

Comment 12

[Justin D'Arcangelo [:justindarc]]

Taking from Wilson -- May need to make a v2.0-only patch.

Comment 13

[Justin D'Arcangelo [:justindarc]]

Attachment: pull-request (v2.0)

Patch rebased for v2.0 -- Carrying R+ forward

Comment 14

[Justin D'Arcangelo [:justindarc]]

Hema, if Bug 1038172 doesn't get uplifted to v2.0, this patch is ready to land.

Comment 15

[Hema Koka [:hema]]

(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.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/7e71fb4d7399e6ca502f252ab259b5462ef41a1d

Comment 17

[[:SalvadorR]]

covered by this test case: https://moztrap.mozilla.org/manage/case/12572/

Comment 18

[KTucker [:KTucker][Inactive 3/4/2016]]

No test case added to moztrap since we have no way of changing the resolution for camera on the Flame.

Comment 19

[KTucker [:KTucker][Inactive 3/4/2016]]

Ignore comment 18. I will be adding a test case to moztrap.

Comment 20

[KTucker [:KTucker][Inactive 3/4/2016]]

Test case added to moztrap:

https://moztrap.mozilla.org/manage/case/14397/