Closed
Bug 1152260
Opened 9 years ago
Closed 9 years ago
[Nexus 5][Camera]The camera can't autofocus, white focus circle always exists and does not change color.
Categories
(Firefox OS Graveyard :: Gaia::Camera, defect)
Tracking
(blocking-b2g:2.2+, firefox38 wontfix, firefox39 wontfix, firefox40 fixed, b2g-v2.2 verified, b2g-master verified)
People
(Reporter: yi.zou, Assigned: aosmond)
References
Details
(Whiteboard: [v2.2-nexus-5-l])
Attachments
(6 files, 4 obsolete files)
5.15 MB,
video/mp4
|
Details | |
470.31 KB,
text/plain
|
Details | |
3.34 MB,
video/mp4
|
Details | |
11.52 KB,
patch
|
aosmond
:
review+
|
Details | Diff | Splinter Review |
10.14 KB,
patch
|
aosmond
:
review+
bajaj
:
approval-mozilla-b2g37+
|
Details | Diff | Splinter Review |
7.16 MB,
video/mp4
|
Details |
[1.Description]: [Nexus5 v2.2 & v3.0]The camera can't autofocus, white focus circle always exists and does not change color. Time:16:54 See attachment:1654.3gp,logcat-1654.txt [2.Testing Steps]: 1. Open camera app. 2. Move the camera around to change focus 3. Waiting for the camera with auto focus [3.Expected Result]: 3. A white focus circle reappears, and after locking the focus, it turns green and disappears. [4.Actual Result]: 3. The camera can't autofocus, white focus circle always exists and does not change color. [5.Reproduction build]: Device: N5 2.2 (affected) Build ID 20150329002502 Gaia Revision 473cd63f53c855299b719285d9b95e3f2910782f Gaia Date 2015-03-27 20:14:43 Gecko Revision https://hg.mozilla.org/releases/mozilla- b2g37_v2_2/rev/4b13c4254e2f Gecko Version 37.0 Device Name hammerhead Firmware(Release) 5.0 Firmware(Incremental) eng.cltbld.20150329.035946 Firmware Date Sun Mar 29 03:59:59 EDT 2015 Bootloader HHZ12d Device: N5 3.0 (affected) Build ID 20150407160201 Gaia Revision 84cbd4391fb7175d5380fa72c04d68873ce77e6d Gaia Date 2015-04-07 17:33:14 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/078128c2600a Gecko Version 40.0a1 Device Name hammerhead Firmware(Release) 5.0 Firmware(Incremental) eng.cltbld.20150407.192944 Firmware Date Tue Apr 7 19:30:00 EDT 2015 Bootloader HHZ12d Device: Flame 2.2 (unaffected) Build ID 20150407162504 Gaia Revision ea735c21bfb0d78333213ff0376fce1eac89ead6 Gaia Date 2015-04-07 20:58:15 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/3f86ddb7f719 Gecko Version 37.0 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150407.195227 Firmware Date Tue Apr 7 19:52:39 EDT 2015 Bootloader L1TC000118D0 Device: Flame 3.0 (unaffected) Build ID 20150407160201 Gaia Revision 84cbd4391fb7175d5380fa72c04d68873ce77e6d Gaia Date 2015-04-07 17:33:14 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/078128c2600a Gecko Version 40.0a1 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150407.193600 Firmware Date Tue Apr 7 19:36:12 EDT 2015 Bootloader L1TC000118D0 [6.Reproduction Frequency]: Always Recurrence,5/5 [7.TCID]: Free Test [8.Note]: This issue doesn't exist on flame.
Reporter | ||
Comment 1•9 years ago
|
||
Reporter | ||
Updated•9 years ago
|
status-b2g-v2.2:
--- → affected
status-b2g-master:
--- → affected
Comment 2•9 years ago
|
||
regressionwindow on V2.2, thanks.
Flags: needinfo?(yi.zou)
Keywords: regressionwindow-wanted
Comment 3•9 years ago
|
||
Hi Eric, The N5 v2.2 oldest nightly build is ‘20150224162516’ and problem can be reproduced on this build by the STR in comment 0. See attachment: RegressionVideo.mp4 Device: Nexus5 v2.2 Build ID 20150224162516 Gaia Revision ca64f2fe145909f31af266b1730874051ba76c78 Gaia Date 2015-02-24 22:06:53 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/16804008c29f Gecko Version 37.0 Device Name hammerhead Firmware(Release) 5.0 Firmware(Incremental) eng.cltbld.20150224.201149 Firmware Date Tue Feb 24 20:12:04 EST 2015 Bootloader HHZ12d
Flags: needinfo?(yi.zou)
Updated•9 years ago
|
QA Whiteboard: [MGSEI-Triage+]
Keywords: regressionwindow-wanted
Comment 4•9 years ago
|
||
Autofocus not working on Nexus, not sure this is an L issue or Nexus only issue, ni RD for clarification.
Flags: needinfo?(dflanagan)
Comment 5•9 years ago
|
||
I can partially reproduce this on 2.2. (The 3.0 build for nexus5-l is badly broken today with graphics bugs, so I couldn't test on that.) For me, the camera does successfully auto-focus, but it does so slowly. And I see that the focus circle never turns green, and the white circle just sits there. Touch-to-focus does work and does produce a green circle. I see the same thing in both photo and video modes. Eric: I can't answer your question, myself, but I bet Mike or Andrew can. If this is an L issue then obviously we need to fix it right away. But if it is specific to the nexus-5 camera driver, then we don't need to fix it, right? Mike or Andrew: do you know if this is a nexus5 thing or a Lollipop thing? Has the auto-focus API changed in Lollipop? It seems to me like it is autofocusing, but we're not responding correctly in Gaia, so I'm wondering if something has changed in the way that the camera driver sends notification of its autofocus results.
Flags: needinfo?(mhabicher)
Flags: needinfo?(dflanagan)
Flags: needinfo?(aosmond)
Assignee | ||
Comment 6•9 years ago
|
||
To my knowledge the API hasn't changed. They added a new version of the camera APIs and declared the old ones (i.e. that which we use) as legacy, but they still should work as before. The video shows that in the continuous auto focus case, we get an auto focus moving event from the driver, but we don't receive an auto focus complete event (which would turn the circle green or red depending on the result of the focus operation). Touch to focus generates an auto focus complete event as one would expect. Empirical testing last summer indicated that we always get a completed event for continuous auto focus attempts. However it is not clearly spelled out in the API documentation that the driver *must* supply that event in that circumstance. The documentation does claim that if we call auto focus in continuous-picture/video mode, we can force a complete event without triggering a new focus attempt (i.e. query the state). We took this out because it was problematic (appeared to trigger an explicit auto focus) and we got the event anyways. But it is possible we called it at the wrong time (code shows it was when the focus was moving, but it should be called when the focus *stops* moving). I can provide a patch to test out this theory (but no nexus 5 device handy to test against).
Flags: needinfo?(aosmond)
Comment 7•9 years ago
|
||
Andrew: I can't tell whether you're proposing a gaia patch or a gecko patch. If gaia, I'd be happy to test it. (And Mike presumably has a nexus5 that he should pass on to you...) I think this is urgent only if we have reason to believe that it will affect other L-based builds. If the nexus 5 is the only one we've got then I guess we just may not know. If it is easy to prepare a patch, then maybe it would be a good idea to have it ready in case we need it.
Flags: needinfo?(aosmond)
Comment 8•9 years ago
|
||
Eric: from Andrew's response it sounds like we don't know if this is specific to the Nexus 5 or is a general L problem. Do we have access to any other Lollipop-based devices to try this out on? Or are there partners we can ask to test this for us on their devices?
Flags: needinfo?(echang)
Assignee | ||
Comment 9•9 years ago
|
||
David: Err forgive my obtuseness :). It will have to be a gecko patch as the required signal doesn't make it up to the DOM layer. While I cannot be certain, I would expect it is rooted in the driver. I'll get the nexus 5 from Mike tomorrow or Monday.
Comment 10•9 years ago
|
||
Clear ni per comment 9. Just a note, we have Nexus 5, let me check if I can get another type of device for reference.
Flags: needinfo?(echang)
Assignee | ||
Comment 11•9 years ago
|
||
Mochitests are broken so hopefully the test works. Tested on flame and the nexus 5, and it is a workaround for the problem. If you look at the AOSP camera application, it doesn't actually use the auto focus API in the way described in the documentation. When it gets an auto focus moving event where the focus has stopped moving, it just assumes success. I have done something very similar here where if we don't get the auto focus complete notification within 1 second of receiving the auto focus moving notification (normally it follows immediately), it will generate the auto focus complete notification on the driver's behalf. If that happens three times, it eliminates the 1 second delay. This is actually a concern we had when we originally fixed the focus issues on flame (that we may not get the auto focus complete notification at all for CAF), but all of the devices we tested worked fine. This is a more robust solution (if sadly not ideal).
Assignee: nobody → aosmond
Flags: needinfo?(mhabicher)
Flags: needinfo?(aosmond)
Attachment #8591909 -
Flags: review?(mhabicher)
Assignee | ||
Updated•9 years ago
|
Flags: in-testsuite+
Assignee | ||
Comment 12•9 years ago
|
||
Move this to GonkCameraControl where it belongs.
Attachment #8591909 -
Attachment is obsolete: true
Attachment #8591909 -
Flags: review?(mhabicher)
Attachment #8592285 -
Flags: review?(mhabicher)
Assignee | ||
Comment 13•9 years ago
|
||
Fix how mAutoFocusCompleteExpired doesn't need to be atomic (was an earlier attempt to avoid the lock).
Attachment #8592285 -
Attachment is obsolete: true
Attachment #8592285 -
Flags: review?(mhabicher)
Attachment #8592286 -
Flags: review?(mhabicher)
Comment 14•9 years ago
|
||
Comment on attachment 8592286 [details] [diff] [review] bug1152260.patch, v2.1 Review of attachment 8592286 [details] [diff] [review]: ----------------------------------------------------------------- r+ with the following issues addressed. ::: dom/camera/GonkCameraControl.cpp @@ +59,5 @@ > } \ > } while(0) > > +static const unsigned long kAutoFocusCompleteTimeoutMs = 1000; > +static const int32_t kAutoFocusCompleteThreshold = 3; I think 'kAutoFocusCompleteTimeoutLimit' would be more descriptive. @@ +1361,5 @@ > + } > + > + if (expiredCount >= kAutoFocusCompleteThreshold) { > + OnAutoFocusComplete(true, true); > + } Could this logic be simplfied a little bit? if (mAutoFocusCompleteTimer) { // ... return; } if (!mAutoFocusPending) { expiredCount = mAutoFocusCompleteExpired; } @@ +1406,5 @@ > + --mAutoFocusCompleteExpired; > + } > + > + if (mAutoFocusCompleteExpired >= kAutoFocusCompleteThreshold || > + mAutoFocusCompleteExpired <= -kAutoFocusCompleteThreshold) Are you trying to catch overflows here? ::: dom/camera/GonkCameraControl.h @@ +19,5 @@ > > #include "base/basictypes.h" > #include "nsRefPtrHashtable.h" > #include "mozilla/ReentrantMonitor.h" > +#include "nsITimer.h" Since we only need nsITimer as a pointer in this header, please move this #include to GonkCameraControl.cpp and add a forward reference to 'class nsITimer' below. @@ +203,5 @@ > > + class AutoFocusMovingTimerCallback : public nsITimerCallback > + { > + public: > + NS_DECL_ISUPPORTS Is this class accessed from multiple threads? If so, this should be NS_DECL_ISUPPORTS_THREADSAFE.
Attachment #8592286 -
Flags: review?(mhabicher) → review+
Assignee | ||
Comment 15•9 years ago
|
||
(In reply to Mike Habicher [:mikeh] from comment #14) > Comment on attachment 8592286 [details] [diff] [review] > bug1152260.patch, v2.1 > > Review of attachment 8592286 [details] [diff] [review]: > ----------------------------------------------------------------- > > r+ with the following issues addressed. > > @@ +1406,5 @@ > > + --mAutoFocusCompleteExpired; > > + } > > + > > + if (mAutoFocusCompleteExpired >= kAutoFocusCompleteThreshold || > > + mAutoFocusCompleteExpired <= -kAutoFocusCompleteThreshold) > > Are you trying to catch overflows here? > Yes it was just a bit of paranoia. I switched them to ==. > ::: dom/camera/GonkCameraControl.cpp > @@ +59,5 @@ > > } \ > > } while(0) > > > > +static const unsigned long kAutoFocusCompleteTimeoutMs = 1000; > > +static const int32_t kAutoFocusCompleteThreshold = 3; > > I think 'kAutoFocusCompleteTimeoutLimit' would be more descriptive. > > @@ +1361,5 @@ > > + } > > + > > + if (expiredCount >= kAutoFocusCompleteThreshold) { > > + OnAutoFocusComplete(true, true); > > + } > > Could this logic be simplfied a little bit? > if (mAutoFocusCompleteTimer) { > // ... > return; > } > if (!mAutoFocusPending) { > expiredCount = mAutoFocusCompleteExpired; > } > > ::: dom/camera/GonkCameraControl.h > @@ +19,5 @@ > > > > #include "base/basictypes.h" > > #include "nsRefPtrHashtable.h" > > #include "mozilla/ReentrantMonitor.h" > > +#include "nsITimer.h" > > Since we only need nsITimer as a pointer in this header, please move this > #include to GonkCameraControl.cpp and add a forward reference to 'class > nsITimer' below. > > @@ +203,5 @@ > > > > + class AutoFocusMovingTimerCallback : public nsITimerCallback > > + { > > + public: > > + NS_DECL_ISUPPORTS > > Is this class accessed from multiple threads? If so, this should be > NS_DECL_ISUPPORTS_THREADSAFE. Done.
Attachment #8592286 -
Attachment is obsolete: true
Attachment #8592358 -
Flags: review+
Assignee | ||
Comment 16•9 years ago
|
||
Fix mochitest. try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4e06275715d6
Attachment #8592358 -
Attachment is obsolete: true
Attachment #8593325 -
Flags: review+
Comment 18•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9688ef4c6f16
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S10 (17apr)
Comment 19•9 years ago
|
||
Are we going to nom this a 2.2 blocker to land the fix to 2.2?
Flags: needinfo?(echang)
Assignee | ||
Comment 20•9 years ago
|
||
NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 1111890 User impact if declined: Focus rings on lollipop builds for nexus 5 will remain persistently on screen. Testing completed: Verified focus rings disappear on nexus 5 on 2.2 with patch applied. Risk to taking this patch (and alternatives if risky): Risk is low. If the runtime detection of missing the auto complete event is faulty, we may post an extra auto focus complete event unexpectedly. The gaia camera application ignores this. String or UUID changes made by this patch: None.
Attachment #8594785 -
Flags: review+
Attachment #8594785 -
Flags: approval-mozilla-b2g37?
Assignee | ||
Updated•9 years ago
|
blocking-b2g: --- → 2.2?
Updated•9 years ago
|
blocking-b2g: 2.2? → 2.2+
Updated•9 years ago
|
Attachment #8594785 -
Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
Updated•9 years ago
|
Flags: needinfo?(echang)
Comment 21•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/35742de4b8b6
status-firefox38:
--- → wontfix
status-firefox39:
--- → wontfix
Comment 22•9 years ago
|
||
According to the STR of Comment 0,this bug has been successfully verified on latest Nightly Flame v2.2&3.0 and Nexus 5 v2.2&3.0. Actual results:The camera can focus automatically, and the white focus circle can change to green highlight when camera is focusing successfully. See attachment: verified_v2.2&3.0.mp4 Reproduce rate: 0/6 ----------------------------------------------------------------------------------------- Device: Flame 2.2 (Pass) Build ID 20150504002502 Gaia Revision 8d14361337e608c8cdf165ea5034db5eda23b618 Gaia Date 2015-05-01 18:23:46 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/cb7cb6597c91 Gecko Version 37.0 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150504.041436 Firmware Date Mon May 4 04:14:48 EDT 2015 Bootloader L1TC000118D0 Device: Flame 3.0 (Pass) Build ID 20150504160201 Gaia Revision 70077825aab2c7a79611befb40a5fe7e610d5443 Gaia Date 2015-05-04 18:09:33 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/102d0e9aa9e1 Gecko Version 40.0a1 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150504.193019 Firmware Date Mon May 4 19:30:30 EDT 2015 Bootloader L1TC000118D0 Device: N5_2.2 (Pass) Build ID 20150504002502 Gaia Revision 8d14361337e608c8cdf165ea5034db5eda23b618 Gaia Date 2015-05-01 18:23:46 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/cb7cb6597c91 Gecko Version 37.0 Device Name hammerhead Firmware(Release) 5.1 Firmware(Incremental) eng.cltbld.20150504.041256 Firmware Date Mon May 4 04:13:14 EDT 2015 Bootloader HHZ12f Device: N5_3.0 (Pass) Build ID 20150504160201 Gaia Revision 70077825aab2c7a79611befb40a5fe7e610d5443 Gaia Date 2015-05-04 18:09:33 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/102d0e9aa9e1 Gecko Version 40.0a1 Device Name hammerhead Firmware(Release) 5.1 Firmware(Incremental) eng.cltbld.20150504.193136 Firmware Date Mon May 4 19:31:54 EDT 2015 Bootloader HHZ12f
Comment 23•9 years ago
|
||
Updated•9 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•