[camera_android_camerax] Allow high-resolution still captures for max preset#11956
[camera_android_camerax] Allow high-resolution still captures for max preset#11956kalyujniy wants to merge 13 commits into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates ResolutionPreset.max to support CameraX high-resolution still capture sizes on Android by introducing an allowedResolutionMode configuration to ResolutionSelector. It adds a separate _imageCaptureResolutionSelector for still image capture, allowing the camera to prefer higher resolution over capture rate. The review feedback suggests passing the preset resolution selector as an argument to _getImageCaptureResolutionSelectorFromPreset to eliminate temporal coupling and preserve existing configuration properties.
…lver Avoid temporal coupling in _getImageCaptureResolutionSelectorFromPreset by passing the preset resolution selector as an argument and copying its properties when building the max-preset ImageCapture selector.
camsim99
left a comment
camsim99
left a comment
There was a problem hiding this comment.
Thank you so much for this change! Without ResolutionSelector.PREFER_HIGHER_RESOLUTION_OVER_CAPTURE_RATE, the plugin was not truly using the highest available resolution.
With that being said, I think this fix should expand beyond ImageCapture. That is, I think all camera use cases should prefer a higher resolution over the capture rate if ResolutionPreset.max is specified. That way, we give developers access to the true highest resolution across the board. Is there some specific reason you think it should only apply to ImageCapture?
| /// | ||
| /// See | ||
| /// https://developer.android.com/reference/kotlin/androidx/camera/core/resolutionselector/ResolutionSelector#PREFER_CAPTURE_RATE_OVER_HIGHER_RESOLUTION(). | ||
| enum ResolutionSelectorAllowedResolutionMode { |
There was a problem hiding this comment.
I would prefer if these were a class of Ints like Surface to better match the actual Android API for clarity.
| /// Allowed resolution mode for [ResolutionSelector]. | ||
| /// | ||
| /// See | ||
| /// https://developer.android.com/reference/kotlin/androidx/camera/core/resolutionselector/ResolutionSelector#PREFER_CAPTURE_RATE_OVER_HIGHER_RESOLUTION(). |
There was a problem hiding this comment.
Nit: put links in the dart doc for each mode individually
| /// For [ResolutionPreset.max], this may differ from [_presetResolutionSelector] | ||
| /// to allow CameraX high-resolution still capture sizes without affecting | ||
| /// preview or image analysis resolution selection. | ||
| ResolutionSelector? _imageCaptureResolutionSelector; |
There was a problem hiding this comment.
See my overall review comment. If you agree, this can just be _presetResolutionSelector when ResolutionPreset.max is specified.
| public ResolutionSelector pigeon_defaultConstructor( | ||
| @Nullable ResolutionFilter resolutionFilter, | ||
| @Nullable ResolutionStrategy resolutionStrategy, | ||
| @Nullable ResolutionSelectorAllowedResolutionMode allowedResolutionMode, |
There was a problem hiding this comment.
See my comment below about this just be an Integer instead of creating a new class.
Use a shared ResolutionSelector with preferHigherResolutionOverCaptureRate for all ResolutionSelector-backed use cases when ResolutionPreset.max is set. Replace the Pigeon enum with nullable int plumbing and Dart int constants documented with Android API links.
Good point. I initially scoped this to I updated the shared I also replaced the Pigeon enum with nullable I manually verified the updated behavior on a real Android device. With |
camsim99
left a comment
camsim99
left a comment
There was a problem hiding this comment.
This LGTM! Thanks for the explanation and the quick changes :) let's get a second reviewer
camsim99
requested review from
a team,
mboetger and
reidbaker
and removed request for
a team and
mboetger
|
This pull request is not mergeable in its current state, likely because of a merge conflict. Pre-submit CI jobs were not triggered. Pushing a new commit to this branch that resolves the issue will result in pre-submit jobs being scheduled. |
|
@kalyujniy Can you take a look at the merge conflicts? |
camsim99
requested review from
a team and
gmackall
and removed request for
a team and
reidbaker
2 participants
Updates
camera_android_cameraxsoResolutionPreset.maxcan select Camera2 high-resolution JPEG still capture sizes on Android. ForResolutionPreset.max,ImageCapturenow usesResolutionSelector.PREFER_HIGHER_RESOLUTION_OVER_CAPTURE_RATE; preview and image analysis keep the existing selector without high-resolution mode.Fixes flutter/flutter#188310
Problem
On some Android devices, Camera2 exposes the largest 4:3 still resolution only through
getHighResolutionOutputSizes(ImageFormat.JPEG). The CameraX backend usedResolutionStrategy.HIGHEST_AVAILABLE_STRATEGYwithoutResolutionSelector.PREFER_HIGHER_RESOLUTION_OVER_CAPTURE_RATE, so CameraX could pick the largest regular JPEG size instead.Observed
takePicture(): 2448x3264After fix
ImageCapture: 4624x3468takePicture(): 3468x4624Risk / tradeoff
High-resolution still capture can be slower and use more memory. Limited to
ResolutionPreset.maxandImageCaptureonly.Note
Generated Pigeon files (
camerax_library.g.dart,CameraXLibrary.g.kt) include formatting churn from local pigeon regeneration; functional changes are limited toResolutionSelectorallowed resolution mode and max-preset ImageCapture selector behavior.Pre-Review Checklist
[shared_preferences]///).Test plan
flutter test test/android_camera_camerax_test.dartdart run script/tool/bin/flutter_plugin_tools.dart format --packages camera_android_camerax --no-java --no-kotlin --no-clang-format --no-swiftdart run script/tool/bin/flutter_plugin_tools.dart analyze --packages camera_android_cameraxdart run script/tool/bin/flutter_plugin_tools.dart dart-test --packages camera_android_cameraxResolutionSelectorTestnative-test --android) — not run locally (no JRE); left to CI