diff --git a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/frame/ByteBufferFrameManagerTest.java b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/frame/ByteBufferFrameManagerTest.java index d3f1242f..78c6321d 100644 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/frame/ByteBufferFrameManagerTest.java +++ b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/frame/ByteBufferFrameManagerTest.java @@ -14,6 +14,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import static org.junit.Assert.assertNotNull; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -56,9 +57,11 @@ public class ByteBufferFrameManagerTest extends BaseTest { int length = manager.getFrameBytes(); 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); - // Release the first frame so it goes back into the pool. + assertNotNull(frame1); + // Since frame1 is already taken and poolSize = 1, getFrame() would return null. + // To create a new frame, freeze the first one. + Frame frame2 = frame1.freeze(); + // Now release the first frame so it goes back into the pool. manager.onFrameReleased(frame1, (byte[]) frame1.getData()); reset(callback); // Release the second. The pool is already full, so onBufferAvailable should not be called @@ -76,6 +79,7 @@ public class ByteBufferFrameManagerTest extends BaseTest { // A camera preview frame comes. Request a frame. byte[] picture = new byte[length]; Frame frame = manager.getFrame(picture, 0, 0); + assertNotNull(frame); // Release the frame and ensure that onBufferAvailable is called. reset(callback); @@ -92,6 +96,7 @@ public class ByteBufferFrameManagerTest extends BaseTest { // A camera preview frame comes. Request a frame. byte[] picture = new byte[length]; Frame frame = manager.getFrame(picture, 0, 0); + assertNotNull(frame); // Don't release the frame. Change the allocation size. manager.setUp(ImageFormat.NV16, new Size(15, 15)); diff --git a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/frame/FrameManagerTest.java b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/frame/FrameManagerTest.java index b553e145..de8aa6a7 100644 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/frame/FrameManagerTest.java +++ b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/frame/FrameManagerTest.java @@ -14,8 +14,12 @@ import org.junit.Test; import org.junit.runner.RunWith; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; @RunWith(AndroidJUnit4.class) @@ -38,9 +42,31 @@ public class FrameManagerTest extends BaseTest { manager.setUp(ImageFormat.NV21, new Size(50, 50)); Frame first = manager.getFrame("foo", 0, 0); + assertNotNull(first); first.release(); Frame second = manager.getFrame("bar", 0, 0); + assertNotNull(second); second.release(); assertEquals(first, second); } + + @Test + public void testGetFrame() { + FrameManager manager = new FrameManager(1, String.class) { + @Override + protected void onFrameDataReleased(@NonNull String data, boolean recycled) { } + + @NonNull + @Override + protected String onCloneFrameData(@NonNull String data) { + return data; + } + }; + manager.setUp(ImageFormat.NV21, new Size(50, 50)); + + Frame first = manager.getFrame("foo", 0, 0); + assertNotNull(first); + Frame second = manager.getFrame("bar", 0, 0); + assertNull(second); + } } diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java b/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java index 493bb396..bbf9d636 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/CameraView.java @@ -2223,7 +2223,7 @@ public class CameraView extends FrameLayout implements LifecycleObserver { mFrameProcessingExecutor.execute(new Runnable() { @Override public void run() { - LOG.v("dispatchFrame: dispatching", frame.getTime(), + LOG.v("dispatchFrame: executing. Passing", frame.getTime(), "to processors."); for (FrameProcessor processor : mFrameProcessors) { try { 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 e96867d6..02553017 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera1Engine.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera1Engine.java @@ -793,10 +793,12 @@ public class Camera1Engine extends CameraBaseEngine implements // Seen this happen in logs. return; } - Frame frame = getFrameManager().getFrame(data, - System.currentTimeMillis(), - getAngles().offset(Reference.SENSOR, Reference.OUTPUT, Axis.RELATIVE_TO_SENSOR)); - getCallback().dispatchFrame(frame); + int rotation = getAngles().offset(Reference.SENSOR, Reference.OUTPUT, + Axis.RELATIVE_TO_SENSOR); + Frame frame = getFrameManager().getFrame(data, System.currentTimeMillis(), rotation); + if (frame != null) { + getCallback().dispatchFrame(frame); + } } //endregion 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 5fe71e99..afaf8839 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera2Engine.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera2Engine.java @@ -539,11 +539,19 @@ public class Camera2Engine extends CameraBaseEngine implements // 4. FRAME PROCESSING if (hasFrameProcessors()) { mFrameProcessingSize = computeFrameProcessingSize(); + // Hard to write down why, but in Camera2 we need a number of Frames that's one less + // than the number of Images. If we let all Images be part of Frames, thus letting all + // Images be used by processor at any given moment, the Camera2 output breaks. + // In fact, if there are no Images available, the sensor BLOCKS until it finds one, + // which is a big issue because processor times become a bottleneck for the preview. + // This is a design flaw in the ImageReader / sensor implementation, as they should + // simply DROP frames written to the surface if there are no Images available. + // Since this is not how things work, we ensure that one Image is always available here. mFrameProcessingReader = ImageReader.newInstance( mFrameProcessingSize.getWidth(), mFrameProcessingSize.getHeight(), mFrameProcessingFormat, - getFrameProcessingPoolSize()); + getFrameProcessingPoolSize() + 1); mFrameProcessingReader.setOnImageAvailableListener(this, null); mFrameProcessingSurface = mFrameProcessingReader.getSurface(); @@ -1407,13 +1415,13 @@ public class Camera2Engine extends CameraBaseEngine implements @EngineThread @Override public void onImageAvailable(ImageReader reader) { - LOG.v("onImageAvailable", "trying to acquire Image."); + LOG.v("onImageAvailable:", "trying to acquire Image."); Image image = null; try { image = reader.acquireLatestImage(); } catch (Exception ignore) { } if (image == null) { - LOG.w("onImageAvailable", "failed to acquire Image!"); + LOG.w("onImageAvailable:", "failed to acquire Image!"); } else if (getState() == CameraState.PREVIEW && !isChangingState()) { // After preview, the frame manager is correctly set up //noinspection unchecked @@ -1422,8 +1430,14 @@ public class Camera2Engine extends CameraBaseEngine implements getAngles().offset(Reference.SENSOR, Reference.OUTPUT, Axis.RELATIVE_TO_SENSOR)); - getCallback().dispatchFrame(frame); + if (frame != null) { + LOG.v("onImageAvailable:", "Image acquired, dispatching."); + getCallback().dispatchFrame(frame); + } else { + LOG.i("onImageAvailable:", "Image acquired, but no free frames. DROPPING."); + } } else { + LOG.i("onImageAvailable:", "Image acquired in wrong state. Closing it now."); image.close(); } } diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/action/LogAction.java b/cameraview/src/main/java/com/otaliastudios/cameraview/engine/action/LogAction.java index 1cb53316..86f36c38 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/action/LogAction.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/engine/action/LogAction.java @@ -9,15 +9,13 @@ import androidx.annotation.NonNull; import androidx.annotation.RequiresApi; import com.otaliastudios.cameraview.CameraLogger; -import com.otaliastudios.cameraview.engine.Camera2Engine; -import com.otaliastudios.cameraview.engine.action.ActionHolder; -import com.otaliastudios.cameraview.engine.action.BaseAction; +import com.otaliastudios.cameraview.engine.CameraEngine; @RequiresApi(Build.VERSION_CODES.LOLLIPOP) public class LogAction extends BaseAction { private final static CameraLogger LOG - = CameraLogger.create(Camera2Engine.class.getSimpleName()); + = CameraLogger.create(CameraEngine.class.getSimpleName()); private String lastLog; 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 65c095fe..31973b10 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/frame/FrameManager.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/frame/FrameManager.java @@ -7,6 +7,7 @@ import com.otaliastudios.cameraview.CameraLogger; import com.otaliastudios.cameraview.size.Size; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import java.util.concurrent.LinkedBlockingQueue; @@ -92,6 +93,9 @@ public abstract class FrameManager { int bitsPerPixel = ImageFormat.getBitsPerPixel(format); long sizeInBits = size.getHeight() * size.getWidth() * bitsPerPixel; mFrameBytes = (int) Math.ceil(sizeInBits / 8.0d); + for (int i = 0; i < getPoolSize(); i++) { + mFrameQueue.offer(new Frame(this)); + } } /** @@ -108,18 +112,14 @@ public abstract class FrameManager { /** * Returns a new Frame for the given data. This must be called * - after {@link #setUp(int, Size)}, which sets the buffer size - * - after the byte buffer given by setUp() 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. + * - after the T data has been filled * * @param data data * @param time timestamp * @param rotation rotation * @return a new frame */ - @NonNull + @Nullable public Frame getFrame(@NonNull T data, long time, int rotation) { if (!isSetUp()) { throw new IllegalStateException("Can't call getFrame() after releasing " + @@ -129,12 +129,13 @@ public abstract class FrameManager { Frame frame = mFrameQueue.poll(); if (frame != null) { LOG.v("getFrame for time:", time, "RECYCLING."); + frame.setContent(data, time, rotation, mFrameSize, mFrameFormat); + return frame; } else { - LOG.v("getFrame for time:", time, "CREATING."); - frame = new Frame(this); + LOG.i("getFrame for time:", time, "NOT AVAILABLE."); + onFrameDataReleased(data, false); + return null; } - frame.setContent(data, time, rotation, mFrameSize, mFrameFormat); - return frame; } /**