Only set preview size before starting preview

Remove recursive call to startPreview when updating preview size.
Avoid invoking preview size parameter updates for all calls to
updateCameraParametersPreference, and only adjust preview size
immediately before starting preview. Also added info logging
for all call paths into startPreview() to better diagnose
sequence of events in logs if we still see any races or
ordering issues.

Bug: 17809876
Change-Id: I626c913bb154c54aaffb451c24a63c6750553a52
(cherry picked from commit 19597371a18921bb03337e4fa24c28dd2d8f06ed)

1 files changed, 62 insertions, 82 deletions

diff --git a/src/com/android/camera/PhotoModule.java b/src/com/android/camera/PhotoModule.java
index bed2684ed..7544e8bfc 100644
--- a/src/com/android/camera/PhotoModule.java
+++ b/src/com/android/camera/PhotoModule.java
@@ -163,13 +163,6 @@ public class PhotoModule
     private boolean mAwbLockSupported;
     private boolean mContinuousFocusSupported;
 
-    /*
-     * If true, attempts to start the preview will be denied.  This ensures that
-     * we never call startPreview multiple times when making changes to
-     * settings.
-     */
-    private boolean mStartPreviewLock = false;
-
     // The degrees of the device rotated clockwise from its natural orientation.
     private int mOrientation = OrientationEventListener.ORIENTATION_UNKNOWN;
 
