You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Ensure the Android robot app releases every analyzed CameraX ImageProxy, including when bitmap initialization, metadata lookup, YUV conversion, or frame processing throws. Keep the proxy open during the synchronous processFrame() callback and snapshot logger frame dimensions before posting the UI update.
Problem and reproduction risk
The analyzer previously called image.close() only after YUV conversion. An exception before that call could leave a frame outstanding and prevent CameraX image analysis from delivering subsequent frames. It also passed an already-closed proxy to processFrame(). LoggerFragment accessed that proxy's dimensions in a later UI callback, which can run after the frame is closed.
The regression tests inject failures during initialization, metadata access, conversion, and processing to verify release and exception propagation without requiring a physical camera.
Changes
Extract the existing analyzer body into a private analyzeFrame() method and close its proxy in finally, covering the entire synchronous processing path.
Preserve the processFrame(Bitmap, ImageProxy) signature and document borrowed ownership: subclasses must not close or retain the proxy; asynchronous work must copy any required metadata before returning.
Capture logger frame width/height synchronously so the queued UI callback does not access the proxy.
Add eight Robolectric regression tests using an ImageProxy fake and a YUV converter shadow, with no new test dependencies. Cover normal ordering, exactly-once release, early return, injected exceptions, and a subsequent analysis invocation after conversion/processing failure.
Exceptions still propagate; this change does not swallow errors or guarantee recovery of a failing converter/executor. The subsequent-frame tests exercise the analyzer again after clearing the injected failure.
Validation
Based on official master at 353da5a103767bee669e038b8785137fa5d8c28a.
./gradlew :robot:testDebugUnitTest :robot:assembleDebug: passed, all 19 robot unit tests and the debug APK build. Existing compileSdk/AGP, deprecated API, navigation/D8, and native-symbol stripping warnings remain.
git diff --check and git diff --cached --check: passed.
Bundled google-java-format-1.7-all-deps.jar direct whole-file checks for CameraFragment.java and CameraFragmentTest.java: passed.
Direct formatter check for the changed LoggerFragment.java method region (lines 658–679, with import sorting/removal disabled): passed. A whole-file check reports existing formatting/import differences in this file, also reproduced on the unchanged base version; unrelated formatting is deliberately preserved.
./gradlew --offline checkStyle: completed successfully, but printed no files were provided because the existing task scans the old app/src/main/java path. This is not counted as effective source-format validation; the direct checks above were used instead.
Inspected all current processFrame() implementations: the logger's queued dimension access was the only reference to proxy methods, and now uses local snapshots.
Suggested device checks (not performed here)
Open a camera-backed page and verify preview/frame processing continues.
Switch between camera-backed pages and switch the front/back camera.
Open the logger page and verify frame dimensions update, including while recording is disabled.
Non-goals
No shopping-cart features, metadata API additions, camera configuration changes, bitmap-reuse changes, asynchronous inference redesign, BLE changes, format-tool fixes, or new dependencies. Physical-device validation is still needed.
@harishthakur52 could you test this on a real robot? Please check camera preview, switching between camera-based pages and front/back cameras, and Logger frame dimensions with recording on and off. CI passes; we’d like device confirmation before merging.
@harishthakur52 could you test this on a real robot? Please check camera preview, switching between camera-based pages and front/back cameras, and Logger frame dimensions with recording on and off. CI passes; we’d like device confirmation before merging.
We have tested on a real phone (Nokia 6.2, Android 11).
Camera preview, switching between camera pages, and front/back camera flipping all work.
The Logger frame size is correct with recording on and off (640 x 360 at Low, 1280 x 720 at Medium), and there were no "Image is already closed" errors. Everything works as expected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ensure the Android robot app releases every analyzed CameraX
ImageProxy, including when bitmap initialization, metadata lookup, YUV conversion, or frame processing throws. Keep the proxy open during the synchronousprocessFrame()callback and snapshot logger frame dimensions before posting the UI update.Problem and reproduction risk
The analyzer previously called
image.close()only after YUV conversion. An exception before that call could leave a frame outstanding and prevent CameraX image analysis from delivering subsequent frames. It also passed an already-closed proxy toprocessFrame().LoggerFragmentaccessed that proxy's dimensions in a later UI callback, which can run after the frame is closed.The regression tests inject failures during initialization, metadata access, conversion, and processing to verify release and exception propagation without requiring a physical camera.
Changes
analyzeFrame()method and close its proxy infinally, covering the entire synchronous processing path.processFrame(Bitmap, ImageProxy)signature and document borrowed ownership: subclasses must not close or retain the proxy; asynchronous work must copy any required metadata before returning.ImageProxyfake and a YUV converter shadow, with no new test dependencies. Cover normal ordering, exactly-once release, early return, injected exceptions, and a subsequent analysis invocation after conversion/processing failure.Exceptions still propagate; this change does not swallow errors or guarantee recovery of a failing converter/executor. The subsequent-frame tests exercise the analyzer again after clearing the injected failure.
Validation
Based on official
masterat353da5a103767bee669e038b8785137fa5d8c28a../gradlew --offline :robot:testDebugUnitTest --tests org.openbot.app.robot.common.CameraFragmentTest: passed, 8 tests../gradlew :robot:testDebugUnitTest :robot:assembleDebug: passed, all 19 robot unit tests and the debug APK build. Existing compileSdk/AGP, deprecated API, navigation/D8, and native-symbol stripping warnings remain.git diff --checkandgit diff --cached --check: passed.google-java-format-1.7-all-deps.jardirect whole-file checks forCameraFragment.javaandCameraFragmentTest.java: passed.LoggerFragment.javamethod region (lines 658–679, with import sorting/removal disabled): passed. A whole-file check reports existing formatting/import differences in this file, also reproduced on the unchanged base version; unrelated formatting is deliberately preserved../gradlew --offline checkStyle: completed successfully, but printedno files were providedbecause the existing task scans the oldapp/src/main/javapath. This is not counted as effective source-format validation; the direct checks above were used instead.processFrame()implementations: the logger's queued dimension access was the only reference to proxy methods, and now uses local snapshots.Suggested device checks (not performed here)
Non-goals
No shopping-cart features, metadata API additions, camera configuration changes, bitmap-reuse changes, asynchronous inference redesign, BLE changes, format-tool fixes, or new dependencies. Physical-device validation is still needed.