Skip to content

Commit daeaf42

Browse files
js2702Ortes
authored andcommitted
[camera_avfoundation] fix race condition when starting image stream on iOS (flutter#8733)
Fixes flutter/flutter#147702 It's not easy to reproduce this with the example application (as shown in the linked issue). We've been able to reproduce it in our own application which makes heavy usage of Platform channels and different native plugins. After digging into the code we notice that: The Dart code that starts listening to the event channel was being executed before initializing the stream in the native side. https://github.com/flutter/packages/blob/9744c9dc4f865cf943776e8c3382b3228b2e7cfd/packages/camera/camera_avfoundation/lib/src/avfoundation_camera.dart#L271 https://github.com/flutter/packages/blob/9744c9dc4f865cf943776e8c3382b3228b2e7cfd/packages/camera/camera_avfoundation/ios/camera_avfoundation/Sources/camera_avfoundation/FLTThreadSafeEventChannel.m#L31 I don't have much experience with Objective-C, so not sure how to create unit tests for that if necessary but I believe this would be an adecuate solution. I've seen other methods in the plugin that use the same strategy passing the completion object. - [] I added new tests to check the change I am making, or this PR is [test-exempt]. - [] All existing and new tests are passing.
1 parent 1f8f89d commit daeaf42

File tree

12 files changed

+158
-65
lines changed

12 files changed

+158
-65
lines changed

packages/camera/camera_avfoundation/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
## 0.9.19+3
2+
3+
* Fixes race condition when starting image stream.
4+
5+
16
## 0.9.19+2
27

38
* Adds the `Camera` Swift protocol.

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
978D90B42D5F630300CD817E /* StreamingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 978D90B32D5F630300CD817E /* StreamingTests.swift */; };
2626
97922B0D2D6380C300A9B4CF /* SampleBufferTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 97922B0C2D6380C300A9B4CF /* SampleBufferTests.swift */; };
2727
979B3DFB2D5B6BC7009BDE1A /* ExceptionCatcher.m in Sources */ = {isa = PBXBuildFile; fileRef = 979B3DFA2D5B6BC7009BDE1A /* ExceptionCatcher.m */; };
28-
979B3DFE2D5B985B009BDE1A /* CameraCaptureSessionQueueRaceConditionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 979B3DFD2D5B985B009BDE1A /* CameraCaptureSessionQueueRaceConditionTests.swift */; };
28+
979B3DFE2D5B985B009BDE1A /* CameraInitRaceConditionsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 979B3DFD2D5B985B009BDE1A /* CameraInitRaceConditionsTests.swift */; };
2929
979B3E002D5B9E6C009BDE1A /* CameraMethodChannelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 979B3DFF2D5B9E6C009BDE1A /* CameraMethodChannelTests.swift */; };
3030
979B3E022D5BA48F009BDE1A /* CameraOrientationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 979B3E012D5BA48F009BDE1A /* CameraOrientationTests.swift */; };
3131
97BD4A0E2D5CC5AE00F857D5 /* CameraSettingsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 97BD4A0D2D5CC5AE00F857D5 /* CameraSettingsTests.swift */; };
@@ -114,7 +114,7 @@
114114
979B3DF92D5B6BA2009BDE1A /* ExceptionCatcher.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ExceptionCatcher.h; sourceTree = "<group>"; };
115115
979B3DFA2D5B6BC7009BDE1A /* ExceptionCatcher.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ExceptionCatcher.m; sourceTree = "<group>"; };
116116
979B3DFC2D5B985B009BDE1A /* RunnerTests-Bridging-Header.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "RunnerTests-Bridging-Header.h"; sourceTree = "<group>"; };
117-
979B3DFD2D5B985B009BDE1A /* CameraCaptureSessionQueueRaceConditionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CameraCaptureSessionQueueRaceConditionTests.swift; sourceTree = "<group>"; };
117+
979B3DFD2D5B985B009BDE1A /* CameraInitRaceConditionsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CameraInitRaceConditionsTests.swift; sourceTree = "<group>"; };
118118
979B3DFF2D5B9E6C009BDE1A /* CameraMethodChannelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CameraMethodChannelTests.swift; sourceTree = "<group>"; };
119119
979B3E012D5BA48F009BDE1A /* CameraOrientationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CameraOrientationTests.swift; sourceTree = "<group>"; };
120120
97BD4A0D2D5CC5AE00F857D5 /* CameraSettingsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CameraSettingsTests.swift; sourceTree = "<group>"; };
@@ -191,7 +191,7 @@
191191
979B3DF92D5B6BA2009BDE1A /* ExceptionCatcher.h */,
192192
979B3DFA2D5B6BC7009BDE1A /* ExceptionCatcher.m */,
193193
979B3DFC2D5B985B009BDE1A /* RunnerTests-Bridging-Header.h */,
194-
979B3DFD2D5B985B009BDE1A /* CameraCaptureSessionQueueRaceConditionTests.swift */,
194+
979B3DFD2D5B985B009BDE1A /* CameraInitRaceConditionsTests.swift */,
195195
979B3DFF2D5B9E6C009BDE1A /* CameraMethodChannelTests.swift */,
196196
979B3E012D5BA48F009BDE1A /* CameraOrientationTests.swift */,
197197
97BD4A0D2D5CC5AE00F857D5 /* CameraSettingsTests.swift */,
@@ -552,7 +552,7 @@
552552
E1ABED6D2D94392700AED9CC /* MockAssetWriterInput.swift in Sources */,
553553
977A25242D5A511600931E34 /* CameraPermissionTests.swift in Sources */,
554554
970ADABE2D6740A900EFDCD9 /* MockWritableData.swift in Sources */,
555-
979B3DFE2D5B985B009BDE1A /* CameraCaptureSessionQueueRaceConditionTests.swift in Sources */,
555+
979B3DFE2D5B985B009BDE1A /* CameraInitRaceConditionsTests.swift in Sources */,
556556
E142F13A2D85940600824824 /* MockCapturePhotoOutput.swift in Sources */,
557557
E12C4FF82D68E85500515E70 /* MockFLTCameraPermissionManager.swift in Sources */,
558558
97922B0D2D6380C300A9B4CF /* SampleBufferTests.swift in Sources */,
Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import XCTest
1111
import camera_avfoundation_objc
1212
#endif
1313

