Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
a978667
fix race condition when starting image stream on iOS
js2702 Feb 27, 2025
494fe89
update pubspec
js2702 Feb 27, 2025
e902881
update changelog
js2702 Feb 27, 2025
c4aa926
Test not working
js2702 Mar 1, 2025
e6dd1a6
fix test
js2702 Mar 1, 2025
423c038
Merge branch 'main' into fix_startstream_race_condition
js2702 Mar 2, 2025
0e16acb
Lint + add async/await
js2702 Mar 2, 2025
364d2d6
Merge tag 'camera_avfoundation-v0.9.18+9' into fix_startstream_race_c…
js2702 Mar 5, 2025
8502b67
remove async from tests
js2702 Mar 5, 2025
7c462d9
fix lint + missing parameter
js2702 Mar 6, 2025
5dc4fb0
Fix lint again
js2702 Mar 6, 2025
680820f
lint
js2702 Mar 6, 2025
d46b835
lint once again
js2702 Mar 6, 2025
2ed081e
Merge remote-tracking branch 'origin/main' into fix_startstream_race_…
js2702 Mar 21, 2025
80b733b
fixes
js2702 Mar 21, 2025
0add70e
empty callback
js2702 Mar 21, 2025
7bd57e5
Merge branch 'main' into fix_startstream_race_condition
js2702 Apr 8, 2025
b50bab8
call completion in all paths
js2702 Apr 8, 2025
42be013
formatting
js2702 Apr 8, 2025
2e78c19
call completion in all paths
js2702 Apr 11, 2025
870f7f5
lint
js2702 Apr 15, 2025
fba0373
Merge branch 'main' into fix_startstream_race_condition
js2702 Apr 29, 2025
a2b3269
separate in function
js2702 Jun 13, 2025
488bf75
Merge remote-tracking branch 'origin/main' into fix_startstream_race_…
js2702 Jun 13, 2025
c5075a0
adapt after merging
js2702 Jun 13, 2025
5dfac20
formatting
js2702 Jun 13, 2025
c0f8e90
rename setup method
davidmartos96 Jun 13, 2025
987f067
update error as param
davidmartos96 Jun 13, 2025
48354d9
lint
js2702 Jun 13, 2025
ce94fad
trigger tests
js2702 Jun 16, 2025
4661cc8
trigger tests
js2702 Jun 16, 2025
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
4 changes: 4 additions & 0 deletions packages/camera/camera_avfoundation/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.9.18+10

* Fix race condition when starting image stream

## 0.9.18+9

