[KK] Continuous auto focus (CAF) not working in Camcorder

VERIFIED FIXED in Firefox 34, Firefox OS v2.1

Status

Firefox OS
Gaia::Camera
--
major
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: Pooja, Assigned: aosmond)

Tracking

({regression})

unspecified
2.1 S5 (26sep)
ARM
Gonk (Firefox OS)
regression
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.1+, firefox33 wontfix, firefox34 fixed, firefox35 fixed, b2g-v2.0 unaffected, b2g-v2.1 verified, b2g-v2.2 verified)

Details

(Whiteboard: [caf priority: p3][CR 721613][CR 744280])

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

4 years ago
STR:
1. Open camera app
2. Switch to Camcorder
3. Try to focus on an object.

Actual behavior:
  CAF is not working

Expected behavior:
  CAF should work 

Device and build details:
Device msm8x26 QRD KK branch

Gaia : 944e5b4582c4efa1b67cd33245dbb8f6aa25d09f
Gecko: a50444fdaa68fefefc3ecc08d97530e13a4742ab
OS Version: 2.1.0
Platform Version : 34.0a2
Build ID: 20140915184008
(Reporter)

Comment 1

4 years ago
[Blocking Requested - why for this release]:
Blocks: 1025317
blocking-b2g: --- → 2.1?
Whiteboard: [CR 721613]

Updated

4 years ago
Whiteboard: [CR 721613] → [caf priority: p3][CR 721613]
QA Wanted - Is this possible to reproduce on a Flame KK 2.1 build?
Keywords: qawanted
Issue is reproducible on Flame 2.1, Flame 2.2 Master, and Open C 2.2 Master.

Observed behavior on Flame: Continuous auto focus is not enabled in video mode. User needs to manually tap on screen to make it focus.

Observed behavior on Open C: Continuous auto focus is not enabled in video mode. Tapping on screen doesn't make the camera focus either. Also it shows a band of artifacts on bottom of screen when in video mode.

Device: Flame 2.1
BuildID: 20140917073957
Gaia: 47939f4c41d0c941e5047e5d1af74a79b7d8e0d5
Gecko: d7ad9b5167d8
Version: 34.0a2 (2.1)
Firmware: v165
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.2 Master
BuildID: 20140916170759
Gaia: 50666fa8bbbf3d346faff24f92ad8140a44a49d0
Gecko: 8252eae8278c
Version: 35.0a1 (2.2 Master)
Firmware: v165
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

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

Issue is NOT reproducible on Flame 2.0.

Observed behavior: Continuous auto focus is enabled in video mode. Device correctly adjusts its focus when moving device away or close to objects without user tapping on screen.

Device: Flame 2.0
BuildID: 20140917073857
Gaia: 31434a3949556171f3565ca47ac2b44e810e95e6
Gecko: e02fe140c0d5
Version: 32.0 (2.0)
Firmware: v165
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
QA Whiteboard: [QAnalyst-Triage?]
status-b2g-v2.0: --- → unaffected
status-b2g-v2.1: --- → affected
status-b2g-v2.2: --- → affected
Flags: needinfo?(jmitchell)
Keywords: qawanted
Forgot to post Open C environmental variables on comment 3:

Device: Open_C 2.2 Master
BuildID: 20140916170759
Gaia: 50666fa8bbbf3d346faff24f92ad8140a44a49d0
Gecko: 8252eae8278c
Version: 35.0a1 (2.2 Master)
Firmware: P821A10v1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: regression, regressionwindow-wanted
QA Contact: pcheng
b2g-inbound regression window:

Last Working Environmental Variables:
Device: Flame
BuildID: 20140625143731
Gaia: 515914905d40d171ea6b927f4c8cbbb93c5bde20
Gecko: ffa21babf90c
Version: 33.0a1
Firmware: V123

First Broken Environmental Variables:
Device: Flame
BuildID: 20140625144906
Gaia: 515914905d40d171ea6b927f4c8cbbb93c5bde20
Gecko: 7a84bd4bb3da
Version: 33.0a1
Firmware: V123

Gaia is the same so it's a Gecko issue.

Gecko pushlog:
http://hg.mozilla.org/integration/b2g-inbound/pushloghtml?fromchange=ffa21babf90c&tochange=7a84bd4bb3da

Caused by Bug 1025197.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: regressionwindow-wanted
broken by  Bug 1025197 - can you take a look Mike?
Blocks: 1025197
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell) → needinfo?(mhabicher)

Comment 7

4 years ago
Blocking Reason: Regression

Andrew, please investigate this one (Mike is out till 22nd)
blocking-b2g: 2.1? → 2.1+
Flags: needinfo?(aosmond)

Updated

4 years ago
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review+]
(Assignee)

Updated

4 years ago
Assignee: nobody → aosmond
(Assignee)

