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 746036ea..b553e145 100644 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/frame/FrameManagerTest.java +++ b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/frame/FrameManagerTest.java @@ -10,17 +10,12 @@ 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.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.times; import static org.mockito.Mockito.verify; @RunWith(AndroidJUnit4.class) @@ -32,7 +27,13 @@ public class FrameManagerTest extends BaseTest { // A 1-pool manager will always recycle the same frame. FrameManager manager = new FrameManager(1, String.class) { @Override - protected void onFrameDataRecycled(@NonNull String data) { } + 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)); diff --git a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/internal/utils/ImageHelperTest.java b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/internal/utils/ImageHelperTest.java deleted file mode 100644 index a75f8438..00000000 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/internal/utils/ImageHelperTest.java +++ /dev/null @@ -1,100 +0,0 @@ -package com.otaliastudios.cameraview.internal.utils; - - -import android.graphics.Bitmap; -import android.graphics.BitmapFactory; -import android.graphics.Canvas; -import android.graphics.Color; -import android.graphics.ImageFormat; -import android.graphics.Paint; -import android.graphics.PorterDuff; -import android.graphics.Rect; -import android.graphics.YuvImage; -import android.media.Image; -import android.media.ImageReader; -import android.os.Handler; -import android.os.Looper; -import android.view.Surface; - -import androidx.annotation.NonNull; -import androidx.test.ext.junit.runners.AndroidJUnit4; -import androidx.test.filters.SmallTest; - -import com.otaliastudios.cameraview.BaseTest; -import com.otaliastudios.cameraview.tools.Op; -import com.otaliastudios.cameraview.tools.SdkExclude; - -import org.junit.Test; -import org.junit.runner.RunWith; - -import java.io.ByteArrayOutputStream; - -import static org.junit.Assert.assertNotNull; - -/** - * Starting from API 29, surface.lockCanvas() sets the surface format to RGBA_8888: - * https://github.com/aosp-mirror/platform_frameworks_base/blob/android10-release/core/jni/android_view_Surface.cpp#L215-L217 . - * For this reason, acquireLatestImage crashes because we requested a different format. - */ -@SdkExclude(minSdkVersion = 29) -@RunWith(AndroidJUnit4.class) -@SmallTest -public class ImageHelperTest extends BaseTest { - - @NonNull - private Image getImage() { - ImageReader reader = ImageReader.newInstance(100, 100, ImageFormat.YUV_420_888, 1); - Surface readerSurface = reader.getSurface(); - final Op imageOp = new Op<>(); - reader.setOnImageAvailableListener(new ImageReader.OnImageAvailableListener() { - @Override - public void onImageAvailable(ImageReader reader) { - Image image = reader.acquireLatestImage(); - if (image != null) imageOp.controller().end(image); - } - }, new Handler(Looper.getMainLooper())); - - // Write on reader surface. - Canvas readerCanvas = readerSurface.lockCanvas(null); - Paint paint = new Paint(Paint.ANTI_ALIAS_FLAG); - paint.setColor(Color.RED); - readerCanvas.drawColor(Color.TRANSPARENT, PorterDuff.Mode.MULTIPLY); - readerCanvas.drawCircle(50, 50, 50, paint); - readerSurface.unlockCanvasAndPost(readerCanvas); - - // Wait - Image image = imageOp.await(5000); - assertNotNull(image); - return image; - } - - @Test - public void testImage() { - Image image = getImage(); - int width = image.getWidth(); - int height = image.getHeight(); - int bitsPerPixel = ImageFormat.getBitsPerPixel(ImageFormat.NV21); - int sizeBits = width * height * bitsPerPixel; - int sizeBytes = (int) Math.ceil(sizeBits / 8.0d); - byte[] bytes = new byte[sizeBytes]; - ImageHelper.convertToNV21(image, bytes); - image.close(); - - // Read the image - YuvImage yuvImage = new YuvImage(bytes, ImageFormat.NV21, width, height, null); - ByteArrayOutputStream jpegStream = new ByteArrayOutputStream(); - yuvImage.compressToJpeg(new Rect(0, 0, width, height), 100, jpegStream); - byte[] jpegByteArray = jpegStream.toByteArray(); - Bitmap bitmap = BitmapFactory.decodeByteArray(jpegByteArray, 0, jpegByteArray.length); - assertNotNull(bitmap); - - // Wanted to do assertions on the color here but it doesn't work. There must be an issue - // with how we are drawing the image in this test, since in real camera, the algorithm works well. - // So for now let's just test that nothing crashes during this process. - // int color = bitmap.getPixel(bitmap.getWidth() - 1, bitmap.getHeight() - 1); - // assertEquals(Color.red(color), 255, 5); - // assertEquals(Color.green(color), 0, 5); - // assertEquals(Color.blue(color), 0, 5); - // assertEquals(Color.alpha(color), 0, 5); - } -} 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 ccb77355..af64078d 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera2Engine.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/engine/Camera2Engine.java @@ -56,12 +56,11 @@ import com.otaliastudios.cameraview.engine.offset.Axis; import com.otaliastudios.cameraview.engine.offset.Reference; import com.otaliastudios.cameraview.engine.options.Camera2Options; import com.otaliastudios.cameraview.engine.orchestrator.CameraState; -import com.otaliastudios.cameraview.frame.ByteBufferFrameManager; import com.otaliastudios.cameraview.frame.Frame; import com.otaliastudios.cameraview.frame.FrameManager; +import com.otaliastudios.cameraview.frame.ImageFrameManager; import com.otaliastudios.cameraview.gesture.Gesture; import com.otaliastudios.cameraview.internal.utils.CropHelper; -import com.otaliastudios.cameraview.internal.utils.ImageHelper; import com.otaliastudios.cameraview.internal.utils.WorkerHandler; import com.otaliastudios.cameraview.picture.Full2PictureRecorder; import com.otaliastudios.cameraview.picture.Snapshot2PictureRecorder; @@ -95,8 +94,6 @@ public class Camera2Engine extends CameraBaseEngine implements // Frame processing private ImageReader mFrameProcessingReader; // need this or the reader surface is collected - private final WorkerHandler mFrameConversionHandler; - private final Object mFrameProcessingImageLock = new Object(); private Surface mFrameProcessingSurface; // Preview @@ -118,7 +115,6 @@ public class Camera2Engine extends CameraBaseEngine implements public Camera2Engine(Callback callback) { super(callback); mManager = (CameraManager) getCallback().getContext().getSystemService(Context.CAMERA_SERVICE); - mFrameConversionHandler = WorkerHandler.get("CameraFrameConversion"); new LogAction().start(this); } @@ -546,9 +542,8 @@ public class Camera2Engine extends CameraBaseEngine implements mFrameProcessingSize.getWidth(), mFrameProcessingSize.getHeight(), getFrameProcessingImageFormat(), - 2); - mFrameProcessingReader.setOnImageAvailableListener(this, - mFrameConversionHandler.getHandler()); + 3); + mFrameProcessingReader.setOnImageAvailableListener(this, null); mFrameProcessingSurface = mFrameProcessingReader.getSurface(); outputSurfaces.add(mFrameProcessingSurface); } else { @@ -682,12 +677,9 @@ public class Camera2Engine extends CameraBaseEngine implements mCaptureSize = null; mFrameProcessingSize = null; if (mFrameProcessingReader != null) { - synchronized (mFrameProcessingImageLock) { - // This call synchronously releases all Images and their underlying properties. - // This can cause a segmentation fault while converting the Image to NV21. - // So we use this lock for the two operations. - mFrameProcessingReader.close(); - } + // WARNING: This call synchronously releases all Images and their underlying + // properties. This can cause issues if the Image is being used. + mFrameProcessingReader.close(); mFrameProcessingReader = null; } if (mPictureReader != null) { @@ -1407,13 +1399,7 @@ public class Camera2Engine extends CameraBaseEngine implements @NonNull @Override protected FrameManager instantiateFrameManager() { - return new ByteBufferFrameManager(2, null); - } - - @NonNull - @Override - public ByteBufferFrameManager getFrameManager() { - return (ByteBufferFrameManager) super.getFrameManager(); + return new ImageFrameManager(2); } /** @@ -1429,42 +1415,24 @@ public class Camera2Engine extends CameraBaseEngine implements @Override public void onImageAvailable(ImageReader reader) { - byte[] data = getFrameManager().getBuffer(); - if (data == null) { - LOG.w("onImageAvailable", "no byte buffer!"); - return; - } LOG.v("onImageAvailable", "trying to acquire Image."); Image image = null; try { image = reader.acquireLatestImage(); } catch (Exception ignore) { } if (image == null) { - LOG.w("onImageAvailable", "we have a byte buffer but no Image!"); - getFrameManager().onBufferUnused(data); - return; - } - LOG.v("onImageAvailable", "we have both a byte buffer and an Image."); - try { - synchronized (mFrameProcessingImageLock) { - ImageHelper.convertToNV21(image, data); - } - } catch (Exception e) { - LOG.w("onImageAvailable", "error while converting."); - getFrameManager().onBufferUnused(data); - image.close(); - return; - } - image.close(); - if (getState() == CameraState.PREVIEW && !isChangingState()) { + LOG.w("onImageAvailable", "failed to acquire Image!"); + } else if (getState() == CameraState.PREVIEW && !isChangingState()) { // After preview, the frame manager is correctly set up - Frame frame = getFrameManager().getFrame(data, + //noinspection unchecked + Frame frame = getFrameManager().getFrame(image, System.currentTimeMillis(), - getAngles().offset(Reference.SENSOR, Reference.OUTPUT, + getAngles().offset(Reference.SENSOR, + Reference.OUTPUT, Axis.RELATIVE_TO_SENSOR)); getCallback().dispatchFrame(frame); } else { - getFrameManager().onBufferUnused(data); + image.close(); } } diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/frame/ByteBufferFrameManager.java b/cameraview/src/main/java/com/otaliastudios/cameraview/frame/ByteBufferFrameManager.java index 805aead4..96ae862c 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/frame/ByteBufferFrameManager.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/frame/ByteBufferFrameManager.java @@ -129,8 +129,8 @@ public class ByteBufferFrameManager extends FrameManager { } @Override - protected void onFrameDataRecycled(@NonNull byte[] data) { - if (data.length == getFrameBytes()) { + protected void onFrameDataReleased(@NonNull byte[] data, boolean recycled) { + if (recycled && data.length == getFrameBytes()) { if (mBufferMode == BUFFER_MODE_DISPATCH) { mBufferCallback.onBufferAvailable(data); } else { @@ -139,6 +139,14 @@ public class ByteBufferFrameManager extends FrameManager { } } + @NonNull + @Override + protected byte[] onCloneFrameData(@NonNull byte[] data) { + byte[] clone = new byte[data.length]; + System.arraycopy(data, 0, clone, 0, data.length); + return clone; + } + /** * Releases all frames controlled by this manager and * clears the pool. 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 89d9ed38..f623c28d 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/frame/Frame.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/frame/Frame.java @@ -74,18 +74,9 @@ public class Frame { public Frame freeze() { ensureHasContent(); Frame other = new Frame(mManager, mDataClass); - if (getDataClass() == byte[].class) { - byte[] input = getData(); - byte[] output = new byte[input.length]; - System.arraycopy(input, 0, output, 0, input.length); - other.setContent(output, mTime, mRotation, mSize, mFormat); - } else if (getDataClass() == Image.class) { - throw new RuntimeException("Cannot freeze() a Image Frame. " + - "Please consider using the frame synchronously in your process() method, " + - "which also gives better performance."); - } else { - throw new RuntimeException("Unknown data class: " + getDataClass()); - } + //noinspection unchecked + Object data = mManager.cloneFrameData(getData()); + other.setContent(data, mTime, mRotation, mSize, mFormat); return other; } 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 bb47fe30..9f42b36c 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/frame/FrameManager.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/frame/FrameManager.java @@ -7,7 +7,6 @@ import com.otaliastudios.cameraview.CameraLogger; import com.otaliastudios.cameraview.size.Size; import androidx.annotation.NonNull; -import androidx.annotation.Nullable; import java.util.concurrent.LinkedBlockingQueue; @@ -138,19 +137,27 @@ public abstract class FrameManager { 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)) { - onFrameDataRecycled(data); - } + boolean recycled = mFrameQueue.offer(frame); + onFrameDataReleased(data, recycled); } /** - * Called when a Frame was recycled and its data is now available. + * Called when a Frame was released and its data is now available. * 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 data data + * @param recycled recycled */ - protected abstract void onFrameDataRecycled(@NonNull T data); + protected abstract void onFrameDataReleased(@NonNull T data, boolean recycled); + + @NonNull + final T cloneFrameData(@NonNull T data) { + return onCloneFrameData(data); + } + + @NonNull + protected abstract T onCloneFrameData(@NonNull T data); /** * Releases all frames controlled by this manager and diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/frame/ImageFrameManager.java b/cameraview/src/main/java/com/otaliastudios/cameraview/frame/ImageFrameManager.java new file mode 100644 index 00000000..9a2e774a --- /dev/null +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/frame/ImageFrameManager.java @@ -0,0 +1,30 @@ +package com.otaliastudios.cameraview.frame; + +import android.media.Image; +import android.os.Build; + +import androidx.annotation.NonNull; +import androidx.annotation.RequiresApi; + +@RequiresApi(Build.VERSION_CODES.KITKAT) +public class ImageFrameManager extends FrameManager { + + public ImageFrameManager(int poolSize) { + super(poolSize, Image.class); + } + + @Override + protected void onFrameDataReleased(@NonNull Image data, boolean recycled) { + try { + data.close(); + } catch (Exception ignore) {} + } + + @NonNull + @Override + protected Image onCloneFrameData(@NonNull Image data) { + throw new RuntimeException("Cannot freeze() an Image Frame. " + + "Please consider using the frame synchronously in your process() method, " + + "which also gives better performance."); + } +} diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/internal/utils/ImageHelper.java b/cameraview/src/main/java/com/otaliastudios/cameraview/internal/utils/ImageHelper.java deleted file mode 100644 index 6f6127a0..00000000 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/internal/utils/ImageHelper.java +++ /dev/null @@ -1,100 +0,0 @@ -package com.otaliastudios.cameraview.internal.utils; - -import android.graphics.ImageFormat; -import android.media.Image; - -import java.nio.ByteBuffer; - -import androidx.annotation.NonNull; -import androidx.annotation.RequiresApi; - -/** - * Conversions for {@link android.media.Image}s into byte arrays. - */ -@RequiresApi(19) -public class ImageHelper { - - /** - * From https://stackoverflow.com/a/52740776/4288782 . - * The result array should have a size that is at least 3/2 * w * h. - * This is correctly computed by {@link com.otaliastudios.cameraview.frame.FrameManager}. - * - * @param image input image - * @param result output array - */ - public static void convertToNV21(@NonNull Image image, @NonNull byte[] result) { - if (image.getFormat() != ImageFormat.YUV_420_888) { - throw new IllegalStateException("CAn only convert from YUV_420_888."); - } - int width = image.getWidth(); - int height = image.getHeight(); - int ySize = width * height; - int uvSize = width * height / 4; - - ByteBuffer yBuffer = image.getPlanes()[0].getBuffer(); // Y - ByteBuffer uBuffer = image.getPlanes()[1].getBuffer(); // U - ByteBuffer vBuffer = image.getPlanes()[2].getBuffer(); // V - - int rowStride = image.getPlanes()[0].getRowStride(); - - if (image.getPlanes()[0].getPixelStride() != 1) { - throw new AssertionError("Something wrong in convertToNV21"); - } - - int pos = 0; - - if (rowStride == width) { // likely - yBuffer.get(result, 0, ySize); - pos += ySize; - } - else { - int yBufferPos = width - rowStride; // not an actual position - for (; pos