From 0b4540463e6345c2405a88bb5fa2bd084b98c23f Mon Sep 17 00:00:00 2001 From: Yuan SY Date: Mon, 5 Oct 2026 18:08:44 +0800 Subject: [PATCH] Fix camera frame release during analysis --- .../app/robot/common/CameraFragment.java | 35 +- .../app/robot/logging/LoggerFragment.java | 7 +- .../app/robot/common/CameraFragmentTest.java | 315 ++++++++++++++++++ 3 files changed, 341 insertions(+), 16 deletions(-) create mode 100644 android/robot/src/test/java/org/openbot/app/robot/common/CameraFragmentTest.java diff --git a/android/robot/src/main/java/org/openbot/app/robot/common/CameraFragment.java b/android/robot/src/main/java/org/openbot/app/robot/common/CameraFragment.java index da07705cdb..7dc11d9907 100644 --- a/android/robot/src/main/java/org/openbot/app/robot/common/CameraFragment.java +++ b/android/robot/src/main/java/org/openbot/app/robot/common/CameraFragment.java @@ -161,20 +161,7 @@ private void bindCameraUseCases() { new ImageAnalysis.Builder().setTargetAspectRatio(AspectRatio.RATIO_16_9).build(); else imageAnalysis = new ImageAnalysis.Builder().setTargetResolution(analyserResolution).build(); - // insert your code here. - imageAnalysis.setAnalyzer( - cameraExecutor, - image -> { - if (bitmapBuffer == null) - bitmapBuffer = - Bitmap.createBitmap(image.getWidth(), image.getHeight(), Bitmap.Config.ARGB_8888); - - rotationDegrees = image.getImageInfo().getRotationDegrees(); - converter.yuvToRgb(image.getImage(), bitmapBuffer); - image.close(); - - processFrame(bitmapBuffer, image); - }); + imageAnalysis.setAnalyzer(cameraExecutor, this::analyzeFrame); try { if (cameraProvider != null) { cameraProvider.unbindAll(); @@ -185,6 +172,21 @@ private void bindCameraUseCases() { } } + @SuppressLint({"UnsafeExperimentalUsageError", "UnsafeOptInUsageError"}) + private void analyzeFrame(ImageProxy image) { + try { + if (bitmapBuffer == null) + bitmapBuffer = + Bitmap.createBitmap(image.getWidth(), image.getHeight(), Bitmap.Config.ARGB_8888); + + rotationDegrees = image.getImageInfo().getRotationDegrees(); + converter.yuvToRgb(image.getImage(), bitmapBuffer); + processFrame(bitmapBuffer, image); + } finally { + image.close(); + } + } + public int getRotationDegrees() { return rotationDegrees; } @@ -238,5 +240,10 @@ public void setAnalyserResolution(Size resolutionSize) { bindCameraUseCases(); } + /** + * Processes a converted frame while its ImageProxy is still open. This base class closes the + * proxy when the synchronous callback returns or throws. Subclasses must not close it or retain + * it for asynchronous work; copy any needed metadata before returning. + */ protected abstract void processFrame(Bitmap image, ImageProxy imageProxy); } diff --git a/android/robot/src/main/java/org/openbot/app/robot/logging/LoggerFragment.java b/android/robot/src/main/java/org/openbot/app/robot/logging/LoggerFragment.java index 404bc61a5c..45d9694940 100644 --- a/android/robot/src/main/java/org/openbot/app/robot/logging/LoggerFragment.java +++ b/android/robot/src/main/java/org/openbot/app/robot/logging/LoggerFragment.java @@ -659,12 +659,15 @@ private void disconnectPhoneController() { protected void processFrame(Bitmap bitmap, ImageProxy image) { ++frameNum; if (binding != null) { - if (isAdded()) + if (isAdded()) { + final int frameWidth = image.getWidth(); + final int frameHeight = image.getHeight(); requireActivity() .runOnUiThread( () -> binding.frameInfo.setText( - String.format(Locale.US, "%d x %d", image.getWidth(), image.getHeight()))); + String.format(Locale.US, "%d x %d", frameWidth, frameHeight))); + } if (!binding.loggerSwitch.isChecked()) return; diff --git a/android/robot/src/test/java/org/openbot/app/robot/common/CameraFragmentTest.java b/android/robot/src/test/java/org/openbot/app/robot/common/CameraFragmentTest.java new file mode 100644 index 0000000000..e01977a16b --- /dev/null +++ b/android/robot/src/test/java/org/openbot/app/robot/common/CameraFragmentTest.java @@ -0,0 +1,315 @@ +package org.openbot.app.robot.common; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +import android.content.Context; +import android.graphics.Bitmap; +import android.graphics.Color; +import android.graphics.ImageFormat; +import android.graphics.Rect; +import android.media.Image; +import androidx.camera.core.ImageInfo; +import androidx.camera.core.ImageProxy; +import androidx.camera.core.impl.TagBundle; +import androidx.camera.core.impl.utils.ExifData; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.openbot.app.robot.utils.YuvToRgbConverter; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; +import org.robolectric.annotation.Implementation; +import org.robolectric.annotation.Implements; +import org.robolectric.shadow.api.Shadow; +import org.robolectric.util.ReflectionHelpers; +import org.robolectric.util.ReflectionHelpers.ClassParameter; + +@RunWith(RobolectricTestRunner.class) +@Config(sdk = 28, shadows = CameraFragmentTest.FakeConverter.class) +public class CameraFragmentTest { + private final List events = new ArrayList<>(); + private TestCameraFragment fragment; + private FakeConverter converter; + + @Before + public void setUp() { + fragment = new TestCameraFragment(); + fragment.events = events; + YuvToRgbConverter realConverter = new YuvToRgbConverter(RuntimeEnvironment.getApplication()); + converter = Shadow.extract(realConverter); + converter.events = events; + ReflectionHelpers.setField(fragment, "converter", realConverter); + } + + @Test + public void processesConvertedFrameBeforeClosingItOnce() { + FakeImage image = new FakeImage(events); + + analyze(image); + + assertEquals(Arrays.asList("convert", "process", "close"), events); + assertEquals(1, fragment.processedFrames); + assertSame(image, fragment.lastImage); + assertEquals(Color.BLUE, fragment.lastBitmap.getPixel(0, 0)); + assertEquals(90, fragment.getRotationDegrees()); + assertClosedOnce(image); + } + + @Test + public void closesFrameWhenBitmapInitializationFails() { + FakeImage image = new FakeImage(events); + image.widthFailure = new IllegalStateException("Cannot read image width"); + + assertAnalysisFailure(image, image.widthFailure); + + assertEquals(0, converter.calls); + assertEquals(0, fragment.processedFrames); + assertClosedOnce(image); + } + + @Test + public void closesFrameWhenRotationLookupFails() { + FakeImage image = new FakeImage(events); + image.rotationFailure = new IllegalStateException("Cannot read rotation"); + + assertAnalysisFailure(image, image.rotationFailure); + + assertEquals(0, converter.calls); + assertEquals(0, fragment.processedFrames); + assertClosedOnce(image); + } + + @Test + public void closesFrameWithoutProcessingWhenConversionFails() { + FakeImage image = new FakeImage(events); + converter.failure = new IllegalArgumentException("Conversion failed"); + + assertAnalysisFailure(image, converter.failure); + + assertEquals(Arrays.asList("convert", "close"), events); + assertEquals(0, fragment.processedFrames); + assertClosedOnce(image); + } + + @Test + public void closesFrameWhenProcessingThrows() { + FakeImage image = new FakeImage(events); + fragment.failure = new IllegalStateException("Processing failed"); + + assertAnalysisFailure(image, fragment.failure); + + assertEquals(Arrays.asList("convert", "process", "close"), events); + assertClosedOnce(image); + } + + @Test + public void closesFrameWhenProcessingReturnsEarly() { + FakeImage image = new FakeImage(events); + fragment.returnEarly = true; + fragment.failure = new IllegalStateException("Must not run after early return"); + + analyze(image); + + assertEquals(Arrays.asList("convert", "process", "close"), events); + assertClosedOnce(image); + } + + @Test + public void canAnalyzeAnotherFrameAfterConversionFailure() { + FakeImage failedImage = new FakeImage(events); + converter.failure = new IllegalArgumentException("Conversion failed"); + assertAnalysisFailure(failedImage, converter.failure); + converter.failure = null; + FakeImage nextImage = new FakeImage(events); + + analyze(nextImage); + + assertEquals(2, converter.calls); + assertEquals(1, fragment.processedFrames); + assertSame(nextImage, fragment.lastImage); + assertClosedOnce(failedImage); + assertClosedOnce(nextImage); + } + + @Test + public void canAnalyzeAnotherFrameAfterProcessingFailure() { + FakeImage failedImage = new FakeImage(events); + fragment.failure = new IllegalStateException("Processing failed"); + assertAnalysisFailure(failedImage, fragment.failure); + Bitmap previousBuffer = fragment.lastBitmap; + fragment.failure = null; + FakeImage nextImage = new FakeImage(events); + + analyze(nextImage); + + assertEquals(2, fragment.processedFrames); + assertSame(nextImage, fragment.lastImage); + assertSame(previousBuffer, fragment.lastBitmap); + assertClosedOnce(failedImage); + assertClosedOnce(nextImage); + } + + private void analyze(ImageProxy image) { + ReflectionHelpers.callInstanceMethod( + fragment, "analyzeFrame", ClassParameter.from(ImageProxy.class, image)); + } + + private void assertAnalysisFailure(FakeImage image, RuntimeException expected) { + Throwable thrown = assertThrows(RuntimeException.class, () -> analyze(image)); + // ReflectionHelpers may wrap the exception thrown by the private analyzer method. + while (thrown.getCause() != null) thrown = thrown.getCause(); + assertSame(expected, thrown); + } + + private static void assertClosedOnce(FakeImage image) { + assertTrue(image.closed); + assertEquals(1, image.closeCount); + } + + private static class TestCameraFragment extends CameraFragment { + List events; + int processedFrames; + Bitmap lastBitmap; + ImageProxy lastImage; + RuntimeException failure; + boolean returnEarly; + + @Override + protected void processFrame(Bitmap bitmap, ImageProxy image) { + assertFalse(((FakeImage) image).closed); + assertEquals(Color.BLUE, bitmap.getPixel(0, 0)); + events.add("process"); + processedFrames++; + lastBitmap = bitmap; + lastImage = image; + if (returnEarly) return; + if (failure != null) throw failure; + } + + @Override + protected void processControllerKeyData(String command) {} + + @Override + protected void processUSBData(String data) {} + } + + @Implements(value = YuvToRgbConverter.class, isInAndroidSdk = false) + public static class FakeConverter { + List events; + int calls; + RuntimeException failure; + + @Implementation + protected void __constructor__(Context context) {} + + @Implementation + protected void yuvToRgb(Image image, Bitmap output) { + calls++; + events.add("convert"); + if (failure != null) throw failure; + output.eraseColor(Color.BLUE); + } + } + + private static class FakeImage implements ImageProxy { + final List events; + boolean closed; + int closeCount; + RuntimeException widthFailure; + RuntimeException rotationFailure; + Rect cropRect = new Rect(0, 0, 4, 3); + + FakeImage(List events) { + this.events = events; + } + + private void checkOpen() { + if (closed) throw new IllegalStateException("Image already closed"); + } + + @Override + public void close() { + closeCount++; + closed = true; + events.add("close"); + } + + @Override + public Rect getCropRect() { + checkOpen(); + return cropRect; + } + + @Override + public void setCropRect(Rect rect) { + checkOpen(); + cropRect = rect; + } + + @Override + public int getFormat() { + checkOpen(); + return ImageFormat.YUV_420_888; + } + + @Override + public int getWidth() { + checkOpen(); + if (widthFailure != null) throw widthFailure; + return 4; + } + + @Override + public int getHeight() { + checkOpen(); + return 3; + } + + @Override + public PlaneProxy[] getPlanes() { + checkOpen(); + return new PlaneProxy[0]; + } + + @Override + public Image getImage() { + checkOpen(); + // The converter shadow does not require a platform Image or RenderScript. + return null; + } + + @Override + public ImageInfo getImageInfo() { + checkOpen(); + return new ImageInfo() { + @Override + public TagBundle getTagBundle() { + return TagBundle.emptyBundle(); + } + + @Override + public long getTimestamp() { + return 0; + } + + @Override + public int getRotationDegrees() { + checkOpen(); + if (rotationFailure != null) throw rotationFailure; + return 90; + } + + @Override + public void populateExifData(ExifData.Builder builder) {} + }; + } + } +}