1051200 - Camera brightness blocked after touch-focusing on dark zone

Status: VERIFIED FIXED

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

Description

[:gerard-majax]

Attachment: 2014-08-09-13-22-19.png

[Blocking Requested - why for this release]:

Reproduced on a Flame running a master built today, I can get the camera in a state where brightness is blocked and everything is nearly all white.

STR:
 0. Launch Camera app, switch to video recording
 1. Point device camera to a dark area, tap to focus
 2. Switch back to picture mode and move to a brighter area

Expected:
 Camera should adjust brightness

Actual:
 Brightness is blocked, camera is unusable.

When back in picture mode, taping on the image to focus again makes the auto brightness to work again

Comment 1

[:gerard-majax]

Actually, STR can be simplified to:

STR:
 0. Launch Camera app, switch to video recording
 1. Point device camera to a dark area
 2. Switch back to picture mode

Brightness is also blocked in this case.

Comment 2

[:gerard-majax]

[Blocking Requested - why for this release]: I gets reproduced on 2.0 also, and it gives bad UX.

Comment 3

[Montoya Clemmons (MClemmons)]

This issue repros on Flame 2.1 but at a lower rate than on Flame 2.0. Following STR from Comment 0 or Comment 1, intermittently the Camera does not adjust for brightness.

Environmental Variables:
Device: Flame Master
BuildID: 20140811040202
Gaia: 19ed3c9e78eaf234cc08484bde6998ae21552ba5
Gecko: a9b43778f0c2
Version: 34.0a1 (Master) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

This issue does not repro on Flame 1.4, Buri 2.1 or Open C 2.0. Camera adjusts for brightness following STR in Comment 0 or Comment 1.

Environmental Variables:
Device: Flame 1.4
BuildID: 20140811081112
Gaia: d47fecdcaa4aefb754561114edd22fb23a92b98d
Gecko: fbee340d0830
Version: 30.0 (1.4) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Environmental Variables:
Device: Buri Master
BuildID: 20140810234109
Gaia: 19ed3c9e78eaf234cc08484bde6998ae21552ba5
Gecko: a9b43778f0c2
Version: 34.0a1 (Master) 
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Environmental Variables:
Device: Open_C Master
BuildID: 20140811040202
Gaia: 19ed3c9e78eaf234cc08484bde6998ae21552ba5
Gecko: a9b43778f0c2
Version: 34.0a1 (Master) 
Firmware Version: P821A10V1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 4

[Dietrich Ayala (:dietrich)]

Triage concludes we should block because this is a regression. We can't ship a new version with degraded functionality from the previous version.

Comment 5

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

This seems to be the same as bug 1048454, which was deemed non-blocking.

Comment 6

[Joshua Mitchell (Inactive)]

The other bug is very similar but I don't think they are the same - additionally the other bug was not a regression(based on the STR) while this one is (regression being a major contributor to a blocking nomination)

Comment 7

[Dietrich Ayala (:dietrich)]

Thanks Joshua!

Mike - As I understood it, this bug was about brightness being stuck in an unusable state due to normal usage. Bug 1048454 was about being able to video another phone screen, which didn't seem like a blocking-worthy use-case.

If these are in fact the same, then please duplicate as needed.

Comment 8

[Jason Smith [:jsmith]]

I think comment 6 addresses my opinion on this.

Comment 9

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

(In reply to Dietrich Ayala (:dietrich) from comment #7)

> Mike - As I understood it, this bug was about brightness being stuck in an
> unusable state due to normal usage. Bug 1048454 was about being able to
> video another phone screen, which didn't seem like a blocking-worthy
> use-case.

My understand of bug 1048454 was that Marcia was using another phone with a white screen as the bright light to blow out the exposure. Although the comments in that bug are a little contradictory, it sounds like this affects v122 and v123. I was unable to reproduce it using v162-3 (which has other problems), but maybe QA will have better (or rather, worse) luck?

I'll mark the other as a dupe.

Comment 11

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

(In reply to Jason Smith [:jsmith] from comment #8)

> I think comment 6 addresses my opinion on this.

I'm looking into whether or not this is a regression due to the addition of touch-focus support in 2.0.

Comment 12

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

I -think- I've figured this out. When we touch-to-focus, we set the focus and light-metering areas of the camera to the area touched[1]. Then we suspend continuous-focus and face-detection[2] for 4 and 2 seconds, respectively. But when we resume face-detection, (and continuous-focus), the focus and metering areas are never cleared! There is a reset()[3] method, but I don't see where it gets called.

Touching-to-focus again clears the stuck state. So too, I presume, would detecting a face, though the latter would be made much more difficult with the extreme exposure setting locked in place.

1. https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/lib/camera/focus.js#L328
2. https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/lib/camera/focus.js#L224
3. https://github.com/mozilla-b2g/gaia/blob/master/apps/camera/js/lib/camera/focus.js#L281

Comment 13

[Diego Marcos [:dmarcos]]

Attachment: Pull Request

The PR resets focus and metering areas when the focus mode changes.

Comment 14

[Diego Marcos [:dmarcos]]

Mike, focus is a complicated feature. The code is well tested but it would be nice if you can torture the patch a little bit to make sure that everything works fine.

Comment 15

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8472087 [details] [review]
Pull Request

LGTM

Comment 16

[Diego Marcos [:dmarcos]]

This is ready to land. I just need confirmation from mikeh that everything works as expected

Comment 17

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

Comment on attachment 8472087 [details] [review]
Pull Request

f+ on fixing the metering area. What I see now is that after the STR, the flash turns on and stays on! In the most recent case, with the flash set to "auto", the flash turned on when I switched back to picture mode and is STILL one (over a minute later).

Comment 18

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

Further to comment 17, app-manager reports app.camera.mozCamera.flashMode == "off". :-o

Comment 19

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

And setting the .flashMode = "off" didn't turn it off. Setting it to "auto" did.

Comment 20

[Diego Marcos [:dmarcos]]

Is it a driver issue? Do you recommend anything I should be doing on the gaia side?

Comment 21

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

Not sure. More investigation needed. Why don't you land this patch and I'll file a new bug for the flash issue?

Comment 22

[Diego Marcos [:dmarcos]]

Landed in master:

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

Comment 23

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/8b1b64ca3347e015d7a57df6d053f95cd26046ca

Comment 24

[SandKing]

This issue has been verified successfully on Flame2.0&2.1
Verify video:"verify_1051200.mp4".

Flame2.0 build
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c756bd8bf3c3
Build-ID        20141130000204
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141130.032432
FW-Date         Sun Nov 30 03:24:44 EST 2014
Bootloader      L1TC00011880

Flame2.1 build:
Gaia-Rev        ccb49abe412c978a4045f0c75abff534372716c4
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/18fb67530b22
Build-ID        20141130001203
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141130.034738
FW-Date         Sun Nov 30 03:47:49 EST 2014
Bootloader      L1TC00011880

Comment 25

[SandKing]

Attachment: Verified video: verify_1051200.mp4