Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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.6.5

* Modifies `stopVideoRecording` to ensure the method only returns when the recorded video finishes saving to a file.
* Adds empty implementation for `setDescriptionWhileRecording` and leaves a todo to add this feature.

## 0.6.4+1

* Adds empty implementation for `prepareForVideoRecording` since this optimization is not used on Android.
Expand Down
12 changes: 4 additions & 8 deletions packages/camera/camera_android_camerax/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ use cases, the plugin behaves according to the following:
video recording and image streaming is supported, but concurrent video recording, image
streaming, and image capture is not supported.

### `setDescriptionWhileRecording` is unimplemented [Issue #148013][148013]
`setDescriptionWhileRecording`, used to switch cameras while recording video, is currently unimplemented.

### 240p resolution configuration for video recording

240p resolution configuration for video recording is unsupported by CameraX,
Expand Down Expand Up @@ -64,11 +67,4 @@ For more information on contributing to this plugin, see [`CONTRIBUTING.md`](CON
[6]: https://developer.android.com/media/camera/camerax/architecture#combine-use-cases
[7]: https://developer.android.com/reference/android/hardware/camera2/CameraMetadata#INFO_SUPPORTED_HARDWARE_LEVEL_3
[8]: https://developer.android.com/reference/android/hardware/camera2/CameraMetadata#INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED
[120462]: https://github.com/flutter/flutter/issues/120462
[125915]: https://github.com/flutter/flutter/issues/125915
[120715]: https://github.com/flutter/flutter/issues/120715
[120468]: https://github.com/flutter/flutter/issues/120468
[120467]: https://github.com/flutter/flutter/issues/120467
[125371]: https://github.com/flutter/flutter/issues/125371
[126477]: https://github.com/flutter/flutter/issues/126477
[127896]: https://github.com/flutter/flutter/issues/127896
[148013]: https://github.com/flutter/flutter/issues/148013
Original file line number Diff line number Diff line change
Expand Up @@ -2144,6 +2144,15 @@ public void create(@NonNull Long identifierArg, @NonNull Reply<Void> callback) {
new ArrayList<Object>(Collections.singletonList(identifierArg)),
channelReply -> callback.reply(null));
}

