1046115 - Flash gets fired automatically when it detects face with flash ON under low light condition

Status: RESOLVED FIXED

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

Description

[Pooja]

Set Flash to ON/Auto and introduce Faces in the Camera FOV, observed the issue that LED flash is getting fired automatically even when Camera preview is kept idle under Lowlight conditions. 

Steps to Repro:
1. Open Camera
2. Go to settings-> set Flash-On/Auto
3. Introduce faces in the Camera FOV (Field of view)
4. Let it detects faces i.e should start showing circles.

Actual Behavior:
Observed the issue that LED flash is getting fired automatically even when Camera preview is kept idle under Lowlight conditions.

Expected:
 Flash should fire only when we  take picture.

Note
1. Issue is not observed if faces are not there in camera FOV
2. Issue is observed without performing TouchAF.

Device Info:

Target: 8x26
Buid Id : 20140729183518
Gaia: 0a864988f5dce7f9f3dea9609e8ef054679c30ff
Gecko : 745b486db495248e4d4503039e374cb8d5bb244f
Platform Version : 32.0

Comment 1

[Pooja]

[Blocking Requested - why for this release]:

Comment 2

[Jason Smith [:jsmith]]

QA Wanted to see if we can reproduce on Flame.

Comment 3

[Joshua Mitchell (Inactive)]

This issue does not repro on 2.1 Flame or 2.0 Flame

Flash only 'goes off' when pressing the shutter button

Device: Flame Master
Build ID: 20140730045709
Gaia: 25e998814ba89f30fe44cd2fdfbb44d160a04641
Gecko: 08c23f12a43e
Version: 34.0a1 (Master)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.0
Build ID: 20140730080807
Gaia: e8425788e0372fbbcc40387b799f0636c041fc14
Gecko: 20c472fc0ee3
Version: 32.0 (2.0)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 4

[Hema Koka [:hema]]

Diego, whats the cause of this issue?

Comment 5

[Diego Marcos [:dmarcos]]

On flame, the flash always fires when focusing (on auto focus, face tracking and touch to focus modes) in low light conditions. This behavior is determined by the driver and we don't have any control over at the gaia level. 

There's a gecko patch (bug 1042072) that will try to mitigate these problems.

I don't know the expected driver behavior on QRD devices. 

Pooja, Does the flash go off also when touching to focus on low light conditions? What about continuous auto focus?

Andrew, Will your patch help with this issue?

Comment 6

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

