[camera_avfoundation] fix race condition when starting image stream on iOS

packages/camera/camera_avfoundation/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.9.18+15
+
+* Fixes race condition when starting image stream.
+
 ## 0.9.18+14
 
 * Creates Swift Package Manager target for Swift implementation.

packages/camera/camera_avfoundation/example/ios/Runner.xcodeproj/project.pbxproj

@@ -25,7 +25,7 @@
 		978D90B42D5F630300CD817E /* StreamingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 978D90B32D5F630300CD817E /* StreamingTests.swift */; };
 		97922B0D2D6380C300A9B4CF /* SampleBufferTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 97922B0C2D6380C300A9B4CF /* SampleBufferTests.swift */; };
 		979B3DFB2D5B6BC7009BDE1A /* ExceptionCatcher.m in Sources */ = {isa = PBXBuildFile; fileRef = 979B3DFA2D5B6BC7009BDE1A /* ExceptionCatcher.m */; };
-		979B3DFE2D5B985B009BDE1A /* CameraCaptureSessionQueueRaceConditionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 979B3DFD2D5B985B009BDE1A /* CameraCaptureSessionQueueRaceConditionTests.swift */; };
+		979B3DFE2D5B985B009BDE1A /* CameraInitRaceConditionsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 979B3DFD2D5B985B009BDE1A /* CameraInitRaceConditionsTests.swift */; };
 		979B3E002D5B9E6C009BDE1A /* CameraMethodChannelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 979B3DFF2D5B9E6C009BDE1A /* CameraMethodChannelTests.swift */; };
 		979B3E022D5BA48F009BDE1A /* CameraOrientationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 979B3E012D5BA48F009BDE1A /* CameraOrientationTests.swift */; };
 		97BD4A0E2D5CC5AE00F857D5 /* CameraSettingsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 97BD4A0D2D5CC5AE00F857D5 /* CameraSettingsTests.swift */; };
@@ -115,7 +115,7 @@
 		979B3DF92D5B6BA2009BDE1A /* ExceptionCatcher.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ExceptionCatcher.h; sourceTree = "<group>"; };
 		979B3DFA2D5B6BC7009BDE1A /* ExceptionCatcher.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ExceptionCatcher.m; sourceTree = "<group>"; };
 		979B3DFC2D5B985B009BDE1A /* RunnerTests-Bridging-Header.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "RunnerTests-Bridging-Header.h"; sourceTree = "<group>"; };
-		979B3DFD2D5B985B009BDE1A /* CameraCaptureSessionQueueRaceConditionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CameraCaptureSessionQueueRaceConditionTests.swift; sourceTree = "<group>"; };
+		979B3DFD2D5B985B009BDE1A /* CameraInitRaceConditionsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CameraInitRaceConditionsTests.swift; sourceTree = "<group>"; };
 		979B3DFF2D5B9E6C009BDE1A /* CameraMethodChannelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CameraMethodChannelTests.swift; sourceTree = "<group>"; };
 		979B3E012D5BA48F009BDE1A /* CameraOrientationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CameraOrientationTests.swift; sourceTree = "<group>"; };
 		97BD4A0D2D5CC5AE00F857D5 /* CameraSettingsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CameraSettingsTests.swift; sourceTree = "<group>"; };
@@ -196,7 +196,7 @@
 				979B3DF92D5B6BA2009BDE1A /* ExceptionCatcher.h */,
 				979B3DFA2D5B6BC7009BDE1A /* ExceptionCatcher.m */,
 				979B3DFC2D5B985B009BDE1A /* RunnerTests-Bridging-Header.h */,
-				979B3DFD2D5B985B009BDE1A /* CameraCaptureSessionQueueRaceConditionTests.swift */,
+				979B3DFD2D5B985B009BDE1A /* CameraInitRaceConditionsTests.swift */,
 				979B3DFF2D5B9E6C009BDE1A /* CameraMethodChannelTests.swift */,
 				979B3E012D5BA48F009BDE1A /* CameraOrientationTests.swift */,
 				97BD4A0D2D5CC5AE00F857D5 /* CameraSettingsTests.swift */,
@@ -557,7 +557,7 @@
 				E1ABED6D2D94392700AED9CC /* MockAssetWriterInput.swift in Sources */,
 				977A25242D5A511600931E34 /* CameraPermissionTests.swift in Sources */,
 				970ADABE2D6740A900EFDCD9 /* MockWritableData.swift in Sources */,
-				979B3DFE2D5B985B009BDE1A /* CameraCaptureSessionQueueRaceConditionTests.swift in Sources */,
+				979B3DFE2D5B985B009BDE1A /* CameraInitRaceConditionsTests.swift in Sources */,
 				E142F13A2D85940600824824 /* MockCapturePhotoOutput.swift in Sources */,
 				E12C4FF82D68E85500515E70 /* MockFLTCameraPermissionManager.swift in Sources */,
 				97922B0D2D6380C300A9B4CF /* SampleBufferTests.swift in Sources */,

packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraCaptureSessionQueueRaceConditionTests.swift → packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraInitRaceConditionsTests.swift

@@ -11,7 +11,7 @@ import XCTest
   @testable import camera_avfoundation_objc
 #endif
 
-final class CameraCaptureSessionQueueRaceConditionTests: XCTestCase {
+final class CameraInitRaceConditionsTests: XCTestCase {
   private func createCameraPlugin() -> CameraPlugin {
     return CameraPlugin(
       registry: MockFlutterTextureRegistry(),
@@ -57,4 +57,39 @@ final class CameraCaptureSessionQueueRaceConditionTests: XCTestCase {
     XCTAssertNotNil(
       cameraPlugin.captureSessionQueue, "captureSessionQueue must not be nil after create method.")
   }
+
+  func testFlutterChannelInitializedWhenStartingImageStream() {
+    let cameraPlugin = createCameraPlugin()
+    let configuration = CameraTestUtils.createTestCameraConfiguration()
+    cameraPlugin.camera = FLTCam(configuration: configuration, error: nil)
+
+    let createExpectation = expectation(description: "create's result block must be called")
+
+    cameraPlugin.createCameraOnSessionQueue(
+      withName: "acamera",
+      settings: FCPPlatformMediaSettings.make(
+        with: .medium,
+        framesPerSecond: nil,
+        videoBitrate: nil,
+        audioBitrate: nil,
+        enableAudio: true
+      )
+    ) { result, error in
+      createExpectation.fulfill()
+    }
+
+    waitForExpectations(timeout: 30, handler: nil)
+
+    // Start stream and wait for its completion.
+    let startStreamExpectation = expectation(
+      description: "startImageStream's result block must be called")
+    cameraPlugin.startImageStream(completion: {
+      _ in
+      startStreamExpectation.fulfill()
+    })
+
+    waitForExpectations(timeout: 30, handler: nil)
+    XCTAssertEqual(cameraPlugin.camera?.isStreamingImages, true)
+  }
+
 }

packages/camera/camera_avfoundation/example/ios/RunnerTests/CameraPluginDelegatingMethodTests.swift

@@ -260,8 +260,9 @@ final class CameraPluginDelegatingMethodTests: XCTestCase {
     let expectation = expectation(description: "Call completed")
 
     var startImageStreamCalled = false
-    mockCamera.startImageStreamStub = { _ in
+    mockCamera.startImageStreamStub = { messenger, completion in
       startImageStreamCalled = true
+      completion(nil)
     }
 
     cameraPlugin.startImageStream { error in

packages/camera/camera_avfoundation/example/ios/RunnerTests/Mocks/MockFLTCam.swift

@@ -38,7 +38,7 @@ final class MockFLTCam: FLTCam {
   var setExposurePointStub: ((FCPPlatformPoint?, ((FlutterError?) -> Void)?) -> Void)?
   var setFocusPointStub: ((FCPPlatformPoint?, ((FlutterError?) -> Void)?) -> Void)?
   var setExposureOffsetStub: ((Double) -> Void)?
-  var startImageStreamStub: ((FlutterBinaryMessenger) -> Void)?
+  var startImageStreamStub: ((FlutterBinaryMessenger, (FlutterError?) -> Void) -> Void)?
   var stopImageStreamStub: (() -> Void)?
   var setZoomLevelStub: ((CGFloat, ((FlutterError?) -> Void)?) -> Void)?
   var setUpCaptureSessionForAudioIfNeededStub: (() -> Void)?
@@ -188,8 +188,11 @@ final class MockFLTCam: FLTCam {
     setExposureOffsetStub?(offset)
   }
 
-  override func startImageStream(with messenger: FlutterBinaryMessenger) {
-    startImageStreamStub?(messenger)
+  override func startImageStream(
+    with messenger: FlutterBinaryMessenger,
+    completion: @escaping (FlutterError?) -> Void
+  ) {
+    startImageStreamStub?(messenger, completion)
   }
 
   override func stopImageStream() {

packages/camera/camera_avfoundation/example/ios/RunnerTests/StreamingTests.swift

@@ -45,14 +45,28 @@ final class StreamingTests: XCTestCase {
 
   func testExceedMaxStreamingPendingFramesCount() {
     let (camera, sampleBuffer) = createCamera()
+    let handlerMock = MockImageStreamHandler()
+
+    let finishStartStreamExpectation = expectation(
+      description: "Finish startStream")
+
+    let messenger = MockFlutterBinaryMessenger()
+    camera.startImageStream(
+      with: messenger, imageStreamHandler: handlerMock,
+      completion: {
+        _ in
+        finishStartStreamExpectation.fulfill()
+      })
+
+    waitForExpectations(timeout: 30, handler: nil)
+
+    // Setup mocked event sink after the stream starts
     let streamingExpectation = expectation(
       description: "Must not call handler over maxStreamingPendingFramesCount")
-    let handlerMock = MockImageStreamHandler()
+
     handlerMock.eventSinkStub = { event in
       streamingExpectation.fulfill()
     }
-    let messenger = MockFlutterBinaryMessenger()
-    camera.startImageStream(with: messenger, imageStreamHandler: handlerMock)
 
     let expectation = XCTKVOExpectation(
       keyPath: "isStreamingImages", object: camera, expectedValue: true)
@@ -69,14 +83,27 @@ final class StreamingTests: XCTestCase {
 
   func testReceivedImageStreamData() {
     let (camera, sampleBuffer) = createCamera()
+    let handlerMock = MockImageStreamHandler()
+
+    let finishStartStreamExpectation = expectation(
+      description: "Finish startStream")
+
+    let messenger = MockFlutterBinaryMessenger()
+    camera.startImageStream(
+      with: messenger, imageStreamHandler: handlerMock,
+      completion: {
+        _ in
+        finishStartStreamExpectation.fulfill()
+      })
+
+    waitForExpectations(timeout: 30, handler: nil)
+
+    // Setup mocked event sink after the stream starts
     let streamingExpectation = expectation(
       description: "Must be able to call the handler again when receivedImageStreamData is called")
-    let handlerMock = MockImageStreamHandler()
     handlerMock.eventSinkStub = { event in
       streamingExpectation.fulfill()
     }
-    let messenger = MockFlutterBinaryMessenger()
-    camera.startImageStream(with: messenger, imageStreamHandler: handlerMock)
 
     let expectation = XCTKVOExpectation(
       keyPath: "isStreamingImages", object: camera, expectedValue: true)

packages/camera/camera_avfoundation/ios/camera_avfoundation/Sources/camera_avfoundation_objc/CameraPlugin.m

@@ -224,8 +224,12 @@ - (void)initializeCamera:(NSInteger)cameraId
 - (void)startImageStreamWithCompletion:(nonnull void (^)(FlutterError *_Nullable))completion {
   __weak typeof(self) weakSelf = self;
   dispatch_async(self.captureSessionQueue, ^{
-    [weakSelf.camera startImageStreamWithMessenger:weakSelf.messenger];
-    completion(nil);
+    typeof(self) strongSelf = weakSelf;
+    if (!strongSelf) {
+      completion(nil);
+      return;
+    }
+    [strongSelf.camera startImageStreamWithMessenger:strongSelf.messenger completion:completion];
   });
 }
 

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

@@ -846,40 +846,49 @@ - (CVPixelBufferRef)copyPixelBuffer {
 - (void)startVideoRecordingWithCompletion:(void (^)(FlutterError *_Nullable))completion
                     messengerForStreaming:(nullable NSObject<FlutterBinaryMessenger> *)messenger {
   if (!_isRecording) {
-    if (messenger != nil) {
-      [self startImageStreamWithMessenger:messenger];
-    }
+    // Main logic to setup the video recording in a block to reuse in the completion block.
+    void (^videoRecordingSetupWithCompletion)(void) = ^void(void) {
+      NSError *error;
+      self->_videoRecordingPath = [self getTemporaryFilePathWithExtension:@"mp4"
+                                                                subfolder:@"videos"
+                                                                   prefix:@"REC_"
+                                                                    error:error];
+      if (error) {
+        completion(FlutterErrorFromNSError(error));
+        return;
+      }
+      if (![self setupWriterForPath:self->_videoRecordingPath]) {
+        completion([FlutterError errorWithCode:@"IOError"
+                                       message:@"Setup Writer Failed"
+                                       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
+      [self->_videoWriter startWriting];
+      self->_isFirstVideoSample = YES;
+      self->_isRecording = YES;
+      self->_isRecordingPaused = NO;
+      self->_videoTimeOffset = CMTimeMake(0, 1);
+      self->_audioTimeOffset = CMTimeMake(0, 1);
+      self->_videoIsDisconnected = NO;
+      self->_audioIsDisconnected = NO;
+      completion(nil);
+    };
 
-    NSError *error;
-    _videoRecordingPath = [self getTemporaryFilePathWithExtension:@"mp4"
-                                                        subfolder:@"videos"
-                                                           prefix:@"REC_"
-                                                            error:error];
-    if (error) {
-      completion(FlutterErrorFromNSError(error));
-      return;
-    }
-    if (![self setupWriterForPath:_videoRecordingPath]) {
-      completion([FlutterError errorWithCode:@"IOError"
-                                     message:@"Setup Writer Failed"
-                                     details:nil]);
+    if (messenger != nil) {
+      [self startImageStreamWithMessenger:messenger
+                               completion:^(FlutterError *_Nullable error) {
+                                 videoRecordingSetupWithCompletion();
+                               }];
       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);
-    _audioTimeOffset = CMTimeMake(0, 1);
-    _videoIsDisconnected = NO;
-    _audioIsDisconnected = NO;
-    completion(nil);
+
+    videoRecordingSetupWithCompletion();
   } else {
     completion([FlutterError errorWithCode:@"Error"
                                    message:@"Video is already recording"
@@ -1176,14 +1185,17 @@ - (void)setExposureOffset:(double)offset {
   [_captureDevice unlockForConfiguration];
 }
 
-- (void)startImageStreamWithMessenger:(NSObject<FlutterBinaryMessenger> *)messenger {
+- (void)startImageStreamWithMessenger:(NSObject<FlutterBinaryMessenger> *)messenger
+                           completion:(void (^)(FlutterError *))completion {
   [self startImageStreamWithMessenger:messenger
                    imageStreamHandler:[[FLTImageStreamHandler alloc]
-                                          initWithCaptureSessionQueue:_captureSessionQueue]];
+                                          initWithCaptureSessionQueue:_captureSessionQueue]
+                           completion:completion];
 }
 
 - (void)startImageStreamWithMessenger:(NSObject<FlutterBinaryMessenger> *)messenger
-                   imageStreamHandler:(FLTImageStreamHandler *)imageStreamHandler {
+                   imageStreamHandler:(FLTImageStreamHandler *)imageStreamHandler
+                           completion:(void (^)(FlutterError *))completion {
   if (!_isStreamingImages) {
     id<FLTEventChannel> eventChannel = [FlutterEventChannel
         eventChannelWithName:@"plugins.flutter.io/camera_avfoundation/imageStream"
@@ -1196,19 +1208,27 @@ - (void)startImageStreamWithMessenger:(NSObject<FlutterBinaryMessenger> *)messen
     [threadSafeEventChannel setStreamHandler:_imageStreamHandler
                                   completion:^{
                                     typeof(self) strongSelf = weakSelf;
-                                    if (!strongSelf) return;
+                                    if (!strongSelf) {
+                                      completion(nil);
+                                      return;
+                                    }
 
                                     dispatch_async(strongSelf.captureSessionQueue, ^{
                                       // cannot use the outter strongSelf
                                       typeof(self) strongSelf = weakSelf;
-                                      if (!strongSelf) return;
+                                      if (!strongSelf) {
+                                        completion(nil);
+                                        return;
+                                      }
 
                                       strongSelf.isStreamingImages = YES;
                                       strongSelf.streamingPendingFramesCount = 0;
+                                      completion(nil);
                                     });
                                   }];
   } else {
     [self reportErrorMessage:@"Images from camera are already streaming!"];
+    completion(nil);
   }
 }
 

packages/camera/camera_avfoundation/ios/camera_avfoundation/Sources/camera_avfoundation_objc/include/camera_avfoundation/FLTCam.h

@@ -99,7 +99,8 @@ NS_ASSUME_NONNULL_BEGIN
        withCompletion:(void (^)(FlutterError *_Nullable))completion
     NS_SWIFT_NAME(setFocusPoint(_:completion:));
 - (void)setExposureOffset:(double)offset;
-- (void)startImageStreamWithMessenger:(NSObject<FlutterBinaryMessenger> *)messenger;
+- (void)startImageStreamWithMessenger:(NSObject<FlutterBinaryMessenger> *)messenger
+                           completion:(nonnull void (^)(FlutterError *_Nullable))completion;
 - (void)stopImageStream;
 - (void)setZoomLevel:(CGFloat)zoom withCompletion:(void (^)(FlutterError *_Nullable))completion;
 - (void)setUpCaptureSessionForAudioIfNeeded;

packages/camera/camera_avfoundation/ios/camera_avfoundation/Sources/camera_avfoundation_objc/include/camera_avfoundation/FLTCam_Test.h

@@ -48,9 +48,9 @@
 - (void)captureOutput:(AVCaptureOutput *)output
     didOutputSampleBuffer:(CMSampleBufferRef)sampleBuffer
            fromConnection:(NSObject<FLTCaptureConnection> *)connection;
-
 /// Start streaming images.
 - (void)startImageStreamWithMessenger:(NSObject<FlutterBinaryMessenger> *)messenger
-                   imageStreamHandler:(FLTImageStreamHandler *)imageStreamHandler;
+                   imageStreamHandler:(FLTImageStreamHandler *)imageStreamHandler
+                           completion:(void (^)(FlutterError *))completion;
 
 @end

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.18+14
+version: 0.9.18+15
 
 environment:
   sdk: ^3.4.0