diff --git a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/CameraIntegration2Test.java b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/CameraIntegration2Test.java index 32ff78a7a..1995eeeed 100644 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/CameraIntegration2Test.java +++ b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/CameraIntegration2Test.java @@ -27,4 +27,9 @@ public class CameraIntegration2Test extends CameraIntegrationTest { protected Engine getEngine() { return Engine.CAMERA2; } + + @Override + public void testFrameProcessing_afterVideo() throws Exception { + super.testFrameProcessing_afterVideo(); + } } 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 2bfbdcfa2..9f95dfe42 100644 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/CameraIntegrationTest.java +++ b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/CameraIntegrationTest.java @@ -136,6 +136,7 @@ private void waitForUiException() throws Throwable { } } + @SuppressWarnings("StatementWithEmptyBody") private CameraOptions openSync(boolean expectSuccess) { camera.open(); final Op open = new Op<>(true); @@ -143,10 +144,11 @@ private CameraOptions openSync(boolean expectSuccess) { CameraOptions result = open.await(DELAY); if (expectSuccess) { assertNotNull("Can open", result); - // Extra wait for the bind state. - // TODO fix this and other while {} in this class in a more elegant way. - //noinspection StatementWithEmptyBody + // Extra wait for the bind and preview state, so we run tests in a fully operational + // state. If we didn't do so, we could have null values, for example, in getPictureSize + // or in getSnapshotSize. while (controller.getBindState() != CameraEngine.STATE_STARTED) {} + while (controller.getPreviewState() != CameraEngine.STATE_STARTED) {} } else { assertNull("Should not open", result); } @@ -194,6 +196,9 @@ public boolean matches(CameraException argument) { video.listen(); result = video.await(DELAY); } + // Sleep another 1000, because camera.isTakingVideo() might return false even + // if the result still has to be dispatched. Rare but could happen. + try { Thread.sleep(1000); } catch (InterruptedException ignore) {} } // Now we should be OK. @@ -684,14 +689,11 @@ public void testCapturePicture_concurrentCalls() throws Exception { assertEquals(1, latch.getCount()); } - @SuppressWarnings("StatementWithEmptyBody") @Test public void testCapturePicture_size() { openSync(true); - // PictureSize can still be null after opened. - // TODO be more elegant - while (camera.getPictureSize() == null) {} Size size = camera.getPictureSize(); + assertNotNull(size); camera.takePicture(); PictureResult result = waitForPictureResult(true); assertNotNull(result); @@ -734,14 +736,11 @@ public void testCaptureSnapshot_concurrentCalls() throws Exception { assertEquals(1, latch.getCount()); } - @SuppressWarnings("StatementWithEmptyBody") @Test public void testCaptureSnapshot_size() { openSync(true); - // SnapshotSize can still be null after opened. - // TODO be more elegant - while (camera.getSnapshotSize() == null) {} Size size = camera.getSnapshotSize(); + assertNotNull(size); camera.takePictureSnapshot(); PictureResult result = waitForPictureResult(true); @@ -764,8 +763,8 @@ private void assert30Frames(FrameProcessor mock) throws Exception { // Expect 30 frames CountDownLatch latch = new CountDownLatch(30); doCountDown(latch).when(mock).process(any(Frame.class)); - boolean did = latch.await(60, TimeUnit.SECONDS); - assertTrue(did); + boolean did = latch.await(15, TimeUnit.SECONDS); + assertTrue("Latch count should be 0: " + latch.getCount(), did); } @Test @@ -811,6 +810,7 @@ public void testFrameProcessing_afterVideo() throws Exception { openSync(true); takeVideoSync(true,4000); waitForVideoResult(true); + assert30Frames(processor); } diff --git a/cameraview/src/test/java/com/otaliastudios/cameraview/frame/FrameManagerTest.java b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/frame/FrameManagerTest.java similarity index 66% rename from cameraview/src/test/java/com/otaliastudios/cameraview/frame/FrameManagerTest.java rename to cameraview/src/androidTest/java/com/otaliastudios/cameraview/frame/FrameManagerTest.java index 513a05fbb..77375047e 100644 --- a/cameraview/src/test/java/com/otaliastudios/cameraview/frame/FrameManagerTest.java +++ b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/frame/FrameManagerTest.java @@ -1,14 +1,20 @@ package com.otaliastudios.cameraview.frame; +import android.graphics.ImageFormat; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import androidx.test.filters.SmallTest; + +import com.otaliastudios.cameraview.BaseTest; import com.otaliastudios.cameraview.size.Size; import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -16,7 +22,9 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -public class FrameManagerTest { +@RunWith(AndroidJUnit4.class) +@SmallTest +public class FrameManagerTest extends BaseTest { private FrameManager.BufferCallback callback; @@ -33,12 +41,12 @@ public void tearDown() { @Test public void testAllocate() { FrameManager manager = new FrameManager(1, callback); - manager.setUp(4, new Size(50, 50)); + manager.setUp(ImageFormat.NV21, new Size(50, 50)); verify(callback, times(1)).onBufferAvailable(any(byte[].class)); reset(callback); manager = new FrameManager(5, callback); - manager.setUp(4, new Size(50, 50)); + manager.setUp(ImageFormat.NV21, new Size(50, 50)); verify(callback, times(5)).onBufferAvailable(any(byte[].class)); } @@ -46,12 +54,12 @@ public void testAllocate() { public void testFrameRecycling() { // A 1-pool manager will always recycle the same frame. FrameManager manager = new FrameManager(1, callback); - manager.setUp(4, new Size(50, 50)); + manager.setUp(ImageFormat.NV21, new Size(50, 50)); - Frame first = manager.getFrame(null, 0, 0, null, 0); + Frame first = manager.getFrame(null, 0, 0); first.release(); - Frame second = manager.getFrame(null, 0, 0, null, 0); + Frame second = manager.getFrame(null, 0, 0); second.release(); assertEquals(first, second); @@ -60,63 +68,51 @@ public void testFrameRecycling() { @Test public void testOnFrameReleased_alreadyFull() { FrameManager manager = new FrameManager(1, callback); - int length = manager.setUp(4, new Size(50, 50)); + int length = manager.setUp(ImageFormat.NV21, new Size(50, 50)); - Frame frame1 = manager.getFrame(new byte[length], 0, 0, null, 0); + Frame frame1 = manager.getFrame(new byte[length], 0, 0); // Since frame1 is already taken and poolSize = 1, a new Frame is created. - Frame frame2 = manager.getFrame(new byte[length], 0, 0, null, 0); + Frame frame2 = manager.getFrame(new byte[length], 0, 0); // Release the first frame so it goes back into the pool. - manager.onFrameReleased(frame1); + manager.onFrameReleased(frame1, frame1.getData()); reset(callback); // Release the second. The pool is already full, so onBufferAvailable should not be called // since this Frame instance will NOT be reused. - manager.onFrameReleased(frame2); + manager.onFrameReleased(frame2, frame2.getData()); verify(callback, never()).onBufferAvailable(frame2.getData()); } @Test public void testOnFrameReleased_sameLength() { FrameManager manager = new FrameManager(1, callback); - int length = manager.setUp(4, new Size(50, 50)); + int length = manager.setUp(ImageFormat.NV21, new Size(50, 50)); // A camera preview frame comes. Request a frame. byte[] picture = new byte[length]; - Frame frame = manager.getFrame(picture, 0, 0, null, 0); + Frame frame = manager.getFrame(picture, 0, 0); // Release the frame and ensure that onBufferAvailable is called. reset(callback); - manager.onFrameReleased(frame); + manager.onFrameReleased(frame, frame.getData()); verify(callback, times(1)).onBufferAvailable(picture); } @Test public void testOnFrameReleased_differentLength() { FrameManager manager = new FrameManager(1, callback); - int length = manager.setUp(4, new Size(50, 50)); + int length = manager.setUp(ImageFormat.NV21, new Size(50, 50)); // A camera preview frame comes. Request a frame. byte[] picture = new byte[length]; - Frame frame = manager.getFrame(picture, 0, 0, null, 0); + Frame frame = manager.getFrame(picture, 0, 0); // Don't release the frame. Change the allocation size. - manager.setUp(2, new Size(15, 15)); + manager.setUp(ImageFormat.NV16, new Size(15, 15)); // Now release the old frame and ensure that onBufferAvailable is NOT called, // because the released data has wrong length. - manager.onFrameReleased(frame); + manager.onFrameReleased(frame, frame.getData()); reset(callback); verify(callback, never()).onBufferAvailable(picture); } - - @Test - public void testRelease() { - FrameManager manager = new FrameManager(1, callback); - int length = manager.setUp(4, new Size(50, 50)); - Frame first = manager.getFrame(new byte[length], 0, 0, null, 0); - first.release(); // Store this frame in the queue. - - // Release the whole manager and ensure it clears the frame. - manager.release(); - assertNull(first.mManager); - } } diff --git a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/gesture/ScrollGestureFinderTest.java b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/gesture/ScrollGestureFinderTest.java index a7d6bb178..4a886ac28 100644 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/gesture/ScrollGestureFinderTest.java +++ b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/gesture/ScrollGestureFinderTest.java @@ -21,6 +21,8 @@ @SmallTest public class ScrollGestureFinderTest extends GestureFinderTest { + private final static long WAIT = 2000; // 500 was too short + @Override protected ScrollGestureFinder createFinder(@NonNull GestureFinder.Controller controller) { return new ScrollGestureFinder(controller); @@ -42,7 +44,7 @@ public void testScrollDisabled() { touchOp.listen(); touchOp.start(); onLayout().perform(swipeUp()); - Gesture found = touchOp.await(500); + Gesture found = touchOp.await(WAIT); assertNull(found); } @@ -50,7 +52,7 @@ private void testScroll(ViewAction scroll, Gesture expected, boolean increasing) touchOp.listen(); touchOp.start(); onLayout().perform(scroll); - Gesture found = touchOp.await(500); + Gesture found = touchOp.await(WAIT); assertEquals(found, expected); // How will this move our parameter? diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java b/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java index 10a7cce9c..adbda8493 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java @@ -2097,28 +2097,30 @@ public void run() { } @Override - public void dispatchFrame(final Frame frame) { + public void dispatchFrame(@NonNull final Frame frame) { + // The getTime() below might crash if developers incorrectly release frames asynchronously. mLogger.v("dispatchFrame:", frame.getTime(), "processors:", mFrameProcessors.size()); if (mFrameProcessors.isEmpty()) { // Mark as released. This instance will be reused. frame.release(); - return; - } - mFrameProcessorsHandler.run(new Runnable() { - @Override - public void run() { - for (FrameProcessor processor : mFrameProcessors) { - try { - processor.process(frame); - } catch (Exception e) { - mLogger.w("dispatchFrame:", - "Error during processor implementation.", - "Can happen when camera is closed while processors are running.", e); + } else { + // Dispatch this frame to frame processors. + mFrameProcessorsHandler.run(new Runnable() { + @Override + public void run() { + mLogger.v("dispatchFrame: dispatching", frame.getTime(), "to processors."); + for (FrameProcessor processor : mFrameProcessors) { + try { + processor.process(frame); + } catch (Exception e) { + // Don't let a single processor crash the processor thread. + mLogger.w("Frame processor crashed:", e); + } } + frame.release(); } - frame.release(); - } - }); + }); + } } @Override 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 5689fcf91..43faf6d7b 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera1Engine.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera1Engine.java @@ -210,7 +210,7 @@ protected Task onStartPreview() { mCamera.setPreviewCallbackWithBuffer(null); // Release anything left mCamera.setPreviewCallbackWithBuffer(this); // Add ourselves - getFrameManager().setUp(ImageFormat.getBitsPerPixel(PREVIEW_FORMAT), mPreviewStreamSize); + getFrameManager().setUp(PREVIEW_FORMAT, mPreviewStreamSize); LOG.i("onStartPreview", "Starting preview with startPreview()."); try { @@ -654,12 +654,15 @@ public void onBufferAvailable(@NonNull byte[] buffer) { } @Override - public void onPreviewFrame(@NonNull byte[] data, Camera camera) { + public void onPreviewFrame(byte[] data, Camera camera) { + if (data == null) { + // Let's test this with an exception. + throw new RuntimeException("Camera1 returns null data from onPreviewFrame! " + + "This would make the frame processors crash later."); + } Frame frame = getFrameManager().getFrame(data, System.currentTimeMillis(), - getAngles().offset(Reference.SENSOR, Reference.OUTPUT, Axis.RELATIVE_TO_SENSOR), - mPreviewStreamSize, - PREVIEW_FORMAT); + getAngles().offset(Reference.SENSOR, Reference.OUTPUT, Axis.RELATIVE_TO_SENSOR)); mCallback.dispatchFrame(frame); } 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 5e928f2dd..8929d36ed 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera2Engine.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera2Engine.java @@ -498,7 +498,7 @@ protected Task onStartPreview() { mPreview.setStreamSize(previewSizeForView.getWidth(), previewSizeForView.getHeight()); mPreview.setDrawRotation(getAngles().offset(Reference.BASE, Reference.VIEW, Axis.ABSOLUTE)); if (hasFrameProcessors()) { - getFrameManager().setUp(ImageFormat.getBitsPerPixel(FRAME_PROCESSING_FORMAT), mFrameProcessingSize); + getFrameManager().setUp(FRAME_PROCESSING_FORMAT, mFrameProcessingSize); } LOG.i("onStartPreview", "Starting preview."); @@ -726,13 +726,38 @@ protected void onTakeVideoSnapshot(@NonNull VideoResult.Stub stub, @NonNull Aspe } @Override - 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) { + public void onVideoRecordingEnd() { + super.onVideoRecordingEnd(); + // When video ends we must stop the recorder and remove the recorder surface from camera outputs. + // This is done in onVideoResult. However, on some devices, order matters. If we stop the recorder + // and AFTER send camera frames to it, the camera will try to fill the recorder "abandoned" + // Surface and on some devices with a poor internal implementation (HW_LEVEL_LEGACY) this crashes. + // So if the conditions are met, we restore here. Issue #549. + boolean needsIssue549Workaround = (mVideoRecorder instanceof Full2VideoRecorder) || + (readCharacteristic(CameraCharacteristics.INFO_SUPPORTED_HARDWARE_LEVEL, -1) + == CameraCharacteristics.INFO_SUPPORTED_HARDWARE_LEVEL_LEGACY); + if (needsIssue549Workaround) { + maybeRestorePreviewTemplateAfterVideo(); + } + } + + @Override + public void onVideoResult(@Nullable VideoResult.Stub result, @Nullable Exception exception) { + super.onVideoResult(result, exception); + maybeRestorePreviewTemplateAfterVideo(); + } + + /** + * Some video recorders might change the camera template to {@link CameraDevice#TEMPLATE_RECORD}. + * After the video is taken, we should restore the template preview, which also means that + * we'll remove any extra surface target that was added by the video recorder. + * + * This method avoids doing this twice by checking the request tag, as set by + * the {@link #createRepeatingRequestBuilder(int)} method. + */ + private void maybeRestorePreviewTemplateAfterVideo() { + int template = (int) mRepeatingRequest.getTag(); + if (template != CameraDevice.TEMPLATE_PREVIEW) { try { createRepeatingRequestBuilder(CameraDevice.TEMPLATE_PREVIEW); addRepeatingRequestBuilderSurfaces(); @@ -741,7 +766,6 @@ protected void onStopVideo() { throw createCameraException(e); } } - super.onStopVideo(); } //endregion @@ -1049,12 +1073,11 @@ public void onImageAvailable(ImageReader reader) { return; } image.close(); - if (getEngineState() == STATE_STARTED) { + if (getPreviewState() == STATE_STARTED) { + // After preview, the frame manager is correctly set up Frame frame = getFrameManager().getFrame(data, System.currentTimeMillis(), - getAngles().offset(Reference.SENSOR, Reference.OUTPUT, Axis.RELATIVE_TO_SENSOR), - mFrameProcessingSize, - FRAME_PROCESSING_FORMAT); + getAngles().offset(Reference.SENSOR, Reference.OUTPUT, Axis.RELATIVE_TO_SENSOR)); mCallback.dispatchFrame(frame); } else { getFrameManager().onBufferUnused(data); 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 9e06934a1..6e5c87f56 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/CameraEngine.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/engine/CameraEngine.java @@ -133,7 +133,7 @@ public interface Callback { void dispatchOnFocusEnd(@Nullable Gesture trigger, boolean success, @NonNull PointF where); void dispatchOnZoomChanged(final float newValue, @Nullable final PointF[] fingers); void dispatchOnExposureCorrectionChanged(float newValue, @NonNull float[] bounds, @Nullable PointF[] fingers); - void dispatchFrame(Frame frame); + void dispatchFrame(@NonNull Frame frame); void dispatchError(CameraException exception); void dispatchOnVideoRecordingStart(); void dispatchOnVideoRecordingEnd(); diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/frame/Frame.java b/cameraview/src/main/java/com/otaliastudios/cameraview/frame/Frame.java index f8607b863..cb9a9d389 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/frame/Frame.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/frame/Frame.java @@ -4,7 +4,6 @@ import com.otaliastudios.cameraview.size.Size; import androidx.annotation.NonNull; -import androidx.annotation.VisibleForTesting; /** * A preview frame to be processed by {@link FrameProcessor}s. @@ -14,7 +13,7 @@ public class Frame { private final static String TAG = Frame.class.getSimpleName(); private final static CameraLogger LOG = CameraLogger.create(TAG); - @VisibleForTesting FrameManager mManager; + private final FrameManager mManager; private byte[] mData = null; private long mTime = -1; @@ -27,28 +26,29 @@ public class Frame { mManager = manager; } + void setContent(@NonNull byte[] data, long time, int rotation, @NonNull Size size, int format) { + this.mData = data; + this.mTime = time; + this.mLastTime = time; + this.mRotation = rotation; + this.mSize = size; + this.mFormat = format; + } + @SuppressWarnings("BooleanMethodIsAlwaysInverted") - private boolean isAlive() { + private boolean hasContent() { return mData != null; } - private void ensureAlive() { - if (!isAlive()) { + private void ensureHasContent() { + if (!hasContent()) { LOG.e("Frame is dead! time:", mTime, "lastTime:", mLastTime); throw new RuntimeException("You should not access a released frame. " + - "If this frame was passed to a FrameProcessor, you can only use its contents synchronously," + + "If this frame was passed to a FrameProcessor, you can only use its contents synchronously, " + "for the duration of the process() method."); } } - void set(@NonNull byte[] data, long time, int rotation, @NonNull Size size, int format) { - this.mData = data; - this.mTime = time; - this.mLastTime = time; - this.mRotation = rotation; - this.mSize = size; - this.mFormat = format; - } @Override public boolean equals(Object obj) { @@ -63,14 +63,13 @@ public boolean equals(Object obj) { * * @return a frozen Frame */ - @SuppressWarnings("WeakerAccess") @NonNull public Frame freeze() { - ensureAlive(); + ensureHasContent(); byte[] data = new byte[mData.length]; System.arraycopy(mData, 0, data, 0, mData.length); Frame other = new Frame(mManager); - other.set(data, mTime, mRotation, mSize, mFormat); + other.setContent(data, mTime, mRotation, mSize, mFormat); return other; } @@ -79,23 +78,18 @@ public Frame freeze() { * that are not useful anymore. */ public void release() { - if (!isAlive()) return; - LOG.v("Frame with time", mTime, "is being released. Has manager:", mManager != null); - - if (mManager != null) { - // If needed, the manager will call releaseManager on us. - mManager.onFrameReleased(this); - } + if (!hasContent()) return; + LOG.v("Frame with time", mTime, "is being released."); + byte[] data = mData; mData = null; mRotation = 0; mTime = -1; mSize = null; mFormat = -1; - } - - // Once this is called, this instance is not usable anymore. - void releaseManager() { - mManager = null; + // After the manager is notified, this frame instance can be taken by + // someone else, possibly from another thread. So this should be the + // last call in this method. If we null data after, we can have issues. + mManager.onFrameReleased(this, data); } /** @@ -104,7 +98,7 @@ void releaseManager() { */ @NonNull public byte[] getData() { - ensureAlive(); + ensureHasContent(); return mData; } @@ -115,7 +109,7 @@ public byte[] getData() { * @return time data */ public long getTime() { - ensureAlive(); + ensureHasContent(); return mTime; } @@ -127,7 +121,7 @@ public long getTime() { * @return clock-wise rotation */ public int getRotation() { - ensureAlive(); + ensureHasContent(); return mRotation; } @@ -138,7 +132,7 @@ public int getRotation() { */ @NonNull public Size getSize() { - ensureAlive(); + ensureHasContent(); return mSize; } @@ -151,7 +145,7 @@ public Size getSize() { * @see android.graphics.ImageFormat */ public int getFormat() { - ensureAlive(); + ensureHasContent(); return mFormat; } } diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/frame/FrameManager.java b/cameraview/src/main/java/com/otaliastudios/cameraview/frame/FrameManager.java index de1b8dd99..0ab689471 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/frame/FrameManager.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/frame/FrameManager.java @@ -1,6 +1,8 @@ package com.otaliastudios.cameraview.frame; +import android.graphics.ImageFormat; + import com.otaliastudios.cameraview.CameraLogger; import com.otaliastudios.cameraview.size.Size; @@ -21,7 +23,7 @@ * Main methods are: * - {@link #setUp(int, Size)}: to set up with size and allocate buffers * - {@link #release()}: to release. After release, a manager can be setUp again. - * - {@link #getFrame(byte[], long, int, Size, int)}: gets a new {@link Frame}. + * - {@link #getFrame(byte[], long, int)}: gets a new {@link Frame}. * * For both byte buffers and frames to get back to the FrameManager pool, all you have to do * is call {@link Frame#release()} when done. @@ -31,12 +33,12 @@ * * 1. {@link #BUFFER_MODE_DISPATCH}: in this mode, as soon as we have a buffer, it is dispatched to * the {@link BufferCallback}. The callback should then fill the buffer, and finally call - * {@link #getFrame(byte[], long, int, Size, int)} to receive a frame. + * {@link #getFrame(byte[], long, int)} to receive a frame. * This is used for Camera1. * * 2. {@link #BUFFER_MODE_ENQUEUE}: in this mode, the manager internally keeps a queue of byte buffers, * instead of handing them to the callback. The users can ask for buffers through {@link #getBuffer()}. - * This buffer can be filled with data and used to get a frame {@link #getFrame(byte[], long, int, Size, int)}, + * This buffer can be filled with data and used to get a frame {@link #getFrame(byte[], long, int)}, * or, in case it was not filled, returned to the queue using {@link #onBufferUnused(byte[])}. * This is used for Camera2. */ @@ -55,6 +57,8 @@ public interface BufferCallback { private final int mPoolSize; private int mBufferSize = -1; + private Size mFrameSize = null; + private int mFrameFormat = -1; private LinkedBlockingQueue mFrameQueue; private LinkedBlockingQueue mBufferQueue; private BufferCallback mBufferCallback; @@ -94,17 +98,22 @@ public FrameManager(int poolSize, @Nullable BufferCallback callback) { /** * Allocates a {@link #mPoolSize} number of buffers. Should be called once - * the preview size and the bitsPerPixel value are known. + * the preview size and the image format value are known. * * This method can be called again after {@link #release()} has been called. * - * @param bitsPerPixel bits per pixel, depends on image format - * @param previewSize the preview size + * @param format the image format + * @param size the frame size * @return the buffer size */ - public int setUp(int bitsPerPixel, @NonNull Size previewSize) { - // TODO throw if called twice without release? - long sizeInBits = previewSize.getHeight() * previewSize.getWidth() * bitsPerPixel; + public int setUp(int format, @NonNull Size size) { + if (isSetUp()) { + // TODO throw or just reconfigure? + } + mFrameSize = size; + mFrameFormat = format; + int bitsPerPixel = ImageFormat.getBitsPerPixel(format); + long sizeInBits = size.getHeight() * size.getWidth() * bitsPerPixel; mBufferSize = (int) Math.ceil(sizeInBits / 8.0d); for (int i = 0; i < mPoolSize; i++) { if (mBufferMode == BUFFER_MODE_DISPATCH) { @@ -116,13 +125,24 @@ public int setUp(int bitsPerPixel, @NonNull Size previewSize) { return mBufferSize; } + /** + * Returns true after {@link #setUp(int, Size)} + * but before {@link #release()}. + * Returns false otherwise. + * + * @return true if set up + */ + private boolean isSetUp() { + return mFrameSize != null; + } + /** * Returns a new byte buffer than can be filled. * This can only be called in {@link #BUFFER_MODE_ENQUEUE} mode! Where the frame * manager also holds a queue of the byte buffers. * * If not null, the buffer returned by this method can be filled and used to get - * a new frame through {@link #getFrame(byte[], long, int, Size, int)}. + * a new frame through {@link #getFrame(byte[], long, int)}. * * @return a buffer, or null */ @@ -143,7 +163,12 @@ public void onBufferUnused(@NonNull byte[] buffer) { if (mBufferMode != BUFFER_MODE_ENQUEUE) { throw new IllegalStateException("Can't call onBufferUnused() when not in BUFFER_MODE_ENQUEUE."); } - mBufferQueue.offer(buffer); + + if (isSetUp()) { + mBufferQueue.offer(buffer); + } else { + LOG.w("onBufferUnused: buffer was returned but we're not set up anymore."); + } } /** @@ -158,53 +183,38 @@ public void onBufferUnused(@NonNull byte[] buffer) { * @param data data * @param time timestamp * @param rotation rotation - * @param previewSize preview size - * @param previewFormat format * @return a new frame */ @NonNull - public Frame getFrame(@NonNull byte[] data, long time, int rotation, @NonNull Size previewSize, int previewFormat) { + public Frame getFrame(@NonNull byte[] data, long time, int rotation) { + if (!isSetUp()) { + throw new IllegalStateException("Can't call getFrame() after releasing or before setUp."); + } + Frame frame = mFrameQueue.poll(); if (frame != null) { - LOG.v("getFrame for time:", time, "RECYCLING.", "Data:", data != null); + LOG.v("getFrame for time:", time, "RECYCLING."); } else { - LOG.v("getFrame for time:", time, "CREATING.", "Data:", data != null); + LOG.v("getFrame for time:", time, "CREATING."); frame = new Frame(this); } - frame.set(data, time, rotation, previewSize, previewFormat); + frame.setContent(data, time, rotation, mFrameSize, mFrameFormat); return frame; } - /** - * Releases all frames controlled by this manager and - * clears the pool. - * In BUFFER_MODE_ENQUEUE, releases also all the buffers. - */ - public void release() { - LOG.w("Releasing all frames!"); - for (Frame frame : mFrameQueue) { - frame.releaseManager(); - frame.release(); - } - mFrameQueue.clear(); - if (mBufferMode == BUFFER_MODE_ENQUEUE) { - mBufferQueue.clear(); - } - mBufferSize = -1; - } - /** * Called by child frames when they are released. + * This might be called from old Frames that belong to an old 'setUp' + * of this FrameManager instance. So the buffer size might be different, + * for instance. + * * @param frame the released frame */ - void onFrameReleased(@NonNull Frame frame) { - byte[] buffer = frame.getData(); - boolean willRecycle = mFrameQueue.offer(frame); - if (!willRecycle) { - // If frame queue is full, let's drop everything. - frame.releaseManager(); - } else { - // If frame will be recycled, let's recycle the buffer as well. + void onFrameReleased(@NonNull Frame frame, @NonNull byte[] buffer) { + if (!isSetUp()) return; + // If frame queue is full, let's drop everything. + // If frame queue accepts this frame, let's recycle the buffer as well. + if (mFrameQueue.offer(frame)) { int currSize = buffer.length; int reqSize = mBufferSize; if (currSize == reqSize) { @@ -216,4 +226,25 @@ void onFrameReleased(@NonNull Frame frame) { } } } + + /** + * Releases all frames controlled by this manager and + * clears the pool. + * In BUFFER_MODE_ENQUEUE, releases also all the buffers. + */ + public void release() { + if (!isSetUp()) { + LOG.w("release called twice. Ignoring."); + return; + } + + LOG.i("release: Clearing the frame and buffer queue."); + mFrameQueue.clear(); + if (mBufferMode == BUFFER_MODE_ENQUEUE) { + mBufferQueue.clear(); + } + mBufferSize = -1; + mFrameSize = null; + mFrameFormat = -1; + } } diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/picture/Snapshot1PictureRecorder.java b/cameraview/src/main/java/com/otaliastudios/cameraview/picture/Snapshot1PictureRecorder.java index c04048210..606ecaaa1 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/picture/Snapshot1PictureRecorder.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/picture/Snapshot1PictureRecorder.java @@ -87,7 +87,7 @@ public void run() { // It seems that the buffers are already cleared here, so we need to allocate again. camera.setPreviewCallbackWithBuffer(null); // Release anything left camera.setPreviewCallbackWithBuffer(mEngine1); // Add ourselves - mEngine1.getFrameManager().setUp(ImageFormat.getBitsPerPixel(mFormat), previewStreamSize); + mEngine1.getFrameManager().setUp(mFormat, previewStreamSize); } }); } diff --git a/cameraview/src/test/java/com/otaliastudios/cameraview/frame/FrameTest.java b/cameraview/src/test/java/com/otaliastudios/cameraview/frame/FrameTest.java index b9fea96f1..3f31548b5 100644 --- a/cameraview/src/test/java/com/otaliastudios/cameraview/frame/FrameTest.java +++ b/cameraview/src/test/java/com/otaliastudios/cameraview/frame/FrameTest.java @@ -15,6 +15,8 @@ import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNull; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -35,23 +37,24 @@ public void tearDown() { @Test public void testEquals() { + // Only time should count. Frame f1 = new Frame(manager); long time = 1000; - f1.set(null, time, 90, null, ImageFormat.NV21); + f1.setContent(new byte[3], time, 90, new Size(5, 5), ImageFormat.NV21); Frame f2 = new Frame(manager); - f2.set(new byte[2], time, 0, new Size(10, 10), ImageFormat.NV21); + f2.setContent(new byte[2], time, 0, new Size(10, 10), ImageFormat.NV21); assertEquals(f1, f2); - f2.set(new byte[2], time + 1, 0, new Size(10, 10), ImageFormat.NV21); + f2.setContent(new byte[2], time + 1, 0, new Size(10, 10), ImageFormat.NV21); assertNotEquals(f1, f2); } @Test public void testReleaseThrows() { final Frame frame = new Frame(manager); - frame.set(new byte[2], 1000, 90, new Size(10, 10), ImageFormat.NV21); + frame.setContent(new byte[2], 1000, 90, new Size(10, 10), ImageFormat.NV21); frame.release(); - verify(manager, times(1)).onFrameReleased(frame); + verify(manager, times(1)).onFrameReleased(eq(frame), any(byte[].class)); assertThrows(new Runnable() { public void run() { frame.getTime(); }}); assertThrows(new Runnable() { public void run() { frame.getFormat(); }}); @@ -69,14 +72,6 @@ private void assertThrows(Runnable runnable) { } } - @Test - public void testReleaseManager() { - Frame frame = new Frame(manager); - assertNotNull(frame.mManager); - frame.releaseManager(); - assertNull(frame.mManager); - } - @Test public void testFreeze() { Frame frame = new Frame(manager); @@ -85,7 +80,7 @@ public void testFreeze() { int rotation = 90; Size size = new Size(10, 10); int format = ImageFormat.NV21; - frame.set(data, time, rotation, size, format); + frame.setContent(data, time, rotation, size, format); Frame frozen = frame.freeze(); assertArrayEquals(data, frozen.getData()); @@ -94,7 +89,7 @@ public void testFreeze() { assertEquals(size, frozen.getSize()); // Mutate the first, ensure that frozen is not affected - frame.set(new byte[]{3, 2, 1}, 50, 180, new Size(1, 1), ImageFormat.JPEG); + frame.setContent(new byte[]{3, 2, 1}, 50, 180, new Size(1, 1), ImageFormat.JPEG); assertArrayEquals(data, frozen.getData()); assertEquals(time, frozen.getTime()); assertEquals(rotation, frozen.getRotation()); diff --git a/docs/_posts/2018-12-20-frame-processing.md b/docs/_posts/2018-12-20-frame-processing.md index 08a7d02d9..864f441e4 100644 --- a/docs/_posts/2018-12-20-frame-processing.md +++ b/docs/_posts/2018-12-20-frame-processing.md @@ -18,7 +18,7 @@ a QR code detector, the cameraView.addFrameProcessor(new FrameProcessor() { @Override @WorkerThread - public void process(Frame frame) { + public void process(@NonNull Frame frame) { byte[] data = frame.getData(); int rotation = frame.getRotation(); long time = frame.getTime(); @@ -33,9 +33,45 @@ For your convenience, the `FrameProcessor` method is run in a background thread in a synchronous fashion. Once the process method returns, internally we will re-use the `Frame` instance and apply new data to it. So: -- you can do your job synchronously in the `process()` method +- you can do your job synchronously in the `process()` method. This is **recommended**. - if you must hold the `Frame` instance longer, use `frame = frame.freeze()` to get a frozen instance - that will not be affected + that will not be affected. This is **discouraged** because it requires copying the whole array. + +### Process synchronously + +Processing synchronously, for the duration of the `process()` method, is the recommended way of using +processors, because it solves different issues: + +- avoids the need of calling `frame = frame.freeze()` which is a very expensive operation +- the engine will **automatically drop frames** if the `process()` method is busy, so you'll only receive frames that you can handle +- we have already allocated a thread for you, so there's no need to create another + +Some frame consumers might have a built-in asynchronous behavior. +But you can still block the `process()` thread until the consumer has returned. + +```java +@Override +@WorkerThread +public void process(@NonNull Frame frame) { + + // EXAMPLE 1: + // Firebase and Google APIs will often return a Task. + // You can use Tasks.await() to complete the task on the current thread. + Tasks.await(firebaseDetector.detectInImage(firebaseImage)); + + // EXAMPLE 2: + // For other async consumers, you can use, for example, a CountDownLatch. + + // Step 1: create the latch. + final CountDownLatch latch = new CountDownLatch(1); + + // Step 2: launch async processing here... + // When processing completes or fails, call latch.countDown(); + + // Step 3: after launching, block the current thread. + latch.await(); +} +``` ### Related APIs