1019965 - [Camera] Implement visual feedback for continuous auto focus

Status: VERIFIED FIXED

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

Comment 1

[Diego Marcos [:dmarcos]]

Attachment: Pull Request

Comment 2

[Justin D'Arcangelo [:justindarc]]

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

looks good!

Comment 3

[Tiffanie Shakespeare [:tif]]

I really like the indicator - I think it really helps with the overall experience of the camera.

One thing  I noticed is that it seems like the camera is still trying to focus when you're in preview? I think ideally we would want stop that, but justindarc mentioned that on less powerful devices it may increase the delay when switching back to camera from preview which isn't a good trade off. 

would there be anything else we could do? Like maybe pause focusing when in preview? 

Thanks Diego for creating this patch!

Comment 4

[Diego Marcos [:dmarcos]]

Updated PR to disable focus when opening gallery. Carrying R+. Waiting for green

Comment 5

[Justin D'Arcangelo [:justindarc]]

(In reply to Diego Marcos [:dmarcos] from comment #4)
> Updated PR to disable focus when opening gallery. Carrying R+. Waiting for
> green

Good idea! I was thinking about shutting down the mozCamera, but that would introduce some serious lag.

Comment 6

[Amy Lee [:amylee] UX]

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

Is it possible to see this work along side the indicator when you capture the photo? Right now when I hit image capture, I get the loading spinner before it captures the image. Thanks!

Comment 7

[Diego Marcos [:dmarcos]]

Sorry but I don't understand what you're saying. When in continuous autofocus (C-AF) the camera doesn't focus when you capture instead it's continuously trying to focus so capturing an image is instantaneous. This how all cameras work.

Comment 8

[Amy Lee [:amylee] UX]

(In reply to Diego Marcos [:dmarcos] from comment #7)
> Sorry but I don't understand what you're saying. When in continuous
> autofocus (C-AF) the camera doesn't focus when you capture instead it's
> continuously trying to focus so capturing an image is instantaneous. This
> how all cameras work.

Every time I take a photo the the screen darkens and the spinner appears. Is this due to lag? The animation for AF and CF should be consistent. In AF, the indicator rotates 45 degrees and turns green when focused.

Comment 9

[Justin D'Arcangelo [:justindarc]]

(In reply to Amy Lee [:amylee] from comment #8)
> (In reply to Diego Marcos [:dmarcos] from comment #7)
> > Sorry but I don't understand what you're saying. When in continuous
> > autofocus (C-AF) the camera doesn't focus when you capture instead it's
> > continuously trying to focus so capturing an image is instantaneous. This
> > how all cameras work.
> 
> Every time I take a photo the the screen darkens and the spinner appears. Is
> this due to lag? The animation for AF and CF should be consistent. In AF,
> the indicator rotates 45 degrees and turns green when focused.

I just noticed that spinner today too. I don't think we should be seeing it unless we're shooting HDR on Flame so that may be a bug? (NI? for wilson)

As far as the animation goes, I think the reason diego made an alternate animation was because our regular focus is a little too *loud* for C-AF. Also, on iOS, the C-AF animation is different and toned-down a bit from the regular tap-to-focus animation (not sure about Android).

Comment 10

[Justin D'Arcangelo [:justindarc]]

To clarify...

Wilson: I think that spinner is appearing because of your patch in Bug 1016492, correct? Should we only be seeing it for HDR on Flame? (it is currently appearing after every picture you take on Flame regardless of mode)

Comment 11

[Wilson Page [:wilsonpage]]

(In reply to Justin D'Arcangelo [:justindarc] from comment #10)
> To clarify...
> 
> Wilson: I think that spinner is appearing because of your patch in Bug
> 1016492, correct? Should we only be seeing it for HDR on Flame? (it is
> currently appearing after every picture you take on Flame regardless of mode)

Yes, that must be my doing :(

Basically the spinner kicks in after a 500ms delay from the mozCamera.takePicture() call. I'm guessing we should increase that timeout so that the spinner appears only after a significant delay, perhaps 1000ms - 1500ms.

If you file a follow-up I will fix first thing tomorrow, apologies!

Comment 12

[Tiffanie Shakespeare [:tif]]

Follow-up bug 1021284 

Let's increase the delay to 1000ms. Thanks Wilson!

(In reply to Wilson Page [:wilsonpage] from comment #11)
> (In reply to Justin D'Arcangelo [:justindarc] from comment #10)
> > To clarify...
> > 
> > Wilson: I think that spinner is appearing because of your patch in Bug
> > 1016492, correct? Should we only be seeing it for HDR on Flame? (it is
> > currently appearing after every picture you take on Flame regardless of mode)
> 
> Yes, that must be my doing :(
> 
> Basically the spinner kicks in after a 500ms delay from the
> mozCamera.takePicture() call. I'm guessing we should increase that timeout
> so that the spinner appears only after a significant delay, perhaps 1000ms -
> 1500ms.
> 
> If you file a follow-up I will fix first thing tomorrow, apologies!

Comment 13

[Diego Marcos [:dmarcos]]

I updated the PR to make consistent the continuous auto focus and touch to focus animations. For the continuous focus case if we want to show the state we have first to fix the problem with multiple calls to mozCamera.autoFocus (bug 1022766)

Comment 14

[Diego Marcos [:dmarcos]]

I want to land this today before branching and later add the state color to the ring once we fix bug 1022766.

Comment 15

[Diego Marcos [:dmarcos]]

I created a follow up bug for the ring color: bug 1022766

Comment 16

[Diego Marcos [:dmarcos]]

Sorry. The follow up bug is bug 1022880

Comment 17

[Tiffanie Shakespeare [:tif]]

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

Seems good. Amy can provide additional feedback about the ring.

Comment 18

[Diego Marcos [:dmarcos]]

Landed on master:

https://github.com/mozilla-b2g/gaia/commit/2d9faa35dfb6b43e58ed09f6c89a3b5705d8ea6f

Please file follow up bugs for any polish or changes

Comment 19

[Ryan VanderMeulen [:RyanVM]]

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

Comment 20

[Lancy(Leave from Mozilla)]

Attachment: Verify_Video_Flame.MP4

This issue has been verified successfully on Flame 2.0 & 2.1.
See attachment: Verify_Video_Flame.MP4
Reproducing rate: 0/20

Flame v2.0 version:
Gaia-Rev        ead3b72a84512750bc5faff4e9e8faa1715c0d05
Gecko-Rev       c715df57d1d1593ede48140ebc88e101f8c3f7da
Build-ID        20141208050313
Version         32.0
Device-Name     jrdhz72_w_ff
FW-Release      4.4.2

Flame v2.1 version:
Gaia-Rev        38e17b0219cbc50a4ad6f51101898f89e513a552
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/8b92c4b8f59a
Build-ID        20141205001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141205.035305
FW-Date         Fri Dec  5 03:53:16 EST 2014
Bootloader      L1TC00011880