[camera] Clean up maxDuration code (#7039)

`maxVideoDuration` was added to the platform interface a long time ago in preparation for adding that feature, but the other parts were never landed. The previous state was: - It has never been implemented for iOS or Android - It has never been settable from the app-facing package, so is always null unless someone uses the platform interface directly, which we don't consider a supported use case. - It cannot be implemented in the CameraX Android implementation. - It was implemented for Windows and web because when those platforms were added much later, nobody realized that the parameter was unused. There is no compelling need for this feature, as clients of the plugin can simply set their own timer to stop recording. Given that, rather than leave the confusing partial state, this marks the option as deprecated at the platform interface layer and warns implementers that it can be ignored. It also removes the implementations from Windows and web in order to reduce implementation complexity, since that code was not reachable from the app-facing API. This does not consider the Windows and web changes to be breaking, even though they arguably could be, because we do not expect clients to be calling platform interface methods directly. Fixes flutter/flutter#150959
flutter · Jul 10, 2024 · 17188b7 · 17188b7
1 parent 007ec66
commit 17188b7
Show file tree

Hide file tree

Showing 41 changed files with 103 additions and 666 deletions.
diff --git a/packages/camera/camera/test/camera_image_stream_test.dart b/packages/camera/camera/test/camera_image_stream_test.dart
@@ -215,11 +215,10 @@ class MockStreamingCameraPlatform extends MockCameraPlatform {
   }
 
   @override
-  Future<XFile> startVideoRecording(int cameraId,
-      {Duration? maxVideoDuration}) {
+  Future<void> startVideoRecording(int cameraId, {Duration? maxVideoDuration}) {
     streamCallLog.add('startVideoRecording');
-    return super
-        .startVideoRecording(cameraId, maxVideoDuration: maxVideoDuration);
+    // Ignore maxVideoDuration, as it is unimplemented and deprecated.
+    return super.startVideoRecording(cameraId);
   }
 
   @override

diff --git a/packages/camera/camera/test/camera_test.dart b/packages/camera/camera/test/camera_test.dart
@@ -1511,14 +1511,17 @@ class MockCameraPlatform extends Mock
       super.noSuchMethod(Invocation.method(#prepareForVideoRecording, null));
 
   @override
-  Future<XFile> startVideoRecording(int cameraId,
-          {Duration? maxVideoDuration}) =>
-      Future<XFile>.value(mockVideoRecordingXFile);
+  Future<void> startVideoRecording(int cameraId, {Duration? maxVideoDuration}) {
+    // Ignore maxVideoDuration, as it is unimplemented and deprecated.
+    return startVideoCapturing(VideoCaptureOptions(cameraId));
+  }
+
+  @override
+  Future<void> startVideoCapturing(VideoCaptureOptions options) async {}
 
   @override
-  Future<void> startVideoCapturing(VideoCaptureOptions options) {
-    return startVideoRecording(options.cameraId,
-        maxVideoDuration: options.maxDuration);
+  Future<XFile> stopVideoRecording(int cameraId) {
+    return Future<XFile>.value(mockVideoRecordingXFile);
   }
 
   @override

diff --git a/packages/camera/camera_android/CHANGELOG.md b/packages/camera/camera_android/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.10.9+8
+
+* Removes unused code related to `maxVideoDuration`.
+
 ## 0.10.9+7
 
 * Updates Android Gradle plugin to 8.5.0.

diff --git a/packages/camera/camera_android/lib/src/android_camera.dart b/packages/camera/camera_android/lib/src/android_camera.dart
@@ -267,8 +267,8 @@ class AndroidCamera extends CameraPlatform {
   @override
   Future<void> startVideoRecording(int cameraId,
       {Duration? maxVideoDuration}) async {
-    return startVideoCapturing(
-        VideoCaptureOptions(cameraId, maxDuration: maxVideoDuration));
+    // Ignore maxVideoDuration, as it is unimplemented and deprecated.
+    return startVideoCapturing(VideoCaptureOptions(cameraId));
   }
 
   @override
@@ -277,7 +277,6 @@ class AndroidCamera extends CameraPlatform {
       'startVideoRecording',
       <String, dynamic>{
         'cameraId': options.cameraId,
-        'maxVideoDuration': options.maxDuration?.inMilliseconds,
         'enableStream': options.streamCallback != null,
       },
     );
@@ -626,15 +625,6 @@ class AndroidCamera extends CameraPlatform {
         cameraEventStreamController.add(CameraClosingEvent(
           cameraId,
         ));
-      case 'video_recorded':
-        final Map<String, Object?> arguments = _getArgumentDictionary(call);
-        cameraEventStreamController.add(VideoRecordedEvent(
-          cameraId,
-          XFile(arguments['path']! as String),
-          arguments['maxVideoDuration'] != null
-              ? Duration(milliseconds: arguments['maxVideoDuration']! as int)
-              : null,
-        ));
       case 'error':
         final Map<String, Object?> arguments = _getArgumentDictionary(call);
         cameraEventStreamController.add(CameraErrorEvent(

diff --git a/packages/camera/camera_android/pubspec.yaml b/packages/camera/camera_android/pubspec.yaml
@@ -3,7 +3,7 @@ description: Android implementation of the camera plugin.
 repository: https://github.com/flutter/packages/tree/main/packages/camera/camera_android
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22
 
-version: 0.10.9+7
+version: 0.10.9+8
 
 environment:
   sdk: ^3.4.0

diff --git a/packages/camera/camera_android/test/android_camera_test.dart b/packages/camera/camera_android/test/android_camera_test.dart
@@ -636,31 +636,6 @@ void main() {
       expect(channel.log, <Matcher>[
         isMethodCall('startVideoRecording', arguments: <String, Object?>{
           'cameraId': cameraId,
-          'maxVideoDuration': null,
-          'enableStream': false,
-        }),
-      ]);
-    });
-
-    test('Should pass maxVideoDuration when starting recording a video',
-        () async {
-      // Arrange
-      final MethodChannelMock channel = MethodChannelMock(
-        channelName: _channelName,
-        methods: <String, dynamic>{'startVideoRecording': null},
-      );
-
-      // Act
-      await camera.startVideoRecording(
-        cameraId,
-        maxVideoDuration: const Duration(seconds: 10),
-      );
-
-      // Assert
-      expect(channel.log, <Matcher>[
-        isMethodCall('startVideoRecording', arguments: <String, Object?>{
-          'cameraId': cameraId,
-          'maxVideoDuration': 10000,
           'enableStream': false,
         }),
       ]);
@@ -685,7 +660,6 @@ void main() {
       expect(channel.log, <Matcher>[
         isMethodCall('startVideoRecording', arguments: <String, Object?>{
           'cameraId': cameraId,
-          'maxVideoDuration': null,
           'enableStream': true,
         }),
       ]);

diff --git a/packages/camera/camera_android_camerax/CHANGELOG.md b/packages/camera/camera_android_camerax/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 0.6.7+1
+
+* Updates README to remove references to `maxVideoDuration`, as it was never
+  visible to app-facing clients, nor was it implemented in `camera_android`.
+
 ## 0.6.7
 
 * Updates AGP version to 8.5.0.

diff --git a/packages/camera/camera_android_camerax/README.md b/packages/camera/camera_android_camerax/README.md
@@ -44,12 +44,12 @@ due to this not currently being supported by CameraX.
 the plugin will fall back to target 480p (`ResolutionPreset.medium`) if configured with
 `ResolutionPreset.low`.
 
-### Setting maximum duration and stream options for video capture
+### Setting stream options for video capture
 
 Calling `startVideoCapturing` with `VideoCaptureOptions` configured with
-`maxVideoDuration` and `streamOptions` is currently unsupported do to the
-limitations of the CameraX library and the platform interface, respectively,
-and thus, those parameters will silently be ignored.
+`streamOptions` is currently unsupported do to
+limitations of the platform interface,
+and thus that parameter will silently be ignored.
 
 ## What requires Android permissions
 

diff --git a/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart b/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart
@@ -1013,16 +1013,15 @@ class AndroidCameraCameraX extends CameraPlatform {
   @override
   Future<void> startVideoRecording(int cameraId,
       {Duration? maxVideoDuration}) async {
-    return startVideoCapturing(
-        VideoCaptureOptions(cameraId, maxDuration: maxVideoDuration));
+    // Ignore maxVideoDuration, as it is unimplemented and deprecated.
+    return startVideoCapturing(VideoCaptureOptions(cameraId));
   }
 
   /// Starts a video recording and/or streaming session.
   ///
   /// Please see [VideoCaptureOptions] for documentation on the
-  /// configuration options. Currently, maxVideoDuration and streamOptions
-  /// are unsupported due to the limitations of CameraX and the platform
-  /// interface, respectively.
+  /// configuration options. Currently streamOptions are unsupported due to
+  /// limitations of the platform interface.
   @override
   Future<void> startVideoCapturing(VideoCaptureOptions options) async {
     if (recording != null) {

diff --git a/packages/camera/camera_android_camerax/pubspec.yaml b/packages/camera/camera_android_camerax/pubspec.yaml
@@ -2,7 +2,7 @@ name: camera_android_camerax
 description: Android implementation of the camera plugin using the CameraX library.
 repository: https://github.com/flutter/packages/tree/main/packages/camera/camera_android_camerax
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22
-version: 0.6.7
+version: 0.6.7+1
 
 environment:
   sdk: ^3.4.0

diff --git a/packages/camera/camera_avfoundation/CHANGELOG.md b/packages/camera/camera_avfoundation/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.9.16+3
+
+* Removes unused `maxVideoDuration` code.
+
 ## 0.9.16+2
 
 * Fixes regression taking a picture in torch mode.

diff --git a/packages/camera/camera_avfoundation/lib/src/avfoundation_camera.dart b/packages/camera/camera_avfoundation/lib/src/avfoundation_camera.dart
@@ -207,8 +207,8 @@ class AVFoundationCamera extends CameraPlatform {
   @override
   Future<void> startVideoRecording(int cameraId,
       {Duration? maxVideoDuration}) async {
-    return startVideoCapturing(
-        VideoCaptureOptions(cameraId, maxDuration: maxVideoDuration));
+    // Ignore maxVideoDuration, as it is unimplemented and deprecated.
+    return startVideoCapturing(VideoCaptureOptions(cameraId));
   }
 
   @override

diff --git a/packages/camera/camera_avfoundation/pubspec.yaml b/packages/camera/camera_avfoundation/pubspec.yaml
@@ -2,7 +2,7 @@ name: camera_avfoundation
 description: iOS implementation of the camera plugin.
 repository: https://github.com/flutter/packages/tree/main/packages/camera/camera_avfoundation
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22
-version: 0.9.16+2
+version: 0.9.16+3
 
 environment:
   sdk: ^3.2.3

diff --git a/packages/camera/camera_web/CHANGELOG.md b/packages/camera/camera_web/CHANGELOG.md
@@ -1,5 +1,8 @@
-## NEXT
+## 0.3.4
 
+* Removes `maxVideoDuration`/`maxDuration`, as the feature was never exposed at
+  the app-facing package level, and is deprecated at the platform interface
+  level.
 * Updates minimum supported SDK version to Flutter 3.16/Dart 3.2.
 
 ## 0.3.3

diff --git a/packages/camera/camera_web/example/integration_test/camera_test.dart b/packages/camera/camera_web/example/integration_test/camera_test.dart
@@ -1069,60 +1069,7 @@ void main() {
           verify(mediaRecorder.start).called(1);
         });
 
-        testWidgets(
-            'starts a video recording '
-            'with maxVideoDuration', (WidgetTester tester) async {
-          const Duration maxVideoDuration = Duration(hours: 1);
-
-          final Camera camera = Camera(
-            textureId: 1,
-            cameraService: cameraService,
-          )
-            ..mediaRecorder = mediaRecorder
-            ..isVideoTypeSupported = isVideoTypeSupported;
-
-          await camera.initialize();
-          await camera.play();
-
-          await camera.startVideoRecording(maxVideoDuration: maxVideoDuration);
-
-          verify(() => mediaRecorder.start(maxVideoDuration.inMilliseconds))
-              .called(1);
-        });
-
         group('throws a CameraWebException', () {
-          testWidgets(
-              'with notSupported error '
-              'when maxVideoDuration is 0 milliseconds or less',
-              (WidgetTester tester) async {
-            final Camera camera = Camera(
-              textureId: 1,
-              cameraService: cameraService,
-            )
-              ..mediaRecorder = mediaRecorder
-              ..isVideoTypeSupported = isVideoTypeSupported;
-
-            await camera.initialize();
-            await camera.play();
-
-            expect(
-              () => camera.startVideoRecording(maxVideoDuration: Duration.zero),
-              throwsA(
-                isA<CameraWebException>()
-                    .having(
-                      (CameraWebException e) => e.cameraId,
-                      'cameraId',
-                      textureId,
-                    )
-                    .having(
-                      (CameraWebException e) => e.code,
-                      'code',
-                      CameraErrorCode.notSupported,
-                    ),
-              ),
-            );
-          });
-
           testWidgets(
               'with notSupported error '
               'when no video types are supported', (WidgetTester tester) async {
@@ -1346,46 +1293,6 @@ void main() {
         });
       });
 
-      group('on video data available', () {
-        late void Function(Event) videoDataAvailableListener;
-
-        setUp(() {
-          when(
-            () => mediaRecorder.addEventListener('dataavailable', any()),
-          ).thenAnswer((Invocation invocation) {
-            videoDataAvailableListener =
-                invocation.positionalArguments[1] as void Function(Event);
-          });
-        });
-
-        testWidgets(
-            'stops a video recording '
-            'if maxVideoDuration is given and '
-            'the recording was not stopped manually',
-            (WidgetTester tester) async {
-          const Duration maxVideoDuration = Duration(hours: 1);
-
-          final Camera camera = Camera(
-            textureId: 1,
-            cameraService: cameraService,
-          )
-            ..mediaRecorder = mediaRecorder
-            ..isVideoTypeSupported = isVideoTypeSupported;
-
-          await camera.initialize();
-          await camera.play();
-          await camera.startVideoRecording(maxVideoDuration: maxVideoDuration);
-
-          when(() => mediaRecorder.state).thenReturn('recording');
-
-          videoDataAvailableListener(FakeBlobEvent(Blob(<Object>[])));
-
-          await Future<void>.microtask(() {});
-
-          verify(mediaRecorder.stop).called(1);
-        });
-      });
-
       group('on video recording stopped', () {
         late void Function(Event) videoRecordingStoppedListener;
 
@@ -1543,7 +1450,6 @@ void main() {
         testWidgets(
             'emits a VideoRecordedEvent '
             'when a video recording is created', (WidgetTester tester) async {
-          const Duration maxVideoDuration = Duration(hours: 1);
           const String supportedVideoType = 'video/webm';
 
           final MockMediaRecorder mediaRecorder = MockMediaRecorder();
@@ -1580,7 +1486,7 @@ void main() {
           final StreamQueue<VideoRecordedEvent> streamQueue =
               StreamQueue<VideoRecordedEvent>(camera.onVideoRecordedEvent);
 
-          await camera.startVideoRecording(maxVideoDuration: maxVideoDuration);
+          await camera.startVideoRecording();
 
           Blob? finalVideo;
           camera.blobBuilder = (List<Blob> blobs, String videoType) {
@@ -1614,11 +1520,6 @@ void main() {
                           'name',
                           finalVideo.hashCode.toString(),
                         ),
-                  )
-                  .having(
-                    (VideoRecordedEvent e) => e.maxVideoDuration,
-                    'maxVideoDuration',
-                    maxVideoDuration,
                   ),
             ),
           );

diff --git a/packages/camera/camera_web/example/integration_test/camera_web_test.dart b/packages/camera/camera_web/example/integration_test/camera_web_test.dart
@@ -2839,9 +2839,7 @@ void main() {
               .thenAnswer((Invocation _) => const Stream<ErrorEvent>.empty());
 
           when(
-            () => camera.startVideoRecording(
-              maxVideoDuration: any(named: 'maxVideoDuration'),
-            ),
+            () => camera.startVideoRecording(),
           ).thenThrow(exception);
 
           final Stream<CameraErrorEvent> eventStream =