-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[camera]writing file on background queue #4721
Conversation
| } | ||
| _enableAudio = enableAudio; | ||
| _captureSessionQueue = captureSessionQueue; | ||
| _photoIOQueue = dispatch_queue_create("io.flutter.camera.photoIOQueue", NULL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
captureSessionQueue was injected because it's also used in CameraPlugin.m
packages/camera/camera/example/ios/RunnerTests/FLTSavePhotoDelegateTests.m
Outdated
Show resolved
Hide resolved
packages/camera/camera/example/ios/RunnerTests/FLTSavePhotoDelegateTests.m
Outdated
Show resolved
Hide resolved
packages/camera/camera/example/ios/RunnerTests/FLTSavePhotoDelegateTests.m
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,37 @@ | |||
| // Copyright 2013 The Flutter Authors. All rights reserved. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is now exposed in the plugin API, I don't think we want that. I think keep most of this in the implementation and only expose FLTSavePhotoDelegate_Test.h in the test module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I didn't add it to the umbrella header so it's not exposed outside of this package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also removed thread safe wrappers from the umbrella header
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right right I forgot this uses a real umbrella header and not one generated by CocoaPods.
I think you're still exposing it in the test module since you're #importing it in the _Test header.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original header and the test header are both exposed for unit test only. I think this is what we want.
jmagman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Writing photo files on background queue. A performance improvement similar to the previous PR that moves sample buffer handling to capture session queue.
Very interesting learning related to mocking NSData: It turned out that we can't
OCMClassMockNSData because XCTest infrastructure itself uses NSData (e.g.XCTRunnerIDESession::logDebugMessage:), which runs on a private queue (com.apple.dt.xctest.default-debug-log-handler). The race condition is pretty common and makes the test very fragile. Took me a while to debug.Issue:
flutter/flutter#96429
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.