1078435 - [Camera][Video] With software home button enabled the top of the screen of the video camera is corrupt

Status: VERIFIED FIXED

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

Description

[[:RJ]]

Attachment: log cat

Description:
With software home button enabled the top of the screen of the video camera appears static and broken  

Repro Steps:
1) Update a Flame to 20141006000205
2) Enable software home button 
3) Go to camera > switch to video camera 
4) switch to front facing camera and back 


Actual:
Top of screen is corrupted 

Expected:
top of screen displayed normally and non static 

Environmental Variables:
Device: Flame 2.1 Kitkat full flash
Build ID: 20141006000205
Gaia: 778ebac47554e1c4b7e9a952d73e850f58123914
Gecko: c4a4b04c617c
Version: 34.0a2 (2.1)
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0


Repro frequency:100% 
See attached: logcat, video : http://youtu.be/875urX8qW6Y

Comment 1

[[:RJ]]

This issue does occur on 2.1 (512 mb)

With software home button enabled the top of the screen of the video camera is static

Flame 2.1 KitKat Base (512mb)(Full Flash)

Environmental Variables:
Device: Flame 2.1
BuildID: 20141006000205
Gaia: 778ebac47554e1c4b7e9a952d73e850f58123914
Gecko: c4a4b04c617c
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

This issue DOES Not occur on Flame 2.2 (319mb), kitkat 2.0 (319mb) full flash

displays correctly

Flame 2.2 KitKat Base (319mb)(Full Flash)

Environmental Variables:
Device: Flame 2.2 Master
BuildID: 20141006040204
Gaia: 470826d13ae130a5c3d572d1029e595105485fb0
Gecko: e0d714f43edc
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0


Flame 2.0 KitKat Base (319mb)(Full Flash)

Environmental Variables:
Device: Flame 2.0
BuildID: 20141006000202
Gaia: 092d2b7678774c8b0b06dca0e0a8119e9eafdec3
Gecko: 69ca61f7edf3
Gonk: 2c909e821d107d414f851e267dedcd7aae2cebf
Version: 32.0 (2.0)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 2

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

After 10 attempts to repro this bug on 2 different devices I could not get this issue to occur. closing as WFM. Please reopen this bug if this issue is run into again.

Comment 3

[Seth Miko [:SMiko]]

Attachment: 2014-10-09-16-20-57.png

I'm seeing this on Flame 2.1 (319mb/Full flash)

Environmental Variables:
Device: Flame 2.1 
Build ID: 20141008000201
Gaia: d71f8804d7229f4b354259d5d8543c25b4796064
Gecko: 7fa82c9acdf2
Version: 34.0a2 Flame 2.1 KK (319mb)
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Repro Steps:
1: Open Settings > Screen lock and enable passcode lock.
2: From Settings enable the software home button.
3: Press power button twice to lock and unlock the phone.
4: When prompted to enter PIN select the camera icon and quickly switch to video.

Actual result: The top 1/4 of the screen is distorted.

Notes:
1: This issue has a higher repro rate after restarting the phone. 

Repro rate 5/10

Comment 4

[Peter Bylenga [:PBylenga]]

QAWanted for branch checks.

Comment 5

[Chris Kreinbring [:CKreinbring]]

Unable to repro with Seth's steps on the 10/8 nightly build using full flash on 319 MB.
Actual result: After tapping the camera/video switch, the viewfinder shows clearly with no corruption.

BuildID: 20141008000201
Gaia: d71f8804d7229f4b354259d5d8543c25b4796064
Gecko: 7fa82c9acdf2
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Platform Version: 34.0a2
Firmware Version: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Leaving qawanted tag for someone else to attempt.

Comment 6

[Yeojin Chung [:YeojinC]]

I was unable to reproduce with the build on Comment 3 as well. 

Environmental Variables:
Device: Flame 2.1 (319mb, Full Flash)
BuildID: 20141008000201
Gaia: d71f8804d7229f4b354259d5d8543c25b4796064
Gecko: 7fa82c9acdf2
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

No graphic issue appeared.
----------------------------
Leaving qawanted keyword for others to try.

Comment 7

[Joshua Mitchell (Inactive)]

Seth - can you try and repro this in the latest?

Comment 8

[Seth Miko [:SMiko]]

Unable to repro on Flame 2.1 (319mb/full flash)

