am 022ce61e: Synchronize CaptureModule camera open/close operations

* commit '022ce61e1c4f338a79585e8656ab57990ecd32c1':
  Synchronize CaptureModule camera open/close operations

5 files changed, 161 insertions, 35 deletions

diff --git a/src/com/android/camera/CaptureModule.java b/src/com/android/camera/CaptureModule.java
index af1fb06fa..9b2bb7bcb 100644
--- a/src/com/android/camera/CaptureModule.java
+++ b/src/com/android/camera/CaptureModule.java
@@ -30,6 +30,7 @@ import android.hardware.SensorManager;
 import android.location.Location;
 import android.net.Uri;
 import android.os.Handler;
+import android.os.HandlerThread;
 import android.os.SystemClock;
 import android.provider.MediaStore;
 import android.view.KeyEvent;
@@ -74,6 +75,8 @@ import com.android.camera2.R;
 import com.android.ex.camera2.portability.CameraAgent.CameraProxy;
 
 import java.io.File;
+import java.util.concurrent.Semaphore;
+import java.util.concurrent.TimeUnit;
 
 /**
  * New Capture module that is made to support photo and video capture on top of
@@ -145,6 +148,9 @@ public class CaptureModule extends CameraModule
      */
     private static final int ON_RESUME_TASKS_DELAY_MSEC = 20;
 
+    /** Timeout for camera open/close operations. */
+    private static final int CAMERA_OPEN_CLOSE_TIMEOUT_MILLIS = 2500;
+
     /** System Properties switch to enable debugging focus UI. */
     private static final boolean CAPTURE_DEBUG_UI = DebugPropertyHelper.showCaptureDebugUI();
 
@@ -170,8 +176,10 @@ public class CaptureModule extends CameraModule
     private CaptureModuleUI mUI;
     /** The camera manager used to open cameras. */
     private OneCameraManager mCameraManager;
-    /** The currently opened camera device. */
+    /** The currently opened camera device, or null if the camera is closed. */
     private OneCamera mCamera;
+    /** Held when opening or closing the camera. */
+    private final Semaphore mCameraOpenCloseLock = new Semaphore(1);
     /** The direction the currently opened camera is facing to. */
     private Facing mCameraFacing = Facing.BACK;
     /** Whether HDR is currently enabled. */
@@ -247,6 +255,8 @@ public class CaptureModule extends CameraModule
 
     /** Main thread handler. */
     private Handler mMainHandler;
+    /** Handler thread for camera-related operations. */
+    private Handler mCameraHandler;
 
     /** Current display rotation in degrees. */
     private int mDisplayRotation;
@@ -298,6 +308,9 @@ public class CaptureModule extends CameraModule
         Log.d(TAG, "init");
         mIsResumeFromLockScreen = isResumeFromLockscreen(activity);
         mMainHandler = new Handler(activity.getMainLooper());
+        HandlerThread thread = new HandlerThread("CaptureModule.mCameraHandler");
+        thread.start();
+        mCameraHandler = new Handler(thread.getLooper());
         mCameraManager = mAppController.getCameraManager();
         mLocationManager = mAppController.getLocationManager();
         mDisplayRotation = CameraUtil.getDisplayRotation(mContext);
@@ -588,6 +601,7 @@ public class CaptureModule extends CameraModule
     @Override
     public void destroy() {
         mCountdownSoundPlayer.release();
+        mCameraHandler.getLooper().quitSafely();
     }
 
     @Override
