[camera_avfoundation] fix stopVideoRecording waiting indefinitely and…

… video lag at start (#7065)

Call to `startWriting` was moved to `startVideoRecordingWithCompletion` to fix situation when completion is not called when `_isRecording` is `YES` but `_videoWriter.status` is `AVAssetWriterStatusUnknown` in `stopVideoRecordingWithCompletion` which can happen if `stopVideoRecordingWithCompletion` is called right after `startVideoRecordingWithCompletion` and `didOutputSampleBuffer` had no chance to call `startWriting` in meantime and to avoid lag which happens when is called `startWriting` between first frame creation and its appending.

Fixes flutter/flutter#132016
Fixes flutter/flutter#151319

packages/camera/camera_avfoundation/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.9.17+2
+
+* Fixes stopVideoRecording waiting indefinitely and lag at start of video.
+
 ## 0.9.17+1
 
 * Fixes a crash due to appending sample buffers when readyForMoreMediaData is NO

packages/camera/camera_avfoundation/example/ios/RunnerTests/FLTCamSampleBufferTests.m

@@ -251,4 +251,91 @@ - (void)testDidOutputSampleBufferMustNotAppendSampleWhenReadyForMoreMediaDataIsN
   CFRelease(videoSample);
 }
 
+- (void)testStopVideoRecordingWithCompletionMustCallCompletion {
+  FLTCam *cam = FLTCreateCamWithCaptureSessionQueue(dispatch_queue_create("testing", NULL));
+
+  id writerMock = OCMClassMock([AVAssetWriter class]);
+  OCMStub([writerMock alloc]).andReturn(writerMock);
+  OCMStub([writerMock initWithURL:OCMOCK_ANY fileType:OCMOCK_ANY error:[OCMArg setTo:nil]])
+      .andReturn(writerMock);
+  __block AVAssetWriterStatus status = AVAssetWriterStatusUnknown;
+  OCMStub([writerMock startWriting]).andDo(^(NSInvocation *invocation) {
+    status = AVAssetWriterStatusWriting;
+  });
+  OCMStub([writerMock status]).andDo(^(NSInvocation *invocation) {
+    [invocation setReturnValue:&status];
+  });
+
+  OCMStub([writerMock finishWritingWithCompletionHandler:[OCMArg checkWithBlock:^(id param) {
+                        XCTAssert(status == AVAssetWriterStatusWriting,
+                                  @"Cannot call finishWritingWithCompletionHandler when status is "
+                                  @"not AVAssetWriterStatusWriting.");
+                        void (^handler)(void) = param;
+                        handler();
+                        return YES;
+                      }]]);
+
+  [cam
+      startVideoRecordingWithCompletion:^(FlutterError *_Nullable error) {
+      }
+                  messengerForStreaming:nil];
+
+  __block BOOL completionCalled = NO;
+  [cam stopVideoRecordingWithCompletion:^(NSString *_Nullable path, FlutterError *_Nullable error) {
+    completionCalled = YES;
+  }];
+  XCTAssert(completionCalled, @"Completion was not called.");
+}
+
+- (void)testStartWritingShouldNotBeCalledBetweenSampleCreationAndAppending {
+  FLTCam *cam = FLTCreateCamWithCaptureSessionQueue(dispatch_queue_create("testing", NULL));
+  CMSampleBufferRef videoSample = FLTCreateTestSampleBuffer();
+
+  id connectionMock = OCMClassMock([AVCaptureConnection class]);
+
+  id writerMock = OCMClassMock([AVAssetWriter class]);
+  OCMStub([writerMock alloc]).andReturn(writerMock);
+  OCMStub([writerMock initWithURL:OCMOCK_ANY fileType:OCMOCK_ANY error:[OCMArg setTo:nil]])
+      .andReturn(writerMock);
+  __block BOOL startWritingCalled = NO;
+  OCMStub([writerMock startWriting]).andDo(^(NSInvocation *invocation) {
+    startWritingCalled = YES;
+  });
+
+  __block BOOL videoAppended = NO;
+  id adaptorMock = OCMClassMock([AVAssetWriterInputPixelBufferAdaptor class]);
+  OCMStub([adaptorMock assetWriterInputPixelBufferAdaptorWithAssetWriterInput:OCMOCK_ANY
+                                                  sourcePixelBufferAttributes:OCMOCK_ANY])
+      .andReturn(adaptorMock);
+  OCMStub([adaptorMock appendPixelBuffer:[OCMArg anyPointer] withPresentationTime:kCMTimeZero])
+      .ignoringNonObjectArgs()
+      .andDo(^(NSInvocation *invocation) {
+        videoAppended = YES;
+      });
+
+  id inputMock = OCMClassMock([AVAssetWriterInput class]);
+  OCMStub([inputMock assetWriterInputWithMediaType:OCMOCK_ANY outputSettings:OCMOCK_ANY])
+      .andReturn(inputMock);
+  OCMStub([inputMock isReadyForMoreMediaData]).andReturn(YES);
+
+  [cam
+      startVideoRecordingWithCompletion:^(FlutterError *_Nullable error) {
+      }
+                  messengerForStreaming:nil];
+
+  BOOL startWritingCalledBefore = startWritingCalled;
+  [cam captureOutput:cam.captureVideoOutput
+      didOutputSampleBuffer:videoSample
+             fromConnection:connectionMock];
+  XCTAssert((startWritingCalledBefore && videoAppended) || (startWritingCalled && !videoAppended),
+            @"The startWriting was called between sample creation and appending.");
+
+  [cam captureOutput:cam.captureVideoOutput
+      didOutputSampleBuffer:videoSample
+             fromConnection:connectionMock];
+  XCTAssert(videoAppended, @"Video was not appended.");
+
+  CFRelease(videoSample);
+}
+
 @end