public void onVideoRecordingFinalized(@NonNull Reply<Void> callback) {
BasicMessageChannel<Object> channel =
new BasicMessageChannel<>(
binaryMessenger,
"dev.flutter.pigeon.PendingRecordingFlutterApi.onVideoRecordingFinalized",
getCodec());
channel.send(null, channelReply -> callback.reply(null));
}
}
/** Generated interface from Pigeon that represents a handler of messages from Flutter. */
public interface RecordingHostApi {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,8 @@ public PendingRecordingFlutterApiImpl(
void create(@NonNull PendingRecording pendingRecording, @Nullable Reply<Void> reply) {
create(instanceManager.addHostCreatedInstance(pendingRecording), reply);
}

void sendVideoRecordingFinalizedEvent(@NonNull Reply<Void> reply) {
super.onVideoRecordingFinalized(reply);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ public class PendingRecordingHostApiImpl implements PendingRecordingHostApi {

@VisibleForTesting @NonNull public CameraXProxy cameraXProxy = new CameraXProxy();

@VisibleForTesting PendingRecordingFlutterApiImpl pendingRecordingFlutterApi;

@VisibleForTesting SystemServicesFlutterApiImpl systemServicesFlutterApi;

@VisibleForTesting RecordingFlutterApiImpl recordingFlutterApi;
Expand All @@ -37,6 +39,8 @@ public PendingRecordingHostApiImpl(
this.context = context;
systemServicesFlutterApi = cameraXProxy.createSystemServicesFlutterApiImpl(binaryMessenger);
recordingFlutterApi = new RecordingFlutterApiImpl(binaryMessenger, instanceManager);
pendingRecordingFlutterApi =
new PendingRecordingFlutterApiImpl(binaryMessenger, instanceManager);
}

/** Sets the context, which is used to get the {@link Executor} needed to start the recording. */
Expand Down Expand Up @@ -77,6 +81,7 @@ public Executor getExecutor() {
@VisibleForTesting
public void handleVideoRecordEvent(@NonNull VideoRecordEvent event) {
if (event instanceof VideoRecordEvent.Finalize) {
pendingRecordingFlutterApi.sendVideoRecordingFinalizedEvent(reply -> {});
VideoRecordEvent.Finalize castedEvent = (VideoRecordEvent.Finalize) event;
if (castedEvent.hasError()) {
String cameraErrorMessage;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public class PendingRecordingTest {
@Mock public RecordingFlutterApiImpl mockRecordingFlutterApi;
@Mock public Context mockContext;
@Mock public SystemServicesFlutterApiImpl mockSystemServicesFlutterApi;
@Mock public PendingRecordingFlutterApiImpl mockPendingRecordingFlutterApi;
@Mock public VideoRecordEvent.Finalize event;
@Mock public Throwable throwable;

Expand Down Expand Up @@ -80,6 +81,7 @@ public void testHandleVideoRecordEventSendsError() {
PendingRecordingHostApiImpl pendingRecordingHostApi =
new PendingRecordingHostApiImpl(mockBinaryMessenger, testInstanceManager, mockContext);
pendingRecordingHostApi.systemServicesFlutterApi = mockSystemServicesFlutterApi;
pendingRecordingHostApi.pendingRecordingFlutterApi = mockPendingRecordingFlutterApi;
final String eventMessage = "example failure message";

when(event.hasError()).thenReturn(true);
Expand All @@ -89,9 +91,25 @@ public void testHandleVideoRecordEventSendsError() {

pendingRecordingHostApi.handleVideoRecordEvent(event);

verify(mockPendingRecordingFlutterApi).sendVideoRecordingFinalizedEvent(any());
verify(mockSystemServicesFlutterApi).sendCameraError(eq(eventMessage), any());
}

@Test
public void handleVideoRecordEvent_SendsVideoRecordingFinalizedEvent() {
PendingRecordingHostApiImpl pendingRecordingHostApi =
new PendingRecordingHostApiImpl(mockBinaryMessenger, testInstanceManager, mockContext);
pendingRecordingHostApi.pendingRecordingFlutterApi = mockPendingRecordingFlutterApi;
final String eventMessage = "example failure message";

when(event.hasError()).thenReturn(false);
doNothing().when(mockSystemServicesFlutterApi).sendCameraError(any(), any());

pendingRecordingHostApi.handleVideoRecordEvent(event);

verify(mockPendingRecordingFlutterApi).sendVideoRecordingFinalizedEvent(any());
}

@Test
public void flutterApiCreateTest() {
final PendingRecordingFlutterApiImpl spyPendingRecordingFlutterApi =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import 'package:camera_platform_interface/camera_platform_interface.dart';
import 'package:flutter/painting.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:integration_test/integration_test.dart';
import 'package:video_player/video_player.dart';

void main() {
IntegrationTestWidgetsFlutterBinding.ensureInitialized();
Expand Down Expand Up @@ -178,4 +179,86 @@ void main() {
}
}
});

testWidgets('Video capture records valid video', (WidgetTester tester) async {
final List<CameraDescription> cameras = await availableCameras();
if (cameras.isEmpty) {
return;
}

final CameraController controller = CameraController(cameras[0],
mediaSettings:
const MediaSettings(resolutionPreset: ResolutionPreset.low));
await controller.initialize();
await controller.prepareForVideoRecording();

await controller.startVideoRecording();
final int recordingStart = DateTime.now().millisecondsSinceEpoch;

sleep(const Duration(seconds: 2));

final XFile file = await controller.stopVideoRecording();
final int recordingTime =
DateTime.now().millisecondsSinceEpoch - recordingStart;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can make this test a bit better by comparing on both sides of the stopRecording call.

Suggested change
sleep(const Duration(seconds: 2));
final XFile file = await controller.stopVideoRecording();
final int recordingTime =
DateTime.now().millisecondsSinceEpoch - recordingStart;
// Test should pass with this at any non zero value.
sleep(const Duration(seconds: 2));
final int preStopTime =
DateTime.now().millisecondsSinceEpoch - recordingStart;
final XFile file = await controller.stopVideoRecording();
final int postStopTime =
DateTime.now().millisecondsSinceEpoch - recordingStart;
....
expect(duration, greaterThan(preStopTime));
expect(duration, lessThan(postStopTime));

Note:
We should be using a package like https://pub.dev/packages/system_clock because DateTime.now is vulnerable to the clock changing while the test is running but that feels out of scope.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The preStopTime check didn't quite work I assume because stopping video recording takes so long but I kept the naming of postStopTime for clarity

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't matter how long stopping recording takes.

The timeline:
start recording
record start time
delay
record prestop time
trigger stop

The comparison for prestop does not depend on the length of time it takes to stop recording.
At minimum the recording should be longer than the pause duration. What is the value of duration when the test fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe the start recording is slow I mean? The duration was 1448 ms (which is also below the 2000 ms of recording), prestop was 2002 ms.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we could also fix this by listening for the VideoRecordEvent.Start event to return. Let me give it a go!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@reidbaker Ok so I added the fix but it didn't quite work still :/ the duration increased but guessing there still some lag time in starting the video. Still left it in though as it is a better approximation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going to land this as-is since I've done all I can to remedy this issue. If we feel like we need this fixed, I can file a bug and escalate to CameraX.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed flutter/flutter#148138 for this.


final File videoFile = File(file.path);
final VideoPlayerController videoController = VideoPlayerController.file(
videoFile,
);
await videoController.initialize();
final int duration = videoController.value.duration.inMilliseconds;
await videoController.dispose();

expect(duration, lessThan(recordingTime));
});

testWidgets('Pause and resume video recording', (WidgetTester tester) async {
final List<CameraDescription> cameras = await availableCameras();
if (cameras.isEmpty) {
return;
}

final CameraController controller = CameraController(cameras[0],
mediaSettings:
const MediaSettings(resolutionPreset: ResolutionPreset.low));
await controller.initialize();
await controller.prepareForVideoRecording();

int startPause;
int timePaused = 0;

await controller.startVideoRecording();
final int recordingStart = DateTime.now().millisecondsSinceEpoch;
sleep(const Duration(milliseconds: 500));

await controller.pauseVideoRecording();
startPause = DateTime.now().millisecondsSinceEpoch;
sleep(const Duration(milliseconds: 500));
await controller.resumeVideoRecording();
timePaused += DateTime.now().millisecondsSinceEpoch - startPause;

sleep(const Duration(milliseconds: 500));

await controller.pauseVideoRecording();
startPause = DateTime.now().millisecondsSinceEpoch;
sleep(const Duration(milliseconds: 500));
await controller.resumeVideoRecording();
timePaused += DateTime.now().millisecondsSinceEpoch - startPause;

sleep(const Duration(milliseconds: 500));

final XFile file = await controller.stopVideoRecording();
final int recordingTime =
DateTime.now().millisecondsSinceEpoch - recordingStart;

final File videoFile = File(file.path);
final VideoPlayerController videoController = VideoPlayerController.file(
videoFile,
);
await videoController.initialize();
final int duration = videoController.value.duration.inMilliseconds;
await videoController.dispose();

expect(duration, lessThan(recordingTime - timePaused));
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,12 @@ class AndroidCameraCameraX extends CameraPlatform {
@visibleForTesting
String? videoOutputPath;

/// Stream queue to pick up finalized viceo recording events in
/// [stopVideoRecording].
final StreamQueue<void> videoRecordingFinalizedStreamQueue =
StreamQueue<void>(
PendingRecording.videoRecordingFinalizedStreamController.stream);

/// Whether or not [preview] has been bound to the lifecycle of the camera by
/// [createCamera].
@visibleForTesting
Expand All @@ -122,7 +128,7 @@ class AndroidCameraCameraX extends CameraPlatform {

/// The prefix used to create the filename for video recording files.
@visibleForTesting
final String videoPrefix = 'MOV';
final String videoPrefix = 'REC';

/// The [ImageCapture] instance that can be configured to capture a still image.
@visibleForTesting
Expand Down Expand Up @@ -777,6 +783,15 @@ class AndroidCameraCameraX extends CameraPlatform {
await _unbindUseCaseFromLifecycle(preview!);
}

/// Sets the active camera while recording.
///
/// Currently unsupported, so is a no-op.
@override
Future<void> setDescriptionWhileRecording(CameraDescription description) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean we have a feature we know will silently stop working when we modify the default to camerax?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does mean that :/ but the fact that it will be a breaking change can mitigate that fact; it will be noted in the README here but I can also add it to the camera/camera CHANGELOG and camera/camera README.

@stuartmorgan any thoughts on this? I realized recently that this was not implemented for camera_android_camerax.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would definitely note the loss of functionality in the CHANGELOG for the camera breaking change.

This is a pretty new feature IIRC, and I would expect relatively niche, so losing it in a breaking change doesn't seem like a major issue. People will always have the possibility of using camera_android (or a fork of it, if/when we stop maintaining it) if they need it.

// TODO(camsim99): Implement this feature, see https://github.com/flutter/flutter/issues/148013.
return Future<void>.value();
}

/// Resume the paused preview for the selected camera.
///
/// [cameraId] not used.
Expand Down Expand Up @@ -955,8 +970,7 @@ class AndroidCameraCameraX extends CameraPlatform {
.setTargetRotation(await proxy.getDefaultDisplayRotation());
}

videoOutputPath =
await SystemServices.getTempFilePath(videoPrefix, '.temp');
videoOutputPath = await SystemServices.getTempFilePath(videoPrefix, '.mp4');
pendingRecording = await recorder!.prepareRecording(videoOutputPath!);
recording = await pendingRecording!.start();

Expand All @@ -979,21 +993,22 @@ class AndroidCameraCameraX extends CameraPlatform {
'Attempting to stop a '
'video recording while no recording is in progress.');
}

/// Stop the active recording and wiat for the video recording to be finalized.
await recording!.close();
await videoRecordingFinalizedStreamQueue.next;
recording = null;
pendingRecording = null;

if (videoOutputPath == null) {
// Stop the current active recording as we will be unable to complete it
// in this error case.
await recording!.close();
recording = null;
pendingRecording = null;
// Handle any errors with finalizing video recording.
throw CameraException(
'INVALID_PATH',
'The platform did not return a path '
'while reporting success. The platform should always '
'return a valid path or report an error.');
}
await recording!.close();
recording = null;
pendingRecording = null;

await _unbindUseCaseFromLifecycle(videoCapture!);
return XFile(videoOutputPath!);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1585,6 +1585,8 @@ abstract class PendingRecordingFlutterApi {

void create(int identifier);

void onVideoRecordingFinalized();

static void setup(PendingRecordingFlutterApi? api,
{BinaryMessenger? binaryMessenger}) {
{
Expand All @@ -1606,6 +1608,21 @@ abstract class PendingRecordingFlutterApi {
});
}
}
{
final BasicMessageChannel<Object?> channel = BasicMessageChannel<Object?>(
'dev.flutter.pigeon.PendingRecordingFlutterApi.onVideoRecordingFinalized',
codec,
binaryMessenger: binaryMessenger);
if (api == null) {
channel.setMessageHandler(null);
} else {
channel.setMessageHandler((Object? message) async {
// ignore message
api.onVideoRecordingFinalized();
return;
});
}
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:async';

import 'package:flutter/services.dart' show BinaryMessenger;
import 'package:meta/meta.dart' show immutable;

Expand Down Expand Up @@ -30,6 +32,10 @@ class PendingRecording extends JavaObject {

late final PendingRecordingHostApiImpl _api;

/// Stream that emits an event when the corresponding video recording is finalized.
static final StreamController<void> videoRecordingFinalizedStreamController =
StreamController<void>.broadcast();

/// Starts the recording, making it an active recording.
Future<Recording> start() {
return _api.startFromInstance(this);
Expand Down Expand Up @@ -100,4 +106,9 @@ class PendingRecordingFlutterApiImpl extends PendingRecordingFlutterApi {
);
});
}

@override
void onVideoRecordingFinalized() {
PendingRecording.videoRecordingFinalizedStreamController.add(null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,8 @@ abstract class PendingRecordingHostApi {
@FlutterApi()
abstract class PendingRecordingFlutterApi {
void create(int identifier);

void onVideoRecordingFinalized();
}

@HostApi(dartHostTestHandler: 'TestRecordingHostApi')
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.6.4+1
version: 0.6.5

environment:
sdk: ^3.1.0
Expand Down
Loading