From eeec7ac222f24a62d25940b5e42e8f68a0ca3b63 Mon Sep 17 00:00:00 2001 From: Mattia Iavarone Date: Wed, 14 Aug 2019 19:24:50 +0200 Subject: [PATCH] Video bugs (#551) * Use onStopVideo callback for restoring preview * Fix takeVideoSnapshot without duration * Add tests * Add comments * Remove extra log line --- .../engine/CameraIntegrationTest.java | 41 +++++++++++++++++-- .../cameraview/engine/Camera2Engine.java | 13 +++--- .../cameraview/engine/CameraEngine.java | 12 ++++-- .../video/encoding/AudioMediaEncoder.java | 4 +- .../video/encoding/MediaEncoder.java | 22 +++++----- .../video/encoding/MediaEncoderEngine.java | 20 ++++----- .../video/encoding/TextureMediaEncoder.java | 6 +-- .../video/encoding/VideoMediaEncoder.java | 2 +- 8 files changed, 81 insertions(+), 39 deletions(-) diff --git a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/CameraIntegrationTest.java b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/CameraIntegrationTest.java index a5b268c7..2bfbdcfa 100644 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/CameraIntegrationTest.java +++ b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/CameraIntegrationTest.java @@ -6,6 +6,7 @@ import android.graphics.Canvas; import android.graphics.PointF; import android.hardware.Camera; import android.os.Build; +import android.os.Handler; import com.otaliastudios.cameraview.BaseTest; import com.otaliastudios.cameraview.CameraException; @@ -526,9 +527,6 @@ public abstract class CameraIntegrationTest extends BaseTest { @Test public void testStartEndVideo() { - // Fails on Travis. Some emulators can't deal with MediaRecorder, - // Error while starting MediaRecorder. java.lang.RuntimeException: start failed. - // as documented. This works locally though. camera.setMode(Mode.VIDEO); openSync(true); takeVideoSync(true, 4000); @@ -543,6 +541,43 @@ public abstract class CameraIntegrationTest extends BaseTest { waitForVideoResult(true); } + @Test + public void testStartEndVideo_withManualStop() { + camera.setMode(Mode.VIDEO); + openSync(true); + takeVideoSync(true); + uiSync(new Runnable() { + @Override + public void run() { + new Handler().postDelayed(new Runnable() { + @Override + public void run() { + camera.stopVideo(); + } + }, 5000); + } + }); + waitForVideoResult(true); + } + + @Test + public void testStartEndVideoSnapshot_withManualStop() { + openSync(true); + takeVideoSnapshotSync(true); + uiSync(new Runnable() { + @Override + public void run() { + new Handler().postDelayed(new Runnable() { + @Override + public void run() { + camera.stopVideo(); + } + }, 5000); + } + }); + waitForVideoResult(true); + } + @Test public void testEndVideo_withoutStarting() { camera.setMode(Mode.VIDEO); diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera2Engine.java b/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera2Engine.java index 2b471bf7..5e928f2d 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera2Engine.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera2Engine.java @@ -726,11 +726,13 @@ public class Camera2Engine extends CameraEngine implements ImageReader.OnImageAv } @Override - public void onVideoResult(@Nullable VideoResult.Stub result, @Nullable Exception exception) { - boolean wasRecordingFullVideo = mVideoRecorder instanceof Full2VideoRecorder; - super.onVideoResult(result, exception); - if (wasRecordingFullVideo) { - // We have to stop all repeating requests and restart them. + protected void onStopVideo() { + // When video ends, we have to restart the repeating request for TEMPLATE_PREVIEW, + // this time without the video recorder surface. We do this before stopping the + // recorder. If we stop first, the camera will try to fill an "abandoned" Surface + // and, on some devices with a poor internal implementation, this crashes. See #549 + boolean isFullVideo = mVideoRecorder instanceof Full2VideoRecorder; + if (isFullVideo) { try { createRepeatingRequestBuilder(CameraDevice.TEMPLATE_PREVIEW); addRepeatingRequestBuilderSurfaces(); @@ -739,6 +741,7 @@ public class Camera2Engine extends CameraEngine implements ImageReader.OnImageAv throw createCameraException(e); } } + super.onStopVideo(); } //endregion diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/CameraEngine.java b/cameraview/src/main/java/com/otaliastudios/cameraview/engine/CameraEngine.java index 8a05f7d3..9e06934a 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/CameraEngine.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/engine/CameraEngine.java @@ -1188,14 +1188,18 @@ public abstract class CameraEngine implements @Override public void run() { LOG.i("stopVideo", "executing.", "isTakingVideo?", isTakingVideo()); - if (mVideoRecorder != null) { - mVideoRecorder.stop(false); - mVideoRecorder = null; - } + onStopVideo(); } }); } + protected void onStopVideo() { + if (mVideoRecorder != null) { + mVideoRecorder.stop(false); + mVideoRecorder = null; + } + } + @CallSuper @Override public void onVideoResult(@Nullable VideoResult.Stub result, @Nullable Exception exception) { diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/AudioMediaEncoder.java b/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/AudioMediaEncoder.java index 9e9dac09..9f03f97e 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/AudioMediaEncoder.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/AudioMediaEncoder.java @@ -62,7 +62,7 @@ public class AudioMediaEncoder extends MediaEncoder { @EncoderThread @Override - protected void onPrepare(@NonNull MediaEncoderEngine.Controller controller, long maxLengthMillis) { + protected void onPrepare(@NonNull MediaEncoderEngine.Controller controller, long maxLengthUs) { final MediaFormat audioFormat = MediaFormat.createAudioFormat( mConfig.mimeType, mConfig.samplingFrequency, @@ -248,7 +248,7 @@ public class AudioMediaEncoder extends MediaEncoder { // See if we reached the max length value. if (!hasReachedMaxLength()) { - boolean didReachMaxLength = (mLastTimeUs - mFirstTimeUs) > getMaxLengthMillis() * 1000L; + boolean didReachMaxLength = (mLastTimeUs - mFirstTimeUs) > getMaxLengthUs(); if (didReachMaxLength && !endOfStream) { LOG.w("read thread - this frame reached the maxLength! deltaUs:", mLastTimeUs - mFirstTimeUs); notifyMaxLengthReached(); diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/MediaEncoder.java b/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/MediaEncoder.java index 12a52743..5cbd4886 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/MediaEncoder.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/MediaEncoder.java @@ -122,7 +122,7 @@ public abstract class MediaEncoder { private MediaCodecBuffers mBuffers; private final Map mPendingEvents = new HashMap<>(); - private long mMaxLengthMillis; + private long mMaxLengthUs; private boolean mMaxLengthReached; private long mStartTimeMillis = 0; // In System.currentTimeMillis() @@ -171,14 +171,14 @@ public abstract class MediaEncoder { * works, we might have {@link #onStop()} or {@link #onStopped()} to be executed before * the previous step has completed. */ - final void prepare(@NonNull final MediaEncoderEngine.Controller controller, final long maxLengthMillis) { + final void prepare(@NonNull final MediaEncoderEngine.Controller controller, final long maxLengthUs) { if (mState >= STATE_PREPARING) { LOG.e(mName, "Wrong state while preparing. Aborting.", mState); return; } mController = controller; mBufferInfo = new MediaCodec.BufferInfo(); - mMaxLengthMillis = maxLengthMillis; + mMaxLengthUs = maxLengthUs; mWorker = WorkerHandler.get(mName); mWorker.getThread().setPriority(Thread.MAX_PRIORITY); LOG.i(mName, "Prepare was called. Posting."); @@ -187,7 +187,7 @@ public abstract class MediaEncoder { public void run() { LOG.i(mName, "Prepare was called. Executing."); setState(STATE_PREPARING); - onPrepare(controller, maxLengthMillis); + onPrepare(controller, maxLengthUs); setState(STATE_PREPARED); } }); @@ -277,10 +277,10 @@ public abstract class MediaEncoder { * At this point subclasses MUST create the {@link #mMediaCodec} object. * * @param controller the muxer controller - * @param maxLengthMillis the maxLength in millis + * @param maxLengthUs the maxLength in microseconds */ @EncoderThread - protected abstract void onPrepare(@NonNull final MediaEncoderEngine.Controller controller, final long maxLengthMillis); + protected abstract void onPrepare(@NonNull MediaEncoderEngine.Controller controller, long maxLengthUs); /** * Start recording. This might be a lightweight operation @@ -465,11 +465,11 @@ public abstract class MediaEncoder { if (!drainAll && !mMaxLengthReached && mFirstTimeUs != Long.MIN_VALUE - && mLastTimeUs - mFirstTimeUs > mMaxLengthMillis * 1000) { + && mLastTimeUs - mFirstTimeUs > mMaxLengthUs) { LOG.w(mName, "DRAINING - Reached maxLength! mLastTimeUs:", mLastTimeUs, "mStartTimeUs:", mFirstTimeUs, "mDeltaUs:", mLastTimeUs - mFirstTimeUs, - "mMaxLengthUs:", mMaxLengthMillis * 1000); + "mMaxLengthUs:", mMaxLengthUs); onMaxLengthReached(); break; } @@ -492,7 +492,7 @@ public abstract class MediaEncoder { protected abstract int getEncodedBitRate(); /** - * Returns the max length setting, in milliseconds, which can be used + * Returns the max length setting, in microseconds, which can be used * to compute the current state and eventually call {@link #notifyMaxLengthReached()}. * This is not a requirement for subclasses - we do this check anyway when draining, * but doing so might be better. @@ -500,8 +500,8 @@ public abstract class MediaEncoder { * @return the max length setting */ @SuppressWarnings("WeakerAccess") - protected long getMaxLengthMillis() { - return mMaxLengthMillis; + protected long getMaxLengthUs() { + return mMaxLengthUs; } /** diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/MediaEncoderEngine.java b/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/MediaEncoderEngine.java index 5164892a..91ae92ba 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/MediaEncoderEngine.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/MediaEncoderEngine.java @@ -146,23 +146,23 @@ public class MediaEncoderEngine { for (MediaEncoder encoder : mEncoders) { bitRate += encoder.getEncodedBitRate(); } - int bytePerSecond = bitRate / 8; - long sizeMaxDuration = (maxSize / bytePerSecond) * 1000L; - - long finalMaxDuration = Long.MAX_VALUE; + int byteRate = bitRate / 8; + long sizeMaxDurationUs = (maxSize / byteRate) * 1000L * 1000L; + long maxDurationUs = maxDuration * 1000L; + long finalMaxDurationUs = Long.MAX_VALUE; if (maxSize > 0 && maxDuration > 0) { - mPossibleEndReason = sizeMaxDuration < maxDuration ? END_BY_MAX_SIZE : END_BY_MAX_DURATION; - finalMaxDuration = Math.min(sizeMaxDuration, maxDuration); + mPossibleEndReason = sizeMaxDurationUs < maxDurationUs ? END_BY_MAX_SIZE : END_BY_MAX_DURATION; + finalMaxDurationUs = Math.min(sizeMaxDurationUs, maxDurationUs); } else if (maxSize > 0) { mPossibleEndReason = END_BY_MAX_SIZE; - finalMaxDuration = sizeMaxDuration; + finalMaxDurationUs = sizeMaxDurationUs; } else if (maxDuration > 0) { mPossibleEndReason = END_BY_MAX_DURATION; - finalMaxDuration = maxDuration; + finalMaxDurationUs = maxDurationUs; } - LOG.w("Computed a max duration of", (finalMaxDuration / 1000F)); + LOG.w("Computed a max duration of", (finalMaxDurationUs / 1000000F)); for (MediaEncoder encoder : mEncoders) { - encoder.prepare(mController, finalMaxDuration); + encoder.prepare(mController, finalMaxDurationUs); } } diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/TextureMediaEncoder.java b/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/TextureMediaEncoder.java index b93328ad..e5b8eb62 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/TextureMediaEncoder.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/TextureMediaEncoder.java @@ -89,12 +89,12 @@ public class TextureMediaEncoder extends VideoMediaEncoder { @EncoderThread @Override - protected void onPrepare(@NonNull MediaEncoderEngine.Controller controller, long maxLengthMillis) { + protected void onPrepare(@NonNull MediaEncoderEngine.Controller controller, long maxLengthUs) { // We rotate the texture using transformRotation. Pass rotation=0 to super so that // no rotation metadata is written into the output file. mTransformRotation = mConfig.rotation; mConfig.rotation = 0; - super.onPrepare(controller, maxLengthMillis); + super.onPrepare(controller, maxLengthUs); mEglCore = new EglCore(mConfig.eglContext, EglCore.FLAG_RECORDABLE); mWindow = new EglWindowSurface(mEglCore, mSurface, true); mWindow.makeCurrent(); @@ -156,7 +156,7 @@ public class TextureMediaEncoder extends VideoMediaEncoder { // Notify we have reached the max length value. if (mFirstTimeUs == Long.MIN_VALUE) mFirstTimeUs = frame.timestampUs(); if (!hasReachedMaxLength()) { - boolean didReachMaxLength = (frame.timestampUs() - mFirstTimeUs) > getMaxLengthMillis() * 1000L; + boolean didReachMaxLength = (frame.timestampUs() - mFirstTimeUs) > getMaxLengthUs(); if (didReachMaxLength) { LOG.w("onEvent -", "frameNumber:", mFrameNumber, diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/VideoMediaEncoder.java b/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/VideoMediaEncoder.java index b542a882..a70e0342 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/VideoMediaEncoder.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/VideoMediaEncoder.java @@ -53,7 +53,7 @@ abstract class VideoMediaEncoder extends MediaEncoder { @EncoderThread @Override - protected void onPrepare(@NonNull MediaEncoderEngine.Controller controller, long maxLengthMillis) { + protected void onPrepare(@NonNull MediaEncoderEngine.Controller controller, long maxLengthUs) { MediaFormat format = MediaFormat.createVideoFormat(mConfig.mimeType, mConfig.width, mConfig.height); // Failing to specify some of these can cause the MediaCodec configure() call to throw an unhelpful exception.