packages/camera/camera_avfoundation/ios/camera_avfoundation/Sources/camera_avfoundation/FLTCam.m

@@ -69,6 +69,7 @@ @interface FLTCam () <AVCaptureVideoDataOutputSampleBufferDelegate,
 @property(strong, nonatomic) AVCaptureVideoDataOutput *videoOutput;
 @property(strong, nonatomic) AVCaptureAudioDataOutput *audioOutput;
 @property(strong, nonatomic) NSString *videoRecordingPath;
+@property(assign, nonatomic) BOOL isFirstVideoSample;
 @property(assign, nonatomic) BOOL isRecording;
 @property(assign, nonatomic) BOOL isRecordingPaused;
 @property(assign, nonatomic) BOOL videoIsDisconnected;
@@ -663,19 +664,19 @@ - (void)captureOutput:(AVCaptureOutput *)output
 
     // ignore audio samples until the first video sample arrives to avoid black frames
     // https://github.com/flutter/flutter/issues/57831
-    if (_videoWriter.status != AVAssetWriterStatusWriting && output != _captureVideoOutput) {
+    if (_isFirstVideoSample && output != _captureVideoOutput) {
       return;
     }
 
     CMTime currentSampleTime = CMSampleBufferGetPresentationTimeStamp(sampleBuffer);
 
-    if (_videoWriter.status != AVAssetWriterStatusWriting) {
-      [_videoWriter startWriting];
+    if (_isFirstVideoSample) {
       [_videoWriter startSessionAtSourceTime:currentSampleTime];
       // fix sample times not being numeric when pause/resume happens before first sample buffer
       // arrives https://github.com/flutter/flutter/issues/132014
       _lastVideoSampleTime = currentSampleTime;
       _lastAudioSampleTime = currentSampleTime;
+      _isFirstVideoSample = NO;
     }
 
     if (output == _captureVideoOutput) {
@@ -834,6 +835,14 @@ - (void)startVideoRecordingWithCompletion:(void (^)(FlutterError *_Nullable))com
                                      details:nil]);
       return;
     }
+    // startWriting should not be called in didOutputSampleBuffer where it can cause state
+    // in which _isRecording is YES but _videoWriter.status is AVAssetWriterStatusUnknown
+    // in stopVideoRecording if it is called after startVideoRecording but before
+    // didOutputSampleBuffer had chance to call startWriting and lag at start of video
+    // https://github.com/flutter/flutter/issues/132016
+    // https://github.com/flutter/flutter/issues/151319
+    [_videoWriter startWriting];
+    _isFirstVideoSample = YES;
     _isRecording = YES;
     _isRecordingPaused = NO;
     _videoTimeOffset = CMTimeMake(0, 1);
@@ -853,19 +862,21 @@ - (void)stopVideoRecordingWithCompletion:(void (^)(NSString *_Nullable,
   if (_isRecording) {
     _isRecording = NO;
 
-    if (_videoWriter.status != AVAssetWriterStatusUnknown) {
-      [_videoWriter finishWritingWithCompletionHandler:^{
-        if (self->_videoWriter.status == AVAssetWriterStatusCompleted) {
-          [self updateOrientation];
-          completion(self->_videoRecordingPath, nil);
-          self->_videoRecordingPath = nil;
-        } else {
-          completion(nil, [FlutterError errorWithCode:@"IOError"
-                                              message:@"AVAssetWriter could not finish writing!"
-                                              details:nil]);
-        }
-      }];
-    }
+    // when _isRecording is YES startWriting was already called so _videoWriter.status
+    // is always either AVAssetWriterStatusWriting or AVAssetWriterStatusFailed and
+    // finishWritingWithCompletionHandler does not throw exception so there is no need
+    // to check _videoWriter.status
+    [_videoWriter finishWritingWithCompletionHandler:^{
+      if (self->_videoWriter.status == AVAssetWriterStatusCompleted) {
+        [self updateOrientation];
+        completion(self->_videoRecordingPath, nil);
+        self->_videoRecordingPath = nil;
+      } else {
+        completion(nil, [FlutterError errorWithCode:@"IOError"
+                                            message:@"AVAssetWriter could not finish writing!"
+                                            details:nil]);
+      }
+    }];
   } else {
     NSError *error =
         [NSError errorWithDomain:NSCocoaErrorDomain

packages/camera/camera_avfoundation/pubspec.yaml

@@ -2,7 +2,7 @@ name: camera_avfoundation
 description: iOS implementation of the camera plugin.
 repository: https://github.com/flutter/packages/tree/main/packages/camera/camera_avfoundation
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22
-version: 0.9.17+1
+version: 0.9.17+2
 
 environment:
   sdk: ^3.2.3