Image Capture Android: only restore preview parameters after photo is taken

media/capture/video/android/java/src/org/chromium/media/VideoCaptureCamera.java

Index: media/capture/video/android/java/src/org/chromium/media/VideoCaptureCamera.java
diff --git a/media/capture/video/android/java/src/org/chromium/media/VideoCaptureCamera.java b/media/capture/video/android/java/src/org/chromium/media/VideoCaptureCamera.java
index 6f407ecf9b7577cf8de9dd51bc5c642ae68ddb7f..56a431aa8866f9e5c9d8a7cd96b3938734ed3060 100644
--- a/media/capture/video/android/java/src/org/chromium/media/VideoCaptureCamera.java
+++ b/media/capture/video/android/java/src/org/chromium/media/VideoCaptureCamera.java
@@ -84,11 +84,13 @@ public class VideoCaptureCamera
     private final Object mPhotoTakenCallbackLock = new Object();
 
     // Storage of takePicture() callback Id. There can be one such request in flight at most, and
-    // needs to be exercised either in case of error or sucess.
+    // needs to be exercised either in case of error or success.
     private long mPhotoTakenCallbackId;
-    private int mPhotoWidth;
-    private int mPhotoHeight;
+
+    private int mPhotoWidth = 0;
+    private int mPhotoHeight = 0;
     private android.hardware.Camera.Area mAreaOfInterest;
+    private android.hardware.Camera.Parameters mPreviewParameters = null;

[Reilly Grant (use Gerrit), 2017/04/18 00:32:12]

In Java everything starts zero-initialized.

[mcasas, 2017/04/18 01:09:38]

Oops too much C++ lately. Done.

 
     private android.hardware.Camera mCamera;
     // Lock to mutually exclude execution of OnPreviewFrame() and {start/stop}Capture().
@@ -160,10 +162,17 @@ public class VideoCaptureCamera
                 }
                 mPhotoTakenCallbackId = 0;
             }
-            android.hardware.Camera.Parameters parameters = getCameraParameters(mCamera);
-            parameters.setRotation(0);
-            mCamera.setParameters(parameters);
-            camera.startPreview();
+            try {
+                Log.d(TAG, "|mPreviewParameters|: %s", mPreviewParameters.flatten());
+                camera.setParameters(mPreviewParameters);
+            } catch (RuntimeException ex) {
+                Log.e(TAG, "onPictureTaken, setParameters() " + ex);
+            }
+            try {
+                camera.startPreview();
+            } catch (RuntimeException ex) {
+                Log.e(TAG, "onPictureTaken, startPreview() " + ex);
+            }

[Reilly Grant (use Gerrit), 2017/04/18 00:32:12]

I think we should do these steps before resetting |mPhotoTakenCallbackId| or
else they could conflict with another takePhoto() call.

[mcasas, 2017/04/18 01:09:38]

Done.

         }
     };
 
@@ -751,44 +760,42 @@ public class VideoCaptureCamera
             if (mPhotoTakenCallbackId != 0) return false;
             mPhotoTakenCallbackId = callbackId;
         }
-
-        android.hardware.Camera.Parameters parameters = getCameraParameters(mCamera);
-        parameters.setRotation(getCameraRotation());
-        final android.hardware.Camera.Size original_size = parameters.getPictureSize();
-
-        List<android.hardware.Camera.Size> supportedSizes = parameters.getSupportedPictureSizes();
-        android.hardware.Camera.Size closestSize = null;
-        int minDiff = Integer.MAX_VALUE;
-        for (android.hardware.Camera.Size size : supportedSizes) {
-            final int diff = ((mPhotoWidth > 0) ? Math.abs(size.width - mPhotoWidth) : 0)
-                    + ((mPhotoHeight > 0) ? Math.abs(size.height - mPhotoHeight) : 0);
-            if (diff < minDiff) {
-                minDiff = diff;
-                closestSize = size;
+        mPreviewParameters = getCameraParameters(mCamera);
+
+        android.hardware.Camera.Parameters photoParameters = getCameraParameters(mCamera);
+        photoParameters.setRotation(getCameraRotation());
+
+        if (mPhotoWidth > 0 || mPhotoHeight > 0) {
+            // |mPhotoWidth| and |mPhotoHeight| can only be set immediately before takePicture(),

[Reilly Grant (use Gerrit), 2017/04/18 00:32:12]

This comment is confusing because |mPhotoWidth| and |mPhotoHeight| are set
earlier. The point is that we shouldn't call mCamera.setParameters() until
immediately before takePicture().

[mcasas, 2017/04/18 01:09:38]

Yeah, it was a leftover of an intermediate step. As a matter of
fact this comment doesn't add any relevant information so
I'll remove it -- it's clear that in this method we're composing
|photoParameters| to be passed to |mCamera|.

+            // otherwise they interfere with the preview format.
+            final List<android.hardware.Camera.Size> supportedSizes =
+                    photoParameters.getSupportedPictureSizes();
+            android.hardware.Camera.Size closestSize = null;
+            int minDiff = Integer.MAX_VALUE;
+            for (android.hardware.Camera.Size size : supportedSizes) {
+                final int diff = ((mPhotoWidth > 0) ? Math.abs(size.width - mPhotoWidth) : 0)
+                        + ((mPhotoHeight > 0) ? Math.abs(size.height - mPhotoHeight) : 0);
+                if (diff < minDiff) {
+                    minDiff = diff;
+                    closestSize = size;
+                }
+            }
+            if (minDiff != Integer.MAX_VALUE) {
+                Log.d(TAG, "requested resolution: (%dx%d); matched (%dx%d)", mPhotoWidth,
+                        mPhotoHeight, closestSize.width, closestSize.height);
+                photoParameters.setPictureSize(closestSize.width, closestSize.height);
             }
-        }
-        Log.d(TAG, "requested resolution: (%dx%d)", mPhotoWidth, mPhotoHeight);
-        if (minDiff != Integer.MAX_VALUE) {
-            Log.d(TAG, " matched (%dx%d)", closestSize.width, closestSize.height);
-            parameters.setPictureSize(closestSize.width, closestSize.height);
         }
 
         try {
-            mCamera.setParameters(parameters);
-            mCamera.takePicture(null, null, null, new CrPictureCallback());
+            Log.d(TAG, "|photoParameters|: %s", photoParameters.flatten());
+            mCamera.setParameters(photoParameters);
         } catch (RuntimeException ex) {
-            Log.e(TAG, "takePicture ", ex);
+            Log.e(TAG, "setParameters " + ex);
             return false;
         }
 
-        // Restore original parameters.
-        parameters.setPictureSize(original_size.width, original_size.height);
-        try {
-            mCamera.setParameters(parameters);
-        } catch (RuntimeException ex) {
-            Log.e(TAG, "takePicture ", ex);
-            return false;
-        }
+        mCamera.takePicture(null, null, null, new CrPictureCallback());
         return true;
     }