14-
final class CameraCaptureSessionQueueRaceConditionTests: XCTestCase {
14+
final class CameraInitRaceConditionsTests: XCTestCase {
1515
private func createCameraPlugin() -> (CameraPlugin, DispatchQueue) {
1616
let captureSessionQueue = DispatchQueue(label: "io.flutter.camera.captureSessionQueue")
1717

@@ -62,4 +62,36 @@ final class CameraCaptureSessionQueueRaceConditionTests: XCTestCase {
6262
XCTAssertNotNil(
6363
captureSessionQueue, "captureSessionQueue must not be nil after create method.")
6464
}
65+
66+
func testFlutterChannelInitializedWhenStartingImageStream() {
67+
let (cameraPlugin, _captureSessionQueue) = createCameraPlugin()
68+
let createExpectation = expectation(description: "create's result block must be called")
69+
70+
cameraPlugin.createCameraOnSessionQueue(
71+
withName: "acamera",
72+
settings: FCPPlatformMediaSettings.make(
73+
with: .medium,
74+
framesPerSecond: nil,
75+
videoBitrate: nil,
76+
audioBitrate: nil,
77+
enableAudio: true
78+
)
79+
) { result, error in
80+
createExpectation.fulfill()
81+
}
82+
83+
waitForExpectations(timeout: 30, handler: nil)
84+
85+
// Start stream and wait for its completion.
86+
let startStreamExpectation = expectation(
87+
description: "startImageStream's result block must be called")
88+
cameraPlugin.startImageStream(completion: {
89+
_ in
90+
startStreamExpectation.fulfill()
91+
})
92+
93+
waitForExpectations(timeout: 30, handler: nil)
94+
XCTAssertEqual(cameraPlugin.camera?.isStreamingImages, true)
95+
}
96+
6597
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,8 +261,9 @@ final class CameraPluginDelegatingMethodTests: XCTestCase {
261261
let expectation = expectation(description: "Call completed")
262262

263263
var startImageStreamCalled = false
264-
mockCamera.startImageStreamStub = { _ in
264+
mockCamera.startImageStreamStub = { messenger, completion in
265265
startImageStreamCalled = true
266+
completion(nil)
266267
}
267268

268269
cameraPlugin.startImageStream { error in

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ final class MockCamera: NSObject, Camera {
3939
var pausePreviewStub: (() -> Void)?
4040
var resumePreviewStub: (() -> Void)?
4141
var setDescriptionWhileRecordingStub: ((String, ((FlutterError?) -> Void)?) -> Void)?
42-
var startImageStreamStub: ((FlutterBinaryMessenger) -> Void)?
42+
var startImageStreamStub: ((FlutterBinaryMessenger, (FlutterError?) -> Void) -> Void)?
4343
var stopImageStreamStub: (() -> Void)?
4444

4545
var dartAPI: FCPCameraEventApi? {
@@ -63,6 +63,7 @@ final class MockCamera: NSObject, Camera {
6363
var videoFormat: FourCharCode = kCVPixelFormatType_32BGRA
6464

6565
var isPreviewPaused: Bool = false
66+
var isStreamingImages: Bool = false
6667

6768
var minimumExposureOffset: CGFloat {
6869
return getMinimumExposureOffsetStub?() ?? 0
@@ -186,8 +187,11 @@ final class MockCamera: NSObject, Camera {
186187
setDescriptionWhileRecordingStub?(cameraName, completion)
187188
}
188189

189-
func startImageStream(with messenger: FlutterBinaryMessenger) {
190-
startImageStreamStub?(messenger)
190+
func startImageStream(
191+
with messenger: FlutterBinaryMessenger,
192+
completion: @escaping (FlutterError?) -> Void
193+
) {
194+
startImageStreamStub?(messenger, completion)
191195
}
192196

193197
func stopImageStream() {

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

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,14 +52,28 @@ final class StreamingTests: XCTestCase {
5252

5353
func testExceedMaxStreamingPendingFramesCount() {
5454
let (camera, testAudioOutput, sampleBuffer, testAudioConnection) = createCamera()
55+
let handlerMock = MockImageStreamHandler()
56+
57+
let finishStartStreamExpectation = expectation(
58+
description: "Finish startStream")
59+
60+
let messenger = MockFlutterBinaryMessenger()
61+
camera.startImageStream(
62+
with: messenger, imageStreamHandler: handlerMock,
63+
completion: {
64+
_ in
65+
finishStartStreamExpectation.fulfill()
66+
})
67+
68+
waitForExpectations(timeout: 30, handler: nil)
69+
70+
// Setup mocked event sink after the stream starts
5571
let streamingExpectation = expectation(
5672
description: "Must not call handler over maxStreamingPendingFramesCount")
57-
let handlerMock = MockImageStreamHandler()
73+
5874
handlerMock.eventSinkStub = { event in
5975
streamingExpectation.fulfill()
6076
}
61-
let messenger = MockFlutterBinaryMessenger()
62-
camera.startImageStream(with: messenger, imageStreamHandler: handlerMock)
6377

6478
waitForQueueRoundTrip(with: DispatchQueue.main)
6579
XCTAssertEqual(camera.isStreamingImages, true)
@@ -74,14 +88,27 @@ final class StreamingTests: XCTestCase {
7488

7589
func testReceivedImageStreamData() {
7690
let (camera, testAudioOutput, sampleBuffer, testAudioConnection) = createCamera()
91+
let handlerMock = MockImageStreamHandler()
92+
93+
let finishStartStreamExpectation = expectation(
94+
description: "Finish startStream")
95+
96+
let messenger = MockFlutterBinaryMessenger()
97+
camera.startImageStream(
98+
with: messenger, imageStreamHandler: handlerMock,
99+
completion: {
100+
_ in
101+
finishStartStreamExpectation.fulfill()
102+
})
103+
104+
waitForExpectations(timeout: 30, handler: nil)
105+
106+
// Setup mocked event sink after the stream starts
77107
let streamingExpectation = expectation(
78108
description: "Must be able to call the handler again when receivedImageStreamData is called")
79-
let handlerMock = MockImageStreamHandler()
80109
handlerMock.eventSinkStub = { event in
81110
streamingExpectation.fulfill()
82111
}
83-
let messenger = MockFlutterBinaryMessenger()
84-
camera.startImageStream(with: messenger, imageStreamHandler: handlerMock)
85112

86113
waitForQueueRoundTrip(with: DispatchQueue.main)
87114
XCTAssertEqual(camera.isStreamingImages, true)

packages/camera/camera_avfoundation/ios/camera_avfoundation/Sources/camera_avfoundation/Camera.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ protocol Camera: FlutterTexture, AVCaptureVideoDataOutputSampleBufferDelegate,
2525
var videoFormat: FourCharCode { get set }
2626

2727
var isPreviewPaused: Bool { get }
28+
var isStreamingImages: Bool { get }
2829

2930
var minimumAvailableZoomFactor: CGFloat { get }
3031
var maximumAvailableZoomFactor: CGFloat { get }
@@ -86,7 +87,8 @@ protocol Camera: FlutterTexture, AVCaptureVideoDataOutputSampleBufferDelegate,
8687
withCompletion: @escaping (_ error: FlutterError?) -> Void
8788
)
8889

89-
func startImageStream(with: FlutterBinaryMessenger)
90+
func startImageStream(
91+
with: FlutterBinaryMessenger, completion: @escaping (_ error: FlutterError?) -> Void)
9092
func stopImageStream()
9193

9294
// Override to make `AVCaptureVideoDataOutputSampleBufferDelegate`/

packages/camera/camera_avfoundation/ios/camera_avfoundation/Sources/camera_avfoundation/CameraPlugin.swift

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -309,9 +309,11 @@ extension CameraPlugin: FCPCameraApi {
309309

310310
public func startImageStream(completion: @escaping (FlutterError?) -> Void) {
311311
captureSessionQueue.async { [weak self] in
312-
guard let strongSelf = self else { return }
313-
strongSelf.camera?.startImageStream(with: strongSelf.messenger)
314-
completion(nil)
312+
guard let strongSelf = self else {
313+
completion(nil)
314+
return
315+
}
316+
strongSelf.camera?.startImageStream(with: strongSelf.messenger, completion: completion)
315317
}
316318
}
317319

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

Lines changed: 61 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ - (void)captureToFileWithCompletion:(void (^)(NSString *_Nullable,
306306
NSString *path = [self getTemporaryFilePathWithExtension:extension
307307
subfolder:@"pictures"
308308
prefix:@"CAP_"
309-
error:error];
309+
error:&error];
310310
if (error) {
311311
completion(nil, FlutterErrorFromNSError(error));
312312
return;
@@ -362,7 +362,7 @@ - (AVCaptureVideoOrientation)getVideoOrientationForDeviceOrientation:
362362
- (NSString *)getTemporaryFilePathWithExtension:(NSString *)extension
363363
subfolder:(NSString *)subfolder
364364
prefix:(NSString *)prefix
365-
error:(NSError *)error {
365+
error:(NSError **)error {
366366
NSString *docDir =
367367
NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES)[0];
368368
NSString *fileDir =
@@ -373,11 +373,11 @@ - (NSString *)getTemporaryFilePathWithExtension:(NSString *)extension
373373

374374
NSFileManager *fm = [NSFileManager defaultManager];
375375
if (![fm fileExistsAtPath:fileDir]) {
376-
[[NSFileManager defaultManager] createDirectoryAtPath:fileDir
377-
withIntermediateDirectories:true
378-
attributes:nil
379-
error:&error];
380-
if (error) {
376+
BOOL success = [[NSFileManager defaultManager] createDirectoryAtPath:fileDir
377+
withIntermediateDirectories:true
378+
attributes:nil
379+
error:error];
380+
if (!success) {
381381
return nil;
382382
}
383383
}
@@ -498,43 +498,50 @@ - (void)dealloc {
498498
[_motionManager stopAccelerometerUpdates];
499499
}
500500

501+
/// Main logic to setup the video recording.
502+
- (void)setUpVideoRecordingWithCompletion:(void (^)(FlutterError *_Nullable))completion {
503+
NSError *error;
504+
_videoRecordingPath = [self getTemporaryFilePathWithExtension:@"mp4"
505+
subfolder:@"videos"
506+
prefix:@"REC_"
507+
error:&error];
508+
if (error) {
509+
completion(FlutterErrorFromNSError(error));
510+
return;
511+
}
512+
if (![self setupWriterForPath:_videoRecordingPath]) {
513+
completion([FlutterError errorWithCode:@"IOError" message:@"Setup Writer Failed" details:nil]);
514+
return;
515+
}
516+
// startWriting should not be called in didOutputSampleBuffer where it can cause state
517+
// in which _isRecording is YES but _videoWriter.status is AVAssetWriterStatusUnknown
518+
// in stopVideoRecording if it is called after startVideoRecording but before
519+
// didOutputSampleBuffer had chance to call startWriting and lag at start of video
520+
// https://github.com/flutter/flutter/issues/132016
521+
// https://github.com/flutter/flutter/issues/151319
522+
[_videoWriter startWriting];
523+
_isFirstVideoSample = YES;
524+
_isRecording = YES;
525+
_isRecordingPaused = NO;
526+
_videoTimeOffset = CMTimeMake(0, 1);
527+
_audioTimeOffset = CMTimeMake(0, 1);
528+
_videoIsDisconnected = NO;
529+
_audioIsDisconnected = NO;
530+
completion(nil);
531+
}
532+
501533
- (void)startVideoRecordingWithCompletion:(void (^)(FlutterError *_Nullable))completion
502534
messengerForStreaming:(nullable NSObject<FlutterBinaryMessenger> *)messenger {
503535
if (!_isRecording) {
504536
if (messenger != nil) {
505-
[self startImageStreamWithMessenger:messenger];
506-
}
507-
508-
NSError *error;
509-
_videoRecordingPath = [self getTemporaryFilePathWithExtension:@"mp4"
510-
subfolder:@"videos"
511-
prefix:@"REC_"
512-
error:error];
513-
if (error) {
514-
completion(FlutterErrorFromNSError(error));
515-
return;
516-
}
517-
if (![self setupWriterForPath:_videoRecordingPath]) {
518-
completion([FlutterError errorWithCode:@"IOError"
519-
message:@"Setup Writer Failed"
520-
details:nil]);
537+
[self startImageStreamWithMessenger:messenger
538+
completion:^(FlutterError *_Nullable error) {
539+
[self setUpVideoRecordingWithCompletion:completion];
540+
}];
521541
return;
522542
}
523-
// startWriting should not be called in didOutputSampleBuffer where it can cause state
524-
// in which _isRecording is YES but _videoWriter.status is AVAssetWriterStatusUnknown
525-
// in stopVideoRecording if it is called after startVideoRecording but before
526-
// didOutputSampleBuffer had chance to call startWriting and lag at start of video
527-
// https://github.com/flutter/flutter/issues/132016
528-
// https://github.com/flutter/flutter/issues/151319
529-
[_videoWriter startWriting];
530-
_isFirstVideoSample = YES;
531-
_isRecording = YES;
532-
_isRecordingPaused = NO;
533-
_videoTimeOffset = CMTimeMake(0, 1);
534-
_audioTimeOffset = CMTimeMake(0, 1);
535-
_videoIsDisconnected = NO;
536-
_audioIsDisconnected = NO;
537-
completion(nil);
543+
544+
[self setUpVideoRecordingWithCompletion:completion];
538545
} else {
539546
completion([FlutterError errorWithCode:@"Error"
540547
message:@"Video is already recording"
@@ -831,14 +838,17 @@ - (void)setExposureOffset:(double)offset {
831838
[_captureDevice unlockForConfiguration];
832839
}
833840

834-
- (void)startImageStreamWithMessenger:(NSObject<FlutterBinaryMessenger> *)messenger {
841+
- (void)startImageStreamWithMessenger:(NSObject<FlutterBinaryMessenger> *)messenger
842+
completion:(void (^)(FlutterError *))completion {
835843
[self startImageStreamWithMessenger:messenger
836844
imageStreamHandler:[[FLTImageStreamHandler alloc]
837-
initWithCaptureSessionQueue:_captureSessionQueue]];
845+
initWithCaptureSessionQueue:_captureSessionQueue]
846+
completion:completion];
838847
}
839848

840849
- (void)startImageStreamWithMessenger:(NSObject<FlutterBinaryMessenger> *)messenger
841-
imageStreamHandler:(FLTImageStreamHandler *)imageStreamHandler {
850+
imageStreamHandler:(FLTImageStreamHandler *)imageStreamHandler
851+
completion:(void (^)(FlutterError *))completion {
842852
if (!_isStreamingImages) {
843853
id<FLTEventChannel> eventChannel = [FlutterEventChannel
844854
eventChannelWithName:@"plugins.flutter.io/camera_avfoundation/imageStream"
@@ -851,19 +861,27 @@ - (void)startImageStreamWithMessenger:(NSObject<FlutterBinaryMessenger> *)messen
851861
[threadSafeEventChannel setStreamHandler:_imageStreamHandler
852862
completion:^{
853863
typeof(self) strongSelf = weakSelf;
854-
if (!strongSelf) return;
864+
if (!strongSelf) {
865+
completion(nil);
866+
return;
867+
}
855868

856869
dispatch_async(strongSelf.captureSessionQueue, ^{
857870
// cannot use the outter strongSelf
858871
typeof(self) strongSelf = weakSelf;
859-
if (!strongSelf) return;
872+
if (!strongSelf) {
873+
completion(nil);
874+
return;
875+
}
860876

861877
strongSelf.isStreamingImages = YES;
862878
strongSelf.streamingPendingFramesCount = 0;
879+
completion(nil);
863880
});
864881
}];
865882
} else {
866883
[self reportErrorMessage:@"Images from camera are already streaming!"];
884+
completion(nil);
867885
}
868886
}
869887

0 commit comments

Comments
 (0)