From a20eb06139771a2ecdb906a96a30a4d361341976 Mon Sep 17 00:00:00 2001 From: cristianoccazinsp <48869228+cristianoccazinsp@users.noreply.github.com> Date: Mon, 9 Sep 2019 09:15:41 -0300 Subject: [PATCH] feat(android): Improve Android Camera1 error and concurrency handling. (#2471) These changes include the following: - use atomic boolean for capturing photo flag just like video - add more exception catching and checks - raise error instead of failing silently if can't capture photo - improve error handling here - synchronize stop to avoid race conditions and crashes - delay params updates (surface) if capturing or recording to avoid bugs - do not allow video or photo capture if already doing video or photo --- .../google/android/cameraview/Camera1.java | 185 ++++++++++++------ .../org/reactnative/camera/CameraModule.java | 30 +-- 2 files changed, 140 insertions(+), 75 deletions(-) diff --git a/android/src/main/java/com/google/android/cameraview/Camera1.java b/android/src/main/java/com/google/android/cameraview/Camera1.java index 16799d5d4..b874fd0a9 100644 --- a/android/src/main/java/com/google/android/cameraview/Camera1.java +++ b/android/src/main/java/com/google/android/cameraview/Camera1.java @@ -86,7 +86,7 @@ class Camera1 extends CameraViewImpl implements MediaRecorder.OnInfoListener, private String mVideoPath; - private boolean mIsRecording; + private final AtomicBoolean mIsRecording = new AtomicBoolean(false); private final SizeMap mPreviewSizes = new SizeMap(); @@ -120,6 +120,8 @@ class Camera1 extends CameraViewImpl implements MediaRecorder.OnInfoListener, private boolean mIsScanning; + private boolean mustUpdateSurface; + private SurfaceTexture mPreviewTexture; Camera1(Callback callback, PreviewImpl preview) { @@ -127,11 +129,7 @@ class Camera1 extends CameraViewImpl implements MediaRecorder.OnInfoListener, preview.setCallback(new PreviewImpl.Callback() { @Override public void onSurfaceChanged() { - if (mCamera != null) { - setUpPreview(); - mIsPreviewActive = false; - adjustCameraParameters(); - } + updateSurface(); } @Override @@ -141,6 +139,25 @@ public void onSurfaceDestroyed() { }); } + private void updateSurface(){ + if (mCamera != null) { + + // do not update surface if we are currently capturing + // since it will break capture events/video due to the + // pause preview calls + // capture callbacks will handle it if needed afterwards. + if(!isPictureCaptureInProgress.get() && !mIsRecording.get()){ + mustUpdateSurface = false; + setUpPreview(); + mIsPreviewActive = false; + adjustCameraParameters(); + } + else{ + mustUpdateSurface = true; + } + } + } + @Override boolean start() { chooseCamera(); @@ -160,25 +177,43 @@ boolean start() { @Override void stop() { - mShowingPreview = false; - if (mMediaRecorder != null) { - mMediaRecorder.stop(); - mMediaRecorder.release(); - mMediaRecorder = null; + // make sure no other threads are trying to do this at the same time + // such as another call to stop() from surface destroyed + // or host destroyed. Should avoid crashes with concurrent calls + + synchronized (this) { + if (mMediaRecorder != null) { + try{ + mMediaRecorder.stop(); + } + catch(RuntimeException e){ + Log.e("CAMERA_1::", "mMediaRecorder.stop() failed", e); + } + + try{ + mMediaRecorder.reset(); + mMediaRecorder.release(); + } + catch(RuntimeException e){ + Log.e("CAMERA_1::", "mMediaRecorder.release() failed", e); + } - if (mIsRecording) { - int deviceOrientation = displayOrientationToOrientationEnum(mDeviceOrientation); - mCallback.onVideoRecorded(mVideoPath, mOrientation != Constants.ORIENTATION_AUTO ? mOrientation : deviceOrientation, deviceOrientation); - mIsRecording = false; + mMediaRecorder = null; + + if (mIsRecording.get()) { + int deviceOrientation = displayOrientationToOrientationEnum(mDeviceOrientation); + mCallback.onVideoRecorded(mVideoPath, mOrientation != Constants.ORIENTATION_AUTO ? mOrientation : deviceOrientation, deviceOrientation); + } } - } - if (mCamera != null) { - mCamera.stopPreview(); - mCamera.setPreviewCallback(null); - } + if (mCamera != null) { + mCamera.stopPreview(); + mCamera.setPreviewCallback(null); + } + mShowingPreview = false; - releaseCamera(); + releaseCamera(); + } } // Suppresses Camera#setPreviewTexture @@ -493,61 +528,77 @@ int displayOrientationToOrientationEnum(int rotation) { } void takePictureInternal(final ReadableMap options) { - if (!isPictureCaptureInProgress.getAndSet(true)) { + // if not capturing already, atomically set it to true + if (!mIsRecording.get() && isPictureCaptureInProgress.compareAndSet(false, true)) { - if (options.hasKey("orientation") && options.getInt("orientation") != Constants.ORIENTATION_AUTO) { - mOrientation = options.getInt("orientation"); - int rotation = orientationEnumToRotation(mOrientation); - mCameraParameters.setRotation(calcCameraRotation(rotation)); - try{ - mCamera.setParameters(mCameraParameters); - } - catch(RuntimeException e ) { - Log.e("CAMERA_1::", "setParameters failed", e); + try{ + if (options.hasKey("orientation") && options.getInt("orientation") != Constants.ORIENTATION_AUTO) { + mOrientation = options.getInt("orientation"); + int rotation = orientationEnumToRotation(mOrientation); + mCameraParameters.setRotation(calcCameraRotation(rotation)); + try{ + mCamera.setParameters(mCameraParameters); + } + catch(RuntimeException e ) { + Log.e("CAMERA_1::", "setParameters failed", e); + } } - } - mCamera.takePicture(null, null, null, new Camera.PictureCallback() { - @Override - public void onPictureTaken(byte[] data, Camera camera) { - isPictureCaptureInProgress.set(false); + mCamera.takePicture(null, null, null, new Camera.PictureCallback() { + @Override + public void onPictureTaken(byte[] data, Camera camera) { + isPictureCaptureInProgress.set(false); + + // this shouldn't be needed and messes up autoFocusPointOfInterest + // camera.cancelAutoFocus(); + + if (options.hasKey("pauseAfterCapture") && !options.getBoolean("pauseAfterCapture")) { + camera.startPreview(); + mIsPreviewActive = true; + if (mIsScanning) { + camera.setPreviewCallback(Camera1.this); + } + } else { + camera.stopPreview(); + mIsPreviewActive = false; + camera.setPreviewCallback(null); + } - // this shouldn't be needed and messes up autoFocusPointOfInterest - // camera.cancelAutoFocus(); + mOrientation = Constants.ORIENTATION_AUTO; + mCallback.onPictureTaken(data, displayOrientationToOrientationEnum(mDeviceOrientation)); - if (options.hasKey("pauseAfterCapture") && !options.getBoolean("pauseAfterCapture")) { - camera.startPreview(); - mIsPreviewActive = true; - if (mIsScanning) { - camera.setPreviewCallback(Camera1.this); + if(mustUpdateSurface){ + updateSurface(); } - } else { - camera.stopPreview(); - mIsPreviewActive = false; - camera.setPreviewCallback(null); } - - mOrientation = Constants.ORIENTATION_AUTO; - mCallback.onPictureTaken(data, displayOrientationToOrientationEnum(mDeviceOrientation)); - } - }); + }); + } + catch(Exception e){ + isPictureCaptureInProgress.set(false); + throw e; + } + } + else{ + throw new IllegalStateException("Camera capture failed. Camera is already capturing."); } } @Override boolean record(String path, int maxDuration, int maxFileSize, boolean recordAudio, CamcorderProfile profile, int orientation) { - if (!mIsRecording) { + + // make sure compareAndSet is last because we are setting it + if (!isPictureCaptureInProgress.get() && mIsRecording.compareAndSet(false, true)) { if (orientation != Constants.ORIENTATION_AUTO) { mOrientation = orientation; } - setUpMediaRecorder(path, maxDuration, maxFileSize, recordAudio, profile); try { + setUpMediaRecorder(path, maxDuration, maxFileSize, recordAudio, profile); mMediaRecorder.prepare(); mMediaRecorder.start(); - mIsRecording = true; return true; - } catch (IOException e) { - e.printStackTrace(); + } catch (Exception e) { + mIsRecording.set(false); + Log.e("CAMERA_1::", "Record start failed", e); return false; } } @@ -556,11 +607,14 @@ boolean record(String path, int maxDuration, int maxFileSize, boolean recordAudi @Override void stopRecording() { - if (mIsRecording) { + if (mIsRecording.compareAndSet(true, false)) { stopMediaRecorder(); if (mCamera != null) { mCamera.lock(); } + if(mustUpdateSurface){ + updateSurface(); + } } } @@ -594,7 +648,7 @@ void setDeviceOrientation(int deviceOrientation) { return; } mDeviceOrientation = deviceOrientation; - if (isCameraOpened() && mOrientation == Constants.ORIENTATION_AUTO && !mIsRecording) { + if (isCameraOpened() && mOrientation == Constants.ORIENTATION_AUTO && !mIsRecording.get() && !isPictureCaptureInProgress.get()) { mCameraParameters.setRotation(calcCameraRotation(deviceOrientation)); try{ mCamera.setParameters(mCameraParameters); @@ -602,7 +656,7 @@ void setDeviceOrientation(int deviceOrientation) { catch(RuntimeException e ) { Log.e("CAMERA_1::", "setParameters failed", e); } - } + } } @Override @@ -763,6 +817,10 @@ private void releaseCamera() { mCamera = null; mPictureSize = null; mCallback.onCameraClosed(); + + // reset these flags + isPictureCaptureInProgress.set(false); + mIsRecording.set(false); } } @@ -1074,6 +1132,7 @@ public void onPreviewFrame(byte[] data, Camera camera) { } private void setUpMediaRecorder(String path, int maxDuration, int maxFileSize, boolean recordAudio, CamcorderProfile profile) { + mMediaRecorder = new MediaRecorder(); mCamera.unlock(); @@ -1107,15 +1166,16 @@ private void setUpMediaRecorder(String path, int maxDuration, int maxFileSize, b mMediaRecorder.setOnInfoListener(this); mMediaRecorder.setOnErrorListener(this); + } private void stopMediaRecorder() { - mIsRecording = false; + if (mMediaRecorder != null) { try { mMediaRecorder.stop(); } catch (RuntimeException ex) { - ex.printStackTrace(); + Log.e("CAMERA_1::", "stopMediaRecorder failed", ex); } mMediaRecorder.reset(); mMediaRecorder.release(); @@ -1130,6 +1190,7 @@ private void stopMediaRecorder() { mCallback.onVideoRecorded(mVideoPath, mOrientation != Constants.ORIENTATION_AUTO ? mOrientation : deviceOrientation, deviceOrientation); mVideoPath = null; + } private void setCamcorderProfile(CamcorderProfile profile, boolean recordAudio) { diff --git a/android/src/main/java/org/reactnative/camera/CameraModule.java b/android/src/main/java/org/reactnative/camera/CameraModule.java index 58cff29a1..891f73bed 100644 --- a/android/src/main/java/org/reactnative/camera/CameraModule.java +++ b/android/src/main/java/org/reactnative/camera/CameraModule.java @@ -208,7 +208,7 @@ private Map getBarCodeConstants() { } }); } - + @ReactMethod public void pausePreview(final int viewTag) { final ReactApplicationContext context = getReactApplicationContext(); @@ -217,7 +217,7 @@ public void pausePreview(final int viewTag) { @Override public void execute(NativeViewHierarchyManager nativeViewHierarchyManager) { final RNCameraView cameraView; - + try { cameraView = (RNCameraView) nativeViewHierarchyManager.resolveView(viewTag); if (cameraView.isCameraOpened()) { @@ -229,7 +229,7 @@ public void execute(NativeViewHierarchyManager nativeViewHierarchyManager) { } }); } - + @ReactMethod public void resumePreview(final int viewTag) { final ReactApplicationContext context = getReactApplicationContext(); @@ -238,7 +238,7 @@ public void resumePreview(final int viewTag) { @Override public void execute(NativeViewHierarchyManager nativeViewHierarchyManager) { final RNCameraView cameraView; - + try { cameraView = (RNCameraView) nativeViewHierarchyManager.resolveView(viewTag); if (cameraView.isCameraOpened()) { @@ -261,14 +261,18 @@ public void takePicture(final ReadableMap options, final int viewTag, final Prom public void execute(NativeViewHierarchyManager nativeViewHierarchyManager) { RNCameraView cameraView = (RNCameraView) nativeViewHierarchyManager.resolveView(viewTag); try { - if (cameraView.isCameraOpened()) { - cameraView.takePicture(options, promise, cacheDirectory); - } else { - promise.reject("E_CAMERA_UNAVAILABLE", "Camera is not running"); - } - } catch (Exception e) { - promise.reject("E_CAMERA_BAD_VIEWTAG", "takePictureAsync: Expected a Camera component"); - } + if (cameraView.isCameraOpened()) { + cameraView.takePicture(options, promise, cacheDirectory); + } else { + promise.reject("E_CAMERA_UNAVAILABLE", "Camera is not running"); + } + } + catch(IllegalStateException e){ + promise.reject("E_CAMERA_UNAVAILABLE", e.getMessage()); + } + catch (Exception e) { + promise.reject("E_CAMERA_BAD_VIEWTAG", e.getMessage()); + } } }); } @@ -353,7 +357,7 @@ public void getAvailablePictureSizes(final String ratio, final int viewTag, fina @Override public void execute(NativeViewHierarchyManager nativeViewHierarchyManager) { final RNCameraView cameraView; - + try { cameraView = (RNCameraView) nativeViewHierarchyManager.resolveView(viewTag); WritableArray result = Arguments.createArray();