[flutter_tools] Add flutter capture command with image/video subcommands - #190953
[flutter_tools] Add flutter capture command with image/video subcommands#190953GhagSagar23 wants to merge 4 commits into
Conversation
…mands (flutter#190587) Adds `flutter capture image` and `flutter capture video` for device screenshot and screen recording. Deprecates `flutter screenshot`. Supported platforms: - Android (adb screenrecord, capped at 180s) - iOS simulator (simctl recordVideo with SIGINT-based stop) - iOS device (devicectl, requires Xcode 27+) - macOS (screencapture)
There was a problem hiding this comment.
Code Review
This pull request introduces a new capture command to the Flutter tool, enabling users to take screenshots (flutter capture image) and record screens (flutter capture video) across Android, iOS, iOS Simulators, and macOS devices, while deprecating the old screenshot command. Feedback on the changes highlights critical issues, including incorrect usage of ProcessSignal.kill (which should be process.kill), potential error masking during Android recording failures, and the need to gracefully handle Ctrl-C signals to ensure recording cleanup and file retrieval can complete. Additionally, suggestions were made to simplify redundant file existence checks and capture stderr on macOS recording failures to improve diagnostics.
…g, stderr capture - Subscribe to ProcessSignal.sigint.watch() to prevent Dart VM immediate exit on Ctrl-C during no-duration recording - Android: nested try/finally to avoid masking screenrecord failure when adb pull fails on non-existent remote file - macOS: capture stderr on screencapture failure for diagnostics - Simplify redundant fs.file(outputFile.path) to outputFile directly
|
|
||
| @override | ||
| String get description => 'Take a screenshot from a connected device.'; | ||
| String get description => |
There was a problem hiding this comment.
Thanks for the contribution!
From the conversation at #190587 (comment) I think instead of deprecating flutter screenshot, rename it to flutter capture with a List<String> get aliases => const <String>['screenshot'], and have it behave the same way as flutter screenshot so there's no behavior difference to the end user. Then you can also add a flutter capture screenshot that does nothing additional. And flutter capture recording (or similar) that does the screen recording logic.
| const String _kOut = 'out'; | ||
|
|
||
| class CaptureImageCommand extends FlutterCommand { | ||
| CaptureImageCommand({required this.fs}) { |
There was a problem hiding this comment.
This is missing the --type and --vm-service-url options.
| await process.exitCode; | ||
| return; | ||
| } on Exception catch (error) { | ||
| final errorMessage = error.toString(); |
There was a problem hiding this comment.
I think you could DRY out the boilerplate around devicectl issues to share the logic with takeScreenshot.
Summary
Implements
flutter capture imageandflutter capture videoas the new unified interface for device screenshots and screen recording (#190587).flutter capture image— wraps per-device screenshot logic (replacesflutter screenshot)flutter capture video— wraps platform-native screen recording with--duration/-dand--out/-oflagsflutter screenshot— marked deprecated, points users toflutter capture imagePlatform support
adb screenrecordsimctl recordVideodevicectldevicectlscreencapture -xscreencapture -v-Vflag for durationKey design decisions
--durationflag OR Ctrl-C to stoptry/finallyensuresadb pullruns even on Ctrl-CTest plan
capture_command_test.dart(no device, unsupported device, success, duration, invalid duration)flutter screenshotstill works with deprecation noticeFixes #190587