Device: Flame 2.1
BuildID: 20141012001201
Gaia: d18e130216cd3960cd327179364d9f71e42debda
Gecko: 610ee0e6a776
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 9

[Joshua Mitchell (Inactive)]

Several Unable-to-repro's including the re-opener tester.  Re-closing as WFM - please reopen if this is seen again (hopefully consistently)

Comment 10

[Andrew Osmond [:aosmond] (he/him)]

Attachment: STR to show static in camera preview in video mode

I was able to reproduce this consistently on flame v184 and v188 on recent b2g-inbound and aurora builds.

STR:
1) Go to developer menu, enable software home button.
2) Open camera app.
3) Take a picture.
4) Switch to recording mode.
5) Static is shown. If a recording is made, it will show in both the preview and the recorded video.

Comment 11

[Joshua Mitchell (Inactive)]

Confirmed to Repro on the latest 2.2 using the STR from comment 10. Also repro's in 2.1

Device: Flame 2.2
Build ID: 20141020055012
Gaia: dc496d04907dd314f9736ff78bab3bd27156f79a
Gecko: f2d7d694aae5
Version: 36.0a1 (2.2)
Firmware Version: v184
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Device: Flame 2.1
Build ID: 20141020105810
Gaia: dc26d4a1c28682137130c62058399b43a4ea86b4
Gecko: 7e96ff7c7a64
Version: 34.0 (2.1)
Firmware Version: v184
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

------------------------------------------------------------------------------------------
This issue does NOT repro in 2.0  - possibly due to the SHB not being fully implemented in 2.0 -

Device: Flame 2.0
Build ID: 20141020025114
Gaia: 63b56a7a7453726b9e12ad1afe02c68c83c5aeca
Gecko: 09b9387be5ad
Version: 32.0 (2.0)
Firmware Version: v184
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

--------------------------------------------------------------------------------------

Issue does NOT repro on the earliest 2.1 build so this is a regression within the 2.1 branch

Build ID: 20140904062538
Gaia: a47ecb6368c015dd72148acde26413fd90ba3136
Gecko: ffb144a500a4
Version: 34.0a2 (2.1)
Firmware Version: v184
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 12

[Joshua Mitchell (Inactive)]

[Blocking Requested - why for this release]: Very bad UX, Regression in camera app


adding regression-window-wanted : use STR from comment 10

Comment 13

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

Static "noisy" or static "not-moving"? Make the summary explicit.

Comment 14

[Joshua Mitchell (Inactive)]

B2G-Inbound Regression Window:

Last Working:
Device: Flame 2.2
Build ID: 20140916122258
Gaia: d3510be2067ff39ce07e72268ae510279ee7688e
Gecko: 2fbe0c8881d1
Version: 35.0a1 (2.2)
Firmware Version: v184
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

First Broken:
Device: Flame 2.2
Build ID: 20140916123756
Gaia: d3510be2067ff39ce07e72268ae510279ee7688e
Gecko: 6b7ab2f8fc1f
Version: 35.0a1 (2.2)
Firmware Version: v184
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Gaia/Gecko Swap
Same Gaia's indicating a Gecko issue

Gecko pushlog: http://hg.mozilla.org/integration/b2g-inbound/pushloghtml?fromchange=2fbe0c8881d1&tochange=6b7ab2f8fc1f


Broken by Bug 1014877 - Can you take a look Mike?

Comment 15

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

Andrew is going to investigate.

Comment 16

[bhavana bajaj [:bajaj]]

Can we backout out the change in https://bugzilla.mozilla.org/show_bug.cgi?id=1014877 in 2.1 to avoid this regression? This was also align with what was agreed upon here : https://bugzilla.mozilla.org/show_bug.cgi?id=1014877#c57.

Josh just as an example, fallouts are one reason we want to avoid risky late landings

Comment 17

[bhavana bajaj [:bajaj]]

Not sure if you want this to be backed out on 2.0M too, leaving that to josh.

Comment 18

[Hubert Lu[:hlu] <[e-mail]>]

It could NOT be reproduced on Woodduck 2.0m. 

Gaia-Rev        ff24db33f8ebb275b200030035b851701b617b80
Gecko-Rev       e7df4dde2d9dbedee942333d34eaea2afe32bebc
Build ID        20141022050313
Version         32.0
Device Name     soul35
FW-Release      4.4.2
FW-Incremental  1413925519
FW-Date         Wed Oct 22 05:05:43 CST 2014

Comment 19

[Josh Cheng [:josh]]

