From a31dd1d57c18307a9a20486a8e87473aa257f923 Mon Sep 17 00:00:00 2001 From: Mattia Iavarone Date: Mon, 6 Nov 2017 23:35:14 +0100 Subject: [PATCH] Recycle buffers and Frame instances (#94) --- .../cameraview/CameraCallbacksTest.java | 7 +- .../cameraview/IntegrationTest.java | 44 ++++++- .../cameraview/MockCameraController.java | 5 + .../com/otaliastudios/cameraview/Camera1.java | 29 +++- .../com/otaliastudios/cameraview/Camera2.java | 5 + .../cameraview/CameraController.java | 4 +- .../otaliastudios/cameraview/CameraView.java | 34 +++-- .../com/otaliastudios/cameraview/Frame.java | 20 ++- .../cameraview/FrameManager.java | 93 +++++++++++++ .../cameraview/FrameManagerTest.java | 124 ++++++++++++++++++ .../otaliastudios/cameraview/FrameTest.java | 38 +++++- 11 files changed, 364 insertions(+), 39 deletions(-) create mode 100644 cameraview/src/main/java/com/otaliastudios/cameraview/FrameManager.java create mode 100644 cameraview/src/test/java/com/otaliastudios/cameraview/FrameManagerTest.java diff --git a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraCallbacksTest.java b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraCallbacksTest.java index b6f4e871..e8d9f83b 100644 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraCallbacksTest.java +++ b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/CameraCallbacksTest.java @@ -300,10 +300,11 @@ public class CameraCallbacksTest extends BaseTest { @Test public void testProcessFrame() { - completeTask().when(processor).process(any(Frame.class)); - camera.mCameraCallbacks.dispatchFrame(new byte[]{0, 1, 2, 3}, 1000, 90, new Size(1, 1), 0); + Frame mock = mock(Frame.class); + completeTask().when(processor).process(mock); + camera.mCameraCallbacks.dispatchFrame(mock); assertNotNull(task.await(200)); - verify(processor, times(1)).process(any(Frame.class)); + verify(processor, times(1)).process(mock); } } diff --git a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/IntegrationTest.java b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/IntegrationTest.java index f67b05e3..9d196b5a 100644 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/IntegrationTest.java +++ b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/IntegrationTest.java @@ -508,19 +508,51 @@ public class IntegrationTest extends BaseTest { //region Frame Processing + 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(4, TimeUnit.SECONDS); + assertTrue(did); + } + @Test - public void testFrameProcessing() throws Exception { + public void testFrameProcessing_simple() throws Exception { FrameProcessor processor = mock(FrameProcessor.class); camera.addFrameProcessor(processor); + camera.start(); + waitForOpen(true); + assert30Frames(processor); + } + + @Test + public void testFrameProcessing_afterSnapshot() throws Exception { + FrameProcessor processor = mock(FrameProcessor.class); + camera.addFrameProcessor(processor); camera.start(); waitForOpen(true); - // Expect 30 frames - CountDownLatch latch = new CountDownLatch(30); - doCountDown(latch).when(processor).process(any(Frame.class)); - boolean did = latch.await(4, TimeUnit.SECONDS); - assertTrue(did); + // In Camera1, snapshots will clear the preview callback + // Ensure we restore correctly + camera.captureSnapshot(); + waitForPicture(true); + + assert30Frames(processor); + } + + @Test + public void testFrameProcessing_afterRestart() throws Exception { + FrameProcessor processor = mock(FrameProcessor.class); + camera.addFrameProcessor(processor); + camera.start(); + waitForOpen(true); + camera.stop(); + waitForClose(true); + camera.start(); + waitForOpen(true); + + assert30Frames(processor); } //endregion diff --git a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/MockCameraController.java b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/MockCameraController.java index dee10dab..daae144a 100644 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/MockCameraController.java +++ b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/MockCameraController.java @@ -134,4 +134,9 @@ public class MockCameraController extends CameraController { public void onSurfaceAvailable() { } + + @Override + public void onBufferAvailable(byte[] buffer) { + + } } diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/Camera1.java b/cameraview/src/main/java/com/otaliastudios/cameraview/Camera1.java index 50620430..64a3afed 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/Camera1.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/Camera1.java @@ -1,5 +1,6 @@ package com.otaliastudios.cameraview; +import android.graphics.ImageFormat; import android.graphics.PointF; import android.graphics.Rect; import android.graphics.SurfaceTexture; @@ -109,6 +110,11 @@ class Camera1 extends CameraController implements Camera.PreviewCallback { invertPreviewSizes ? mPreviewSize.getHeight() : mPreviewSize.getWidth(), invertPreviewSizes ? mPreviewSize.getWidth() : mPreviewSize.getHeight() ); + + mCamera.setPreviewCallbackWithBuffer(null); // This clears the buffers + mCamera.setPreviewCallbackWithBuffer(this); // Reset + mFrameManager.allocate(ImageFormat.getBitsPerPixel(mPreviewFormat), mPreviewSize); + LOG.i("onSurfaceChanged:", "Restarting preview."); mCamera.startPreview(); LOG.i("onSurfaceChanged:", "Restarted preview."); @@ -149,7 +155,9 @@ class Camera1 extends CameraController implements Camera.PreviewCallback { mCamera.setParameters(params); } - mCamera.setPreviewCallback(this); // Frame processing + mCamera.setPreviewCallbackWithBuffer(null); // Release anything left + mCamera.setPreviewCallbackWithBuffer(this); // Add ourselves + mFrameManager.allocate(ImageFormat.getBitsPerPixel(mPreviewFormat), mPreviewSize); LOG.i("setup:", "Starting preview with startPreview()."); mCamera.startPreview(); @@ -195,6 +203,8 @@ class Camera1 extends CameraController implements Camera.PreviewCallback { Exception error = null; LOG.i("onStop:", "About to clean up."); mHandler.get().removeCallbacks(mPostFocusResetRunnable); + mFrameManager.release(); + if (mCamera != null) { LOG.i("onStop:", "Clean up.", "Ending video?", mIsCapturingVideo); @@ -203,7 +213,7 @@ class Camera1 extends CameraController implements Camera.PreviewCallback { try { LOG.i("onStop:", "Clean up.", "Stopping preview."); mCamera.stopPreview(); - mCamera.setPreviewCallback(null); + mCamera.setPreviewCallbackWithBuffer(null); LOG.i("onStop:", "Clean up.", "Stopped preview."); } catch (Exception e) { LOG.w("onStop:", "Clean up.", "Exception while stopping preview."); @@ -243,6 +253,12 @@ class Camera1 extends CameraController implements Camera.PreviewCallback { return false; } + @Override + public void onBufferAvailable(byte[] buffer) { + if (isCameraAvailable()) { + mCamera.addCallbackBuffer(buffer); + } + } @Override void setSessionType(SessionType sessionType) { @@ -501,7 +517,11 @@ class Camera1 extends CameraController implements Camera.PreviewCallback { mIsCapturingImage = false; } }); - mCamera.setPreviewCallback(Camera1.this); + + // It seems that the buffers are already cleared here, so we need to allocate again. + mCamera.setPreviewCallbackWithBuffer(null); // Release anything left + mCamera.setPreviewCallbackWithBuffer(this); // Add ourselves + mFrameManager.allocate(ImageFormat.getBitsPerPixel(mPreviewFormat), mPreviewSize); mCamera.setPreviewCallbackWithBuffer(Camera1.this); } }); return true; @@ -509,11 +529,12 @@ class Camera1 extends CameraController implements Camera.PreviewCallback { @Override public void onPreviewFrame(byte[] data, Camera camera) { - mCameraCallbacks.dispatchFrame(data, + Frame frame = mFrameManager.getFrame(data, System.currentTimeMillis(), computeExifRotation(), mPreviewSize, mPreviewFormat); + mCameraCallbacks.dispatchFrame(frame); } @Override diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/Camera2.java b/cameraview/src/main/java/com/otaliastudios/cameraview/Camera2.java index 472b623c..c03e1423 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/Camera2.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/Camera2.java @@ -125,4 +125,9 @@ class Camera2 extends CameraController { boolean startAutoFocus(@Nullable Gesture gesture, PointF point) { return false; } + + @Override + public void onBufferAvailable(byte[] buffer) { + + } } diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraController.java b/cameraview/src/main/java/com/otaliastudios/cameraview/CameraController.java index cde43096..3a519552 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraController.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/CameraController.java @@ -8,7 +8,7 @@ import android.support.annotation.WorkerThread; import java.io.File; -abstract class CameraController implements CameraPreview.SurfaceCallback { +abstract class CameraController implements CameraPreview.SurfaceCallback, FrameManager.BufferCallback { private static final String TAG = CameraController.class.getSimpleName(); private static final CameraLogger LOG = CameraLogger.create(TAG); @@ -36,6 +36,7 @@ abstract class CameraController implements CameraPreview.SurfaceCallback { protected ExtraProperties mExtraProperties; protected CameraOptions mOptions; + protected FrameManager mFrameManager; protected int mDisplayOffset; protected int mDeviceOrientation; @@ -67,6 +68,7 @@ abstract class CameraController implements CameraPreview.SurfaceCallback { }); } }); + mFrameManager = new FrameManager(2, this); } void setPreview(CameraPreview cameraPreview) { diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java b/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java index b685da20..863fbed9 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java @@ -1353,7 +1353,7 @@ public class CameraView extends FrameLayout { void dispatchOnFocusEnd(@Nullable Gesture trigger, boolean success, PointF where); void dispatchOnZoomChanged(final float newValue, final PointF[] fingers); void dispatchOnExposureCorrectionChanged(float newValue, float[] bounds, PointF[] fingers); - void dispatchFrame(byte[] frame, long time, int rotation, Size size, int previewFormat); + void dispatchFrame(Frame frame); } private class Callbacks implements CameraCallbacks { @@ -1365,9 +1365,6 @@ public class CameraView extends FrameLayout { private Integer mDisplayOffset; private Integer mDeviceOrientation; - // Frame processing - private Frame mFrame; - Callbacks() {} @Override @@ -1606,21 +1603,22 @@ public class CameraView extends FrameLayout { } @Override - public void dispatchFrame(final byte[] frame, final long time, final int rotation, - final Size size, final int previewFormat) { - if (mFrameProcessors.isEmpty()) return; - mLogger.v("dispatchFrame", time, rotation, "processors:", mFrameProcessors.size()); - if (mFrame == null) mFrame = new Frame(); - mFrameProcessorsHandler.post(new Runnable() { - @Override - public void run() { - mFrame.set(frame, time, rotation, size, previewFormat); - for (FrameProcessor processor : mFrameProcessors) { - processor.process(mFrame); + public void dispatchFrame(final Frame frame) { + if (mFrameProcessors.isEmpty()) { + // Mark as released. This instance will be reused. + frame.release(); + } else { + mLogger.v("dispatchFrame:", frame.getTime(), "processors:", mFrameProcessors.size()); + mFrameProcessorsHandler.post(new Runnable() { + @Override + public void run() { + for (FrameProcessor processor : mFrameProcessors) { + processor.process(frame); + } + frame.release(); } - } - }); - + }); + } } } diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/Frame.java b/cameraview/src/main/java/com/otaliastudios/cameraview/Frame.java index f7bc677c..7df639c1 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/Frame.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/Frame.java @@ -1,17 +1,23 @@ package com.otaliastudios.cameraview; +import android.support.annotation.NonNull; + /** * A preview frame to be processed by {@link FrameProcessor}s. */ public class Frame { + /* for tests */ FrameManager mManager; + private byte[] mData = null; private long mTime = -1; private int mRotation = 0; private Size mSize = null; private int mFormat = -1; - Frame() {} + Frame(@NonNull FrameManager manager) { + mManager = manager; + } void set(byte[] data, long time, int rotation, Size size, int format) { this.mData = data; @@ -37,7 +43,7 @@ public class Frame { public Frame freeze() { byte[] data = new byte[mData.length]; System.arraycopy(mData, 0, data, 0, mData.length); - Frame other = new Frame(); + Frame other = new Frame(mManager); other.set(data, mTime, mRotation, mSize, mFormat); return other; } @@ -47,6 +53,11 @@ public class Frame { * that are not useful anymore. */ public void release() { + if (mManager != null) { + // If needed, the manager will call releaseManager on us. + mManager.onFrameReleased(this); + } + mData = null; mRotation = 0; mTime = -1; @@ -54,6 +65,11 @@ public class Frame { mFormat = -1; } + // Once this is called, this instance is not usable anymore. + void releaseManager() { + mManager = null; + } + /** * Returns the frame data. * @return the frame data diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/FrameManager.java b/cameraview/src/main/java/com/otaliastudios/cameraview/FrameManager.java new file mode 100644 index 00000000..d192de2a --- /dev/null +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/FrameManager.java @@ -0,0 +1,93 @@ +package com.otaliastudios.cameraview; + + +import java.util.concurrent.LinkedBlockingQueue; + +/** + * This class manages the allocation of buffers and Frame objects. + * We are interested in both recycling byte[] buffers so they are not allocated for each + * preview frame, and in recycling Frame instances (so we don't instantiate a lot). + * + * For this, we keep a mPoolSize integer that defines the size of instances to keep. + * Whether this does make sense, it depends on how slow the frame processors are. + * If they are very slow, it is possible that some frames will be skipped. + * + * - byte[] buffer pool: + * this is not kept here, because Camera1 internals already have one that we can't control, but + * it should be OK. The only thing we do is allocate mPoolSize buffers when requested. + * - Frame pool: + * We keep a list of mPoolSize recycled instances, to be reused when a new buffer is available. + */ +class FrameManager { + + interface BufferCallback { + void onBufferAvailable(byte[] buffer); + } + + private int mPoolSize; + private int mBufferSize; + private BufferCallback mCallback; + private LinkedBlockingQueue mQueue; + + FrameManager(int poolSize, BufferCallback callback) { + mPoolSize = poolSize; + mCallback = callback; + mQueue = new LinkedBlockingQueue<>(mPoolSize); + mBufferSize = -1; + } + + void release() { + for (Frame frame : mQueue) { + frame.releaseManager(); + frame.release(); + } + mQueue.clear(); + mBufferSize = -1; + } + + void onFrameReleased(Frame frame) { + byte[] buffer = frame.getData(); + boolean willRecycle = mQueue.offer(frame); + if (!willRecycle) { + frame.releaseManager(); + } + if (buffer != null && mCallback != null) { + int currSize = buffer.length; + int reqSize = mBufferSize; + if (currSize == reqSize) { + mCallback.onBufferAvailable(buffer); + } + } + } + + /** + * Returns a new Frame for the given data. This must be called + * - after {@link #allocate(int, Size)}, which sets the buffer size + * - after the byte buffer given by allocate() has been filled. + * If this is called X times in a row without releasing frames, it will allocate + * X frames and that's bad. Callers must wait for the preview buffer to be available. + * + * In Camera1, this is always respected thanks to its internals. + * + * @return a new frame + */ + Frame getFrame(byte[] data, long time, int rotation, Size previewSize, int previewFormat) { + Frame frame = mQueue.poll(); + if (frame == null) frame = new Frame(this); + frame.set(data, time, rotation, previewSize, previewFormat); + return frame; + } + + int allocate(int bitsPerPixel, Size previewSize) { + mBufferSize = getBufferSize(bitsPerPixel, previewSize); + for (int i = 0; i < mPoolSize; i++) { + mCallback.onBufferAvailable(new byte[mBufferSize]); + } + return mBufferSize; + } + + private int getBufferSize(int bitsPerPixel, Size previewSize) { + long sizeInBits = previewSize.getHeight() * previewSize.getWidth() * bitsPerPixel; + return (int) Math.ceil(sizeInBits / 8.0d) + 1; + } +} diff --git a/cameraview/src/test/java/com/otaliastudios/cameraview/FrameManagerTest.java b/cameraview/src/test/java/com/otaliastudios/cameraview/FrameManagerTest.java new file mode 100644 index 00000000..98959d3a --- /dev/null +++ b/cameraview/src/test/java/com/otaliastudios/cameraview/FrameManagerTest.java @@ -0,0 +1,124 @@ +package com.otaliastudios.cameraview; + + +import android.graphics.ImageFormat; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class FrameManagerTest { + + private FrameManager.BufferCallback callback; + + @Before + public void setUp() { + callback = mock(FrameManager.BufferCallback.class); + } + + @After + public void tearDown() { + callback = null; + } + + @Test + public void testAllocate() { + FrameManager manager = new FrameManager(1, callback); + manager.allocate(4, new Size(50, 50)); + verify(callback, times(1)).onBufferAvailable(any(byte[].class)); + reset(callback); + + manager = new FrameManager(5, callback); + manager.allocate(4, new Size(50, 50)); + verify(callback, times(5)).onBufferAvailable(any(byte[].class)); + } + + @Test + public void testFrameRecycling() { + // A 1-pool manager will always recycle the same frame. + FrameManager manager = new FrameManager(1, callback); + manager.allocate(4, new Size(50, 50)); + + Frame first = manager.getFrame(null, 0, 0, null, 0); + first.release(); + + Frame second = manager.getFrame(null, 0, 0, null, 0); + second.release(); + + assertEquals(first, second); + } + + @Test + public void testOnFrameReleased_nullBuffer() { + FrameManager manager = new FrameManager(1, callback); + manager.allocate(4, new Size(50, 50)); + reset(callback); + + Frame frame = manager.getFrame(null, 0, 0, null, 0); + manager.onFrameReleased(frame); + verify(callback, never()).onBufferAvailable(frame.getData()); + } + + @Test + public void testOnFrameReleased_sameLength() { + FrameManager manager = new FrameManager(1, callback); + int length = manager.allocate(4, 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); + + // Release the frame and ensure that onBufferAvailable is called. + reset(callback); + manager.onFrameReleased(frame); + verify(callback, times(1)).onBufferAvailable(picture); + } + + @Test + public void testOnFrameReleased_differentLength() { + FrameManager manager = new FrameManager(1, callback); + int length = manager.allocate(4, 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); + + // Don't release the frame. Change the allocation size. + manager.allocate(2, 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); + reset(callback); + verify(callback, never()).onBufferAvailable(picture); + } + + @Test + public void testRelease() { + FrameManager manager = new FrameManager(1, callback); + int length = manager.allocate(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.getData()); + assertNull(first.mManager); + } +} diff --git a/cameraview/src/test/java/com/otaliastudios/cameraview/FrameTest.java b/cameraview/src/test/java/com/otaliastudios/cameraview/FrameTest.java index ce3755da..13892402 100644 --- a/cameraview/src/test/java/com/otaliastudios/cameraview/FrameTest.java +++ b/cameraview/src/test/java/com/otaliastudios/cameraview/FrameTest.java @@ -3,8 +3,11 @@ package com.otaliastudios.cameraview; import android.graphics.ImageFormat; +import org.junit.After; +import org.junit.Before; import org.junit.Test; +import static junit.framework.Assert.assertNotNull; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -12,11 +15,27 @@ import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + public class FrameTest { + private FrameManager manager; + + @Before + public void setUp() { + manager = mock(FrameManager.class); + } + + @After + public void tearDown() { + manager = null; + } + @Test public void testDefaults() { - Frame frame = new Frame(); + Frame frame = new Frame(manager); assertEquals(frame.getTime(), -1); assertEquals(frame.getFormat(), -1); assertEquals(frame.getRotation(), 0); @@ -26,10 +45,10 @@ public class FrameTest { @Test public void testEquals() { - Frame f1 = new Frame(); + Frame f1 = new Frame(manager); long time = 1000; f1.set(null, time, 90, null, ImageFormat.NV21); - Frame f2 = new Frame(); + Frame f2 = new Frame(manager); f2.set(new byte[2], time, 0, new Size(10, 10), ImageFormat.NV21); assertEquals(f1, f2); @@ -39,7 +58,7 @@ public class FrameTest { @Test public void testRelease() { - Frame frame = new Frame(); + Frame frame = new Frame(manager); frame.set(new byte[2], 1000, 90, new Size(10, 10), ImageFormat.NV21); frame.release(); @@ -48,11 +67,20 @@ public class FrameTest { assertEquals(frame.getRotation(), 0); assertNull(frame.getData()); assertNull(frame.getSize()); + verify(manager, times(1)).onFrameReleased(frame); + } + + @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(); + Frame frame = new Frame(manager); byte[] data = new byte[]{0, 1, 5, 0, 7, 3, 4, 5}; long time = 1000; int rotation = 90;