Comment 8

4 years ago
While it is not 100% reproducible for me, it does happen often enough that I think I've captured the problem. When auto focus stops moving in continuous-video mode, the camera app attempts to auto focus explicitly. This is permitted behaviour according to the AOSP documentation [1]. While the driver appears to kick off the focus, it would not return until it was very close to an object, and even then the focus would fail in the end.

The AOSP documentation [2] suggests that it is intended for UI updates and its camera application [3] follows this behaviour. I changed our application to follow this and the performance is much more satisfactory on flame.

[1] http://developer.android.com/reference/android/hardware/Camera.Parameters.html#FOCUS_MODE_CONTINUOUS_VIDEO
[2] http://developer.android.com/reference/android/hardware/Camera.AutoFocusMoveCallback.html
[3] https://android.googlesource.com/platform/packages/apps/Camera2.git/+/master/src/com/android/camera/FocusOverlayManager.java line 305

It is possible that setting the recording hint aggravated the driver somehow (the change marked as introducing the regression). Previously we would let the driver set it internally, and only reset it after ending a recording. But my belief is that that code is correct :).
Status: NEW → ASSIGNED
(Assignee)

Comment 9

4 years ago
Created attachment 8491783 [details] [review]
Gaia pull request, v1

piwei: Could you try out the pull request and let me know if resolves the problem for you? The bug *should* affect 2.0 (and 1.4) but since I couldn't reproduce 100% of the time, so I want to make sure I am fixing the right problem :).
Attachment #8491783 - Flags: feedback?(pcheng)
Flags: needinfo?(mhabicher)
Flags: needinfo?(aosmond)
Comment on attachment 8491783 [details] [review]
Gaia pull request, v1

This patch fixes the continuous auto focus issue. I'm now seeing the video recorder continuously making effort to focus.

However I am seeing another issue where sometimes when it's having difficulty focusing, the viewfinder/screen would continuously go very dark and very bright until it successfully focuses, which seems a bit odd to me. I could add a video if needed.
Attachment #8491783 - Flags: feedback?(pcheng) → feedback+
(Assignee)

Comment 11