No reproducible on Woodduck 2.0M.

Comment 20

[Andrew Osmond [:aosmond] (he/him)]

Having tested various camera configurations, this appears to be limited when using a specific resolution preview resolution 800x480 in conjunction with the 720p recording profile. When the camera app is forced to use that resolution, the static is reproducible *with* or *without* the software home button and appears to be a bug in the driver. Given that the static appears in both the preview and in the recording, the latter of which largely avoids Gecko code and uses primarily Gonk/AOSP components, it is highly unlikely this is an issue in our graphics stack.

The reason it happens with a software home button is because the available display size is reduced, and the algorithm which uses that information to select a preview size finds 800x480 the best match instead of the usual 864x480.

As to the resolution:
1) I will create a patch shortly to allow filtering of acceptable preview sizes; we can have it remove 800x480 and the problem should disappear.
2) We request the camera vendor to investigate the root cause independently.

Comment 21

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Add support for build property to ignore specific preview sizes, v1

Comment 22

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Configure build property to ignore broken preview sizes on flame, v1

Comment 23

[Michael Wu [:mwu]]

Comment on attachment 8509654 [details] [review]
Configure build property to ignore broken preview sizes on flame, v1

Please get the vendor to disable/investigate this on their side.

Comment 24

[Andrew Osmond [:aosmond] (he/him)]

Inder: See comment 10 for details on the issue and attachment 8509654 [details] [review] and attachment 8509653 [details] [diff] [review] for a workaround. Could you please let us know if issues with 800x480 are known, if a fix is forthcoming, or if the workaround above is necessary (at least in the short term) and putting attachment 8509654 [details] [review] in your tree?

Comment 25

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

Comment on attachment 8509653 [details] [diff] [review]
Add support for build property to ignore specific preview sizes, v1

Review of attachment 8509653 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/camera/GonkCameraParameters.cpp
@@ +982,5 @@
> +          break;
> +        }
> +      }
> +    }
> +  }

Please cache the results of this operation in something like mSupportedVideoSizes so that this doesn't need to be filtered every time.

Comment 26

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

I guess you want to needinfo to Wesly (TAM) for reaching out to vendor.
Forwarding the ni?

Comment 27

[Wesly Huang (EPM)]

Hi Youlong:

pls check this multiple-times-reopened issue, also comment#20 & comment#24 are some analysis and thing need your clarification here (might need to check with QCT as well), pls check and update here. Will add this into our tracking issue list as well, thanks.

Comment 28

[youlong.jiang]

