Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[camera]manages ios camera's orientation states on capture session queue #4748

Merged
fluttergithubbot merged 2 commits into from
Feb 8, 2022
Merged

[camera]manages ios camera's orientation states on capture session queue #4748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cbeacff
[camera]manages ios camera's orientation states on capture session queue
hellohuanlin Feb 7, 2022
447dd23
typo
hellohuanlin Feb 7, 2022
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/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.9.4+11

* Manages iOS camera's orientation-related states on a background queue to prevent potential race conditions.

## 0.9.4+10

* iOS performance improvement by moving file writing from the main queue to a background IO queue.
Expand Down
22 changes: 22 additions & 0 deletions packages/camera/camera/example/ios/RunnerTests/CameraOrientationTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,28 @@ - (void)testOrientationNotifications {
OCMVerifyAll(mockMessenger);
}

- (void)testOrientationUpdateMustBeOnCaptureSessionQueue {
XCTestExpectation *queueExpectation = [self
expectationWithDescription:@"Orientation update must happen on the capture session queue"];

CameraPlugin *camera = [[CameraPlugin alloc] initWithRegistry:nil messenger:nil];
const char *captureSessionQueueSpecific = "capture_session_queue";
dispatch_queue_set_specific(camera.captureSessionQueue, captureSessionQueueSpecific,
(void *)captureSessionQueueSpecific, NULL);
FLTCam *mockCam = OCMClassMock([FLTCam class]);
camera.camera = mockCam;
OCMStub([mockCam setDeviceOrientation:UIDeviceOrientationLandscapeLeft])
.andDo(^(NSInvocation *invocation) {
if (dispatch_get_specific(captureSessionQueueSpecific)) {
[queueExpectation fulfill];
}
});

[camera orientationChanged:
[self createMockNotificationForOrientation:UIDeviceOrientationLandscapeLeft]];
[self waitForExpectationsWithTimeout:1 handler:nil];
}

- (void)rotate:(UIDeviceOrientation)deviceOrientation
expectedChannelOrientation:(NSString *)channelOrientation
cameraPlugin:(CameraPlugin *)cameraPlugin
Expand Down
12 changes: 6 additions & 6 deletions packages/camera/camera/ios/Classes/CameraPlugin.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
@interface CameraPlugin ()
@property(readonly, nonatomic) FLTThreadSafeTextureRegistry *registry;
@property(readonly, nonatomic) NSObject<FlutterBinaryMessenger> *messenger;
@property(readonly, nonatomic) FLTCam *camera;
@property(readonly, nonatomic) FLTThreadSafeMethodChannel *deviceEventMethodChannel;
@end

Expand Down Expand Up @@ -69,11 +68,12 @@ - (void)orientationChanged:(NSNotification *)note {
return;
}

if (_camera) {
[_camera setDeviceOrientation:orientation];
}

[self sendDeviceOrientation:orientation];
dispatch_async(self.captureSessionQueue, ^{
// `FLTCam::setDeviceOrientation` must be called on capture session queue.
[self.camera setDeviceOrientation:orientation];
// `CameraPlugin::sendDeviceOrientation` can be called on any queue.
[self sendDeviceOrientation:orientation];
});
}

- (void)sendDeviceOrientation:(UIDeviceOrientation)orientation {
Expand Down
6 changes: 5 additions & 1 deletion packages/camera/camera/ios/Classes/CameraPlugin_Test.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@
// This header is available in the Test module. Import via "@import camera.Test;"

#import <camera/CameraPlugin.h>
#import <camera/FLTCam.h>
#import <camera/FLTThreadSafeFlutterResult.h>

/// Methods exposed for unit testing.
@interface CameraPlugin ()

// All FLTCam's state access and capture session related operations should be on run on this queue.
/// All FLTCam's state access and capture session related operations should be on run on this queue.
@property(nonatomic, strong) dispatch_queue_t captureSessionQueue;

/// An internal camera object that manages camera's state and performs camera operations.
@property(nonatomic, strong) FLTCam *camera;
Comment on lines +17 to +18
Copy link
Copy Markdown
Member

@jmagman jmagman Feb 7, 2022

Choose a reason for hiding this comment

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

I don't love that this is exposed as readwrite now, but I can't think of a better way to do it since a test-only convenience initializer isn't great either since FLTCam is created later.

Copy link
Copy Markdown
Contributor Author

@hellohuanlin hellohuanlin Feb 7, 2022

Choose a reason for hiding this comment

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

Oh this was supposed to be readwrite in the first place. The old readonly worked before only because we used the ivar and bypassed the attribute's restriction:

      _camera = cam;

Copy link
Copy Markdown
Member

@jmagman jmagman Feb 8, 2022

Choose a reason for hiding this comment

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

readwrite in the implementation though, not to, say, consumers of the plugin (who can also import the test module). But it's fine I don't have a better idea for testing this.

Copy link
Copy Markdown
Contributor Author

@hellohuanlin hellohuanlin Feb 8, 2022

Choose a reason for hiding this comment

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

yeah should be fine. we already have many internal APIs exposed under this test module (eg CameraPlugin_Test.h). it's very unlikely for non-test code to import it.


/// Inject @p FlutterTextureRegistry and @p FlutterBinaryMessenger for unit testing.
- (instancetype)initWithRegistry:(NSObject<FlutterTextureRegistry> *)registry
messenger:(NSObject<FlutterBinaryMessenger> *)messenger
Expand Down
2 changes: 1 addition & 1 deletion packages/camera/camera/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ description: A Flutter plugin for controlling the camera. Supports previewing
Dart.
repository: https://github.com/flutter/plugins/tree/main/packages/camera/camera
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22
version: 0.9.4+10
version: 0.9.4+11

environment:
sdk: ">=2.14.0 <3.0.0"
Expand Down