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

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.media;
 
 import android.annotation.TargetApi;
 import android.content.Context;
 import android.graphics.ImageFormat;
 import android.graphics.Rect;
@@ skipping 66 matching lines @@
                 }
             }
             return ImageFormat.YV12;
         }
     }
 
     private int mExpectedFrameSize;
     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().
     private ReentrantLock mPreviewBufferLock = new ReentrantLock();
     // True when native code has started capture.
     private boolean mIsRunning;
 
     private int[] mGlTextures;
     private SurfaceTexture mSurfaceTexture;
 
@@ skipping 51 matching lines @@
     private class CrPictureCallback implements android.hardware.Camera.PictureCallback {
         @Override
         public void onPictureTaken(byte[] data, android.hardware.Camera camera) {
             synchronized (mPhotoTakenCallbackLock) {
                 if (mPhotoTakenCallbackId != 0) {
                     nativeOnPhotoTaken(
                             mNativeVideoCaptureDeviceAndroid, mPhotoTakenCallbackId, data);
                 }
                 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.

         }
     };
 
     static int getNumberOfCameras() {
         return android.hardware.Camera.getNumberOfCameras();
     }
 
     static int getCaptureApiType(int id) {
         if (VideoCaptureCamera.getCameraInfo(id) == null) {
             return VideoCaptureApi.UNKNOWN;
@@ skipping 567 matching lines @@
         if (mCamera == null || !mIsRunning) {
             Log.e(TAG, "takePhoto: mCamera is null or is not running");
             return false;
         }
 
         // Only one picture can be taken at once.
         synchronized (mPhotoTakenCallbackLock) {
             if (mPhotoTakenCallbackId != 0) return false;
             mPhotoTakenCallbackId = callbackId;
         }
+        mPreviewParameters = getCameraParameters(mCamera);
 
-        android.hardware.Camera.Parameters parameters = getCameraParameters(mCamera);
-        parameters.setRotation(getCameraRotation());
-        final android.hardware.Camera.Size original_size = parameters.getPictureSize();
+        android.hardware.Camera.Parameters photoParameters = getCameraParameters(mCamera);
+        photoParameters.setRotation(getCameraRotation());
 
-        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;
+        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;
+                }
             }
-        }
-        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);
+            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);
+            }
         }
 
         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;
     }
 
     @Override
     public void deallocate() {
         if (mCamera == null) return;
 
         stopCapture();
         try {
             mCamera.setPreviewTexture(null);
@@ skipping 23 matching lines @@
                         getCameraRotation());
             }
         } finally {
             mPreviewBufferLock.unlock();
             if (camera != null) {
                 camera.addCallbackBuffer(data);
             }
         }
     }
 }