@@ -624,6 +617,7 @@ public class PhotoModule
                     Keys.KEY_USER_SELECTED_ASPECT_RATIO);
                 Log.e(TAG, "aspect ratio after setting it to true=" + aspectRatio);
                 if (newAspectRatio != currentAspectRatio) {
+                    Log.i(TAG, "changing aspect ratio from dialog");
                     stopPreview();
                     startPreview();
                     mUI.setRunnableForNextFrame(dialogHandlingFinishedRunnable);
@@ -637,6 +631,7 @@ public class PhotoModule
 
     @Override
     public void onPreviewUIReady() {
+        Log.i(TAG, "onPreviewUIReady");
         startPreview();
     }
 
@@ -1037,6 +1032,7 @@ public class PhotoModule
 
         @Override
         public void onPictureTaken(final byte[] originalJpegData, final CameraProxy camera) {
+            Log.i(TAG, "onPictureTaken");
             mAppController.setShutterEnabled(true);
             if (mPaused) {
                 return;
@@ -1368,7 +1364,7 @@ public class PhotoModule
 
     @Override
     public void onCameraAvailable(CameraProxy cameraProxy) {
-        Log.v(TAG, "onCameraAvailable");
+        Log.i(TAG, "onCameraAvailable");
         if (mPaused) {
             return;
         }
@@ -1385,21 +1381,13 @@ public class PhotoModule
 
         // Do camera parameter dependent initialization.
         mCameraSettings = mCameraDevice.getSettings();
-        // HACK: The call to setCameraParameters(UPDATE_PARAM_ALL) may
-        // eventually recurse back into startPreview().
-        // To avoid calling startPreview() twice, first acquire
-        // mStartPreviewLock.
-        mStartPreviewLock = true;
-        try {
-            setCameraParameters(UPDATE_PARAM_ALL);
-            // Set a listener which updates camera parameters based
-            // on changed settings.
-            SettingsManager settingsManager = mActivity.getSettingsManager();
-            settingsManager.addListener(this);
-            mCameraPreviewParamsReady = true;
-        } finally {
-            mStartPreviewLock = false;
-        }
+
+        setCameraParameters(UPDATE_PARAM_ALL);
+        // Set a listener which updates camera parameters based
+        // on changed settings.
+        SettingsManager settingsManager = mActivity.getSettingsManager();
+        settingsManager.addListener(this);
+        mCameraPreviewParamsReady = true;
 
         startPreview();
 
@@ -1414,6 +1402,7 @@ public class PhotoModule
 
     @Override
     public void onCaptureRetake() {
+        Log.i(TAG, "onCaptureRetake");
         if (mPaused) {
             return;
         }
@@ -1928,6 +1917,7 @@ public class PhotoModule
 
     /** Only called by UI thread. */
     private void setupPreview() {
+        Log.i(TAG, "setupPreview");
         mFocusManager.resetTouchFocus();
         startPreview();
     }
@@ -1962,59 +1952,55 @@ public class PhotoModule
      * The start/stop preview should only run on the UI thread.
      */
     private void startPreview() {
-        // HACK: The call to setCameraParameters(UPDATE_PARAM_ALL) may
-        // eventually recurse back into startPreview().
-        // To avoid calling startPreview() twice, we must acquire
-        // mStartPreviewLock.
-        if (mStartPreviewLock || mCameraDevice == null) {
+        if (mCameraDevice == null) {
+            Log.i(TAG, "attempted to start preview before camera device");
             // do nothing
             return;
         }
-        mStartPreviewLock = true;
-        try {
-            if (!checkPreviewPreconditions()) {
-                return;
-            }
 
-            mCameraDevice.setErrorCallback(mHandler, mErrorCallback);
-            setDisplayOrientation();
+        if (!checkPreviewPreconditions()) {
+            return;
+        }
 
-            if (!mSnapshotOnIdle) {
-                // If the focus mode is continuous autofocus, call cancelAutoFocus
-                // to resume it because it may have been paused by autoFocus call.
-                if (mFocusManager.getFocusMode(mCameraSettings.getCurrentFocusMode()) ==
-                        CameraCapabilities.FocusMode.CONTINUOUS_PICTURE) {
-                    mCameraDevice.cancelAutoFocus();
-                }
-                mFocusManager.setAeAwbLock(false); // Unlock AE and AWB.
+        mCameraDevice.setErrorCallback(mHandler, mErrorCallback);
+        setDisplayOrientation();
+
+        if (!mSnapshotOnIdle) {
+            // If the focus mode is continuous autofocus, call cancelAutoFocus
+            // to resume it because it may have been paused by autoFocus call.
+            if (mFocusManager.getFocusMode(mCameraSettings.getCurrentFocusMode()) ==
+                    CameraCapabilities.FocusMode.CONTINUOUS_PICTURE) {
+                mCameraDevice.cancelAutoFocus();
             }
-            setCameraParameters(UPDATE_PARAM_ALL);
-            mCameraDevice.setPreviewTexture(mActivity.getCameraAppUI().getSurfaceTexture());
-
-            Log.i(TAG, "startPreview");
-            // If we're using API2 in portability layers, don't use startPreviewWithCallback()
-            // b/17576554
-            CameraAgent.CameraStartPreviewCallback startPreviewCallback =
-                new CameraAgent.CameraStartPreviewCallback() {
-                    @Override
-                    public void onPreviewStarted() {
-                        mFocusManager.onPreviewStarted();
-                        PhotoModule.this.onPreviewStarted();
-                        SessionStatsCollector.instance().previewActive(true);
-                        if (mSnapshotOnIdle) {
-                            mHandler.post(mDoSnapRunnable);
-                        }
+            mFocusManager.setAeAwbLock(false); // Unlock AE and AWB.
+        }
+        setCameraParameters(UPDATE_PARAM_ALL);
+
+        updateParametersPictureSize();
+
+        mCameraDevice.setPreviewTexture(mActivity.getCameraAppUI().getSurfaceTexture());
+
+        Log.i(TAG, "startPreview");
+        // If we're using API2 in portability layers, don't use startPreviewWithCallback()
+        // b/17576554
+        CameraAgent.CameraStartPreviewCallback startPreviewCallback =
+            new CameraAgent.CameraStartPreviewCallback() {
+                @Override
+                public void onPreviewStarted() {
+                    mFocusManager.onPreviewStarted();
+                    PhotoModule.this.onPreviewStarted();
+                    SessionStatsCollector.instance().previewActive(true);
+                    if (mSnapshotOnIdle) {
+                        mHandler.post(mDoSnapRunnable);
                     }
-                };
-            if (GservicesHelper.useCamera2ApiThroughPortabilityLayer(mActivity)) {
-                mCameraDevice.startPreview();
-                startPreviewCallback.onPreviewStarted();
-            } else {
-                mCameraDevice.startPreviewWithCallback(new Handler(Looper.getMainLooper()),
-                        startPreviewCallback);
-            }
-        } finally {
-            mStartPreviewLock = false;
+                }
+            };
+        if (GservicesHelper.useCamera2ApiThroughPortabilityLayer(mActivity)) {
+            mCameraDevice.startPreview();
+            startPreviewCallback.onPreviewStarted();
+        } else {
+            mCameraDevice.startPreviewWithCallback(new Handler(Looper.getMainLooper()),
+                    startPreviewCallback);
         }
     }
 
@@ -2130,9 +2116,6 @@ public class PhotoModule
                         CameraCapabilities.FocusMode.CONTINUOUS_PICTURE
         );
 
-        // Set picture size.
-        updateParametersPictureSize();
-
         // Set JPEG quality.
         updateParametersPictureQuality();
 
@@ -2150,8 +2133,14 @@ public class PhotoModule
         }
     }
 
+    /**
+     * This method sets picture size parameters. Size parameters should only be
+     * set when the preview is stopped, and so this method is only invoked in
+     * {@link #startPreview()} just before starting the preview.
+     */
     private void updateParametersPictureSize() {
         if (mCameraDevice == null) {
+            Log.w(TAG, "attempting to set picture size without caemra device");
             return;
         }
 
@@ -2187,16 +2176,7 @@ public class PhotoModule
             Log.v(TAG, "setting preview size. optimal: " + optimalSize + "original: " + original);
             mCameraSettings.setPreviewSize(optimalSize);
 
-            // Zoom related settings will be changed for different preview
-            // sizes, so set and read the parameters to get latest values
-            if (mHandler.getLooper() == Looper.myLooper()) {
-                Log.v(TAG, "matched looper, setting up preview");
-                // On UI thread only, not when camera starts up
-                setupPreview();
-            } else {
-                Log.v(TAG, "no looper match, directly applying settings");
-                mCameraDevice.applySettings(mCameraSettings);
-            }
+            mCameraDevice.applySettings(mCameraSettings);
             mCameraSettings = mCameraDevice.getSettings();
         }