diff --git a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraViewTest.java b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraViewTest.java index 7bdc790c..055c4557 100644 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraViewTest.java +++ b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraViewTest.java @@ -135,7 +135,7 @@ public class CameraViewTest extends BaseTest { public void testDestroy() { cameraView.destroy(); verify(mockPreview, times(1)).onDestroy(); - verify(mockController, times(1)).destroy(); + verify(mockController, times(1)).destroy(true); } //region testDefaults 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 10a2eaba..74c8e5a2 100644 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/CameraIntegrationTest.java +++ b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/CameraIntegrationTest.java @@ -630,17 +630,26 @@ public abstract class CameraIntegrationTest extends Base @Test public void testEndVideo_withMaxSize() { camera.setMode(Mode.VIDEO); - camera.setVideoSize(SizeSelectors.smallest()); - camera.setVideoMaxSize(1000*1000); + camera.setVideoSize(SizeSelectors.maxArea(480 * 360)); openSync(true); + // Assuming video frame rate is 12... + //noinspection ConstantConditions + camera.setVideoBitRate((int) estimateVideoBitRate(camera.getVideoSize(), 12)); + camera.setVideoMaxSize(estimateVideoBytes(camera.getVideoBitRate(), 4000)); takeVideoSync(true); waitForVideoResult(true); } @Test public void testEndVideoSnapshot_withMaxSize() { - camera.setVideoMaxSize(1000*1000); openSync(true); + camera.setSnapshotMaxWidth(480); + camera.setSnapshotMaxHeight(480); + camera.setPreviewFrameRate(12F); + //noinspection ConstantConditions + camera.setVideoBitRate((int) estimateVideoBitRate(camera.getSnapshotSize(), + (int) camera.getPreviewFrameRate())); + camera.setVideoMaxSize(estimateVideoBytes(camera.getVideoBitRate(), 4000)); takeVideoSnapshotSync(true); waitForVideoResult(true); } @@ -865,11 +874,11 @@ public abstract class CameraIntegrationTest extends Base //region Frame Processing - private void assert30Frames(@NonNull FrameProcessor mock) throws Exception { - // Expect 30 frames - CountDownLatch latch = new CountDownLatch(30); + private void assert15Frames(@NonNull FrameProcessor mock) throws Exception { + // Expect 15 frames. Time is very high because currently Camera2 keeps a very low FPS. + CountDownLatch latch = new CountDownLatch(15); doCountDown(latch).when(mock).process(any(Frame.class)); - boolean did = latch.await(15, TimeUnit.SECONDS); + boolean did = latch.await(30, TimeUnit.SECONDS); assertTrue("Latch count should be 0: " + latch.getCount(), did); } @@ -879,7 +888,7 @@ public abstract class CameraIntegrationTest extends Base camera.addFrameProcessor(processor); openSync(true); - assert30Frames(processor); + assert15Frames(processor); } @Test @@ -893,7 +902,7 @@ public abstract class CameraIntegrationTest extends Base camera.takePictureSnapshot(); waitForPictureResult(true); - assert30Frames(processor); + assert15Frames(processor); } @Test @@ -904,7 +913,7 @@ public abstract class CameraIntegrationTest extends Base closeSync(true); openSync(true); - assert30Frames(processor); + assert15Frames(processor); } @@ -917,7 +926,7 @@ public abstract class CameraIntegrationTest extends Base takeVideoSync(true,4000); waitForVideoResult(true); - assert30Frames(processor); + assert15Frames(processor); } @Test @@ -930,7 +939,7 @@ public abstract class CameraIntegrationTest extends Base camera.addFrameProcessor(processor); openSync(true); - assert30Frames(processor); + assert15Frames(processor); } public class FreezeReleaseFrameProcessor implements FrameProcessor { @@ -969,4 +978,15 @@ public abstract class CameraIntegrationTest extends Base } //endregion + + @SuppressWarnings("SameParameterValue") + private static long estimateVideoBitRate(@NonNull Size size, int frameRate) { + // Nasty estimate for a LQ video + return Math.round(0.05D * size.getWidth() * size.getHeight() * frameRate); + } + + @SuppressWarnings("SameParameterValue") + private static long estimateVideoBytes(long bitRate, long millis) { + return Math.round(bitRate * (millis / 1000D) / 8D); + } } diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java b/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java index 154f8c9f..23acf314 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java @@ -808,7 +808,7 @@ public class CameraView extends FrameLayout implements LifecycleObserver { if (mInEditor) return; clearCameraListeners(); clearFrameProcessors(); - mCameraEngine.destroy(); + mCameraEngine.destroy(true); if (mCameraPreview != null) mCameraPreview.onDestroy(); } diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera1Engine.java b/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera1Engine.java index ee583f84..9ce706b1 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera1Engine.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera1Engine.java @@ -457,7 +457,7 @@ public class Camera1Engine extends CameraEngine implements public void setFlash(@NonNull Flash flash) { final Flash old = mFlash; mFlash = flash; - mFlashTask = mOrchestrator.scheduleStateful("flash", + mFlashTask = mOrchestrator.scheduleStateful("flash (" + flash + ")", CameraState.ENGINE, new Runnable() { @Override @@ -508,7 +508,8 @@ public class Camera1Engine extends CameraEngine implements public void setWhiteBalance(@NonNull WhiteBalance whiteBalance) { final WhiteBalance old = mWhiteBalance; mWhiteBalance = whiteBalance; - mWhiteBalanceTask = mOrchestrator.scheduleStateful("white balance", + mWhiteBalanceTask = mOrchestrator.scheduleStateful( + "white balance (" + whiteBalance + ")", CameraState.ENGINE, new Runnable() { @Override @@ -522,7 +523,11 @@ public class Camera1Engine extends CameraEngine implements private boolean applyWhiteBalance(@NonNull Camera.Parameters params, @NonNull WhiteBalance oldWhiteBalance) { if (mCameraOptions.supports(mWhiteBalance)) { + // If this lock key is present, the engine can throw when applying the + // parameters, not sure why. Since we never lock it, this should be + // harmless for the rest of the engine. params.setWhiteBalance(mMapper.mapWhiteBalance(mWhiteBalance)); + params.remove("auto-whitebalance-lock"); return true; } mWhiteBalance = oldWhiteBalance; @@ -533,7 +538,7 @@ public class Camera1Engine extends CameraEngine implements public void setHdr(@NonNull Hdr hdr) { final Hdr old = mHdr; mHdr = hdr; - mHdrTask = mOrchestrator.scheduleStateful("hdr", + mHdrTask = mOrchestrator.scheduleStateful("hdr (" + hdr + ")", CameraState.ENGINE, new Runnable() { @Override @@ -557,7 +562,7 @@ public class Camera1Engine extends CameraEngine implements public void setZoom(final float zoom, @Nullable final PointF[] points, final boolean notify) { final float old = mZoomValue; mZoomValue = zoom; - mZoomTask = mOrchestrator.scheduleStateful("zoom", + mZoomTask = mOrchestrator.scheduleStateful("zoom (" + zoom + ")", CameraState.ENGINE, new Runnable() { @Override @@ -589,7 +594,8 @@ public class Camera1Engine extends CameraEngine implements @Nullable final PointF[] points, final boolean notify) { final float old = mExposureCorrectionValue; mExposureCorrectionValue = EVvalue; - mExposureCorrectionTask = mOrchestrator.scheduleStateful("exposure correction", + mExposureCorrectionTask = mOrchestrator.scheduleStateful( + "exposure correction (" + EVvalue + ")", CameraState.ENGINE, new Runnable() { @Override @@ -629,7 +635,8 @@ public class Camera1Engine extends CameraEngine implements public void setPlaySounds(boolean playSounds) { final boolean old = mPlaySounds; mPlaySounds = playSounds; - mPlaySoundsTask = mOrchestrator.scheduleStateful("play sounds", + mPlaySoundsTask = mOrchestrator.scheduleStateful( + "play sounds (" + playSounds + ")", CameraState.ENGINE, new Runnable() { @Override @@ -665,7 +672,8 @@ public class Camera1Engine extends CameraEngine implements public void setPreviewFrameRate(float previewFrameRate) { final float old = previewFrameRate; mPreviewFrameRate = previewFrameRate; - mPreviewFrameRateTask = mOrchestrator.scheduleStateful("preview fps", + mPreviewFrameRateTask = mOrchestrator.scheduleStateful( + "preview fps (" + previewFrameRate + ")", CameraState.ENGINE, new Runnable() { @Override 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 79f7363f..e348ac60 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/CameraEngine.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/engine/CameraEngine.java @@ -85,8 +85,9 @@ import java.util.concurrent.TimeUnit; * S3 and S4 are also performed. * - {@link #stop(boolean)}: ASYNC - stops everything: undoes S4, then S3, then S2. * - {@link #restart()}: ASYNC - completes a stop then a start. - * - {@link #destroy()}: SYNC - performs a {@link #stop(boolean)} that will go on no matter what, - * without throwing. Makes the engine unusable and clears resources. + * - {@link #destroy(boolean)}: SYNC - performs a {@link #stop(boolean)} that will go on no matter + * what, without throwing. Makes the engine unusable and clears + * resources. * * THREADING * Subclasses should always execute code on the thread given by {@link #mHandler}. @@ -268,48 +269,45 @@ public abstract class CameraEngine implements * @param throwable the throwable * @param fromExceptionHandler true if coming from exception handler */ - private void handleException(@NonNull Thread thread, + private void handleException(@NonNull final Thread thread, final @NonNull Throwable throwable, final boolean fromExceptionHandler) { - if (!(throwable instanceof CameraException)) { - // This is unexpected, either a bug or something the developer should know. - // Release and crash the UI thread so we get bug reports. - LOG.e("EXCEPTION:", "Unexpected exception! Scheduling destroy..."); - mCrashHandler.post(new Runnable() { - @Override - public void run() { - LOG.e("EXCEPTION:", "Unexpected exception! Executing destroy..."); - destroy(); - // Throws an unchecked exception without unnecessary wrapping. - if (throwable instanceof RuntimeException) { - throw (RuntimeException) throwable; - } else { - throw new RuntimeException(throwable); - } - } - }); - return; - } - - final CameraException cameraException = (CameraException) throwable; - LOG.e("EXCEPTION:", "Received CameraException.", - "Engine state:", getState(), "Current thread:", Thread.currentThread()); + // 1. If this comes from the exception handler, the thread has crashed. Replace it. + // I'm not sure if this can even be called now that all actions are wrapped into Tasks. if (fromExceptionHandler) { - // Got to restart the handler. LOG.e("EXCEPTION:", "Handler thread is gone. Replacing."); thread.interrupt(); mHandler = WorkerHandler.get("CameraViewEngine"); mHandler.getThread().setUncaughtExceptionHandler(new CrashExceptionHandler()); } - LOG.e("EXCEPTION:", "Dispatching it to CameraListener."); - mCallback.dispatchError(cameraException); - if (cameraException.isUnrecoverable()) { - LOG.e("EXCEPTION:", "Since exception is unrecoverable, we stop(true)", - "to release resources and make it easy to retry."); - // Stop everything (if needed) without notifying teardown errors. - stop(true); - } + // 2. Depending on the exception, we must destroy(false|true) to release resources, and + // notify the outside, either with the callback or by crashing the app. + LOG.e("EXCEPTION:", "Scheduling on the crash handler..."); + mCrashHandler.post(new Runnable() { + @Override + public void run() { + if (throwable instanceof CameraException) { + CameraException exception = (CameraException) throwable; + if (exception.isUnrecoverable()) { + LOG.e("EXCEPTION:", "Got CameraException. " + + "Since it is unrecoverable, executing destroy(false)."); + destroy(false); + } + LOG.e("EXCEPTION:", "Got CameraException. Dispatching to callback."); + mCallback.dispatchError(exception); + } else { + LOG.e("EXCEPTION:", "Unexpected error! Executing destroy(true)."); + destroy(true); + LOG.e("EXCEPTION:", "Unexpected error! Throwing."); + if (throwable instanceof RuntimeException) { + throw (RuntimeException) throwable; + } else { + throw new RuntimeException(throwable); + } + } + } + }); } //endregion @@ -334,14 +332,21 @@ public abstract class CameraEngine implements * Calls {@link #stop(boolean)} and waits for it. * Not final due to mockito requirements. * + * If forever is true, this also releases resources and the engine will not be in a + * functional state after. If forever is false, this really is just a synchronous stop. + * * NOTE: Should not be called on the orchestrator thread! This would cause deadlocks due to us * awaiting for {@link #stop(boolean)} to return. */ - public void destroy() { - LOG.i("DESTROY:", "state:", getState(), "thread:", Thread.currentThread()); - // Prevent CameraEngine leaks. Don't set to null, or exceptions - // inside the standard stop() method might crash the main thread. - mHandler.getThread().setUncaughtExceptionHandler(new NoOpExceptionHandler()); + public void destroy(boolean forever) { + LOG.i("DESTROY:", "state:", getState(), + "thread:", Thread.currentThread(), + "forever:", forever); + if (forever) { + // Prevent CameraEngine leaks. Don't set to null, or exceptions + // inside the standard stop() method might crash the main thread. + mHandler.getThread().setUncaughtExceptionHandler(new NoOpExceptionHandler()); + } // Stop if needed, synchronously and silently. // Cannot use Tasks.await() because we might be on the UI thread. final CountDownLatch latch = new CountDownLatch(1); diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/video/FullVideoRecorder.java b/cameraview/src/main/java/com/otaliastudios/cameraview/video/FullVideoRecorder.java index df4ebb35..a54aa494 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/video/FullVideoRecorder.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/video/FullVideoRecorder.java @@ -76,6 +76,7 @@ public abstract class FullVideoRecorder extends VideoRecorder { @SuppressWarnings("SameParameterValue") private boolean prepareMediaRecorder(@NonNull VideoResult.Stub stub, boolean applyEncodersConstraints) { + LOG.i("prepareMediaRecorder:", "Preparing on thread", Thread.currentThread()); // 1. Create reference and ask for the CamcorderProfile mMediaRecorder = new MediaRecorder(); mProfile = getCamcorderProfile(stub); @@ -210,10 +211,14 @@ public abstract class FullVideoRecorder extends VideoRecorder { // Would do this with max duration as well but there's no such callback. mMediaRecorder.setMaxFileSize(stub.maxSize <= 0 ? stub.maxSize : Math.round(stub.maxSize / 0.9D)); + LOG.i("prepareMediaRecorder:", "Increased max size from", stub.maxSize, "to", + Math.round(stub.maxSize / 0.9D)); mMediaRecorder.setMaxDuration(stub.maxDuration); mMediaRecorder.setOnInfoListener(new MediaRecorder.OnInfoListener() { @Override public void onInfo(MediaRecorder mediaRecorder, int what, int extra) { + LOG.i("OnInfoListener:", "Received info", what, extra, + "Thread: ", Thread.currentThread()); switch (what) { case MediaRecorder.MEDIA_RECORDER_INFO_MAX_DURATION_REACHED: mResult.endReason = VideoResult.REASON_MAX_DURATION_REACHED; @@ -226,6 +231,15 @@ public abstract class FullVideoRecorder extends VideoRecorder { } } }); + mMediaRecorder.setOnErrorListener(new MediaRecorder.OnErrorListener() { + @Override + public void onError(MediaRecorder mr, int what, int extra) { + LOG.e("OnErrorListener: got error", what, extra, ". Stopping."); + mResult = null; + mError = new RuntimeException("MediaRecorder error: " + what + " " + extra); + stop(false); + } + }); // 8. Prepare the Recorder try { @@ -265,7 +279,9 @@ public abstract class FullVideoRecorder extends VideoRecorder { if (mMediaRecorder != null) { dispatchVideoRecordingEnd(); try { + LOG.i("stop:", "Stopping MediaRecorder..."); mMediaRecorder.stop(); + LOG.i("stop:", "Stopped MediaRecorder."); } catch (Exception e) { // This can happen if stopVideo() is called right after takeVideo() // (in which case we don't care). Or when prepare()/start() have failed for @@ -278,7 +294,9 @@ public abstract class FullVideoRecorder extends VideoRecorder { } } try { + LOG.i("stop:", "Releasing MediaRecorder..."); mMediaRecorder.release(); + LOG.i("stop:", "Released MediaRecorder."); } catch (Exception e) { mResult = null; if (mError == null) {