From bcf9446a175d8f5090a2832296e5e42d71b1a601 Mon Sep 17 00:00:00 2001 From: Igor Murashkin Date: Tue, 9 Sep 2014 11:50:23 -0700 Subject: portability: Fix a bug where long AF callbacks caused ISE timeouts Bug: 17403384 Change-Id: I2f452f79ffb4c0c3327ea5bf7db3f9d26e98ff51 --- .../portability/AndroidCamera2AgentImpl.java | 19 ++++++ .../portability/AndroidCameraAgentImpl.java | 19 ++++++ .../ex/camera2/portability/CameraActions.java | 70 ++++++++++++++++++++++ .../ex/camera2/portability/CameraAgent.java | 39 +++++++++--- .../ex/camera2/portability/CameraStateHolder.java | 5 ++ 5 files changed, 144 insertions(+), 8 deletions(-) diff --git a/camera2/portability/src/com/android/ex/camera2/portability/AndroidCamera2AgentImpl.java b/camera2/portability/src/com/android/ex/camera2/portability/AndroidCamera2AgentImpl.java index 62cb700..88be114 100644 --- a/camera2/portability/src/com/android/ex/camera2/portability/AndroidCamera2AgentImpl.java +++ b/camera2/portability/src/com/android/ex/camera2/portability/AndroidCamera2AgentImpl.java @@ -168,6 +168,7 @@ class AndroidCamera2AgentImpl extends CameraAgent { private CameraOpenCallback mOpenCallback; private int mCameraIndex; private String mCameraId; + private int mCancelAfPending = 0; // Available in CAMERA_UNCONFIGURED state and above: private CameraDevice mCamera; @@ -208,6 +209,7 @@ class AndroidCamera2AgentImpl extends CameraAgent { @Override public void handleMessage(final Message msg) { super.handleMessage(msg); + Log.v(TAG, "handleMessage - action = '" + CameraActions.stringify(msg.what) + "'"); try { switch(msg.what) { case CameraActions.OPEN_CAMERA: @@ -360,6 +362,11 @@ class AndroidCamera2AgentImpl extends CameraAgent { } case CameraActions.AUTO_FOCUS: { + if (mCancelAfPending > 0) { + Log.v(TAG, "handleMessage - Ignored AUTO_FOCUS because there was " + + mCancelAfPending + " pending CANCEL_AUTO_FOCUS messages"); + break; // ignore AF because a CANCEL_AF is queued after this + } // We only support locking the focus while a preview is being displayed. // However, it can be requested multiple times in succession; the effect of // the subsequent invocations is determined by the focus mode defined in the @@ -437,6 +444,9 @@ class AndroidCamera2AgentImpl extends CameraAgent { } case CameraActions.CANCEL_AUTO_FOCUS: { + // Ignore all AFs that were already queued until we see + // a CANCEL_AUTO_FOCUS_FINISH + mCancelAfPending++; // Why would you want to unlock the lens if it isn't already locked? if (mCameraState.getState() < AndroidCamera2StateHolder.CAMERA_PREVIEW_ACTIVE) { @@ -462,6 +472,13 @@ class AndroidCamera2AgentImpl extends CameraAgent { break; } + case CameraActions.CANCEL_AUTO_FOCUS_FINISH: { + // Stop ignoring AUTO_FOCUS messages unless there are additional + // CANCEL_AUTO_FOCUSes that were added + mCancelAfPending--; + break; + } + case CameraActions.SET_AUTO_FOCUS_MOVE_CALLBACK: { mPassiveAfCallback = (CameraAFMoveCallback) msg.obj; break; @@ -628,6 +645,8 @@ class AndroidCamera2AgentImpl extends CameraAgent { sCameraExceptionCallback.onCameraException((RuntimeException) ex); }}); } + } finally { + WaitDoneBundle.unblockSyncWaiters(msg); } } diff --git a/camera2/portability/src/com/android/ex/camera2/portability/AndroidCameraAgentImpl.java b/camera2/portability/src/com/android/ex/camera2/portability/AndroidCameraAgentImpl.java index 949ac62..3704f59 100644 --- a/camera2/portability/src/com/android/ex/camera2/portability/AndroidCameraAgentImpl.java +++ b/camera2/portability/src/com/android/ex/camera2/portability/AndroidCameraAgentImpl.java @@ -242,6 +242,7 @@ class AndroidCameraAgentImpl extends CameraAgent { private Camera mCamera; private int mCameraId; private ParametersCache mParameterCache; + private int mCancelAfPending = 0; private class CaptureCallbacks { public final ShutterCallback mShutter; @@ -323,6 +324,7 @@ class AndroidCameraAgentImpl extends CameraAgent { @Override public void handleMessage(final Message msg) { super.handleMessage(msg); + Log.v(TAG, "handleMessage - action = '" + CameraActions.stringify(msg.what) + "'"); try { switch (msg.what) { case CameraActions.OPEN_CAMERA: { @@ -451,17 +453,32 @@ class AndroidCameraAgentImpl extends CameraAgent { } case CameraActions.AUTO_FOCUS: { + if (mCancelAfPending > 0) { + Log.v(TAG, "handleMessage - Ignored AUTO_FOCUS because there was " + + mCancelAfPending + " pending CANCEL_AUTO_FOCUS messages"); + break; // ignore AF because a CANCEL_AF is queued after this + } mCameraState.setState(AndroidCameraStateHolder.CAMERA_FOCUSING); mCamera.autoFocus((AutoFocusCallback) msg.obj); break; } case CameraActions.CANCEL_AUTO_FOCUS: { + // Ignore all AFs that were already queued until we see + // a CANCEL_AUTO_FOCUS_FINISH + mCancelAfPending++; mCamera.cancelAutoFocus(); mCameraState.setState(AndroidCameraStateHolder.CAMERA_IDLE); break; } + case CameraActions.CANCEL_AUTO_FOCUS_FINISH: { + // Stop ignoring AUTO_FOCUS messages unless there are additional + // CANCEL_AUTO_FOCUSes that were added + mCancelAfPending--; + break; + } + case CameraActions.SET_AUTO_FOCUS_MOVE_CALLBACK: { setAutoFocusMoveCallback(mCamera, msg.obj); break; @@ -585,6 +602,8 @@ class AndroidCameraAgentImpl extends CameraAgent { } }); } + } finally { + WaitDoneBundle.unblockSyncWaiters(msg); } } diff --git a/camera2/portability/src/com/android/ex/camera2/portability/CameraActions.java b/camera2/portability/src/com/android/ex/camera2/portability/CameraActions.java index aae122b..03bfc03 100644 --- a/camera2/portability/src/com/android/ex/camera2/portability/CameraActions.java +++ b/camera2/portability/src/com/android/ex/camera2/portability/CameraActions.java @@ -42,6 +42,7 @@ class CameraActions { public static final int CANCEL_AUTO_FOCUS = 302; public static final int SET_AUTO_FOCUS_MOVE_CALLBACK = 303; public static final int SET_ZOOM_CHANGE_LISTENER = 304; + public static final int CANCEL_AUTO_FOCUS_FINISH = 305; // Face detection public static final int SET_FACE_DETECTION_LISTENER = 461; public static final int START_FACE_DETECTION = 462; @@ -52,4 +53,73 @@ class CameraActions { public static final int SET_DISPLAY_ORIENTATION = 502; // Capture public static final int CAPTURE_PHOTO = 601; + + public static String stringify(int action) { + switch (action) { + case OPEN_CAMERA: + return "OPEN_CAMERA"; + case RELEASE: + return "RELEASE"; + case RECONNECT: + return "RECONNECT"; + case UNLOCK: + return "UNLOCK"; + case LOCK: + return "LOCK"; + case SET_PREVIEW_TEXTURE_ASYNC: + return "SET_PREVIEW_TEXTURE_ASYNC"; + case START_PREVIEW_ASYNC: + return "START_PREVIEW_ASYNC"; + case STOP_PREVIEW: + return "STOP_PREVIEW"; + case SET_PREVIEW_CALLBACK_WITH_BUFFER: + return "SET_PREVIEW_CALLBACK_WITH_BUFFER"; + case ADD_CALLBACK_BUFFER: + return "ADD_CALLBACK_BUFFER"; + case SET_PREVIEW_DISPLAY_ASYNC: + return "SET_PREVIEW_DISPLAY_ASYNC"; + case SET_PREVIEW_CALLBACK: + return "SET_PREVIEW_CALLBACK"; + case SET_ONE_SHOT_PREVIEW_CALLBACK: + return "SET_ONE_SHOT_PREVIEW_CALLBACK"; + case SET_PARAMETERS: + return "SET_PARAMETERS"; + case GET_PARAMETERS: + return "GET_PARAMETERS"; + case REFRESH_PARAMETERS: + return "REFRESH_PARAMETERS"; + case APPLY_SETTINGS: + return "APPLY_SETTINGS"; + case AUTO_FOCUS: + return "AUTO_FOCUS"; + case CANCEL_AUTO_FOCUS: + return "CANCEL_AUTO_FOCUS"; + case SET_AUTO_FOCUS_MOVE_CALLBACK: + return "SET_AUTO_FOCUS_MOVE_CALLBACK"; + case SET_ZOOM_CHANGE_LISTENER: + return "SET_ZOOM_CHANGE_LISTENER"; + case CANCEL_AUTO_FOCUS_FINISH: + return "CANCEL_AUTO_FOCUS_FINISH"; + case SET_FACE_DETECTION_LISTENER: + return "SET_FACE_DETECTION_LISTENER"; + case START_FACE_DETECTION: + return "START_FACE_DETECTION"; + case STOP_FACE_DETECTION: + return "STOP_FACE_DETECTION"; + case SET_ERROR_CALLBACK: + return "SET_ERROR_CALLBACK"; + case ENABLE_SHUTTER_SOUND: + return "ENABLE_SHUTTER_SOUND"; + case SET_DISPLAY_ORIENTATION: + return "SET_DISPLAY_ORIENTATION"; + case CAPTURE_PHOTO: + return "CAPTURE_PHOTO"; + default: + return "UNKNOWN(" + action + ")"; + } + } + + private CameraActions() { + throw new AssertionError(); + } } diff --git a/camera2/portability/src/com/android/ex/camera2/portability/CameraAgent.java b/camera2/portability/src/com/android/ex/camera2/portability/CameraAgent.java index b624b47..475523d 100644 --- a/camera2/portability/src/com/android/ex/camera2/portability/CameraAgent.java +++ b/camera2/portability/src/com/android/ex/camera2/portability/CameraAgent.java @@ -23,6 +23,7 @@ import android.hardware.Camera.OnZoomChangeListener; import android.os.Build; import android.os.Handler; import android.os.Looper; +import android.os.Message; import android.view.SurfaceHolder; import com.android.ex.camera2.portability.debug.Log; @@ -541,8 +542,8 @@ public abstract class CameraAgent { getDispatchThread().runJobSync(new Runnable() { @Override public void run() { - getCameraHandler().sendEmptyMessage(CameraActions.STOP_PREVIEW); - getCameraHandler().post(bundle.mUnlockRunnable); + getCameraHandler().obtainMessage(CameraActions.STOP_PREVIEW, bundle) + .sendToTarget(); }}, bundle.mWaitLock, CAMERA_OPERATION_TIMEOUT_MS, "stop preview"); } @@ -602,14 +603,18 @@ public abstract class CameraAgent { /** * Cancels the auto-focus process. + * + *

This action has the highest priority and will get processed before anything + * else that is pending. Moreover, any pending auto-focuses that haven't yet + * began will also be ignored.

*/ public void cancelAutoFocus() { - getDispatchThread().runJob(new Runnable() { - @Override - public void run() { - getCameraHandler().removeMessages(CameraActions.AUTO_FOCUS); - getCameraHandler().sendEmptyMessage(CameraActions.CANCEL_AUTO_FOCUS); - }}); + // Do not use the dispatch thread since we want to avoid a wait-cycle + // between applySettingsHelper which waits until the state is not FOCUSING. + // cancelAutoFocus should get executed asap, set the state back to idle. + getCameraHandler().sendMessageAtFrontOfQueue( + getCameraHandler().obtainMessage(CameraActions.CANCEL_AUTO_FOCUS)); + getCameraHandler().sendEmptyMessage(CameraActions.CANCEL_AUTO_FOCUS_FINISH); } /** @@ -778,6 +783,9 @@ public abstract class CameraAgent { /** * Applies the settings to the camera device. * + *

If the camera is either focusing or capturing; settings applications + * will be (asynchronously) deferred until those operations complete.

+ * * @param settings The settings to use on the device. * @return Whether the settings can be applied. */ @@ -851,5 +859,20 @@ public abstract class CameraAgent { } }}; } + + /** + * Notify all synchronous waiters waiting on message completion with {@link #mWaitLock}. + * + *

This assumes that the message was sent with {@code this} as the {@code Message#obj}. + * Otherwise the message is ignored.

+ */ + /*package*/ static void unblockSyncWaiters(Message msg) { + if (msg == null) return; + + if (msg.obj instanceof WaitDoneBundle) { + WaitDoneBundle bundle = (WaitDoneBundle)msg.obj; + bundle.mUnlockRunnable.run(); + } + } } } diff --git a/camera2/portability/src/com/android/ex/camera2/portability/CameraStateHolder.java b/camera2/portability/src/com/android/ex/camera2/portability/CameraStateHolder.java index 35ae51c..c8d82b6 100644 --- a/camera2/portability/src/com/android/ex/camera2/portability/CameraStateHolder.java +++ b/camera2/portability/src/com/android/ex/camera2/portability/CameraStateHolder.java @@ -30,6 +30,9 @@ public abstract class CameraStateHolder { } public synchronized void setState(int state) { + if (mState != state) { + Log.v(TAG, "setState - state = " + Integer.toBinaryString(state)); + } mState = state; this.notifyAll(); } @@ -83,6 +86,7 @@ public abstract class CameraStateHolder { * reached. */ public boolean waitForStates(final int states) { + Log.v(TAG, "waitForStates - states = " + Integer.toBinaryString(states)); return waitForCondition(new ConditionChecker() { @Override public boolean success() { @@ -100,6 +104,7 @@ public abstract class CameraStateHolder { * reached. */ public boolean waitToAvoidStates(final int states) { + Log.v(TAG, "waitToAvoidStates - states = " + Integer.toBinaryString(states)); return waitForCondition(new ConditionChecker() { @Override public boolean success() { -- cgit v1.2.3