Skip to content

[camerax] Small fixes to starting/stopping video capture #6068

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/camera/camera_android_camerax/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 0.5.0+32

* Removes all remaining `unawaited` calls to fix potential race conditions and updates the
camera state when video capture starts.

## 0.5.0+31

* Wraps CameraX classes needed to set capture request options, which is needed to implement setting the exposure mode.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,9 +357,9 @@ class AndroidCameraCameraX extends CameraPlatform {
@override
Future<void> 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.
Expand Down Expand Up @@ -645,6 +645,7 @@ class AndroidCameraCameraX extends CameraPlatform {
if (!(await processCameraProvider!.isBound(videoCapture!))) {
camera = await processCameraProvider!
.bindToLifecycle(cameraSelector!, <UseCase>[videoCapture!]);
await _updateCameraInfoAndLiveCameraState(options.cameraId);
}

// Set target rotation to default CameraX rotation only if capture
Expand Down Expand Up @@ -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(
Expand All @@ -690,7 +691,7 @@ 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();
recording = null;
pendingRecording = null;
return XFile(videoOutputPath!);
Expand Down Expand Up @@ -813,7 +814,7 @@ class AndroidCameraCameraX extends CameraPlatform {
/// Removes the previously set analyzer on the [imageAnalysis] instance, since
/// image information should no longer be streamed.
FutureOr<void> _onFrameStreamCancel() async {
unawaited(imageAnalysis!.clearAnalyzer());
await imageAnalysis!.clearAnalyzer();
}

/// Converts between Android ImageFormat constants and [ImageFormatGroup]s.
Expand Down
2 changes: 1 addition & 1 deletion packages/camera/camera_android_camerax/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<CameraException>()));
await expectLater(() async {
await camera.initializeCamera(3);
}, throwsA(isA<CameraException>()));
});

test('initializeCamera sends expected CameraInitializedEvent', () async {
Expand Down Expand Up @@ -1006,26 +1008,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<CameraState>.detached(onChanged: onChanged));

const int cameraId = 17;
const String outputPath = '/temp/MOV123.temp';

Expand All @@ -1039,12 +1052,30 @@ void main() {
.thenAnswer((_) async => false);
when(camera.processCameraProvider!.bindToLifecycle(
camera.cameraSelector!, <UseCase>[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!, <UseCase>[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<dynamic>),
isTrue);

// Verify recording is started.
expect(camera.pendingRecording, equals(mockPendingRecording));
expect(camera.recording, mockRecording);
});
Expand All @@ -1056,6 +1087,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);
Expand All @@ -1069,6 +1102,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<CameraState>.detached(onChanged: onChanged));

const int cameraId = 17;
const String outputPath = '/temp/MOV123.temp';

Expand All @@ -1082,7 +1120,11 @@ void main() {
.thenAnswer((_) async => false);
when(camera.processCameraProvider!.bindToLifecycle(
camera.cameraSelector!, <UseCase>[camera.videoCapture!]))
.thenAnswer((_) async => MockCamera());
.thenAnswer((_) async => mockCamera);
when(mockCamera.getCameraInfo())
.thenAnswer((_) => Future<CameraInfo>.value(mockCameraInfo));
when(mockCameraInfo.getCameraState())
.thenAnswer((_) async => MockLiveCameraState());

await camera.startVideoCapturing(const VideoCaptureOptions(cameraId));

Expand Down Expand Up @@ -1267,9 +1309,14 @@ 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);
});
Expand All @@ -1284,47 +1331,57 @@ void main() {
camera.recording = null;
camera.videoOutputPath = videoOutputPath;

expect(
() => camera.stopVideoRecording(0), throwsA(isA<CameraException>()));
await expectLater(() async {
await camera.stopVideoRecording(0);
}, throwsA(isA<CameraException>()));
});
});

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<CameraException>()));
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<CameraException>()));
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;
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';

final XFile file = await camera.stopVideoRecording(0);
expect(file.path, videoOutputPath);
// Set directly for test versus calling createCamera and startVideoCapturing.
camera.processCameraProvider = processCameraProvider;
camera.recording = recording;
camera.videoCapture = videoCapture;
camera.videoOutputPath = videoOutputPath;

expect(
() => camera.stopVideoRecording(0), throwsA(isA<CameraException>()));
});
final XFile file = await camera.stopVideoRecording(0);
expect(file.path, videoOutputPath);

await expectLater(() async {
await camera.stopVideoRecording(0);
}, throwsA(isA<CameraException>()));
});

test(
Expand Down