(In reply to Diego Marcos [:dmarcos] from comment #5)
> On flame, the flash always fires when focusing (on auto focus, face tracking
> and touch to focus modes) in low light conditions. This behavior is
> determined by the driver and we don't have any control over at the gaia
> level. 
> 
> There's a gecko patch (bug 1042072) that will try to mitigate these problems.
> 
> I don't know the expected driver behavior on QRD devices. 
> 
> Pooja, Does the flash go off also when touching to focus on low light
> conditions? What about continuous auto focus?
> 
> Andrew, Will your patch help with this issue?

The patch in bug 1042072 will only fix the case where the flash never turns off because the driver doesn't like us interrupting the auto focus. However it will not have an impact on the driver deciding to use flash in the first place (unwanted but otherwise performed correctly and turns off when complete).

Comment 7

[Hema Koka [:hema]]

Blocker from a user experience point of view (camera continuously flashes as it focuses on Flame)

Comment 8

[Diego Marcos [:dmarcos]]

This is an OEM bug. NI Mike to confirm this.

Comment 9

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

(In reply to Diego Marcos [:dmarcos] from comment #8)

> This is an OEM bug. NI Mike to confirm this.

Yes: the Flame camera may fire the flash on its own (in low-light conditions) if flashMode is set to 'auto' and focusMode is set to 'continuous-picture|video', or if it's set to 'auto' and we call autoFocus(). Sounds like face detection exhibits the same behaviour.

Comment 10

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

Ah, and it sounds like the QRD is doing it as well.

Pooja, we set the camera's focus mode to 'continuous-picture' by default, so the camera triggers its own auto-focus cycles without us having to poke it. This is probably what's causing the flash to go off. The behaviour is internal to the driver.

Comment 11

[Diego Marcos [:dmarcos]]

This is Flame/QRD specific. For instance, Nexus 4 doesn't trigger flash while focusing in low light conditions.

Comment 12

[Pooja]

(In reply to Diego Marcos [:dmarcos] from comment #5)
> 
> Pooja, Does the flash go off also when touching to focus on low light
> conditions? 

Yes Flash goes off with Touch focus, but its expected behavior. Isn't it?

> What about continuous auto focus?
 Didn't get it. Do you mean  continuously keeping preview idle with on flash mode.
  If So yes

Comment 13

[Diego Marcos [:dmarcos]]

> 
> > What about continuous auto focus?
>  Didn't get it. Do you mean  continuously keeping preview idle with on flash
> mode.
>   If So yes

Does the flash also go off when the camera tries to focus on continuous auto focus mode?

Comment 14

[bhargavg1]

(In reply to Diego Marcos [:dmarcos] from comment #13)
> > 
> > > What about continuous auto focus?
> >  Didn't get it. Do you mean  continuously keeping preview idle with on flash
> > mode.
> >   If So yes
> 
> Does the flash also go off when the camera tries to focus on continuous auto
> focus mode?

Diego, have a quick question looks like we are trying to do AF even after we start recording. I believe normally AF is required only when user touches the preview or just before the capture.

Comment 15

[bhargavg1]

(In reply to bhargavg1 from comment #14)
> (In reply to Diego Marcos [:dmarcos] from comment #13)
> > > 
> Diego, have a quick question looks like we are trying to do AF even after we
> start recording. I believe normally AF is required only when user touches
> the preview or just before the capture.

Also because of the AF while recording we seem to be consuming more bus bandwidth

Comment 16

[Pooja]

(In reply to Diego Marcos [:dmarcos] from comment #13)
> > 
> Does the flash also go off when the camera tries to focus on continuous auto
> focus mode?

Hi Diego, With continuous auto focus mode the issue doesn't reproduced. Flash got fired automatically only when device detects faces in face detection focus mode.

To be sure about that i tried with changing focus "faceDetection :false" in apps/camera/js/config/config.js And so this issue doesn't reproduced.

Comment 17

[Pooja]

Marking Ni to Diego for Bhargav question comment#14

Comment 18

[Diego Marcos [:dmarcos]]

(In reply to bhargavg1 from comment #14)
> (In reply to Diego Marcos [:dmarcos] from comment #13)
> > > 
> > > > What about continuous auto focus?
> > >  Didn't get it. Do you mean  continuously keeping preview idle with on flash
> > > mode.
> > >   If So yes
> > 
> > Does the flash also go off when the camera tries to focus on continuous auto
> > focus mode?
> 
> Diego, have a quick question looks like we are trying to do AF even after we
> start recording. I believe normally AF is required only when user touches
> the preview or just before the capture.

Are you talking about continuous auto focus? Android does continuous auto focus while recording video iOS doesn't. Do you see anything wrong with the current behavior?

Comment 19

[Pooja]

I guess Bhargav means Focus mode changes from continuous-picture to autofocus mode and remains same.

That is why the LED gets fired because.
In (Flash Auto + Low light) or Flash on conditions, when Auto focus is triggered from application, internally flash gets fired. This is a feature – LED assisted AF.

Also attached logs.

Comment 20

[Pooja]

Attachment: Logs for flash fires Automatically in low light condition.

Comment 21

[Pooja]

Hi Diego,

Here in this case App is changing focus mode to auto whenever it detects faces. And so as per my above #comment19 flash fires.

Below log message indicates that app has requested to change focus-mode  from continous-picture to "Auto"

E/QCamera2HWI(  205): Pooja dumping params part 2 :-areas=(178,-259,384,16,1000);focus-distances=0.283767,0.382002,0.584262;focus-mode=auto;focus-mode

E/mm-camera-sensor(  300): port_sensor_handle_upstream_module_event:629 MCT_EVENT_MODULE_LED_AF_UPDATE  Indicates LED fired

Please take a look on logs and suggest why are we changing focus mode to Auto

Comment 22

[Pooja]

Attachment: adb logcat when flash fires automatically in low light condition

Comment 23

[Diego Marcos [:dmarcos]]

After detecting faces we change to auto (for a few seconds) so continuous auto focus doesn't override the focus area and the faces stay focused while touch to focus remains available. It gives the user the opportunity to take a picture with the faces on focus before continuous auto focus kicks back in.

Why does switching to auto focus mode trigger the flash? On auto focus mode the camera should only lock focus when the shutter is pressed and not when the mode kicks in. Just switching to auto focus mode should not actually trying to focus the image and trigger the flash as a consequence. ni Mike for second opinion.

Comment 24

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

(In reply to Diego Marcos [:dmarcos] from comment #23)
 
> Why does switching to auto focus mode trigger the flash? On auto focus mode
> the camera should only lock focus when the shutter is pressed and not when
> the mode kicks in. Just switching to auto focus mode should not actually
> trying to focus the image and trigger the flash as a consequence. ni Mike
> for second opinion.

Didn't we determine that this was (incorrect and) Flame-specific behaviour?

Comment 25

[Diego Marcos [:dmarcos]]

(In reply to Mike Habicher [:mikeh] from comment #24)
> (In reply to Diego Marcos [:dmarcos] from comment #23)
>  
> > Why does switching to auto focus mode trigger the flash? On auto focus mode
> > the camera should only lock focus when the shutter is pressed and not when
> > the mode kicks in. Just switching to auto focus mode should not actually
> > trying to focus the image and trigger the flash as a consequence. ni Mike
> > for second opinion.
> 
> Didn't we determine that this was (incorrect and) Flame-specific behaviour?

That's correct but this bug is file against QRD. This is very messy. There's no consistency about how drivers behave across devices. We should have a solid spec somewhere.

Comment 26

[Diego Marcos [:dmarcos]]

After talking to Mike. QRD device seems to be behaving in an unexpected manner:

- Auto focus mode should not trigger a focus cycle when the mode is enabled. 

From Android Camera API:

"Auto-focus mode. Applications should call autoFocus(AutoFocusCallback) to start the focus in this mode"

[1] http://developer.android.com/reference/android/hardware/Camera.Parameters.html#FOCUS_MODE_AUTO

This is an OEM issue.

ni Hema so we can bring the attention of the appropriate people.

Comment 27

[Hema Koka [:hema]]

(In reply to Diego Marcos [:dmarcos] from comment #26)
> After talking to Mike. QRD device seems to be behaving in an unexpected
> manner:
> 
> - Auto focus mode should not trigger a focus cycle when the mode is enabled. 
> 
> From Android Camera API:
> 
> "Auto-focus mode. Applications should call autoFocus(AutoFocusCallback) to
> start the focus in this mode"
> 
> [1]
> http://developer.android.com/reference/android/hardware/Camera.Parameters.
> html#FOCUS_MODE_AUTO
> 
> This is an OEM issue.
> 
> ni Hema so we can bring the attention of the appropriate people.

Inder, can we get your input on this please?

Thanks
Hema

Comment 28

[Inder]

Hema -- discussed this with Bhargav. Looks like there is misunderstanding on how the LED assisted AF feature works. Lets clarify it offline. Bhargav will start an email thread on it.

Comment 29

[vasanth]

Just to clarify some questions.

> Why *no* need to switch from Continuous Auto Focus to Auto Focus during Face Detection?
Our platform's CAF is FD aware and capable of focusing faces automatically.
So no need to switch to AF during FD in our platform.

> Why does switching to AF mode trigger the flash?
Quality of AF in low light condition is not so good. 
To improve this, we have come up with LED assisted AF feature which may not be present in other platforms.

Comment 30

[Diego Marcos [:dmarcos]]

(In reply to vasanth from comment #29)
> Just to clarify some questions.
> 
> > Why *no* need to switch from Continuous Auto Focus to Auto Focus during Face Detection?
> Our platform's CAF is FD aware and capable of focusing faces automatically.
> So no need to switch to AF during FD in our platform.

The rest of the devices we have don't automatically focus on detected faces. How can we detect and accommodate the different driver behavior of various devices in gaia? What behaviors should Mozilla support in our code base and what should be supported by partners? I think we should come with a focus spec that we commit to support. Mike, What do you think?

> 
> > Why does switching to AF mode trigger the flash?
> Quality of AF in low light condition is not so good. 
> To improve this, we have come up with LED assisted AF feature which may not
> be present in other platforms.

There's no problem on AF mode triggering the flash on a focus cycle. The question is, Why a focus cycle is triggered when switching to AF? Different drivers are implementing AF in different manners. There's confusion between AF-A, AF-S and AF-C:

http://www.slrphotographyguide.com/camera/nikon-digital-slr/focus-modes.shtml

Comment 31

[Yeojin Chung [:YeojinC]]

New test case needs to be added. There is no existing test case.

Comment 32

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

Okay, as I understand it, the premises of the problem are these:

1. the camera supports Face Detection ("FD");
2. the camera supports Continuous Auto-Focus ("CAF") wherein the camera regularly triggers focus cycles ("FC") without app intervention;
3. the camera supports Auto-Focus ("AF") wherein the app must call autoFocus() to trigger FC;
4. when in AF mode, FD sets the focus areas but does not automatically triggers FC on detected faces;
5. when in CAF mode, FD sets the focus areas and automatically triggers FC on detected faces;
6. when FC is triggered, if the flash mode ("FM") is set to "on", the flash WILL fire;
7. when FC is triggered, if FM is set to "auto", the flash MAY fire in low-light conditions;
8. when FC is triggered, if FM is set to "off", the flash will not fire, ever;
9. the Mozilla Camera app runs with focus mode ("focus") set to CAF;
10. the Mozilla Camera app responds to the detected faces by:
    a. changing focus from CAF to AF;
    b. stopping FD;
    c. setting the focus- and metering-areas to the largest detected face;
    d. setting FM to "off";
    e. calling autoFocus() to trigger FC;
    f. restoring FM in the autoFocus() callback;
    g. changing from AF back to CAF.

If any of the above premises are incorrect, please let me know. But based on them, we have the following observed behaviour:

a. FD triggers a CAF FC (premise 5) inside the driver, which may fire the flash (premises 6 and 7); or
b. FC may happen without FD (premise 2), which may also fire the flash (premises 6 and 7);
c. since (a) happens within the driver, premise 10 is irrelevant.

The expected behaviour (from the submitter) is:

i. FD does not trigger FC, or if it does, it doesn't fire the flash;
ii. before a picture is taken, FC is triggered;
iii. FC in (ii) may fire the flash.

If that is correct, then we have a few possible solutions:

A. break premise 5: modify the driver so that when in CAF, FD does not automatically trigger FC--from premise 10, we can already handle this in the Camera app;
B. break premises 6 and 7: modify the driver so that when CAF triggers FC, it does not fire the flash;
C. break premise 9: do not run the Camera app with it set to CAF, but use AF instead;
D. (ab)use premise 8: run the app with FM ALWAYS set to "off", regardless of what the UI shows, and when the user presses the shutter button, quickly set FM to the setting in the UI, call autoFocus() to trigger FC, and call takePicture() to take the picture.

None of the above solutions is ideal. A though C may affect CAF performance in low-light conditions; D may increase the latency of the shutter, something we are trying to minimize by using CAF in the first place.

We believe option (A), usign premise 10, is how at least one other partner has implemented their solution.

If option (A) is too drastic, my preference is for option (B), since that seems the least intrusive. Possible approaches:

I. handling CAF differently, so that the flash doesn't get fired, regardless of its mode; or
II. exposing a new FM, e.g. "auto-no-continuous" and "on-no-continuous" that gives the app the option of suppressing firing the flash when in CAF.

The logic to handle this could be (apologies--I'm running out of unique symbol sets):

B1. set focus to CAF;
B2. set FM to "auto-no-continous" or "on-no-continuous";
B3. camera runs FD/CAF without flash;
B4. Camera app gets autoFocusMoving() callbacks and calls autoFocus() to determine if FC succeeded or not, updates the UI, and keeps track of success/failure;
B5. when the user presses the shutter button, the Camera app checks last success/failure, and if failure, calls autoFocus() which causes the camera to run FC WITH the flash if needed;
B6. in the autoFocus() callback, the app calls takePicture().

Alternatively, with option (D):

D1. set focus to CAF;
D2. set FM to "off";
D3. camera runs FD/CAF without flash;
D4. Camera app gets autoFocusMoving() callbacks and calls autoFocus() to determine if FC succeeded or not, updates the UI, and keeps track of success/failure;
D5. when the user presses the shutter button, the Camera app checks last success/failure, and if success OR the UI FM is "off", calls takePicture(); if failure, and the UI FM is not "off", the app sets the correct FM and calls autoFocus() which causes the camera to run FC WITH the flash if needed;
D6. in the autoFocus() callback, the app calls takePicture().

Finally, in option (C), we undo bug 925192 and return to using AF.

There may be other solutions, but just so we can keep things clear, there are a lot of ambiguous terms in this dicussion, so please use the clauses symbols (they're case-sensitive!) and terms I've defined above, and provide clear definitions for any new ones.

Comment 33

[Hema Koka [:hema]]

Out of the possible solutions...

A. break premise 5: modify the driver so that when in CAF, FD does not automatically trigger FC--from premise 10, we can already handle this in the Camera app;
B. break premises 6 and 7: modify the driver so that when CAF triggers FC, it does not fire the flash;
C. break premise 9: do not run the Camera app with it set to CAF, but use AF instead;
D. (ab)use premise 8: run the app with FM ALWAYS set to "off", regardless of what the UI shows, and when the user presses the shutter button, quickly set FM to the setting in the UI, call autoFocus() to trigger FC, and call takePicture() to take the picture.

Option A or B seems like a viable option (Option C for switching back to AF may not be possible because of explicit asks by partners and option D creates latency issues). 

Inder,

Can you evaluate driver level solutions (A and B) that Mike is suggesting? Also confirm if this is a QRD specific issue only? Will we run into it on other 2.0 devices like open2, madai? (adding qawanted to check)

Sri, 

NI Sri to see if option C is still a possibility given there was a huge push to get focus set to Continuous Auto Focus.

Thanks
Hema

Comment 34

[Joshua Mitchell (Inactive)]

QA-Wanted dept does not have open2 or Madai devices. 

Buri and Open-C devices do not have flashes.

Comment 35

[Hema Koka [:hema]]

Does QA team have proper KK build for Flame? Can we please test this out on KK Flame build while Inder evaluates the possible solutions laid out by Mike. 

Justin just tried it with the eng kk build on his flame and camera hardware seems to say that flash itself is not detected. This probably is because of Flame specific KK build/v162-3 issues. Few other issues that we ran into https://bugzilla.mozilla.org/show_bug.cgi?id=1050467 and https://bugzilla.mozilla.org/show_bug.cgi?id=1050469


Thanks
Hema

Comment 36

[No-Jun Park [:njpark]]

In KK build, a number of features are still missing.
Touch to focus is not working (Bug 1050467)
Weird artifacts are shown on viewfiner (Bug 1050469)
Flash isn't working (Bug 1054515)

Because of these issues, and since flash does not fire at all under KK build per Bug 1050469, this bug is not reproducible in current Flame KK build.

Comment 37

[Inder]

Hema -- We found some details and existing driver's CAF is FD aware and capable of focusing faces automatically. So there is no need to switch to AF during FD in the camera app. That's an extra processing which is not needed here. Can we please get it fixed asap.

Comment 38

[William Hsu [:whsu]]

(In reply to No-Jun Park [:njpark] from comment #36)
> In KK build, a number of features are still missing.
> Touch to focus is not working (Bug 1050467)
> Weird artifacts are shown on viewfiner (Bug 1050469)
> Flash isn't working (Bug 1054515)
> 
> Because of these issues, and since flash does not fire at all under KK build
> per Bug 1050469, this bug is not reproducible in current Flame KK build.

Flame KK v2.0 has the same problems as No-Jun mentioned.
So, I also cannot check this issue on Flame KK. (Attach the screenshots)

By the way, we have encountered the similar issues on Flame JB. FYI.
- Bug 1036312 - Camera flash flashes when in background
- Bug 1036389 - [Flame][Master::v2.1] Auto-focus triggers flash light

Comment 39

[Mike Lien[:mlien]]

verify with Flame KK (1)only base image v165 and (2)v165 + v2.0 gaia/gecko
Result(1): cannot reproduce this issue
Result(2): can reproduce this issue

Comment 40

[Mike Lien[:mlien]]

(In reply to Mike Lien[:mlien] from comment #39)
> verify with Flame KK (1)only base image v165 and (2)v165 + v2.0 gaia/gecko
> Result(1): cannot reproduce this issue
> Result(2): can reproduce this issue

gaia/gecko info.
Gaia      d889984833025f208cfd3f3c2c37c87940a529dc
Gecko     6329352ca531b977979451e77e5862af485388b2
BuildID   20140815143240
Version   32.0

Comment 41

[Hema Koka [:hema]]

Justin will investigate since we got a new flame base image over the weekend and is reproducible with that and 2.0 codebase per previous comment

Comment 42

[Tony Chung [:tchung]]

ni? No-jun to try this again on flame base v165 with latest 2.0 build.

Comment 43

[No-Jun Park [:njpark]]

In the KK 2.0 version with v165, the problem is slightly different.
Following the steps in the original STR, the bug does not reproduce. HOWEVER,
the flash does not come on even when flash is set to ON.  Seems that flash is not functioning in this build

Gaia      d889984833025f208cfd3f3c2c37c87940a529dc
Gecko     6329352ca531b977979451e77e5862af485388b2
BuildID   20140815143240
Version   32.0
ro.build.version.incremental=18
ro.build.date=Thu Aug 14 19:29:46 CST 2014

Comment 44

[No-Jun Park [:njpark]]

Raised Bug 1051164 for flash non-firing issue

Comment 45

[No-Jun Park [:njpark]]

Sorr(In reply to No-Jun Park [:njpark] from comment #44)
> Raised Bug 1051164 for flash non-firing issue

Sorry, that should be Bug 1055164

Comment 46

[Hema Koka [:hema]]

(In reply to Inder from comment #37)
> Hema -- We found some details and existing driver's CAF is FD aware and
> capable of focusing faces automatically. So there is no need to switch to AF
> during FD in the camera app. That's an extra processing which is not needed
> here. Can we please get it fixed asap.

Based on this additional input, it looks like we are left with solution D from comment 32. Justin is working on it -- he is going to test how long it takes for the camera's flash state to change and if it impacts the shutter latency

Comment 47

[Joshua Mitchell (Inactive)]

(In reply to Tony Chung [:tchung] from comment #42)
> ni? No-jun to try this again on flame base v165 with latest 2.0 build.

Tony - can you elaborate on your QA-Wanted tag from comment 42? Did you want us to also check flame base v165?

Comment 48

[Tony Chung [:tchung]]

(In reply to Joshua Mitchell [:Joshua_M] from comment #47)
> (In reply to Tony Chung [:tchung] from comment #42)
> > ni? No-jun to try this again on flame base v165 with latest 2.0 build.
> 
> Tony - can you elaborate on your QA-Wanted tag from comment 42? Did you want
> us to also check flame base v165?

yeah, check it against v165, plus put 2.0 gaia/gecko on top of it.   You can ignore v162-3 instead.

Thanks,
Tony

Comment 49

[Hema Koka [:hema]]

Update based on call with CAF and Justin's test: We cannot go with solution option D because of latency issues that it is causing (from test results). Also looks like we missed comment 23 where Diego says that we switch to AF for a few seconds. He is going to try removing this. 

Thanks
Hema

Comment 50

[David Flanagan [:djf]]

(In reply to Hema Koka [:hema] [OOO 8/20 to 8/27] from comment #46)
> (In reply to Inder from comment #37)
> > Hema -- We found some details and existing driver's CAF is FD aware and
> > capable of focusing faces automatically. So there is no need to switch to AF
> > during FD in the camera app. That's an extra processing which is not needed
> > here. Can we please get it fixed asap.
> 
> Based on this additional input, it looks like we are left with solution D
> from comment 32. Justin is working on it -- he is going to test how long it
> takes for the camera's flash state to change and if it impacts the shutter
> latency

Hema: What Inder writes in comment #37 appears to be a request based on Mike's "Premise 10" from comment #32.  But it does not provide the information you requested in comment #33. If I'm reading this correctly, Mike's solutions A and B should still be on the table.

Inder: Could you comment on the driver-level fixes A and B that Mike proposed in comment #32, please?  Also, please clarify whether you think that what you suggest in comment #37 would solve this bug. It sounds to me like it is a performance issue but not directly related to the flash on face-detect issue.

Mike and Diego: Could either of you comment on "Premise 10" in comment #32? Why is it that the camera leaves C-AF mode and goes to AF mode when a face is detected? Is this required to make the Flame or Madai device work? Can we do as Inder suggests in comment #37 and just remain in C-AF mode all the time?

As a general thing, I think our work on this and similar bugs would be made much easier if we had documentation for the camera driver from CAF, or if Mike could sign an NDA with CAF and get access to the driver source code. With so much of our camera work, we're left trying to guess how the driver works.

Comment 51

[Justin D'Arcangelo [:justindarc]]

Attachment: pull-request (v2.0)

Pooja/Inder: Please give feedback on whether or not this patch resolves the issue on your QRD. Thanks!

Comment 52

[No-Jun Park [:njpark]]

Per Hema's suggestion, tried local KK 2.0 build.  The bug in Comment 44 does not reproduce anymore, and the original bug is not reproducible as well in both 289MB and 319MB config.

Version Info:
Gaia      1a215917df01bb815811f33665bd3fdca4130708
Gecko     d52b2995e5ea00867eeb425c10131c96e84bd55a
BuildID   20140818143909
Version   32.0
ro.build.version.incremental=18
ro.build.date=Thu Aug 14 19:29:46 CST 2014

Comment 53

[Justin D'Arcangelo [:justindarc]]

(In reply to No-Jun Park [:njpark] from comment #52)
> Per Hema's suggestion, tried local KK 2.0 build.  The bug in Comment 44 does
> not reproduce anymore, and the original bug is not reproducible as well in
> both 289MB and 319MB config.
> 
> Version Info:
> Gaia      1a215917df01bb815811f33665bd3fdca4130708
> Gecko     d52b2995e5ea00867eeb425c10131c96e84bd55a
> BuildID   20140818143909
> Version   32.0
> ro.build.version.incremental=18
> ro.build.date=Thu Aug 14 19:29:46 CST 2014

On my Flame, the bug was reproducible for the very first face detected only. Every subsequent face detected *did not* trigger the flash. Regardless, from what Inder has described, it sounds like we were abusing the face detection/focus API. Also, Andrew has noted that in the AOSP Camera app, the app only displays faces and makes no attempt to trigger focus on them.

Comment 54

[Hema Koka [:hema]]

(In reply to David Flanagan [:djf] from comment #50)

> 
> Hema: What Inder writes in comment #37 appears to be a request based on
> Mike's "Premise 10" from comment #32.  But it does not provide the
> information you requested in comment #33. If I'm reading this correctly,
> Mike's solutions A and B should still be on the table.
> 

David, Solutions A and B are off the table per call with Inder. So what Justin is trying now is removing the explicit switch to AF that is triggering the flash once the faces are detected (see comment 23) and see if that fixes the issue. We need to ensure that this works with other drivers that we support.

Comment 55

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

I was unable to reproduce this issue on the latest 2.0 build with the v165 kk base.  The flash did not go off except when I took a picture.  I also did not see No-Jun's issue where the flash does not work at all.

Environmental Variables:
Device: Flame 2.0
BuildID: 20140815143240
Gaia: d889984833025f208cfd3f3c2c37c87940a529dc
Gecko: 6329352ca531b977979451e77e5862af485388b2
Version: 32.0 (2.0) 
Firmware Version: v165
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 56

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

Test case added in moztrap:

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

Comment 57

[Pooja]

(In reply to Justin D'Arcangelo [:justindarc] from comment #51)
> Created attachment 8474770 [details] [review]
> pull-request (v2.0)
> 
> Pooja/Inder: Please give feedback on whether or not this patch resolves the
> issue on your QRD. Thanks!

Thanks for the change Justin. It works. Issue is not reproducible with it.

Comment 58

[Pooja]

Comment on attachment 8474770 [details] [review]
pull-request (v2.0)

Issue is not reproducible with change :)

Comment 59

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

Comment on attachment 8474770 [details] [review]
pull-request (v2.0)

And this doesn't break touch-to-focus?

Comment 60

[Justin D'Arcangelo [:justindarc]]

(In reply to Mike Habicher [:mikeh] from comment #59)
> Comment on attachment 8474770 [details] [review]
> pull-request (v2.0)
> 
> And this doesn't break touch-to-focus?

Good catch! It hadn't broken touch-to-focus when I was testing it, but right before I submitted the PR, I deleted the entire updateFocusArea() method because it looked like it wasn't needed anymore. Now that I've added it back in, touch-to-focus should also work in addition to fixing this issue.

Comment 61

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8474770 [details] [review]
pull-request (v2.0)

Diego: Please review. This patch fixes the issue on QRD.

Comment 62

[Diego Marcos [:dmarcos]]

Comment on attachment 8474770 [details] [review]
pull-request (v2.0)

This patch is assuming that the phone automatically focuses on faces. Is this a consistent behavior across all devices we want to support? My JB flame for instance does not automatically focuses on faces.

Comment 63

[Justin D'Arcangelo [:justindarc]]

(In reply to Diego Marcos [:dmarcos] from comment #62)
> Comment on attachment 8474770 [details] [review]
> pull-request (v2.0)
> 
> This patch is assuming that the phone automatically focuses on faces. Is
> this a consistent behavior across all devices we want to support? My JB
> flame for instance does not automatically focuses on faces.

This is a good question. According to Comment 37, Inder says that the driver's C-AF is FD aware. I don't know for certain if this will always be the case. If not, then we should probably put this code behind a build-time flag.

Comment 64

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

(In reply to Diego Marcos [:dmarcos] from comment #62)

> This patch is assuming that the phone automatically focuses on faces. Is
> this a consistent behavior across all devices we want to support? My JB
> flame for instance does not automatically focuses on faces.

The Android documentation says that the camera will automatically focus on any detected faces once FD is started. Conversely, any device not exhibiting it is "broken" by Android's standards.

http://developer.android.com/reference/android/hardware/Camera.html#startFaceDetection%28%29

"When the face detection is running, setWhiteBalance(String), setFocusAreas(List), and setMeteringAreas(List) have no effect. The camera uses the detected faces to do auto-white balance, auto exposure, and autofocus."

Comment 65

[bhavana bajaj [:bajaj]]

(In reply to Diego Marcos [:dmarcos] from comment #62)
> Comment on attachment 8474770 [details] [review]
> pull-request (v2.0)
> 
> This patch is assuming that the phone automatically focuses on faces. Is
> this a consistent behavior across all devices we want to support? My JB
> flame for instance does not automatically focuses on faces.

Lets not worry about JB for 2.0 as all the update's/releases for 2.0 will have full system update including update to KK per CAF.

Comment 66

[Diego Marcos [:dmarcos]]

Comment on attachment 8474770 [details] [review]
pull-request (v2.0)

I suggested some changes on GH

Comment 67

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8474770 [details] [review]
pull-request (v2.0)

Diego: Addressed review comments and fixed unit tests. Please re-review, thanks!

Comment 68

[Diego Marcos [:dmarcos]]

Comment on attachment 8474770 [details] [review]
pull-request (v2.0)

The patch looks great but I want to make sure we are solving this properly. I see the circles for the detected faces but the image doesn't seem to focus on them. Also it's weird to see the continuous auto focus ring at the same time than the circles for the faces. We have to handle it more gracefully.

Comment 69

[Justin D'Arcangelo [:justindarc]]

(In reply to Diego Marcos [:dmarcos] from comment #68)
> Comment on attachment 8474770 [details] [review]
> pull-request (v2.0)
> 
> The patch looks great but I want to make sure we are solving this properly.
> I see the circles for the detected faces but the image doesn't seem to focus
> on them. Also it's weird to see the continuous auto focus ring at the same
> time than the circles for the faces. We have to handle it more gracefully.

According to comment 57, this patch works on QRD. I'm not sure if the QRD has a capability that the Flame does not? Flagging Inder for more info.

Also, Diego, this is marked as a 2.0+ blocker, so I believe it needs to land today? What do you propose we should do here?

Comment 70

[Justin D'Arcangelo [:justindarc]]

Attachment: pull-request (master)

Patch for master (v2.1) -- Carrying R+ forward

Comment 71

[Justin D'Arcangelo [:justindarc]]

Landed on master:

https://github.com/mozilla-b2g/gaia/commit/17e16d894b80ac45a078e35f3cdf1272f388e325

Comment 72

[Justin D'Arcangelo [:justindarc]]

Landed on v2.0:

https://github.com/mozilla-b2g/gaia/commit/7cd93d61aed02d8a41e1ef17145618500bd26ac2