Fix tests

cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/CameraIntegration1Test.java

@@ -3,6 +3,7 @@ package com.otaliastudios.cameraview.engine;
 import com.otaliastudios.cameraview.controls.Engine;
 
 import org.junit.Ignore;
+import org.junit.Test;
 import org.junit.runner.RunWith;
 
 import androidx.annotation.NonNull;
@@ -17,8 +18,8 @@ import androidx.test.filters.LargeTest;
  */
 @RunWith(AndroidJUnit4.class)
 @LargeTest
-@Ignore
-public class Camera2IntegrationTest extends CameraIntegrationTest {
+// @Ignore
+public class CameraIntegration1Test extends CameraIntegrationTest {
 
     @NonNull
     @Override

cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/CameraIntegration2Test.java

@@ -17,8 +17,8 @@ import androidx.test.filters.LargeTest;
  */
 @RunWith(AndroidJUnit4.class)
 @LargeTest
-@Ignore
-public class Camera1IntegrationTest extends CameraIntegrationTest {
+// @Ignore
+public class CameraIntegration2Test extends CameraIntegrationTest {
 
     @NonNull
     @Override

cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/CameraIntegrationTest.java

@@ -92,6 +92,8 @@ public abstract class CameraIntegrationTest extends BaseTest {
                     }
                 };
                 listener = mock(CameraListener.class);
+                camera.setExperimental(true);
+                camera.setEngine(getEngine());
                 camera.addCameraListener(listener);
                 rule.getActivity().inflate(camera);
 
@@ -117,8 +119,10 @@ public abstract class CameraIntegrationTest extends BaseTest {
     }
 
     private void waitForUiException() throws Throwable {
-        Throwable throwable = uiExceptionOp.await(2500);
-        if (throwable != null) throw throwable;
+        Throwable throwable = uiExceptionOp.await(5000);
+        if (throwable != null) {
+            throw throwable;
+        }
     }
 
     private CameraOptions waitForOpen(boolean expectSuccess) {
@@ -128,6 +132,8 @@ public abstract class CameraIntegrationTest extends BaseTest {
         CameraOptions result = open.await(4000);
         if (expectSuccess) {
             assertNotNull("Can open", result);
+            // Extra wait for the bind state.
+            while (controller.getBindState() != CameraEngine.STATE_STARTED) {}
         } else {
             assertNull("Should not open", result);
         }
@@ -149,7 +155,7 @@ public abstract class CameraIntegrationTest extends BaseTest {
     private void waitForVideoEnd(boolean expectSuccess) {
         final Op<Boolean> video = new Op<>(true);
         doEndTask(video, true).when(listener).onVideoTaken(any(VideoResult.class));
-        Boolean result = video.await(8000);
+        Boolean result = video.await(12000);
         if (expectSuccess) {
             assertNotNull("Should end video", result);
         } else {
@@ -173,7 +179,7 @@ public abstract class CameraIntegrationTest extends BaseTest {
         controller.mStartVideoOp.listen();
         File file = new File(context().getFilesDir(), "video.mp4");
         camera.takeVideo(file);
-        controller.mStartVideoOp.await(400);
+        controller.mStartVideoOp.await(1000);
     }
 
     //region test open/close
@@ -593,7 +599,7 @@ public abstract class CameraIntegrationTest extends BaseTest {
         // Expect 30 frames
         CountDownLatch latch = new CountDownLatch(30);
         doCountDown(latch).when(mock).process(any(Frame.class));
-        boolean did = latch.await(4, TimeUnit.SECONDS);
+        boolean did = latch.await(60, TimeUnit.SECONDS);
         assertTrue(did);
     }
 
@@ -640,7 +646,6 @@ public abstract class CameraIntegrationTest extends BaseTest {
         waitForOpen(true);
         camera.takeVideo(new File(context().getFilesDir(), "video.mp4"), 4000);
         waitForVideoEnd(true);
-
         assert30Frames(processor);
     }
 

cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/MockCameraEngine.java

@@ -143,9 +143,15 @@ public class MockCameraEngine extends CameraEngine {
 
     @Override
     public void takePicture(@NonNull PictureResult.Stub stub) {
+        super.takePicture(stub);
         mPictureCaptured = true;
     }
 
+    @Override
+    protected void onTakePicture(@NonNull PictureResult.Stub stub) {
+
+    }
+
     @Override
     protected void onTakePictureSnapshot(@NonNull PictureResult.Stub stub, @NonNull AspectRatio viewAspectRatio) {
 

cameraview/src/androidTest/java/com/otaliastudios/cameraview/picture/PictureRecorderTest.java

@@ -31,7 +31,7 @@ public class PictureRecorderTest extends BaseTest {
             }
         };
         recorder.take();
-        Mockito.verify(listener, Mockito.times(1)).onPictureResult(result);
+        Mockito.verify(listener, Mockito.times(1)).onPictureResult(result, null);
         assertNull(recorder.mListener);
         assertNull(recorder.mResult);
     }

cameraview/src/androidTest/java/com/otaliastudios/cameraview/video/VideoRecorderTest.java

@@ -27,17 +27,19 @@ public class VideoRecorderTest extends BaseTest {
     public void testRecorder() throws Exception {
         VideoResult.Stub result = createStub();
         VideoRecorder.VideoResultListener listener = Mockito.mock(VideoRecorder.VideoResultListener.class);
-        VideoRecorder recorder = new VideoRecorder(result, listener) {
-            public void start() {}
-            public void stop() {
+        VideoRecorder recorder = new VideoRecorder(listener) {
+            @Override
+            protected void onStart() { }
+
+            @Override
+            protected void onStop() {
                 dispatchResult();
             }
         };
-        recorder.start();
+        recorder.start(result);
         recorder.stop();
-        Mockito.verify(listener, Mockito.times(1)).onVideoResult(result, null);
-        assertNull(recorder.mListener);
-        assertNull(recorder.mResult);
+        Mockito.verify(listener, Mockito.times(1))
+                .onVideoResult(result, null);
     }
 
     private VideoResult.Stub createStub() throws Exception {

cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java

@@ -713,6 +713,14 @@ public class CameraView extends FrameLayout implements LifecycleObserver {
 
     //region Public APIs for controls
 
+    /**
+     * Sets the experimental flag which occasionally can enable
+     * new, unstable beta features.
+     * @param experimental true to enable new features
+     */
+    public void setExperimental(boolean experimental) {
+        mExperimental = experimental;
+    }
 
     /**
      * Shorthand for the appropriate set* method.

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

@@ -13,6 +13,8 @@ import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 import androidx.annotation.WorkerThread;
+
+import android.util.Log;
 import android.view.SurfaceHolder;
 
 import com.google.android.gms.tasks.Task;
@@ -506,6 +508,7 @@ public class Camera1Engine extends CameraEngine implements
 
     @Override
     public void onVideoResult(@Nullable VideoResult.Stub result, @Nullable Exception exception) {
+        super.onVideoResult(result, exception);
         if (result == null) {
             // Something went wrong, lock the camera again.
             mCamera.lock();

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

@@ -503,11 +503,13 @@ public class Camera2Engine extends CameraEngine implements ImageReader.OnImageAv
         if (mFullVideoPendingStub != null) {
             // Do not call takeVideo/onTakeVideo. It will reset some stub parameters that the recorder sets.
             // Also we are posting this so that doTakeVideo sees a started preview.
+            LOG.i("onStartPreview", "Posting doTakeVideo call.");
             final VideoResult.Stub stub = mFullVideoPendingStub;
             mFullVideoPendingStub = null;
             mHandler.post(new Runnable() {
                 @Override
                 public void run() {
+                    LOG.i("onStartPreview", "Executing doTakeVideo call.");
                     doTakeVideo(stub);
                 }
             });
@@ -522,6 +524,7 @@ public class Camera2Engine extends CameraEngine implements ImageReader.OnImageAv
     @NonNull
     @Override
     protected Task<Void> onStopPreview() {
+        LOG.i("onStopPreview:", "About to clean up.");
         if (mVideoRecorder != null) {
             // This should synchronously call onVideoResult that will reset the repeating builder
             // to the PREVIEW template. This is very important.
@@ -546,6 +549,7 @@ public class Camera2Engine extends CameraEngine implements ImageReader.OnImageAv
         mRepeatingRequest = null;
         mAutoFocusPoint = null;
         mAutoFocusGesture = null;
+        LOG.i("onStopPreview:", "Returning.");
         return Tasks.forResult(null);
     }
 
@@ -553,6 +557,7 @@ public class Camera2Engine extends CameraEngine implements ImageReader.OnImageAv
     @NonNull
     @Override
     protected Task<Void> onStopBind() {
+        LOG.i("onStopBind:", "About to clean up.");
         if (mFullVideoPersistentSurface != null) {
             mFullVideoPersistentSurface.release();
             mFullVideoPersistentSurface = null;
@@ -572,6 +577,7 @@ public class Camera2Engine extends CameraEngine implements ImageReader.OnImageAv
         }
         mSession.close();
         mSession = null;
+        LOG.i("onStopBind:", "Returning.");
         return Tasks.forResult(null);
     }
 
@@ -650,11 +656,13 @@ public class Camera2Engine extends CameraEngine implements ImageReader.OnImageAv
     @WorkerThread
     @Override
     protected void onTakeVideo(@NonNull VideoResult.Stub stub) {
+        LOG.i("onTakeVideo", "called.");
         stub.rotation = offset(REF_SENSOR, REF_OUTPUT);
         stub.size = flip(REF_SENSOR, REF_OUTPUT) ? mCaptureSize.flip() : mCaptureSize;
         if (!Full2VideoRecorder.SUPPORTS_PERSISTENT_SURFACE) {
             // On API 21 and 22, we must restart the session at each time.
             // Save the pending data and restart the session.
+            LOG.w("onTakeVideo", "calling restartBind.");
             mFullVideoPendingStub = stub;
             restartBind();
         } else {

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

@@ -41,6 +41,7 @@ import com.otaliastudios.cameraview.size.SizeSelector;
 import com.otaliastudios.cameraview.size.SizeSelectors;
 import com.otaliastudios.cameraview.video.VideoRecorder;
 
+import androidx.annotation.CallSuper;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
@@ -53,6 +54,7 @@ import java.util.List;
 import java.util.concurrent.Callable;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.Executor;
+import java.util.concurrent.TimeUnit;
 
 
 /**
@@ -477,22 +479,25 @@ public abstract class CameraEngine implements
         mHandler.run(new Runnable() {
             @Override
             public void run() {
-                LOG.i("restartBind", "executing.");
+                LOG.w("restartBind", "executing stopPreview.");
                 stopPreview(false).continueWithTask(mHandler.getExecutor(), new Continuation<Void, Task<Void>>() {
                     @Override
                     public Task<Void> then(@NonNull Task<Void> task) {
+                        LOG.w("restartBind", "executing stopBind.");
                         return stopBind(false);
                     }
                 }).onSuccessTask(mHandler.getExecutor(), new SuccessContinuation<Void, Void>() {
                     @NonNull
                     @Override
                     public Task<Void> then(@Nullable Void aVoid) {
+                        LOG.w("restartBind", "executing startBind.");
                         return startBind();
                     }
                 }).onSuccessTask(mHandler.getExecutor(), new SuccessContinuation<Void, Void>() {
                     @NonNull
                     @Override
                     public Task<Void> then(@Nullable Void aVoid) {
+                        LOG.w("restartBind", "executing startPreview.");
                         return startPreview();
                     }
                 });
@@ -667,7 +672,12 @@ public abstract class CameraEngine implements
             }
         });
         try {
-            latch.await();
+            boolean success = latch.await(3, TimeUnit.SECONDS);
+            if (!success) {
+                LOG.e("Probably some deadlock in destroy.",
+                        "Current thread:", Thread.currentThread(),
+                        "Handler thread: ", mHandler.getThread());
+            }
         } catch (InterruptedException ignore) {}
     }
 
@@ -685,8 +695,11 @@ public abstract class CameraEngine implements
         mHandler.run(new Runnable() {
             @Override
             public void run() {
-                LOG.i("Start:", "executing runnable. State:", getEngineStateName());
-                if (mAllStep.isStoppingOrStopped()) {
+                LOG.w("Start:", "executing runnable. AllState is", mAllStep.getState());
+                // It's better to schedule anyway. allStep might be STARTING and we might be tempted to early return here,
+                // But the truth is that there might be a stop already scheduled when the STARTING op ends.
+                // if (mAllStep.isStoppingOrStopped()) {
+                //     LOG.i("Start:", "executing runnable. AllState is STOPPING or STOPPED, so we schedule a start.");
                     mAllStep.doStart(false, new Callable<Task<Void>>() {
                         @Override
                         public Task<Void> call() {
@@ -711,10 +724,11 @@ public abstract class CameraEngine implements
                             });
                         }
                     });
-                } else {
-                    // NOTE: this returns early if we were STARTING.
-                    outTask.trySetResult(null);
-                }
+                // } else {
+                //     // NOTE: this returns early if we were STARTING.
+                //     LOG.i("Start:", "executing runnable. AllState is STARTING or STARTED, so we return early.");
+                //     outTask.trySetResult(null);
+                // }
             }
         });
         return outTask.getTask();
@@ -732,8 +746,11 @@ public abstract class CameraEngine implements
         mHandler.run(new Runnable() {
             @Override
             public void run() {
-                LOG.i("Stop:", "executing runnable. State:", getEngineStateName());
-                if (mAllStep.isStartedOrStarting()) {
+                LOG.w("Stop:", "executing runnable. AllState is", mAllStep.getState());
+                // It's better to schedule anyway. allStep might be STOPPING and we might be tempted to early return here,
+                // But the truth is that there might be a start already scheduled when the STOPPING op ends.
+                // if (mAllStep.isStartedOrStarting()) {
+                //     LOG.i("Stop:", "executing runnable. AllState is STARTING or STARTED, so we schedule a stop.");
                     mAllStep.doStop(swallowExceptions, new Callable<Task<Void>>() {
                         @Override
                         public Task<Void> call() {
@@ -761,10 +778,11 @@ public abstract class CameraEngine implements
                             });
                         }
                     });
-                } else {
-                    // NOTE: this returns early if we were STOPPING.
-                    outTask.trySetResult(null);
-                }
+                // } else {
+                //     // NOTE: this returns early if we were STOPPING.
+                //     LOG.i("Stop:", "executing runnable. AllState is STOPPING or STOPPED, so we return early.");
+                //     outTask.trySetResult(null);
+                // }
             }
         });
         return outTask.getTask();
@@ -933,17 +951,18 @@ public abstract class CameraEngine implements
 
     //region picture and video control
 
-    public final void takePicture(final @NonNull PictureResult.Stub stub) {
+    /* not final for tests */
+    public void takePicture(final @NonNull PictureResult.Stub stub) {
         LOG.v("takePicture", "scheduling");
         mHandler.run(new Runnable() {
             @Override
             public void run() {
-                LOG.v("takePicture", "performing. Engine:", getEngineStateName(), "isTakingPicture:", isTakingPicture());
+                LOG.v("takePicture", "performing. BindState:", getBindState(), "isTakingPicture:", isTakingPicture());
                 if (mMode == Mode.VIDEO) {
                     // Could redirect to takePictureSnapshot, but it's better if people know what they are doing.
                     throw new IllegalStateException("Can't take hq pictures while in VIDEO mode");
                 }
-                if (getEngineState() < STATE_STARTED) return;
+                if (getBindState() < STATE_STARTED) return;
                 if (isTakingPicture()) return;
                 stub.isSnapshot = false;
                 stub.location = mLocation;
@@ -964,8 +983,8 @@ public abstract class CameraEngine implements
         mHandler.run(new Runnable() {
             @Override
             public void run() {
-                LOG.v("takePictureSnapshot", "performing. Engine:", getEngineStateName(), "isTakingPicture:", isTakingPicture());
-                if (getEngineState() < STATE_STARTED) return;
+                LOG.v("takePictureSnapshot", "performing. BindState:", getBindState(), "isTakingPicture:", isTakingPicture());
+                if (getBindState() < STATE_STARTED) return;
                 if (isTakingPicture()) return;
                 stub.location = mLocation;
                 stub.isSnapshot = true;
@@ -1000,8 +1019,8 @@ public abstract class CameraEngine implements
         mHandler.run(new Runnable() {
             @Override
             public void run() {
-                LOG.v("takeVideo", "performing. Engine:", getEngineStateName(), "isTakingVideo:", isTakingVideo());
-                if (getEngineState() < STATE_STARTED) { mStartVideoOp.end(null); return; }
+                LOG.v("takeVideo", "performing. BindState:", getBindState(), "isTakingVideo:", isTakingVideo());
+                if (getBindState() < STATE_STARTED) { mStartVideoOp.end(null); return; }
                 if (isTakingVideo()) { mStartVideoOp.end(null); return; }
                 if (mMode == Mode.PICTURE) {
                     throw new IllegalStateException("Can't record video while in PICTURE mode");
@@ -1032,8 +1051,8 @@ public abstract class CameraEngine implements
         mHandler.run(new Runnable() {
             @Override
             public void run() {
-                LOG.v("takeVideoSnapshot", "performing. Engine:", getEngineStateName(), "isTakingVideo:", isTakingVideo());
-                if (getEngineState() < STATE_STARTED) { mStartVideoOp.end(null); return; }
+                LOG.v("takeVideoSnapshot", "performing. BindState:", getBindState(), "isTakingVideo:", isTakingVideo());
+                if (getBindState() < STATE_STARTED) { mStartVideoOp.end(null); return; }
                 if (isTakingVideo()) { mStartVideoOp.end(null); return; }
                 stub.file = file;
                 stub.isSnapshot = true;
@@ -1059,11 +1078,13 @@ public abstract class CameraEngine implements
                 LOG.i("stopVideo", "executing.", "isTakingVideo?", isTakingVideo());
                 if (mVideoRecorder != null) {
                     mVideoRecorder.stop();
+                    mVideoRecorder = null;
                 }
             }
         });
     }
 
+    @CallSuper
     @Override
     public void onVideoResult(@Nullable VideoResult.Stub result, @Nullable Exception exception) {
         mVideoRecorder = null;

cameraview/src/main/java/com/otaliastudios/cameraview/video/Full1VideoRecorder.java

@@ -2,6 +2,7 @@ package com.otaliastudios.cameraview.video;
 
 import android.hardware.Camera;
 import android.media.MediaRecorder;
+import android.util.Log;
 
 import com.otaliastudios.cameraview.CameraLogger;
 import com.otaliastudios.cameraview.VideoResult;
@@ -44,9 +45,9 @@ public class Full1VideoRecorder extends FullVideoRecorder {
     }
 
     @Override
-    protected void onStop() {
+    protected void dispatchResult() {
         // Restore frame processing.
         mCamera.setPreviewCallbackWithBuffer(mEngine);
-        super.onStop();
+        super.dispatchResult();
     }
 }

cameraview/src/main/java/com/otaliastudios/cameraview/video/VideoRecorder.java

@@ -76,7 +76,9 @@ public abstract class VideoRecorder {
     @CallSuper
     protected void dispatchResult() {
         mIsRecording = false;
-        if (mListener != null) mListener.onVideoResult(mResult, mError);
+        if (mListener != null) {
+            mListener.onVideoResult(mResult, mError);
+        }
         mResult = null;
         mError = null;
     }