Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[camera]Fix CameraPreview freezes during startVideoRecording on iOS #1023

Merged
merged 8 commits into from
Mar 13, 2019
Merged

[camera]Fix CameraPreview freezes during startVideoRecording on iOS #1023

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
56e9eec
Fix CameraPreview freezes during startVideoRecording on iOS
cvolzke4 Jan 2, 2019
a140c07
Fix CameraPreview freezes during startVideoRecording on iOS - address…
cvolzke4 Jan 8, 2019
6dd5e2e
Fix CameraPreview freezes during startVideoRecording on iOS - address…
cvolzke4 Feb 21, 2019
8f28873
Fix CameraPreview freezes during startVideoRecording on iOS - address…
cvolzke4 Feb 27, 2019
b33f2af
Revert "Fix CameraPreview freezes during startVideoRecording on iOS -…
cvolzke4 Feb 27, 2019
98b5e39
Fix CameraPreview freezes during startVideoRecording on iOS - address…
cvolzke4 Feb 27, 2019
6e7e3bc
Merge branch 'master' into issue/25961-1
cvolzke4 Mar 8, 2019
9c346d4
version bump
bparrishMines Mar 13, 2019
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
4 changes: 4 additions & 0 deletions packages/camera/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.4.3

* Add capability to prepare the capture session for video recording on iOS.

## 0.4.2

* Add sensor orientation value to `CameraDescription`.
Expand Down
6 changes: 6 additions & 0 deletions packages/camera/android/src/main/java/io/flutter/plugins/camera/CameraPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,12 @@ public void onMethodCall(MethodCall call, final Result result) {
camera.takePicture((String) call.argument("path"), result);
break;
}
case "prepareForVideoRecording":
{
// This optimization is not required for Android.
result.success(null);
break;
}
case "startVideoRecording":
{
final String filePath = call.argument("filePath");
Expand Down
5 changes: 3 additions & 2 deletions packages/camera/ios/Classes/CameraPlugin.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,9 +467,7 @@ - (void)startVideoRecordingAtPath:(NSString *)path result:(FlutterResult)result
_eventSink(@{@"event" : @"error", @"errorDescription" : @"Setup Writer Failed"});
return;
}
[_captureSession stopRunning];
Copy link
Contributor Author

@cvolzke4 cvolzke4 Jan 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

This stopRunning() / startRunning() pair interferes with the CameraPreview view, causing it to freeze momentarily. Are they needed?

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 you are right that they are not needed.
I no longer have an iOS setup to test it. Does recording work without them?

Copy link
Contributor

Choose a reason for hiding this comment

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

I checked out their branch and it everything still works when it's removed on my iphone 6.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Maurice for checking that. It also seems to work on my iPhone 7.

_isRecording = YES;
[_captureSession startRunning];
result(nil);
} else {
_eventSink(@{@"event" : @"error", @"errorDescription" : @"Video is already recording!"});
Expand Down Expand Up @@ -726,6 +724,9 @@ - (void)handleMethodCallAsync:(FlutterMethodCall *)call result:(FlutterResult)re
[_camera close];
_dispatchQueue = nil;
result(nil);
} else if ([@"prepareForVideoRecording" isEqualToString:call.method]) {
[_camera setUpCaptureSessionForAudio];
result(nil);
} else if ([@"startVideoRecording" isEqualToString:call.method]) {
[_camera startVideoRecordingAtPath:call.arguments[@"filePath"] result:result];
} else if ([@"stopVideoRecording" isEqualToString:call.method]) {
Expand Down
18 changes: 18 additions & 0 deletions packages/camera/lib/camera.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,24 @@ class CameraController extends ValueNotifier<CameraValue> {
return _creatingCompleter.future;
}

/// Prepare the capture session for video recording.
///
/// Use of this method is optional, but it may be called for performance
/// reasons on iOS.
///
/// Preparing audio can cause a minor delay in the CameraPreview view on iOS.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: leave an empty line after the first line in dart-doc comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

/// If video recording is intended, calling this early eliminates this delay
/// that would otherwise be experienced when video recording is started.
/// This operation is a no-op on Android.
///
/// Throws a [CameraException] if the prepare fails.
Future<void> prepareForVideoRecording() async {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there some way we can avoid exposing this on the dart side?
Could we always call this as part of the initialization of the camera itself or is that too costly if the camera is only started much later?
It seems annoying for the user to have several initialization steps to do - and it also exposes platform specifics, so it would be nice to avoid if possible.

Copy link
Contributor Author

@cvolzke4 cvolzke4 Jan 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

There are three use cases to consider:

  1. Start preview, capture still image (takePicture()).
  2. Start preview, capture video+audio recording (startVideoRecording()).
  3. Start preview,
    Capture still image,
    Switch to 'video preview' (call prepareForVideoRecording() here),
    Start video+audio recording (expect no delay).

Today in flutter, use cases #2 and #3 cause a delay in the preview when startVideoRecording() is started.

Use case #3 without this delay would look like this (iPhone native camera app):
https://drive.google.com/open?id=17OAkOADgcSMgyNtg6TkN6Gj3fYkXAiye0Q

This demo doesn't show a delay when starting video capture, presumably because audio capture is initialized by the native app when the user taps the 'Video' button while fuzzing the preview view. This isn't possible in flutter without a prepareForVideoRecording(), or initializing audio early.

Initializing audio early is an option, but isn't ideal:

  • Use case Add url-launcher plugin #1 would require audio permissions.
  • Use case #3 would have undue delay on preview init, if the user only intends to capture a still image.

prepareForVideoRecording() is an optional optimization, giving the developer the option of choosing when to take on the delay in the preview view.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any more thoughts on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not have both?

Can we manage some initialization state (presumably on iOS side?) so that if the user has not called prepareForVideoRecording(), it still works? My guess is that majority of our users won't care about that delay so forcing them to call an additional API is harsh. For the remaining users, having this fine tuned control is great.

We should also document this behavior (that this is an optional call).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

prepareForVideoRecording() is intended to be an optional call, it would only be required for users that care about performance in the above way.

I've added a comment to make this more clear.

// TODO(amirh): remove this on when the invokeMethod update makes it to stable Flutter.
// https://github.com/flutter/flutter/issues/26431
// ignore: strong_mode_implicit_dynamic_method
await _channel.invokeMethod('prepareForVideoRecording');
}

/// Listen to events from the native plugins.
///
/// A "cameraClosing" event is sent when the camera is closed automatically by the system (for example when the app go to background). The plugin will try to reopen the camera automatically but any ongoing recording will end.
Expand Down
2 changes: 1 addition & 1 deletion packages/camera/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: camera
description: A Flutter plugin for getting information about and controlling the
camera on Android and iOS. Supports previewing the camera feed, capturing images, capturing video,
and streaming image buffers to dart.
version: 0.4.2
version: 0.4.3
authors:
- Flutter Team <[email protected]>
- Luigi Agosti <[email protected]>
Expand Down