[camera_android_camerax] Allow usage of all use cases concurrently#10732
[camera_android_camerax] Allow usage of all use cases concurrently#10732camsim99 wants to merge 8 commits intoflutter:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request correctly removes obsolete logic that prevented concurrent use of camera features like video recording, preview, and image analysis/capture. This simplification is made possible by updates in the underlying CameraX library, which now supports these concurrent operations. The changes primarily involve deleting hardware-level check code from startVideoCapturing in android_camera_camerax.dart and removing the corresponding tests that verified the old, restrictive behavior. The pubspec.yaml and CHANGELOG.md have also been updated to reflect these changes. The pull request improves code maintainability by removing unnecessary complexity.
There was a problem hiding this comment.
Code Review
This pull request removes outdated restrictions on concurrent camera use cases, which simplifies the codebase and enables more flexible camera functionality. It also fixes an issue in the integration tests where video recording could fail by replacing blocking sleep calls with non-blocking Future.delayed, which is a great improvement for test stability. The corresponding documentation and tests have been updated to reflect these changes. I have one suggestion to make the changelog more comprehensive.
| @@ -1,3 +1,7 @@ | |||
| ## 0.7.1 | |||
|
|
|||
| * Removes outdated restrictions against concurrent camera use cases. | |||
There was a problem hiding this comment.
This is a great summary of the main change! To make the changelog even more helpful for users, consider also mentioning the fix for the video recording issue (#157181). The repository's CHANGELOG style guide also recommends linking to the fixed issues.
How about something like this?
| * Removes outdated restrictions against concurrent camera use cases. | |
| * Removes outdated restrictions against concurrent camera use cases (fixes flutter/flutter#150387). | |
| * Fixes an issue where video recording could fail immediately after starting on some devices (fixes flutter/flutter#157181). |
References
- The repository's CHANGELOG style guide recommends that if a change fixes one or more issues, they should be listed at the end of the description in parentheses. This change also includes a significant bug fix that would be valuable to mention for users. (link)
Removes guardrails to avoid previously unsupported concurrent camera use cases (this was changed in a later version of CameraX that came after the guards were implemented. See #6608 for details). Fixes flutter/flutter#150387.
As a bonus, also fixes flutter/flutter#157181 by:
Futures instead of calls tosleepwhich I theorize was blocking calls happening on the native side, so thesleepcalls were not actually giving time for the camera to initializeI tracked down these fixes by noticing we actually are not surfacing video recording errors; we actually were getting the ERROR_NO_VALID_DATA error. Filed an issue to expose all video recording errors like this for easier debugging in the future: flutter/flutter#182960.
Pre-Review Checklist
[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2 ↩3