Frame improvements (#572)

* Fix #544

* Improve Frames behavior and error messages

* Improve Frames documentation

* Fix tests

* Fix video crashes

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

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

@@ -136,6 +136,7 @@ public abstract class CameraIntegrationTest extends BaseTest {
         }
     }
 
+    @SuppressWarnings("StatementWithEmptyBody")
     private CameraOptions openSync(boolean expectSuccess) {
         camera.open();
         final Op<CameraOptions> open = new Op<>(true);
@@ -143,10 +144,11 @@ public abstract class CameraIntegrationTest extends BaseTest {
         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 abstract class CameraIntegrationTest extends BaseTest {
                 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 abstract class CameraIntegrationTest extends BaseTest {
         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 abstract class CameraIntegrationTest extends BaseTest {
         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 @@ 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(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 abstract class CameraIntegrationTest extends BaseTest {
         openSync(true);
         takeVideoSync(true,4000);
         waitForVideoResult(true);
+
         assert30Frames(processor);
     }
 

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.reset;
 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 class FrameManagerTest {
     @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 class FrameManagerTest {
     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 class FrameManagerTest {
     @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);
-    }
 }

cameraview/src/androidTest/java/com/otaliastudios/cameraview/gesture/ScrollGestureFinderTest.java

@@ -21,6 +21,8 @@ import static org.junit.Assert.assertTrue;
 @SmallTest
 public class ScrollGestureFinderTest extends GestureFinderTest<ScrollGestureFinder> {
 
+    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 class ScrollGestureFinderTest extends GestureFinderTest<ScrollGestureFind
         touchOp.listen();
         touchOp.start();
         onLayout().perform(swipeUp());
-        Gesture found = touchOp.await(500);
+        Gesture found = touchOp.await(WAIT);
         assertNull(found);
     }
 
@@ -50,7 +52,7 @@ public class ScrollGestureFinderTest extends GestureFinderTest<ScrollGestureFind
         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?

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

@@ -2097,28 +2097,30 @@ public class CameraView extends FrameLayout implements LifecycleObserver {
         }
 
         @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

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

@@ -210,7 +210,7 @@ public class Camera1Engine extends CameraEngine implements
 
         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 class Camera1Engine extends CameraEngine implements
     }
 
     @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);
     }
 

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

@@ -498,7 +498,7 @@ public class Camera2Engine extends CameraEngine implements ImageReader.OnImageAv
         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 @@ public class Camera2Engine extends CameraEngine implements ImageReader.OnImageAv
     }
 
     @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 @@ public class Camera2Engine extends CameraEngine implements ImageReader.OnImageAv
                 throw createCameraException(e);
             }
         }
-        super.onStopVideo();
     }
 
     //endregion
@@ -1049,12 +1073,11 @@ public class Camera2Engine extends CameraEngine implements ImageReader.OnImageAv
             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);

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

@@ -133,7 +133,7 @@ public abstract class CameraEngine implements
         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();

cameraview/src/main/java/com/otaliastudios/cameraview/frame/Frame.java

@@ -4,7 +4,6 @@ import com.otaliastudios.cameraview.CameraLogger;
 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 class Frame {
      *
      * @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 class Frame {
      * 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 @@ public class Frame {
      */
     @NonNull
     public byte[] getData() {
-        ensureAlive();
+        ensureHasContent();
         return mData;
     }
 
@@ -115,7 +109,7 @@ public class Frame {
      * @return time data
      */
     public long getTime() {
-        ensureAlive();
+        ensureHasContent();
         return mTime;
     }
 
@@ -127,7 +121,7 @@ public class Frame {
      * @return clock-wise rotation
      */
     public int getRotation() {
-        ensureAlive();
+        ensureHasContent();
         return mRotation;
     }
 
@@ -138,7 +132,7 @@ public class Frame {
      */
     @NonNull
     public Size getSize() {
-        ensureAlive();
+        ensureHasContent();
         return mSize;
     }
 
@@ -151,7 +145,7 @@ public class Frame {
      * @see android.graphics.ImageFormat
      */
     public int getFormat() {
-        ensureAlive();
+        ensureHasContent();
         return mFormat;
     }
 }

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 @@ import java.util.concurrent.LinkedBlockingQueue;
  * 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 @@ import java.util.concurrent.LinkedBlockingQueue;
  *
  * 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 class FrameManager {
 
     private final int mPoolSize;
     private int mBufferSize = -1;
+    private Size mFrameSize = null;
+    private int mFrameFormat = -1;
     private LinkedBlockingQueue<Frame> mFrameQueue;
     private LinkedBlockingQueue<byte[]> mBufferQueue;
     private BufferCallback mBufferCallback;
@@ -94,17 +98,22 @@ public class FrameManager {
 
     /**
      * 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 class FrameManager {
         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 class FrameManager {
         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 class FrameManager {
      * @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 @@ public class FrameManager {
             }
         }
     }
+
+    /**
+     * 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;
+    }
 }

cameraview/src/main/java/com/otaliastudios/cameraview/picture/Snapshot1PictureRecorder.java

@@ -87,7 +87,7 @@ public class Snapshot1PictureRecorder extends PictureRecorder {
                 // 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);
             }
         });
     }

cameraview/src/test/java/com/otaliastudios/cameraview/frame/FrameTest.java

@@ -15,6 +15,8 @@ import static org.junit.Assert.assertEquals;
 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 class FrameTest {
 
     @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 @@ public class FrameTest {
         }
     }
 
-    @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 class FrameTest {
         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 class FrameTest {
         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());

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