Closed Bug 986024 Opened 6 years ago Closed 6 years ago

[Camera][Gecko] Expose cancelAutoFocus()

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:1.4+, firefox29 wontfix, firefox30 fixed, firefox31 fixed, b2g-v1.4 fixed, b2g-v2.0 fixed)

RESOLVED FIXED
1.4 S6 (25apr)
blocking-b2g 1.4+
Tracking Status
firefox29 --- wontfix
firefox30 --- fixed
firefox31 --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: mikeh, Assigned: mikeh)

References

Details

Attachments

(2 files, 2 obsolete files)

Expose cancelAutoFocus() so that the Camera app can resume continuous auto-focus, per http://developer.android.com/reference/android/hardware/Camera.Parameters.html#FOCUS_MODE_CONTINUOUS_PICTURE
David, if you're comfortable building your own Gecko, this patch should apply on top of a recent b2g-inbound (and likely m-c) tree.
Attachment #8405625 - Flags: feedback?(dflanagan)
Comment on attachment 8405625 [details] [diff] [review]
Expose resumeContinuousFocus() method to JS/DOM, v1

Dave, would you mind reviewing the non-DOM changes?
Johnny, do you have a minute to look over CameraControl.webidl and DOMCameraControl.* ?
Attachment #8405625 - Flags: review?(jst)
Attachment #8405625 - Flags: review?(dhylands)
Comment on attachment 8405625 [details] [diff] [review]
Expose resumeContinuousFocus() method to JS/DOM, v1

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

Just one minor comment...

::: dom/camera/CameraControlListener.h
@@ +94,5 @@
>      kInStopRecording,
>      kInSetConfiguration,
>      kInStartPreview,
>      kInStopPreview,
> +    kInResumeContinuousFocus,

Shouldn't there also be a corresponding string added to OnError/OnUserError strings?
Attachment #8405625 - Flags: review?(dhylands) → review+
Comment on attachment 8405625 [details] [diff] [review]
Expose resumeContinuousFocus() method to JS/DOM, v1

Works for me. I would love, love, love to get this in 1.4. We've got a deadline today to land stuff before QC starts testing the 1.4 camera.
Attachment #8405625 - Flags: feedback?(dflanagan) → feedback+
Comment on attachment 8405625 [details] [diff] [review]
Expose resumeContinuousFocus() method to JS/DOM, v1

bz, sorry to ask so last-minute, but is there any chance you could last-minute review this one today? It's a pretty formulaic change that just exposes a new camera method to the DOM. No arguments, no callbacks.
Attachment #8405625 - Flags: review?(jst) → review?(bzbarsky)
(In reply to Dave Hylands [:dhylands] (away - back Tue April 15) from comment #3)
>
> Shouldn't there also be a corresponding string added to OnError/OnUserError
> strings?

Yes, thanks! I'll fix that up before landing.
Comment on attachment 8405625 [details] [diff] [review]
Expose resumeContinuousFocus() method to JS/DOM, v1

Hmm.  The AutoFocus API with this change is a bit bizarre, in that it can call the error callback before the call returns of an existing autofocus is in progress.  Is that behavior really as-designed?  That's definitely not the sort of behavior people would expect out of a callback in general...
Comment on attachment 8405625 [details] [diff] [review]
Expose resumeContinuousFocus() method to JS/DOM, v1

r=me if we call the error callback async.
Attachment #8405625 - Flags: review?(bzbarsky) → review+
Assignee: nobody → mhabicher
Incorporate review feedback: fix error strings, make "AlreadyInProgress" callbacks async. Carrying r+en forward.

try-server push: https://tbpl.mozilla.org/?tree=Try&rev=67dd594012d2&showall=1
Attachment #8405625 - Attachment is obsolete: true
Attachment #8406418 - Flags: review+
Attachment #8406418 - Flags: feedback+
Comment on attachment 8405625 [details] [diff] [review]
Expose resumeContinuousFocus() method to JS/DOM, v1

This is a low-risk patch that we'd like to get into v1.4
Attachment #8405625 - Flags: approval-gaia-v1.4+
The linux opt build is fine, but the debug build is failing for a reason I don't understand--looks like a MOZ_CRASH during the -build- stage.

Here's a non-Linux try-server push:
https://tbpl.mozilla.org/?tree=Try&rev=a75408c4f90d&showall=1
Both the linux (comment 11) and linux64 (comment 12) DEBUG builds are failing on this error:

Executing /builds/slave/try-l64-d-00000000000000000000/build/obj-firefox/dist/bin/xpcshell -g /builds/slave/try-l64-d-00000000000000000000/build/obj-firefox/dist/bin/ -a /builds/slave/try-l64-d-00000000000000000000/build/obj-firefox/dist/bin/ -f /builds/slave/try-l64-d-00000000000000000000/build/toolkit/mozapps/installer/precompile_cache.js -e precompile_startupcache("resource://gre/");
  ...
[4896] ###!!! ABORT: file /builds/slave/try-l64-d-00000000000000000000/build/ipc/chromium/src/base/message_loop.h, line 517
Hit MOZ_CRASH() at /builds/slave/try-l64-d-00000000000000000000/build/memory/mozalloc/mozalloc_abort.cpp:30
  ...
mozpack.errors.ErrorMessage: Error: Error while running startup cache precompilation

The Win32 DEBUG build worked just fine, as did the MacOSX variants.
I see "Error while running startup cache precompilation" mentioned in bug 872439 and bug 902221. Not sure if/how they're related to this issue.

The Windows 7 and 8 variants are also building fine.

I've kicked off the linux builds one more time, but it's well past my bedtime now, and my time to try to fix this. Will look again in the morning.
With only some extra logging statements added, this builds:
https://tbpl.mozilla.org/?tree=Try&rev=0823b01425db

Trying again, in case the try server has decided to be sane again:
https://tbpl.mozilla.org/?tree=Try&rev=d404d74a3cc4
https://hg.mozilla.org/mozilla-central/rev/7151a7e50317
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S1 (9may)
Doesn't apply to Aurora. Also not sure if non-blockers are being taken for v1.4 anymore, so you may want to request blocking status on this bug if you want it uplifted.
Flags: needinfo?(mhabicher)
blocking-b2g: --- → 1.4?
Flags: needinfo?(mhabicher)
Version of the patch rebased for aurora, pending blocking-1.4+.
Attachment #8407677 - Flags: review+
blocking-b2g: 1.4? → 1.4+
You need to log in before you can comment on or make changes to this bug.