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

cvolzke4 · 2019-01-02T09:18:10Z

See flutter/flutter#25961 for more information.

cvolzke4 · 2019-01-02T09:19:23Z

packages/camera/ios/Classes/CameraPlugin.m

@@ -288,9 +288,7 @@ - (void)startVideoRecordingAtPath:(NSString *)path result:(FlutterResult)result
      _eventSink(@{@"event" : @"error", @"errorDescription" : @"Setup Writer Failed"});
      return;
    }
-    [_captureSession stopRunning];


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

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?

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

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

sigurdm

Thanks for looking into this!
I have a few comments, but generally this looks good.
I'm no longer on the flutter team, so would wait for @bparrishMines to approve and land this.

sigurdm · 2019-01-07T08:35:41Z

packages/camera/ios/Classes/CameraPlugin.m

@@ -515,4 +516,4 @@ - (void)handleMethodCall:(FlutterMethodCall *)call result:(FlutterResult)result
  }
 }

-@end
+@end


nit: this lost a newline.

sigurdm · 2019-01-07T08:42:44Z

packages/camera/lib/camera.dart

@@ -222,6 +222,16 @@ class CameraController extends ValueNotifier<CameraValue> {
    return _creatingCompleter.future;
  }

+  /// Prepare the capture session for video recording.
+  /// Preparing audio can cause a minor delay in the CameraPreview view on iOS.


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

sigurdm · 2019-01-07T08:43:38Z

packages/camera/lib/camera.dart

+  /// that would otherwise be experienced when video recording is started.
+  ///
+  /// Throws a [CameraException] if the prepare fails.
+  Future<void> prepareForVideoRecording() async {


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.

There are three use cases to consider:

Start preview, capture still image (takePicture()).

Start preview, capture video+audio recording (startVideoRecording()).

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.

Any more thoughts on this?

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).

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.

sigurdm · 2019-01-07T09:17:33Z

packages/camera/ios/Classes/CameraPlugin.m

@@ -288,9 +288,7 @@ - (void)startVideoRecordingAtPath:(NSString *)path result:(FlutterResult)result
      _eventSink(@{@"event" : @"error", @"errorDescription" : @"Setup Writer Failed"});
      return;
    }
-    [_captureSession stopRunning];


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?

bparrishMines

PR looks good. I tested it on my iphone 6 and the freeze is now significantly shorter with this addition.

bparrishMines · 2019-01-07T23:09:42Z

packages/camera/ios/Classes/CameraPlugin.m

@@ -288,9 +288,7 @@ - (void)startVideoRecordingAtPath:(NSString *)path result:(FlutterResult)result
      _eventSink(@{@"event" : @"error", @"errorDescription" : @"Setup Writer Failed"});
      return;
    }
-    [_captureSession stopRunning];


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

bparrishMines · 2019-01-07T23:13:29Z

packages/camera/android/src/main/java/io/flutter/plugins/camera/CameraPlugin.java

@@ -202,6 +202,12 @@ public void onMethodCall(MethodCall call, final Result result) {
          camera.takePicture((String) call.argument("path"), result);
          break;
        }
+      case "prepareForVideoRecording":
+        {
+          // not yet implemented for Android


Would this need to be implemented on Android? If it does, this would require a // Todo: ...If not, I would include that this method does nothing on Android in the documentation on the dart side.

I haven't tried the camera on Android, but if it would only be required there if it suffers from the same freezing issue.

I've updated the comment re Android no-op.

… review comments

… review comment

packages/camera/lib/camera.dart

… review comment

… address review comment" This reverts commit 8f28873.

… review comment

…lutter#1023)

Fix CameraPreview freezes during startVideoRecording on iOS

56e9eec

googlebot added the cla: yes label Jan 2, 2019

cvolzke4 commented Jan 2, 2019

View reviewed changes

cvolzke4 mentioned this pull request Jan 2, 2019

CameraPreview freezes during startVideoRecording on iOS flutter/flutter#25961

Closed

mehmetf requested a review from bparrishMines January 5, 2019 20:40

sigurdm reviewed Jan 7, 2019

View reviewed changes

bparrishMines reviewed Jan 7, 2019

View reviewed changes

cvolzke4 added 2 commits January 8, 2019 12:11

Fix CameraPreview freezes during startVideoRecording on iOS - address…

a140c07

… review comments

Fix CameraPreview freezes during startVideoRecording on iOS - address…

6dd5e2e

… review comment

mehmetf approved these changes Feb 21, 2019

View reviewed changes

mehmetf reviewed Feb 21, 2019

View reviewed changes

packages/camera/lib/camera.dart Show resolved Hide resolved

cyanglaz changed the title ~~Fix CameraPreview freezes during startVideoRecording on iOS~~ [camera]Fix CameraPreview freezes during startVideoRecording on iOS Feb 23, 2019

cvolzke4 and others added 5 commits February 27, 2019 16:16

Fix CameraPreview freezes during startVideoRecording on iOS - address…

8f28873

… review comment

Revert "Fix CameraPreview freezes during startVideoRecording on iOS -…

b33f2af

… address review comment" This reverts commit 8f28873.

Fix CameraPreview freezes during startVideoRecording on iOS - address…

98b5e39

… review comment

Merge branch 'master' into issue/25961-1

6e7e3bc

version bump

9c346d4

cvolzke4 requested a review from mklim as a code owner March 13, 2019 00:48

bparrishMines merged commit 236885b into flutter:master Mar 13, 2019

Akachu pushed a commit to Akachu/flutter_camera that referenced this pull request Apr 27, 2020

[camera]Fix CameraPreview freezes during startVideoRecording on iOS (f…

b7c81a1

…lutter#1023)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

cvolzke4 commented Jan 2, 2019

cvolzke4 Jan 2, 2019 •

edited

Loading

sigurdm Jan 7, 2019

bparrishMines Jan 7, 2019

cvolzke4 Jan 8, 2019

sigurdm left a comment

sigurdm Jan 7, 2019

cvolzke4 Jan 8, 2019

sigurdm Jan 7, 2019

cvolzke4 Jan 8, 2019

sigurdm Jan 7, 2019

cvolzke4 Jan 8, 2019 •

edited

Loading

cvolzke4 Feb 5, 2019

mehmetf Feb 18, 2019

cvolzke4 Feb 21, 2019

sigurdm Jan 7, 2019

bparrishMines left a comment

bparrishMines Jan 7, 2019

bparrishMines Jan 7, 2019

cvolzke4 Jan 8, 2019 •

edited

Loading

                 }
               }
-              @end
+              @end

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

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

Conversation

cvolzke4 commented Jan 2, 2019

cvolzke4 Jan 2, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sigurdm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cvolzke4 Jan 8, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bparrishMines left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cvolzke4 Jan 8, 2019 • edited Loading

Choose a reason for hiding this comment

cvolzke4 Jan 2, 2019 •

edited

Loading

cvolzke4 Jan 8, 2019 •

edited

Loading

cvolzke4 Jan 8, 2019 •

edited

Loading