(In reply to Wesly Huang from comment #27)
> Hi Youlong:
> 
> pls check this multiple-times-reopened issue, also comment#20 & comment#24
> are some analysis and thing need your clarification here (might need to
> check with QCT as well), pls check and update here. Will add this into our
> tracking issue list as well, thanks.

hi wesly -

how about just flash v188 without gaia/gecko v2.1?

we'll try to reproduce this issue internally and feedback asap.

tks.

Comment 29

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

Comment on attachment 8509653 [details] [diff] [review]
Add support for build property to ignore specific preview sizes, v1

Review of attachment 8509653 [details] [diff] [review]:
-----------------------------------------------------------------

Hmm, how did this get re-r?-ed?

r+ with notes below.

::: dom/camera/GonkCameraParameters.cpp
@@ +180,5 @@
> +  snprintf(propname, sizeof(propname), "ro.moz.cam.%d.del_preview_sizes", aCameraId);
> +  if (__system_property_get(propname, prop) > 0) {
> +    nsresult rv = ParseListAsArray(prop, mRemovedPreviewSizes);
> +    if (NS_FAILED(rv)) {
> +      NS_WARNING("Failed to parse removed preview sizes");

Please include 'propname' in the warning, so that someone looking at the log output will have a clue as to where to look.

Comment 30

[youlong.jiang]

Dear wesly -

per your comments, we've test on v188 more than 30+ times, and not reproduce this issue.

also according to comment29, I think current base image doesn't match your dom code. in case that we didn't find ro.moz.cam.* property in our build.

tks.

Comment 31

[Andrew Osmond [:aosmond] (he/him)]

(In reply to youlong.jiang from comment #30)
> Dear wesly -
> 
> per your comments, we've test on v188 more than 30+ times, and not reproduce
> this issue.
> 
> also according to comment29, I think current base image doesn't match your
> dom code. in case that we didn't find ro.moz.cam.* property in our build.
> 
> tks.

Youlong: Are you using the gaia/gecko that comes with v188? This issue is not present on 2.0, as indicated in comment 11, which is what is loaded with our v188 image. In order to reproduce you will need to load a 2.1, 2.2 or master gaia/gecko build from the last month or so (see comment 14 for the actual change). If you did load 2.1+, please let us know what the specific versions/hashes you loaded and I will make sure there isn't another related un-regression :).

While different versions would suggest that it is a gaia/gecko issue, I pointed out in comment 20 that with the software home button, we now use a different resolution than we have historically. This resolution was never tested before on flame. You should be able to reproduce whatever gaia/gecko combination if you limit the preview sizes to 800x480 from the camera driver.

The new ro.moz.cam.* option that Mike was referring to in comment 29 is a proposed change to work around the bug in the platform. It hasn't yet made it into the tree, because ideally we would like to fix the problem in the platform.

Comment 32

[youlong.jiang]

(In reply to Andrew Osmond [:aosmond] from comment #31)
> (In reply to youlong.jiang from comment #30)
> > Dear wesly -
> > 
> > per your comments, we've test on v188 more than 30+ times, and not reproduce
> > this issue.
> > 
> > also according to comment29, I think current base image doesn't match your
> > dom code. in case that we didn't find ro.moz.cam.* property in our build.
> > 
> > tks.
> 
> Youlong: Are you using the gaia/gecko that comes with v188? This issue is
> not present on 2.0, as indicated in comment 11, which is what is loaded with
> our v188 image. In order to reproduce you will need to load a 2.1, 2.2 or
> master gaia/gecko build from the last month or so (see comment 14 for the
> actual change). If you did load 2.1+, please let us know what the specific
> versions/hashes you loaded and I will make sure there isn't another related
> un-regression :).
> 
> While different versions would suggest that it is a gaia/gecko issue, I
> pointed out in comment 20 that with the software home button, we now use a
> different resolution than we have historically. This resolution was never
> tested before on flame. You should be able to reproduce whatever gaia/gecko
> combination if you limit the preview sizes to 800x480 from the camera driver.
> 
> The new ro.moz.cam.* option that Mike was referring to in comment 29 is a
> proposed change to work around the bug in the platform. It hasn't yet made
> it into the tree, because ideally we would like to fix the problem in the
> platform.

hi andrew -

I don't understand why you would configure a new resolution in up-layer. but from our view, camera preview size should follow our screen resolution, and I'm not sure correct it whether would lead to other issue.

tks.

Comment 33

[Andrew Osmond [:aosmond] (he/him)]

(In reply to youlong.jiang from comment #32)
> 
> hi andrew -
> 
> I don't understand why you would configure a new resolution in up-layer. but
> from our view, camera preview size should follow our screen resolution, and
> I'm not sure correct it whether would lead to other issue.
> 
> tks.

If you follow the STR, go into the developer menu and enable the software home button on flame, then you lose part of the screen real estate to display that button (see the bottom of attachment 8502803 [details]). That reduces the pixels available to the camera preview in the application.

The application goes through the list of supported resolutions by the camera and finds the best match for the space available on the screen. Normally on flame that would be 854x480. With the SHB it becomes 800x480. From my own testing by forcing the app to select different ones, all of the other preview sizes work fine, and only 800x480 is broken.

Why do you think the camera preview size should follow screen resolution? Is there some sort of coupling between the two? The camera driver should provide frames with the configured resolution and let us worry about how to display those frames. The case where we don't use the full screen for the camera preview is legitimate for phones without a physical home button, as an example.

If the camera driver is unable to support 800x480 for some reason, we are fine with that, as long as it doesn't list it in the supported preview sizes.

Comment 34

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Add support for build property to ignore specific preview sizes, v2

Comment 35

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

Comment on attachment 8515112 [details] [diff] [review]
Add support for build property to ignore specific preview sizes, v2

Review of attachment 8515112 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with nit fixed :)

::: dom/camera/GonkCameraParameters.h
@@ +226,5 @@
>    // Return values:
>    //  - NS_OK on success;
>    //  - NS_ERROR_NOT_IMPLEMENTED if 'aKey' is invalid;
>    //  - NS_ERROR_NOT_AVAILABLE if a valid value could not be returned.
> +  template<class T> nsresult ParseListAsArray(const char *aStart, nsTArray<T>& aArray);

'const char* aStart'

Comment 36

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Add support for build property to ignore specific preview sizes, v3 [carry r=mikeh]

Updated patch based on review feedback.

youlong: ping

Comment 37

[youlong.jiang]

(In reply to Andrew Osmond [:aosmond] from comment #36)
> Created attachment 8522279 [details] [diff] [review]
> Add support for build property to ignore specific preview sizes, v3 [carry
> r=mikeh]
> 
> Updated patch based on review feedback.
> 
> youlong: ping

Dears -

we've tried to custom 800*480,a fixed resolution in hal interface. But the result is preview not ok. now we have two uncertain factors, 
1. maybe our tmp modify is not correct that affect the normal work
2. really not support 800*480 in low-layer

next step, we'll custom another worked resolution in previous method, to check our summarize is No.1 or No.2

tks.

Comment 38

[youlong.jiang]

(In reply to youlong.jiang from comment #37)
> (In reply to Andrew Osmond [:aosmond] from comment #36)
> > Created attachment 8522279 [details] [diff] [review]
> > Add support for build property to ignore specific preview sizes, v3 [carry
> > r=mikeh]
> > 
> > Updated patch based on review feedback.
> > 
> > youlong: ping
> 
> Dears -
> 
> we've tried to custom 800*480,a fixed resolution in hal interface. But the
> result is preview not ok. now we have two uncertain factors, 
> 1. maybe our tmp modify is not correct that affect the normal work
> 2. really not support 800*480 in low-layer
> 
> next step, we'll custom another worked resolution in previous method, to
> check our summarize is No.1 or No.2
> 
> tks.

hi all -

we confirm that low-layer provide support for 800*480, and found that must re-enable 800*480 in gecko part, detail as below:

Gecko/dom/camera/GonkRecorderProfiles.def

how about your status? could you pls have a check

tks.

Comment 39

[Andrew Osmond [:aosmond] (he/him)]

youlong: I think I see what you mean. If force the preview size to be 800x480 and the recording profile to be wvga it seems to work on b2g-inbound. Merely adding it on 2.1 has no effect, because our selection algorithms still prefer 800x480 + 720p over the new profiles.

However this appears to have been resolved in mozilla-central by bug 1098660 which updated our video/preview size selection algorithms. I need to look at testing/landing that change on v2.1.

Comment 40

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Enhance preview selection algorithm in gaia, v1 -- v2.1 *only*

Changing from original thought of pulling in gecko patches to creating a custom gaia patch as the changes required at this level are 1) much simpler, 2) have a lower risk of landing, and 3) doesn't require a ton of dependencies (with their own shortcomings/bugs).

Moving forward on 2.2, this logic will move fully into gecko as per bug 1104913.

Comment 41

[Wesly Huang (EPM)]

remove POVB since component moved to Gaia-Camera.

Comment 42

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Enhance preview selection algorithm in gaia, v2 -- v2.1 *only*

Comment 43

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

Comment on attachment 8532230 [details] [review]
Enhance preview selection algorithm in gaia, v2 -- v2.1 *only*

LGTM.

Comment 44

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8532230 [details] [review]
Enhance preview selection algorithm in gaia, v2 -- v2.1 *only*

Andrew: One nit, but I have a question about the one conditional statement on whether it should be an `&&` instead of an `||`. The comment above the code in question seems to be indicating that it should be `&&`.

Comment 45

[Andrew Osmond [:aosmond] (he/him)]

(In reply to Justin D'Arcangelo [:justindarc] from comment #44)
> Comment on attachment 8532230 [details] [review]
> Enhance preview selection algorithm in gaia, v2 -- v2.1 *only*
> 
> Andrew: One nit, but I have a question about the one conditional statement
> on whether it should be an `&&` instead of an `||`. The comment above the
> code in question seems to be indicating that it should be `&&`.

I think it makes sense if you invert the expression (using De Morgan's law). i.e.

if previewSize.width > resolution.width || previewSize.width > resolution.height, drop preview size

becomes

if previewSize.width <= resolution.width && previewSize.width <= resolution.height, keep preview size

Comment 46

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8532230 [details] [review]
Enhance preview selection algorithm in gaia, v2 -- v2.1 *only*

Minor nit in GitHub about updating the code comments to reflect the change.

Tested on Flame. LGTM!

Comment 47

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/gaia/commit/3919250c8884439a9faaddfa18057d8b8c7f6002

Comment 48

[Ryan VanderMeulen [:RyanVM]]

Why did this land on v2.1 without approval?
https://wiki.mozilla.org/Release_Management/B2G_Landing#Landing_Procedure_5
> All others must have approval‑mozilla‑b2g34+ / approval-gaia-v2.1+ to land (including bugs marked as blocking-b2g:2.1+) 

Also setting v2.2 to wontfix per comment 40.

Comment 49

[Ryan VanderMeulen [:RyanVM]]

Reverted.

v2.1: https://github.com/mozilla-b2g/gaia/commit/f03a0c46ec5153f60cb29b8efb298d8cef0a3dd5

Andrew, you'll need to request v2.1 approval on this before it can re-land.

Comment 51

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 8532230 [details] [review]
Enhance preview selection algorithm in gaia, v2 -- v2.1 *only*

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): N/A
[User impact] if declined: In some screen resolution and camera configurations, the device may display and record partially corrupted video streams.
[Testing completed]: Manually tested front and back cameras with and without the software home button to confirm preview and recordings are uncorrupted. Updated unit tests to verify algorithm selects correct resolutions.
[Risk to taking this patch] (and alternatives if risky): The preview resolution will now be different for video recording. Some devices besides flame may have a possible combination of recording sizes and preview sizes which are incompatible under the new algorithm. I deem this to be a rather low risk given we tightening the constraints rather than loosening, and more closely following the behaviour of Android based platforms.
[String changes made]: None.

Comment 52

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Enhance preview selection algorithm in gaia, v3 (v2.1 *only*) -- carries r=justindarc,mikeh

Updated branch to latest 2.1 and recreated pull request as the original was merged/reverted.

Comment 53

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 8546031 [details] [review]
Enhance preview selection algorithm in gaia, v3 (v2.1 *only*) -- carries r=justindarc,mikeh

Put this on the right attachment, duh ;).

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): N/A
[User impact] if declined: In some screen resolution and camera configurations, the device may display and record partially corrupted video streams.
[Testing completed]: Manually tested front and back cameras with and without the software home button to confirm preview and recordings are uncorrupted. Updated unit tests to verify algorithm selects correct resolutions.
[Risk to taking this patch] (and alternatives if risky): The preview resolution will now be different for video recording. Some devices besides flame may have a possible combination of recording sizes and preview sizes which are incompatible under the new algorithm. I deem this to be a rather low risk given we are tightening the constraints rather than loosening, and more closely following the behaviour of Android based platforms.
[String changes made]: None.

Comment 54

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/b87be345aa48cab5e4b752892f8cd2c2cb30427b

Comment 55

[Vincent Liu[:vliu]]

https://github.com/mozilla-b2g/gaia/commit/b87be345aa48cab5e4b752892f8cd2c2cb30427b

Comment 56

[Coler]

This bug has been verified as "pass" on latest build of Flame 2.1, 2.1S(256&512MB) by STR as comment 0, comment 3 and comment 10.

Actual result:The viewfinder is displayed normally with software home button enabled.

See attachment:Verify.3gp
Reproducing rate: 0/10

Device:Flame 2.1 build(Pass):
Build ID               20150617001205
Gaia Revision          f8b848c82d1ed589f7a1eb5cc099830c867ff1d4
Gaia Date              2015-06-08 09:48:23
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/0ebea88c344d
Gecko Version          34.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150617.035603
Firmware Date          Wed Jun 17 03:56:15 EDT 2015
Bootloader             L1TC000118D0

Device:Flame 2.1S_256MB build(Pass):
Build ID               20150617001203
Gaia Revision          4196ca9119b4cde8353130165de90c0ffa6060a3
Gaia Date              2015-06-17 03:30:30
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/8614617a0eb9
Gecko Version          34.0
Device Name            scx15_sp7715ga
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150617.034659
Firmware Date          Wed Jun 17 03:47:12 EDT 2015

Device:Flame 2.1S_512MB build(Pass):
Build ID               20150617001203
Gaia Revision          4196ca9119b4cde8353130165de90c0ffa6060a3
Gaia Date              2015-06-17 03:30:30
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/8614617a0eb9
Gecko Version          34.0
Device Name            scx15_sp7715ea
Firmware(Release)      4.4.2
Firmware(Incremental)  122
Firmware Date          Thu Feb  5 12:42:58 CST 2015

Comment 57

[Coler]

Attachment: Verify.3gp