@@ -725,6 +739,11 @@ public class CaptureModule extends CameraModule
     // PhotoModule uses FocusOverlayManager which uses API1/portability
     // logic and coordinates.
     private void triggerFocusAtScreenCoord(int x, int y) {
+        if (mCamera == null) {
+            // If we receive this after the camera is closed, do nothing.
+            return;
+        }
+
         mTapToFocusWaitForActiveScan = true;
         // Show UI immediately even though scan has not started yet.
         float minEdge = Math.min(mPreviewArea.width(), mPreviewArea.height());
@@ -1205,16 +1224,37 @@ public class CaptureModule extends CameraModule
     private void openCameraAndStartPreview() {
         // Only enable HDR on the back camera
         boolean useHdr = mHdrEnabled && mCameraFacing == Facing.BACK;
+
+        try {
+            // TODO Given the current design, we cannot guarantee that one of
+            // CaptureReadyCallback.onSetupFailed or onReadyForCapture will
+            // be called (see below), so it's possible that
+            // mCameraOpenCloseLock.release() is never called under extremely
+            // rare cases.  If we leak the lock, this timeout ensures that we at
+            // least crash so we don't deadlock the app.
+            if (!mCameraOpenCloseLock.tryAcquire(CAMERA_OPEN_CLOSE_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS)) {
+                throw new RuntimeException("Time out waiting to acquire camera-open lock.");
+            }
+        } catch (InterruptedException e) {
+            throw new RuntimeException("Interrupted while waiting to acquire camera-open lock.", e);
+        }
         mCameraManager.open(mCameraFacing, useHdr, getPictureSizeFromSettings(),
                 new OpenCallback() {
                     @Override
                     public void onFailure() {
                         Log.e(TAG, "Could not open camera.");
                         mCamera = null;
+                        mCameraOpenCloseLock.release();
                         mAppController.showErrorAndFinish(R.string.cannot_connect_camera);
                     }
 
                     @Override
+                    public void onCameraClosed() {
+                        mCamera = null;
+                        mCameraOpenCloseLock.release();
+                    }
+
+                    @Override
                     public void onCameraOpened(final OneCamera camera) {
                         Log.d(TAG, "onCameraOpened: " + camera);
                         mCamera = camera;
@@ -1232,32 +1272,67 @@ public class CaptureModule extends CameraModule
                                 new CaptureReadyCallback() {
                                     @Override
                                     public void onSetupFailed() {
+                                        // We must release this lock here, before posting
+                                        // to the main handler since we may be blocked
+                                        // in pause(), getting ready to close the camera.
+                                        mCameraOpenCloseLock.release();
                                         Log.e(TAG, "Could not set up preview.");
-                                        mCamera.close(null);
-                                        mCamera = null;
-                                        // TODO: Show an error message and exit.
+                                        mMainHandler.post(new Runnable() {
+                                           @Override
+                                           public void run() {
+                                               if (mCamera == null) {
+                                                   Log.d(TAG, "Camera closed, aborting.");
+                                                   return;
+                                               }
+                                               mCamera.close(null);
+                                               mCamera = null;
+                                               // TODO: Show an error message and exit.
+                                           }
+                                        });
                                     }
 
                                     @Override
                                     public void onReadyForCapture() {
-                                        Log.d(TAG, "Ready for capture.");
-                                        onPreviewStarted();
-                                        // Enable zooming after preview has
-                                        // started.
-                                        mUI.initializeZoom(mCamera.getMaxZoom());
-                                        mCamera.setFocusStateListener(CaptureModule.this);
-                                        mCamera.setReadyStateChangedListener(CaptureModule.this);
+                                        // We must release this lock here, before posting
+                                        // to the main handler since we may be blocked
+                                        // in pause(), getting ready to close the camera.
+                                        mCameraOpenCloseLock.release();
+                                        mMainHandler.post(new Runnable() {
+                                           @Override
+                                           public void run() {
+                                               Log.d(TAG, "Ready for capture.");
+                                               if (mCamera == null) {
+                                                   Log.d(TAG, "Camera closed, aborting.");
+                                                   return;
+                                               }
+                                               onPreviewStarted();
+                                               // Enable zooming after preview has
+                                               // started.
+                                               mUI.initializeZoom(mCamera.getMaxZoom());
+                                               mCamera.setFocusStateListener(CaptureModule.this);
+                                               mCamera.setReadyStateChangedListener(CaptureModule.this);
+                                           }
+                                        });
                                     }
                                 });
                     }
-                });
+                }, mCameraHandler);
     }
 
     private void closeCamera() {
-        if (mCamera != null) {
-            mCamera.setFocusStateListener(null);
-            mCamera.close(null);
-            mCamera = null;
+        try {
+            mCameraOpenCloseLock.acquire();
+        } catch(InterruptedException e) {
+            throw new RuntimeException("Interrupted while waiting to acquire camera-open lock.", e);
+        }
+        try {
+            if (mCamera != null) {
+                mCamera.setFocusStateListener(null);
+                mCamera.close(null);
+                mCamera = null;
+            }
+        } finally {
+            mCameraOpenCloseLock.release();
         }
     }
 
diff --git a/src/com/android/camera/one/OneCamera.java b/src/com/android/camera/one/OneCamera.java
index a22c889b2..593dea4cd 100644
--- a/src/com/android/camera/one/OneCamera.java
+++ b/src/com/android/camera/one/OneCamera.java
@@ -98,6 +98,12 @@ public interface OneCamera {
          * Called if opening the camera failed.
          */
         public void onFailure();
+
+        /**
+         * Called if the camera is closed or disconnected while attempting to
+         * open.
+         */
+        public void onCameraClosed();
     }
 
     /**
diff --git a/src/com/android/camera/one/OneCameraManager.java b/src/com/android/camera/one/OneCameraManager.java
index 4c78f3576..ee505ae34 100644
--- a/src/com/android/camera/one/OneCameraManager.java
+++ b/src/com/android/camera/one/OneCameraManager.java
@@ -21,6 +21,7 @@ import android.hardware.camera2.CameraAccessException;
 import android.hardware.camera2.CameraCharacteristics;
 import android.hardware.camera2.CameraManager;
 import android.os.Build;
+import android.os.Handler;
 import android.util.DisplayMetrics;
 import android.view.WindowManager;
 
@@ -42,6 +43,9 @@ public abstract class OneCameraManager {
      * Attempts to open the camera facing the given direction with the given
      * capture size.
      *
+     * Exactly one call will always be made to a single method in the provided
+     * {@link OpenCallback}.
+     *
      * @param facing which camera to open. The first camera found in the given
      *            direction will be opened.
      * @param enableHdr if an HDR feature exists, open a camera that supports it
@@ -49,9 +53,10 @@ public abstract class OneCameraManager {
      *            sizes.
      * @param callback this listener is called when the camera was opened or
      *            when it failed to open.
+     * @param handler the handler on which callback methods are invoked.
      */
     public abstract void open(Facing facing, boolean enableHdr, Size captureSize,
-            OpenCallback callback);
+            OpenCallback callback, Handler handler);
 
     /**
      * Returns whether the device has a camera facing the given direction.
diff --git a/src/com/android/camera/one/v1/OneCameraManagerImpl.java b/src/com/android/camera/one/v1/OneCameraManagerImpl.java
index 0be46f261..839ca5aa2 100644
--- a/src/com/android/camera/one/v1/OneCameraManagerImpl.java
+++ b/src/com/android/camera/one/v1/OneCameraManagerImpl.java
@@ -16,6 +16,8 @@
 
 package com.android.camera.one.v1;
 
+import android.os.Handler;
+
 import com.android.camera.one.OneCamera.Facing;
 import com.android.camera.one.OneCamera.OpenCallback;
 import com.android.camera.one.OneCameraManager;
@@ -28,7 +30,7 @@ import com.android.camera.util.Size;
 public class OneCameraManagerImpl extends OneCameraManager {
 
     @Override
-    public void open(Facing facing, boolean enableHdr, Size pictureSize, OpenCallback callback) {
+    public void open(Facing facing, boolean enableHdr, Size pictureSize, OpenCallback callback, Handler handler) {
         throw new RuntimeException("Not implemented yet.");
     }
 
diff --git a/src/com/android/camera/one/v2/OneCameraManagerImpl.java b/src/com/android/camera/one/v2/OneCameraManagerImpl.java
index 0f2247704..249e4ecef 100644
--- a/src/com/android/camera/one/v2/OneCameraManagerImpl.java
+++ b/src/com/android/camera/one/v2/OneCameraManagerImpl.java
@@ -21,6 +21,7 @@ import android.hardware.camera2.CameraAccessException;
 import android.hardware.camera2.CameraCharacteristics;
 import android.hardware.camera2.CameraDevice;
 import android.hardware.camera2.CameraManager;
+import android.os.Handler;
 import android.util.DisplayMetrics;
 
 import com.android.camera.SoundPlayer;
@@ -62,44 +63,81 @@ public class OneCameraManagerImpl extends OneCameraManager {
 
     @Override
     public void open(Facing facing, final boolean useHdr, final Size pictureSize,
-            final OpenCallback openCallback) {
+            final OpenCallback openCallback, Handler handler) {
         try {
             final String cameraId = getCameraId(facing);
             Log.i(TAG, "Opening Camera ID " + cameraId);
             mCameraManager.openCamera(cameraId, new CameraDevice.StateCallback() {
+                // We may get multiple calls to StateCallback, but only the
+                // first callback indicates the status of the camera-opening
+                // operation.  For example, we may receive onOpened() and later
+                // onClosed(), but only the first should be relayed to
+                // openCallback.
+                private boolean isFirstCallback = true;
 
                 @Override
                 public void onDisconnected(CameraDevice device) {
-                    // TODO, Re-route through the camera instance?
+                    if (isFirstCallback) {
+                        isFirstCallback = false;
+                        // If the camera is disconnected before it is opened
+                        // then we must call close.
+                        device.close();
+                        openCallback.onCameraClosed();
+                    }
+                }
+
+                @Override
+                public void onClosed(CameraDevice device) {
+                    if (isFirstCallback) {
+                        isFirstCallback = false;
+                        openCallback.onCameraClosed();
+                    }
                 }
 
                 @Override
                 public void onError(CameraDevice device, int error) {
-                    openCallback.onFailure();
+                    if (isFirstCallback) {
+                        isFirstCallback = false;
+                        device.close();
+                        openCallback.onFailure();
+                    }
                 }
 
                 @Override
                 public void onOpened(CameraDevice device) {
-                    try {
-                        CameraCharacteristics characteristics = mCameraManager
-                                .getCameraCharacteristics(device.getId());
-                        // TODO: Set boolean based on whether HDR+ is enabled.
-                        OneCamera oneCamera = OneCameraCreator.create(mContext, useHdr, device,
-                                characteristics, pictureSize, mMaxMemoryMB, mDisplayMetrics,
-                                mSoundPlayer);
-                        openCallback.onCameraOpened(oneCamera);
-                    } catch (CameraAccessException e) {
-                        Log.d(TAG, "Could not get camera characteristics");
-                        openCallback.onFailure();
+                    if (isFirstCallback) {
+                        isFirstCallback = false;
+                        try {
+                            CameraCharacteristics characteristics = mCameraManager
+                                    .getCameraCharacteristics(device.getId());
+                            // TODO: Set boolean based on whether HDR+ is enabled.
+                            OneCamera oneCamera = OneCameraCreator.create(mContext, useHdr, device,
+                                    characteristics, pictureSize, mMaxMemoryMB, mDisplayMetrics,
+                                    mSoundPlayer);
+                            openCallback.onCameraOpened(oneCamera);
+                        } catch (CameraAccessException e) {
+                            Log.d(TAG, "Could not get camera characteristics");
+                            openCallback.onFailure();
+                        }
                     }
                 }
-            }, null);
+            }, handler);
         } catch (CameraAccessException ex) {
             Log.e(TAG, "Could not open camera. " + ex.getMessage());
-            openCallback.onFailure();
+            handler.post(new Runnable() {
+                @Override
+                public void run() {
+                    openCallback.onFailure();
+                }
+            });
         } catch (UnsupportedOperationException ex) {
             Log.e(TAG, "Could not open camera. " + ex.getMessage());
-            openCallback.onFailure();
+            handler.post(new Runnable() {
+                @Override
+                public void run() {
+                    openCallback.onFailure();
+                }
+            });
         }
     }