Video bugs (#551)

* Use onStopVideo callback for restoring preview

* Fix takeVideoSnapshot without duration

* Add tests

* Add comments

* Remove extra log line

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);

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

cameraview/src/main/java/com/otaliastudios/cameraview/engine/CameraEngine.java

@@ -1188,13 +1188,17 @@ public abstract class CameraEngine implements
             @Override
             public void run() {
                 LOG.i("stopVideo", "executing.", "isTakingVideo?", isTakingVideo());
+                onStopVideo();
+            }
+        });
+    }
+
+    protected void onStopVideo() {
         if (mVideoRecorder != null) {
             mVideoRecorder.stop(false);
             mVideoRecorder = null;
         }
     }
-        });
-    }
 
     @CallSuper
     @Override

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();

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<String, AtomicInteger> 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;
     }
 
     /**

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);
         }
     }
 

cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/TextureMediaEncoder.java

@@ -89,12 +89,12 @@ public class TextureMediaEncoder extends VideoMediaEncoder<TextureConfig> {
 
     @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<TextureConfig> {
             // 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,

cameraview/src/main/java/com/otaliastudios/cameraview/video/encoding/VideoMediaEncoder.java

@@ -53,7 +53,7 @@ abstract class VideoMediaEncoder<C extends VideoConfig> 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.