[camera_android_camerax] Prefer higher resolution for max preset - #11956
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. |
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.
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.
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
There was a problem hiding this comment.
This LGTM! Thanks for the explanation and the quick changes :) let's get a second reviewer
|
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? |
b7392ab to
f1e3b7c
Compare
Done, thanks! I updated the changelog and PR description wording to reflect the expanded scope. This PR updates the CameraX
|
|
autosubmit label was removed for flutter/packages/11956, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label. |
bde2ade to
7e51b76
Compare
|
@camsim99 I noticed autosubmit was removed due to a |
|
autosubmit label was removed for flutter/packages/11956, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label. |
|
This is currently blocked by the flutter/flutter tree I believe (or at least it's the same failure https://ci.chromium.org/ui/p/flutter/builders/prod/Linux%20plugin_test_android_variants/4862/overview |
It looks like the flutter/flutter tree issue has been resolved and all checks are green now. The PR is approved and mergeable. Could you please re-apply the autosubmit label when convenient? |
|
@kalyujniy Ah sorry for the delay! Thanks for flagging :) |
…r#190351) flutter/packages@7d17fc8...5351d8c 2026-07-30 katelovett@google.com [material_ui, cupertino_ui] Add migration bridge utilities (flutter/packages#12314) 2026-07-30 269567208+reidbaker-agent@users.noreply.github.com Enforce scoped validation tools in AGENTS.md with file-based trap evals (flutter/packages#12297) 2026-07-30 engine-flutter-autoroll@skia.org Manual roll Flutter from c83f80b to 2a230d1 (6 revisions) (flutter/packages#12325) 2026-07-30 engine-flutter-autoroll@skia.org Manual roll Flutter from 0f02463 to c83f80b (7 revisions) (flutter/packages#12318) 2026-07-30 kirill.webmail@yandex.com [camera_android_camerax] Prefer higher resolution for max preset (flutter/packages#11956) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
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