[camera] Implement setDescription with android camerax #10117
Conversation
![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 implements the ability to switch cameras (setDescription) while a video is recording on Android when using CameraX. This is achieved by introducing a new enablePersistentRecording parameter to the startVideoRecording method. When enabled, the recording session persists through camera switches and lifecycle events. The changes include updating the CameraController to handle this new parameter, adding relevant documentation, and modifying an integration test to verify that recording continues uninterrupted when the camera is switched.
| /// Currently a no-op on platforms other than Android. | ||
| Future<void> startVideoRecording({ | ||
| onLatestImageAvailable? onAvailable, | ||
| bool enablePersistentRecording = true, |
There was a problem hiding this comment.
Setting enablePersistentRecording to true by default changes the existing behavior of video recording. As the documentation states, a persistent recording will ignore events that would normally stop it, such as lifecycle events. This is a behavioral breaking change for users who might be relying on the previous behavior.
For a patch release, it's best to avoid such breaking changes. I recommend changing the default value to false to make this new persistent recording behavior opt-in. This maintains backward compatibility, and developers who need to switch cameras during recording can explicitly enable it.
If you apply this change, please remember to update the integration test in packages/camera/camera/example/integration_test/camera_test.dart to explicitly call startVideoRecording(enablePersistentRecording: true) to ensure the test for the new functionality continues to pass.
| bool enablePersistentRecording = true, | |
| bool enablePersistentRecording = false, |
blackorbs-dev
changed the title
|
@bparrishMines please review |
There was a problem hiding this comment.
Sorry this fell off my radar. LGTM with changelog update.
cc @camsim99 for secondary review.
packages/camera/camera/CHANGELOG.md
Outdated
| ## NEXT | ||
| ## 0.11.2+1 | ||
|
|
||
| * Implement setDescriptionWhileRecording with android camerax (Fixes flutter/flutter#148013). |
There was a problem hiding this comment.
This doesn't describe the change to this package. The change for camerax has already landed. This should be something like:
Adds support to disable persistent recording on Android. See CameraController.startVideoRecording(enablePersistentRecording).
packages/camera/camera/pubspec.yaml
Outdated
| repository: https://github.com/flutter/packages/tree/main/packages/camera/camera | ||
| issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22 | ||
| version: 0.11.2 | ||
| version: 0.11.2+1 |
There was a problem hiding this comment.
@bparrishMines Correct me if I'm wrong but I think this should at least be a minor bump to 0.11.3 if not major (0.12.0) as this bumps the version of the interface and camera_android_camerax required. I would think 0.12.0.
There was a problem hiding this comment.
Yes this should bump to 0.11.3. Bumping to 0.12.0 would require a breaking change, but bumping dependencies doesn't usually warrant a breaking change.
packages/camera/camera/CHANGELOG.md
Outdated
| ## NEXT | ||
| ## 0.12.0 | ||
|
|
||
| * Adds support to disable persistent recording on Android. See `CameraController.startVideoRecording(enablePersistentRecording)`. |
There was a problem hiding this comment.
Minor optional nit:
| * Adds support to disable persistent recording on Android. See `CameraController.startVideoRecording(enablePersistentRecording)`. | |
| * Adds support to configure persistent recording on Android. See `CameraController.startVideoRecording(enablePersistentRecording)`. |
|
It looks like this will also need to bump the pubspec that matches |
Wait what needs to be bumped here? |
IIUC this PR is bumping the |
|
@bparrishMines please review |
3 participants
setDescription allows switching camera while a video recording is in progress. This feature was only working with camera2 setup, but the camera plugin uses camerax by default. So, this PR includes update to address the issue.
Fixes #148013
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