* Backfills unit tests for `CameraPlugin` class.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,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 */; };
Expand Down Expand Up @@ -127,7 +127,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>"; };
Expand Down Expand Up @@ -184,7 +184,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 */,
Expand Down Expand Up @@ -542,7 +542,8 @@
970ADABE2D6740A900EFDCD9 /* MockWritableData.swift in Sources */,
7F8FD2292D4BFABF001AF2C1 /* MockGlobalEventApi.m in Sources */,
7FD582352D57D97C003B1200 /* MockCaptureDeviceFormat.m in Sources */,
979B3DFE2D5B985B009BDE1A /* CameraCaptureSessionQueueRaceConditionTests.swift in Sources */,
979B3DFE2D5B985B009BDE1A /* CameraInitRaceConditionsTests.swift in Sources */,
7F29EB222D269ED500740257 /* MockEventChannel.m in Sources */,
7F8FD22F2D4D0B88001AF2C1 /* MockFlutterBinaryMessenger.m in Sources */,
E12C4FF82D68E85500515E70 /* MockFLTCameraPermissionManager.swift in Sources */,
97922B0D2D6380C300A9B4CF /* SampleBufferTests.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import XCTest

@testable import camera_avfoundation

final class CameraCaptureSessionQueueRaceConditionTests: XCTestCase {
final class CameraInitRaceConditionsTests: XCTestCase {
private func createCameraPlugin() -> CameraPlugin {
return CameraPlugin(
registry: MockFlutterTextureRegistry(),
Expand Down Expand Up @@ -52,4 +52,41 @@ final class CameraCaptureSessionQueueRaceConditionTests: XCTestCase {
XCTAssertNotNil(
cameraPlugin.captureSessionQueue, "captureSessionQueue must not be nil after create method.")
}

func testFlutterChannelInitializedWhenStartingImageStream() {
let cameraPlugin = createCameraPlugin()
let configuration = FLTCreateTestCameraConfiguration()
cameraPlugin.camera = FLTCreateCamWithConfiguration(configuration)

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)

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,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,
withCompletion: {
_ 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)
Expand All @@ -64,14 +78,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,
withCompletion: {
_ 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,7 @@ - (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);
[weakSelf.camera startImageStreamWithMessenger:weakSelf.messenger withCompletion:completion];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be converted to a strongSelf, and a codepath added that calls completion(nil) if strongSelf is nil. As written, this will never call completion if weakSelf is nil, which violates the API contract of platform channel completion handlers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made some changes as you suggested, hopefully is OK now.

});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,7 @@ - (void)startVideoRecordingWithCompletion:(void (^)(FlutterError *_Nullable))com
messengerForStreaming:(nullable NSObject<FlutterBinaryMessenger> *)messenger {
if (!_isRecording) {
if (messenger != nil) {
[self startImageStreamWithMessenger:messenger];
[self startImageStreamWithMessenger:messenger withCompletion:completion];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is completion being passed to a method and also used below? This looks like it will call completion multiple times, which has undefined behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, unfortunately providing nil for the completion seems to break the interfaces because _test interface is included in the FLTCam.m. My Objective-C knowledge is quite limited so I put an empty callback there which also works.

If you want a nullable completion I would appreciate if you could finish up this small change.

}

NSError *error;
Expand Down Expand Up @@ -1168,14 +1168,17 @@ - (void)setExposureOffset:(double)offset {
[_captureDevice unlockForConfiguration];
}

- (void)startImageStreamWithMessenger:(NSObject<FlutterBinaryMessenger> *)messenger {
- (void)startImageStreamWithMessenger:(NSObject<FlutterBinaryMessenger> *)messenger
withCompletion:(void (^)(FlutterError *))completion {
[self startImageStreamWithMessenger:messenger
imageStreamHandler:[[FLTImageStreamHandler alloc]
initWithCaptureSessionQueue:_captureSessionQueue]];
initWithCaptureSessionQueue:_captureSessionQueue]
withCompletion:completion];
}

- (void)startImageStreamWithMessenger:(NSObject<FlutterBinaryMessenger> *)messenger
imageStreamHandler:(FLTImageStreamHandler *)imageStreamHandler {
imageStreamHandler:(FLTImageStreamHandler *)imageStreamHandler
withCompletion:(void (^)(FlutterError *))completion {
if (!_isStreamingImages) {
id<FLTEventChannel> eventChannel = [FlutterEventChannel
eventChannelWithName:@"plugins.flutter.io/camera_avfoundation/imageStream"
Expand All @@ -1197,6 +1200,7 @@ - (void)startImageStreamWithMessenger:(NSObject<FlutterBinaryMessenger> *)messen

strongSelf.isStreamingImages = YES;
strongSelf.streamingPendingFramesCount = 0;
completion(nil);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the format seems off

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want the \n removed there?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh sorry the github web ui shows it weirdly. This is good.

});
}];
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,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
withCompletion:(nonnull void (^)(FlutterError *_Nullable))completion;
- (void)stopImageStream;
- (void)setZoomLevel:(CGFloat)zoom withCompletion:(void (^)(FlutterError *_Nullable))completion;
- (void)setUpCaptureSessionForAudioIfNeeded;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,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
withCompletion:(void (^)(FlutterError *))completion;

@end
2 changes: 1 addition & 1 deletion packages/camera/camera_avfoundation/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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+9
version: 0.9.18+10

environment:
sdk: ^3.4.0
Expand Down