From 5bfdeeed63dc3cc0926d50c11a71ab0e4f0a63cd Mon Sep 17 00:00:00 2001 From: camsim99 Date: Tue, 6 Feb 2024 12:49:19 -0800 Subject: [PATCH 1/5] Make fixes and bump version --- packages/camera/camera_android_camerax/CHANGELOG.md | 5 +++++ .../lib/src/android_camera_camerax.dart | 12 +++++++----- packages/camera/camera_android_camerax/pubspec.yaml | 2 +- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/camera/camera_android_camerax/CHANGELOG.md b/packages/camera/camera_android_camerax/CHANGELOG.md index 9db3eadff53..2b84aa8ae8e 100644 --- a/packages/camera/camera_android_camerax/CHANGELOG.md +++ b/packages/camera/camera_android_camerax/CHANGELOG.md @@ -1,3 +1,8 @@ +## 0.5.0+32 + +* Removes all `unawaited` calls to fix https://github.com/flutter/flutter/issues/132499, updates the + camera state when video capture starts, and unbinds video capture case when video recording stops. + ## 0.5.0+31 * Wraps CameraX classes needed to set capture request options, which is needed to implement setting the exposure mode. 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 index a6834280178..f1cfc0289e7 100644 --- a/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart +++ b/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart @@ -357,9 +357,9 @@ class AndroidCameraCameraX extends CameraPlatform { @override Future dispose(int cameraId) async { preview?.releaseFlutterSurfaceTexture(); - unawaited(liveCameraState?.removeObservers()); + await liveCameraState?.removeObservers(); processCameraProvider?.unbindAll(); - unawaited(imageAnalysis?.clearAnalyzer()); + await imageAnalysis?.clearAnalyzer(); } /// The camera has been initialized. @@ -645,6 +645,7 @@ class AndroidCameraCameraX extends CameraPlatform { if (!(await processCameraProvider!.isBound(videoCapture!))) { camera = await processCameraProvider! .bindToLifecycle(cameraSelector!, [videoCapture!]); + await _updateCameraInfoAndLiveCameraState(options.cameraId); } // Set target rotation to default CameraX rotation only if capture @@ -681,7 +682,7 @@ class AndroidCameraCameraX extends CameraPlatform { if (videoOutputPath == null) { // Stop the current active recording as we will be unable to complete it // in this error case. - unawaited(recording!.close()); + await recording!.close(); recording = null; pendingRecording = null; throw CameraException( @@ -690,7 +691,8 @@ class AndroidCameraCameraX extends CameraPlatform { 'while reporting success. The platform should always ' 'return a valid path or report an error.'); } - unawaited(recording!.close()); + await recording!.close(); + await _unbindUseCaseFromLifecycle(videoCapture!); recording = null; pendingRecording = null; return XFile(videoOutputPath!); @@ -813,7 +815,7 @@ class AndroidCameraCameraX extends CameraPlatform { /// Removes the previously set analyzer on the [imageAnalysis] instance, since /// image information should no longer be streamed. FutureOr _onFrameStreamCancel() async { - unawaited(imageAnalysis!.clearAnalyzer()); + await imageAnalysis!.clearAnalyzer(); } /// Converts between Android ImageFormat constants and [ImageFormatGroup]s. diff --git a/packages/camera/camera_android_camerax/pubspec.yaml b/packages/camera/camera_android_camerax/pubspec.yaml index f40f6a3d55d..c9da1786693 100644 --- 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.5.0+31 +version: 0.5.0+32 environment: sdk: ">=3.0.0 <4.0.0" From 5182ce0899fbeaa9887618a688b68f4889571e20 Mon Sep 17 00:00:00 2001 From: camsim99 Date: Wed, 7 Feb 2024 11:56:15 -0800 Subject: [PATCH 2/5] Fixing tests --- .../camera_android_camerax/CHANGELOG.md | 2 +- .../lib/src/android_camera_camerax.dart | 5 +- .../test/android_camera_camerax_test.dart | 60 +++++++++++++++++-- 3 files changed, 60 insertions(+), 7 deletions(-) diff --git a/packages/camera/camera_android_camerax/CHANGELOG.md b/packages/camera/camera_android_camerax/CHANGELOG.md index 2b84aa8ae8e..f521d539bf8 100644 --- a/packages/camera/camera_android_camerax/CHANGELOG.md +++ b/packages/camera/camera_android_camerax/CHANGELOG.md @@ -1,6 +1,6 @@ ## 0.5.0+32 -* Removes all `unawaited` calls to fix https://github.com/flutter/flutter/issues/132499, updates the +* Removes all remaining `unawaited` calls to fix https://github.com/flutter/flutter/issues/132499, updates the camera state when video capture starts, and unbinds video capture case when video recording stops. ## 0.5.0+31 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 index f1cfc0289e7..b98669097b7 100644 --- a/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart +++ b/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart @@ -680,11 +680,14 @@ class AndroidCameraCameraX extends CameraPlatform { 'video recording while no recording is in progress.'); } if (videoOutputPath == null) { + print('hi'); // Stop the current active recording as we will be unable to complete it // in this error case. await recording!.close(); + // await _unbindUseCaseFromLifecycle(videoCapture!); recording = null; pendingRecording = null; + print('hi'); throw CameraException( 'INVALID_PATH', 'The platform did not return a path ' @@ -692,7 +695,7 @@ class AndroidCameraCameraX extends CameraPlatform { 'return a valid path or report an error.'); } await recording!.close(); - await _unbindUseCaseFromLifecycle(videoCapture!); + // await _unbindUseCaseFromLifecycle(videoCapture!); recording = null; pendingRecording = null; return XFile(videoOutputPath!); diff --git a/packages/camera/camera_android_camerax/test/android_camera_camerax_test.dart b/packages/camera/camera_android_camerax/test/android_camera_camerax_test.dart index 415c5ee6448..bb571c2aa41 100644 --- a/packages/camera/camera_android_camerax/test/android_camera_camerax_test.dart +++ b/packages/camera/camera_android_camerax/test/android_camera_camerax_test.dart @@ -1006,26 +1006,37 @@ void main() { group('video recording', () { test( - 'startVideoCapturing binds video capture use case and starts the recording', + 'startVideoCapturing binds video capture use case, updates saved camera instance and its properties, and starts the recording', () async { // Set up mocks and constants. final AndroidCameraCameraX camera = AndroidCameraCameraX(); final MockPendingRecording mockPendingRecording = MockPendingRecording(); final MockRecording mockRecording = MockRecording(); + final MockCamera mockCamera = MockCamera(); + final MockCamera newMockCamera = MockCamera(); + final MockCameraInfo mockCameraInfo = MockCameraInfo(); + final MockLiveCameraState mockLiveCameraState = MockLiveCameraState(); + final MockLiveCameraState newMockLiveCameraState = MockLiveCameraState(); final TestSystemServicesHostApi mockSystemServicesApi = MockTestSystemServicesHostApi(); TestSystemServicesHostApi.setup(mockSystemServicesApi); // Set directly for test versus calling createCamera. camera.processCameraProvider = MockProcessCameraProvider(); - camera.camera = MockCamera(); + camera.camera = mockCamera; camera.recorder = MockRecorder(); camera.videoCapture = MockVideoCapture(); camera.cameraSelector = MockCameraSelector(); + camera.liveCameraState = mockLiveCameraState; // Ignore setting target rotation for this test; tested seprately. camera.captureOrientationLocked = true; + // Tell plugin to create detached Observer when camera info updated. + camera.proxy = CameraXProxy( + createCameraStateObserver: (void Function(Object) onChanged) => + Observer.detached(onChanged: onChanged)); + const int cameraId = 17; const String outputPath = '/temp/MOV123.temp'; @@ -1039,12 +1050,30 @@ void main() { .thenAnswer((_) async => false); when(camera.processCameraProvider!.bindToLifecycle( camera.cameraSelector!, [camera.videoCapture!])) - .thenAnswer((_) async => camera.camera!); + .thenAnswer((_) async => newMockCamera); + when(newMockCamera.getCameraInfo()) + .thenAnswer((_) async => mockCameraInfo); + when(mockCameraInfo.getCameraState()) + .thenAnswer((_) async => newMockLiveCameraState); await camera.startVideoCapturing(const VideoCaptureOptions(cameraId)); + // Verify VideoCapture UseCase is bound and camera & its properties + // are updated. verify(camera.processCameraProvider!.bindToLifecycle( camera.cameraSelector!, [camera.videoCapture!])); + expect(camera.camera, equals(newMockCamera)); + expect(camera.cameraInfo, equals(mockCameraInfo)); + verify(mockLiveCameraState.removeObservers()); + expect( + await testCameraClosingObserver( + camera, + cameraId, + verify(newMockLiveCameraState.observe(captureAny)).captured.single + as Observer), + isTrue); + + // Verify recording is started. expect(camera.pendingRecording, equals(mockPendingRecording)); expect(camera.recording, mockRecording); }); @@ -1056,6 +1085,8 @@ void main() { final AndroidCameraCameraX camera = AndroidCameraCameraX(); final MockPendingRecording mockPendingRecording = MockPendingRecording(); final MockRecording mockRecording = MockRecording(); + final MockCamera mockCamera = MockCamera(); + final MockCameraInfo mockCameraInfo = MockCameraInfo(); final TestSystemServicesHostApi mockSystemServicesApi = MockTestSystemServicesHostApi(); TestSystemServicesHostApi.setup(mockSystemServicesApi); @@ -1069,6 +1100,11 @@ void main() { // Ignore setting target rotation for this test; tested seprately. camera.captureOrientationLocked = true; + // Tell plugin to create detached Observer when camera info updated. + camera.proxy = CameraXProxy( + createCameraStateObserver: (void Function(Object) onChanged) => + Observer.detached(onChanged: onChanged)); + const int cameraId = 17; const String outputPath = '/temp/MOV123.temp'; @@ -1082,7 +1118,11 @@ void main() { .thenAnswer((_) async => false); when(camera.processCameraProvider!.bindToLifecycle( camera.cameraSelector!, [camera.videoCapture!])) - .thenAnswer((_) async => MockCamera()); + .thenAnswer((_) async => mockCamera); + when(mockCamera.getCameraInfo()) + .thenAnswer((_) => Future.value(mockCameraInfo)); + when(mockCameraInfo.getCameraState()) + .thenAnswer((_) async => MockLiveCameraState()); await camera.startVideoCapturing(const VideoCaptureOptions(cameraId)); @@ -1253,7 +1293,9 @@ void main() { verifyNoMoreInteractions(recording); }); - test('stopVideoRecording stops the recording', () async { + test( + 'stopVideoRecording stops the recording and unbinds video capture use case', + () async { final AndroidCameraCameraX camera = AndroidCameraCameraX(); final MockRecording recording = MockRecording(); final MockProcessCameraProvider processCameraProvider = @@ -1267,11 +1309,19 @@ void main() { camera.videoCapture = videoCapture; camera.videoOutputPath = videoOutputPath; + // Tell plugin that videoCapture use case was bound to start recording. + when(camera.processCameraProvider!.isBound(videoCapture)) + .thenAnswer((_) async => true); + final XFile file = await camera.stopVideoRecording(0); expect(file.path, videoOutputPath); + // Verify that recording stops. verify(recording.close()); verifyNoMoreInteractions(recording); + + // Verify video capture use case is unbound. + verify(camera.processCameraProvider!.unbind([videoCapture])); }); test( From 12163d17387caa053936f431f9de5dba97a5df98 Mon Sep 17 00:00:00 2001 From: camsim99 Date: Thu, 8 Feb 2024 10:50:19 -0800 Subject: [PATCH 3/5] Fix code + tests --- .../lib/src/android_camera_camerax.dart | 6 +- .../test/android_camera_camerax_test.dart | 83 +++++++++++-------- 2 files changed, 51 insertions(+), 38 deletions(-) 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 index b98669097b7..b4145c23e14 100644 --- a/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart +++ b/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart @@ -680,14 +680,12 @@ class AndroidCameraCameraX extends CameraPlatform { 'video recording while no recording is in progress.'); } if (videoOutputPath == null) { - print('hi'); // Stop the current active recording as we will be unable to complete it // in this error case. await recording!.close(); - // await _unbindUseCaseFromLifecycle(videoCapture!); + await _unbindUseCaseFromLifecycle(videoCapture!); recording = null; pendingRecording = null; - print('hi'); throw CameraException( 'INVALID_PATH', 'The platform did not return a path ' @@ -695,7 +693,7 @@ class AndroidCameraCameraX extends CameraPlatform { 'return a valid path or report an error.'); } await recording!.close(); - // await _unbindUseCaseFromLifecycle(videoCapture!); + await _unbindUseCaseFromLifecycle(videoCapture!); recording = null; pendingRecording = null; return XFile(videoOutputPath!); diff --git a/packages/camera/camera_android_camerax/test/android_camera_camerax_test.dart b/packages/camera/camera_android_camerax/test/android_camera_camerax_test.dart index bb571c2aa41..b9b52afa8ae 100644 --- a/packages/camera/camera_android_camerax/test/android_camera_camerax_test.dart +++ b/packages/camera/camera_android_camerax/test/android_camera_camerax_test.dart @@ -662,7 +662,9 @@ void main() { 'initializeCamera throws a CameraException when createCamera has not been called before initializedCamera', () async { final AndroidCameraCameraX camera = AndroidCameraCameraX(); - expect(() => camera.initializeCamera(3), throwsA(isA())); + await expectLater(() async { + await camera.initializeCamera(3); + }, throwsA(isA())); }); test('initializeCamera sends expected CameraInitializedEvent', () async { @@ -1334,47 +1336,60 @@ void main() { camera.recording = null; camera.videoOutputPath = videoOutputPath; - expect( - () => camera.stopVideoRecording(0), throwsA(isA())); + await expectLater(() async { + await camera.stopVideoRecording(0); + }, throwsA(isA())); }); + }); - test( - 'stopVideoRecording throws a camera exception if ' - 'videoOutputPath is null, and sets recording to null', () async { - final AndroidCameraCameraX camera = AndroidCameraCameraX(); - final MockRecording recording = MockRecording(); + test( + 'stopVideoRecording throws a camera exception if ' + 'videoOutputPath is null, and sets recording to null', () async { + final AndroidCameraCameraX camera = AndroidCameraCameraX(); + final MockRecording mockRecording = MockRecording(); + final MockVideoCapture mockVideoCapture = MockVideoCapture(); - // Set directly for test versus calling startVideoCapturing. - camera.recording = recording; - camera.videoOutputPath = null; + // Set directly for test versus calling startVideoCapturing. + camera.processCameraProvider = MockProcessCameraProvider(); + camera.recording = mockRecording; + camera.videoOutputPath = null; + camera.videoCapture = mockVideoCapture; - expect( - () => camera.stopVideoRecording(0), throwsA(isA())); - expect(camera.recording, null); - }); + // Tell plugin that videoCapture use case was bound to start recording. + when(camera.processCameraProvider!.isBound(mockVideoCapture)) + .thenAnswer((_) async => true); - test( - 'calling stopVideoRecording twice stops the recording ' - 'and then throws a CameraException', () async { - final AndroidCameraCameraX camera = AndroidCameraCameraX(); - final MockRecording recording = MockRecording(); - final MockProcessCameraProvider processCameraProvider = - MockProcessCameraProvider(); - final MockVideoCapture videoCapture = MockVideoCapture(); - const String videoOutputPath = '/test/output/path'; + await expectLater(() async { + await camera.stopVideoRecording(0); + }, throwsA(isA())); + expect(camera.recording, null); - // Set directly for test versus calling createCamera and startVideoCapturing. - camera.processCameraProvider = processCameraProvider; - camera.recording = recording; - camera.videoCapture = videoCapture; - camera.videoOutputPath = videoOutputPath; + // Verify video capture use case is unbound. + verify(camera.processCameraProvider!.unbind([mockVideoCapture])); + }); - final XFile file = await camera.stopVideoRecording(0); - expect(file.path, videoOutputPath); + test( + 'calling stopVideoRecording twice stops the recording ' + 'and then throws a CameraException', () async { + final AndroidCameraCameraX camera = AndroidCameraCameraX(); + final MockRecording recording = MockRecording(); + final MockProcessCameraProvider processCameraProvider = + MockProcessCameraProvider(); + final MockVideoCapture videoCapture = MockVideoCapture(); + const String videoOutputPath = '/test/output/path'; - expect( - () => camera.stopVideoRecording(0), throwsA(isA())); - }); + // Set directly for test versus calling createCamera and startVideoCapturing. + camera.processCameraProvider = processCameraProvider; + camera.recording = recording; + camera.videoCapture = videoCapture; + camera.videoOutputPath = videoOutputPath; + + final XFile file = await camera.stopVideoRecording(0); + expect(file.path, videoOutputPath); + + await expectLater(() async { + await camera.stopVideoRecording(0); + }, throwsA(isA())); }); test( From 4320f2f6c0dce1c691b3ec2eb2a30cb62ff9e255 Mon Sep 17 00:00:00 2001 From: camsim99 Date: Thu, 8 Feb 2024 15:06:59 -0800 Subject: [PATCH 4/5] Update changelog --- packages/camera/camera_android_camerax/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/camera/camera_android_camerax/CHANGELOG.md b/packages/camera/camera_android_camerax/CHANGELOG.md index f521d539bf8..0c92955966c 100644 --- a/packages/camera/camera_android_camerax/CHANGELOG.md +++ b/packages/camera/camera_android_camerax/CHANGELOG.md @@ -1,6 +1,6 @@ ## 0.5.0+32 -* Removes all remaining `unawaited` calls to fix https://github.com/flutter/flutter/issues/132499, updates the +* Removes all remaining `unawaited` calls to fix potential race conditions, updates the camera state when video capture starts, and unbinds video capture case when video recording stops. ## 0.5.0+31 From bddff068d3045b6bd62e38d2e78cb2f56105e8d6 Mon Sep 17 00:00:00 2001 From: camsim99 Date: Thu, 8 Feb 2024 16:27:32 -0800 Subject: [PATCH 5/5] Remove unbind calls --- packages/camera/camera_android_camerax/CHANGELOG.md | 4 ++-- .../lib/src/android_camera_camerax.dart | 2 -- .../test/android_camera_camerax_test.dart | 10 +--------- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/packages/camera/camera_android_camerax/CHANGELOG.md b/packages/camera/camera_android_camerax/CHANGELOG.md index 0c92955966c..a96bfcefd99 100644 --- a/packages/camera/camera_android_camerax/CHANGELOG.md +++ b/packages/camera/camera_android_camerax/CHANGELOG.md @@ -1,7 +1,7 @@ ## 0.5.0+32 -* Removes all remaining `unawaited` calls to fix potential race conditions, updates the - camera state when video capture starts, and unbinds video capture case when video recording stops. +* Removes all remaining `unawaited` calls to fix potential race conditions and updates the + camera state when video capture starts. ## 0.5.0+31 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 index b4145c23e14..b05788d68a4 100644 --- a/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart +++ b/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart @@ -683,7 +683,6 @@ class AndroidCameraCameraX extends CameraPlatform { // Stop the current active recording as we will be unable to complete it // in this error case. await recording!.close(); - await _unbindUseCaseFromLifecycle(videoCapture!); recording = null; pendingRecording = null; throw CameraException( @@ -693,7 +692,6 @@ class AndroidCameraCameraX extends CameraPlatform { 'return a valid path or report an error.'); } await recording!.close(); - await _unbindUseCaseFromLifecycle(videoCapture!); recording = null; pendingRecording = null; return XFile(videoOutputPath!); diff --git a/packages/camera/camera_android_camerax/test/android_camera_camerax_test.dart b/packages/camera/camera_android_camerax/test/android_camera_camerax_test.dart index b9b52afa8ae..bc6a0d1c6a3 100644 --- a/packages/camera/camera_android_camerax/test/android_camera_camerax_test.dart +++ b/packages/camera/camera_android_camerax/test/android_camera_camerax_test.dart @@ -1295,9 +1295,7 @@ void main() { verifyNoMoreInteractions(recording); }); - test( - 'stopVideoRecording stops the recording and unbinds video capture use case', - () async { + test('stopVideoRecording stops the recording', () async { final AndroidCameraCameraX camera = AndroidCameraCameraX(); final MockRecording recording = MockRecording(); final MockProcessCameraProvider processCameraProvider = @@ -1321,9 +1319,6 @@ void main() { // Verify that recording stops. verify(recording.close()); verifyNoMoreInteractions(recording); - - // Verify video capture use case is unbound. - verify(camera.processCameraProvider!.unbind([videoCapture])); }); test( @@ -1363,9 +1358,6 @@ void main() { await camera.stopVideoRecording(0); }, throwsA(isA())); expect(camera.recording, null); - - // Verify video capture use case is unbound. - verify(camera.processCameraProvider!.unbind([mockVideoCapture])); }); test(