4 years ago
(In reply to Pi Wei Cheng [:piwei] from comment #10)
> Comment on attachment 8491783 [details] [review]
> Gaia pull request, v1
> 
> This patch fixes the continuous auto focus issue. I'm now seeing the video
> recorder continuously making effort to focus.
> 
> However I am seeing another issue where sometimes when it's having
> difficulty focusing, the viewfinder/screen would continuously go very dark
> and very bright until it successfully focuses, which seems a bit odd to me.
> I could add a video if needed.

I've seen this before independent of my change, but only on KK; v165 is what I have been working with. Did you test this on JB or KK?
(In reply to Andrew Osmond [:aosmond] from comment #11)
> I've seen this before independent of my change, but only on KK; v165 is what
> I have been working with. Did you test this on JB or KK?

I was on KK (v180) base when testing the patch.
Depends on: 994912
We currently call autofocus when the onAutoFocusMoving callback is invoked. It's the only way we had to determine if CAF succeded or failed to focus a subject and show a red or green focus ring accordingly.

bug 994912 is introducing events instead of callbacks and will make onAutoFocusMoving unnecessary solving this bug as well.
(In reply to Diego Marcos [:dmarcos] from comment #13)
> We currently call autofocus when the onAutoFocusMoving callback is invoked.
> It's the only way we had to determine if CAF succeded or failed to focus a
> subject and show a red or green focus ring accordingly.
> 
> bug 994912 is introducing events instead of callbacks and will make
> onAutoFocusMoving unnecessary solving this bug as well.

bug 994912 will not land in time for 2.1, so we need a way to resolve this regression with  minimal risk
(Assignee)

Comment 15

4 years ago
Created attachment 8492445 [details] [diff] [review]
Gecko, add support for auto focus completed cb, v1

Bug 994912 will be committed on Monday but maybe it is a bit large / risky to pull into 2.1. Created gecko patch to add necessary functionality for apps to get desired auto focus state. Updated gaia pull request to follow.
Attachment #8492445 - Flags: review?(dhylands)
(Assignee)

Comment 17

4 years ago
Created attachment 8492449 [details] [review]
Gaia pull request, v2

Updated gaia pull request to rely upon the new auto focus completed method added in the gecko patch.
Attachment #8491783 - Attachment is obsolete: true
Attachment #8492449 - Flags: review?(dmarcos)
(Assignee)

Comment 18

4 years ago
Created attachment 8492455 [details] [diff] [review]
Gecko, add support for auto focus completed cb, v2

gecko try: https://tbpl.mozilla.org/?tree=Try&rev=89247df58b68

Don't be hasty. I reused a previous debugging hg patch queue and forgot there was still stuff in the tree. Mea culpa.
Attachment #8492445 - Attachment is obsolete: true
Attachment #8492445 - Flags: review?(dhylands)
Attachment #8492455 - Flags: review?(dhylands)
Comment on attachment 8492455 [details] [diff] [review]
Gecko, add support for auto focus completed cb, v2

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

r=me with the test issue resolved.

::: dom/camera/test/test_bug1022766.html
@@ +39,2 @@
>    checkForDone: function test_checkForDone() {
> +    if (Camera.firstCallFailed && Camera.secondCallSucceeded && Camera.callbackTriggered) {

This says callbackTriggered but a couple lines above this you set callbackTrigger (no ed).

You should add a test for the callback not being called and make sure that calbackTriggered is still false.
Attachment #8492455 - Flags: review?(dhylands) → review+
(Assignee)

Comment 20

4 years ago
Created attachment 8493035 [details] [diff] [review]
Gecko, add support for auto focus completed cb, v3

(In reply to Dave Hylands [:dhylands] from comment #19)
> Comment on attachment 8492455 [details] [diff] [review]
> Gecko, add support for auto focus completed cb, v2
> 
> Review of attachment 8492455 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me with the test issue resolved.
> 
> ::: dom/camera/test/test_bug1022766.html
> @@ +39,2 @@
> >    checkForDone: function test_checkForDone() {
> > +    if (Camera.firstCallFailed && Camera.secondCallSucceeded && Camera.callbackTriggered) {
> 
> This says callbackTriggered but a couple lines above this you set
> callbackTrigger (no ed).
> 
> You should add a test for the callback not being called and make sure that
> calbackTriggered is still false.

Nit fixed.

gecko try: https://tbpl.mozilla.org/?tree=Try&rev=e060b4f64641
Attachment #8492455 - Attachment is obsolete: true
Attachment #8493035 - Flags: review+
Blocks: 1068528

Updated

4 years ago
Summary: Continuous auto focus (CAF) not working in Camcorder → [KK] Continuous auto focus (CAF) not working in Camcorder
Blocks: 1072461
Comment on attachment 8493035 [details] [diff] [review]
Gecko, add support for auto focus completed cb, v3

Andrew, if we want to land this, you'll need a DOM-peer to review the WebIDL change.
(Assignee)

Comment 22

4 years ago
Comment on attachment 8493035 [details] [diff] [review]
Gecko, add support for auto focus completed cb, v3

bz: This should be a quick review compared to bug 994912 :). This is a temporary addition which that bug deprecates (because ideally we won't put 994912 in 2.1 since it is so big).
Attachment #8493035 - Flags: review+ → review?(bzbarsky)
aosmond, dmarcos: with the above Gecko and Gaia patches applied to a Nexus 4, I see CAF working properly, with no spurious flash fires in varying lighting conditions.

I do, however, still see an initial CAF-moving circle appear and go green when I switch to video mode; after it goes away, CAF still works but the circle never reappears.
Comment on attachment 8493035 [details] [diff] [review]
Gecko, add support for auto focus completed cb, v3

r=me
Attachment #8493035 - Flags: review?(bzbarsky) → review+
(Assignee)

Updated

4 years ago
No longer depends on: 994912
Attachment #8492449 - Flags: review?(dmarcos) → review+
(Assignee)

Comment 25

4 years ago
The gecko change is safe to check in on its own, but the gaia change needs to go in after it.
Keywords: checkin-needed
landed in https://hg.mozilla.org/integration/b2g-inbound/rev/7ea7a02ce2be

and gaia master: https://github.com/mozilla-b2g/gaia/commit/d9106e0e0b50e262cc6b16b98a068c65e5880342
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S5 (26sep)
Please add Gaia v2.1 and aurora approval requests when you get a chance :)
status-b2g-v2.2: affected → fixed
status-firefox33: --- → wontfix
status-firefox34: --- → affected
status-firefox35: --- → fixed
Flags: needinfo?(aosmond)
(Assignee)

Comment 29

4 years ago
Comment on attachment 8493035 [details] [diff] [review]
Gecko, add support for auto focus completed cb, v3

Approval Request Comment
[Feature/regressing bug #]: 1068559
[User impact if declined]: Continuous auto focus performance will be very poor. User may see flash turn on when unexpected.
[Describe test coverage new/current, TBPL]: Augmented test case which verifies we receive new callback added when expected with the emulated camera. Verified manually on flame and nexus4 that it works.
[Risks and why]: It is possible that some drivers behave differently from flame-jb, flame-kk and nexus4 and we won't receive the driver event; we didn't rely upon receiving this event in this context in the past.
[String/UUID change made/needed]: None
Attachment #8493035 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 30

4 years ago
Comment on attachment 8492449 [details] [review]
Gaia pull request, v2

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): 1068559
[User impact] if declined: Continuous auto focus performance will be very poor. User may see flash turn on when unexpected.
[Testing completed]: Added/updated unit test cases for affected modules. Verified manually on flame and nexus4 that CAF works as expected now.
[Risk to taking this patch] (and alternatives if risky): It is possible that some drivers behave differently from flame-jb, flame-kk and nexus4 and we won't receive the driver event; we didn't rely upon receiving this event in this context in the past. That would leave the focus ring displayed on the application some of the time.
[String changes made]: None
Attachment #8492449 - Flags: approval-gaia-v2.1?(hkoka)
Flags: needinfo?(aosmond)

Updated

4 years ago
Attachment #8492449 - Flags: approval-gaia-v2.1?(hkoka) → approval-gaia-v2.1?(fabrice)
Attachment #8492449 - Flags: approval-gaia-v2.1?(fabrice) → approval-gaia-v2.1+

Updated

4 years ago
Attachment #8493035 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Duplicate of this bug: 1008283

Updated

4 years ago
Status: RESOLVED → VERIFIED
(Reporter)

Comment 33

4 years ago
Hi Andrew,

We still see the issue in our latest build in camcorder CAF. 
Triggering for CAF occurs however the focus doesn't happen.
Flags: needinfo?(hkoka)
Flags: needinfo?(aosmond)
(Reporter)

Comment 34

4 years ago
Hi Andrew,

We still see the issue in our latest build in camcorder CAF. 
Triggering for CAF occurs however the focus doesn't happen.


Kindly confirm once and re-open the bug
(Reporter)

Updated

4 years ago
Blocks: 1074290
(Reporter)

Updated

4 years ago
Whiteboard: [caf priority: p3][CR 721613] → [caf priority: p3][CR 721613][CR 744280]
(Reporter)

Updated

4 years ago
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 35

4 years ago
(In reply to Pooja from comment #34)
> Hi Andrew,
> 
> We still see the issue in our latest build in camcorder CAF. 
> Triggering for CAF occurs however the focus doesn't happen.
> 
> 
> Kindly confirm once and re-open the bug

I am unable to confirm on flame v188 with latest b2g-inbound and 2.1 builds. 2.0 does not perform as well as 2.1/b2g-inbound but it still autofocuses, just less frequently. Has the STR changed at all or is it still as in the original description? Could you also let me know the gaia/gecko/etc versions you are using?
Flags: needinfo?(aosmond) → needinfo?(poojas)

Comment 36

4 years ago
Adding qawanted to see if this is reproducible with latest 2.1 build on flame with KK.

Thanks
Hema
Flags: needinfo?(hkoka)
Keywords: qawanted
(In reply to Hema Koka [:hema] from comment #36)
> Adding qawanted to see if this is reproducible with latest 2.1 build on
> flame with KK.

Issue is NOT reproducible on 2.1 KK Flame. Camcorder automatically and continuously makes effort to focus and would show a green ring once it successfully focuses.

Device: Flame (full flash, 512MB mem)
BuildID: 20141028001203
Gaia: a0174f7166745256aaca1cb3aa9f894033fbffa6
Gecko: 43bda3541f6b
Gonk: 6e51d9216901d39d192d9e6dd86a5e15b0641a89
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
QA Whiteboard: [QAnalyst-Triage+][lead-review+] → [QAnalyst-Triage?][lead-review+]
Flags: needinfo?(jmitchell)
Keywords: qawanted
QA Whiteboard: [QAnalyst-Triage?][lead-review+] → [QAnalyst-Triage+][lead-review+]
Flags: needinfo?(jmitchell)
QA Contact: pcheng

Comment 38

4 years ago
Pooja, please check. We are not able to reproduce on Flame KK. 

Thanks
Hema
(Reporter)

Comment 39

4 years ago
Yeas, its not reproducible on flame. 
We debugged it as internal issue.

Thanks everyone for your time and support :)
Flags: needinfo?(poojas)
(Assignee)

Updated

4 years ago
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED

Comment 40

4 years ago
Verified the bug is fixed on 2.2 and 2.1 Flame builds'
Camcorder automatically and continuously makes effort to focus and shows a green ring once it successfully focuses.

Device: Flame 2.2 Master KK
BuildID: 20141030040201
Gaia: 0dfc1996eb583c8b507a82bf6b8319624bba23ea
Gecko: 80e18ff7c7b2
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 36.0a1 (2.2 Master)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Device: Flame 2.1 KK
BuildID: 20141030001201
Gaia: 3e585d8be5e2dffc376f83313299c9b6d53c3db4
Gecko: b643d78a23c6
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+][lead-review+] → [QAnalyst-Triage?]
status-b2g-v2.1: fixed → verified
status-b2g-v2.2: fixed → verified
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.