From 68b993d2c794758dedc2c629079c6fb679d54987 Mon Sep 17 00:00:00 2001 From: Mattia Iavarone Date: Wed, 16 Oct 2019 20:27:23 +0200 Subject: [PATCH] Add synchronization in SnapshotVideoRecorder. Fixes #642 --- .../video/SnapshotVideoRecorder.java | 56 +++++++++++++------ .../cameraview/video/VideoRecorder.java | 42 +++++++++----- 2 files changed, 68 insertions(+), 30 deletions(-) diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/video/SnapshotVideoRecorder.java b/cameraview/src/main/java/com/otaliastudios/cameraview/video/SnapshotVideoRecorder.java index 4d3cde12..be46e182 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/video/SnapshotVideoRecorder.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/video/SnapshotVideoRecorder.java @@ -51,6 +51,7 @@ public class SnapshotVideoRecorder extends VideoRecorder implements RendererFram private static final int STATE_NOT_RECORDING = 1; private MediaEncoderEngine mEncoderEngine; + private final Object mEncoderEngineLock = new Object(); private GlCameraPreview mPreview; private int mCurrentState = STATE_NOT_RECORDING; @@ -82,14 +83,21 @@ public class SnapshotVideoRecorder extends VideoRecorder implements RendererFram dispatchVideoRecordingStart(); } + // Can be called different threads @Override protected void onStop(boolean isCameraShutdown) { if (isCameraShutdown) { - // The renderer callback might never be called. From my tests, it's not. + // The renderer callback might never be called. From my tests, it's not, + // so we can't wait for that callback to stop the encoder engine. LOG.i("Stopping the encoder engine from isCameraShutdown."); mDesiredState = STATE_NOT_RECORDING; mCurrentState = STATE_NOT_RECORDING; - mEncoderEngine.stop(); + synchronized (mEncoderEngineLock) { + if (mEncoderEngine != null) { + mEncoderEngine.stop(); + mEncoderEngine = null; + } + } } else { mDesiredState = STATE_NOT_RECORDING; } @@ -104,12 +112,15 @@ public class SnapshotVideoRecorder extends VideoRecorder implements RendererFram } } + @RendererThread @Override public void onRendererFilterChanged(@NonNull Filter filter) { mCurrentFilter = filter.copy(); - if (mEncoderEngine != null) { - mCurrentFilter.setSize(mResult.size.getWidth(), mResult.size.getHeight()); - mEncoderEngine.notify(TextureMediaEncoder.FILTER_EVENT, mCurrentFilter); + mCurrentFilter.setSize(mResult.size.getWidth(), mResult.size.getHeight()); + synchronized (mEncoderEngineLock) { + if (mEncoderEngine != null) { + mEncoderEngine.notify(TextureMediaEncoder.FILTER_EVENT, mCurrentFilter); + } } } @@ -206,14 +217,16 @@ public class SnapshotVideoRecorder extends VideoRecorder implements RendererFram } // Engine - mEncoderEngine = new MediaEncoderEngine(mResult.file, - videoEncoder, - audioEncoder, - mResult.maxDuration, - mResult.maxSize, - SnapshotVideoRecorder.this); - mEncoderEngine.notify(TextureMediaEncoder.FILTER_EVENT, mCurrentFilter); - mEncoderEngine.start(); + synchronized (mEncoderEngineLock) { + mEncoderEngine = new MediaEncoderEngine(mResult.file, + videoEncoder, + audioEncoder, + mResult.maxDuration, + mResult.maxSize, + SnapshotVideoRecorder.this); + mEncoderEngine.notify(TextureMediaEncoder.FILTER_EVENT, mCurrentFilter); + mEncoderEngine.start(); + } mCurrentState = STATE_RECORDING; } @@ -226,15 +239,22 @@ public class SnapshotVideoRecorder extends VideoRecorder implements RendererFram // NOTE: this is an approximation but it seems to work: frame.timestampMillis = System.currentTimeMillis(); surfaceTexture.getTransformMatrix(frame.transform); - if (mEncoderEngine != null) { // Can happen on teardown. At least it used to. - mEncoderEngine.notify(TextureMediaEncoder.FRAME_EVENT, frame); + synchronized (mEncoderEngineLock) { + if (mEncoderEngine != null) { // Can be null on teardown. + mEncoderEngine.notify(TextureMediaEncoder.FRAME_EVENT, frame); + } } } if (mCurrentState == STATE_RECORDING && mDesiredState == STATE_NOT_RECORDING) { LOG.i("Stopping the encoder engine."); mCurrentState = STATE_NOT_RECORDING; - mEncoderEngine.stop(); + synchronized (mEncoderEngineLock) { + if (mEncoderEngine != null) { + mEncoderEngine.stop(); + mEncoderEngine = null; + } + } } } @@ -280,7 +300,9 @@ public class SnapshotVideoRecorder extends VideoRecorder implements RendererFram mOverlayDrawer.release(); mOverlayDrawer = null; } - mEncoderEngine = null; + synchronized (mEncoderEngineLock) { + mEncoderEngine = null; + } dispatchResult(); } } diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/video/VideoRecorder.java b/cameraview/src/main/java/com/otaliastudios/cameraview/video/VideoRecorder.java index 12513740..62814e46 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/video/VideoRecorder.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/video/VideoRecorder.java @@ -50,6 +50,7 @@ public abstract class VideoRecorder { @SuppressWarnings("WeakerAccess") protected Exception mError; private int mState; + private final Object mStateLock = new Object(); /** * Creates a new video recorder. @@ -66,12 +67,14 @@ public abstract class VideoRecorder { * @param stub the video stub */ public final void start(@NonNull VideoResult.Stub stub) { - if (mState != STATE_IDLE) { - LOG.e("start:", "called twice, or while stopping! " + - "Ignoring. state:", mState); - return; + synchronized (mStateLock) { + if (mState != STATE_IDLE) { + LOG.e("start:", "called twice, or while stopping! " + + "Ignoring. state:", mState); + return; + } + mState = STATE_RECORDING; } - mState = STATE_RECORDING; mResult = stub; onStart(); } @@ -81,12 +84,15 @@ public abstract class VideoRecorder { * @param isCameraShutdown whether this is a full shutdown, camera is being closed */ public final void stop(boolean isCameraShutdown) { - if (mState == STATE_IDLE) { - LOG.e("stop:", "called twice, or called before start! " + - "Ignoring. isCameraShutdown:", isCameraShutdown); - return; + synchronized (mStateLock) { + if (mState == STATE_IDLE) { + // Do not check for STOPPING! See onStop(). + LOG.e("stop:", "called twice, or called before start! " + + "Ignoring. isCameraShutdown:", isCameraShutdown); + return; + } + mState = STATE_STOPPING; } - mState = STATE_STOPPING; onStop(isCameraShutdown); } @@ -96,11 +102,19 @@ public abstract class VideoRecorder { */ public boolean isRecording() { // true if not idle. - return mState != STATE_IDLE; + synchronized (mStateLock) { + return mState != STATE_IDLE; + } } protected abstract void onStart(); + /** + * Should stop recording as fast as possible. This can be called twice because the + * shutdown boolean might be different. + * + * @param isCameraShutdown whether camera is shutting down + */ protected abstract void onStop(boolean isCameraShutdown); /** @@ -108,8 +122,10 @@ public abstract class VideoRecorder { * either with some error (null result) or with the actual stub, filled. */ protected final void dispatchResult() { - if (!isRecording()) return; - mState = STATE_IDLE; + synchronized (mStateLock) { + if (!isRecording()) return; + mState = STATE_IDLE; + } onDispatchResult(); if (mListener != null) { mListener.onVideoResult(mResult, mError);