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

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

Merged
bparrishMines merged 8 commits into from
Mar 13, 2019
Merged

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

Changes from 1 commit
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
Prev Previous commit
Next Next commit
Fix CameraPreview freezes during startVideoRecording on iOS - address… 
… review comment
  • Loading branch information
@cvolzke4
cvolzke4 committed Feb 27, 2019
commit 98b5e398dfae717140f1e6930578842b375ba8b9
3 changes: 3 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 @@ -234,6 +234,9 @@ class CameraController extends ValueNotifier<CameraValue> {
///
/// Throws a [CameraException] if the prepare fails.
Future<void> prepareForVideoRecording() async {
Copy link
Contributor

@sigurdm sigurdm Jan 7, 2019

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

@cvolzke4 cvolzke4 Feb 5, 2019

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

@mehmetf mehmetf Feb 18, 2019

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

@cvolzke4 cvolzke4 Feb 21, 2019

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');
}

Expand Down