-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[camera] Sample buffer handling on session queue #4709
[camera] Sample buffer handling on session queue #4709
Conversation
3abb010
to
8b908cb
Compare
8b908cb
to
e68a118
Compare
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 with nits.
Can you also do some manual testing of these before you merge? Want some validation before customers pick up the new version.
orientation:UIDeviceOrientationPortrait | ||
captureSessionQueue:captureSessionQueue | ||
error:nil]; | ||
XCTAssertEqual(captureSessionQueue, cam.captureVideoOutput.sampleBufferCallbackQueue); |
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.
To use isEqual:
and not ==
.
XCTAssertEqual(captureSessionQueue, cam.captureVideoOutput.sampleBufferCallbackQueue); | |
XCTAssertEqualObjects(captureSessionQueue, cam.captureVideoOutput.sampleBufferCallbackQueue); |
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.
Never mind these are dispatch_queues
not NSObject
s.
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.
iirc a few years ago apple made dispatch_queues objc objects. So both should be fine. Most likely isEqual
will simply call ==
internally
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.
found this flag to turn on/off: OS_OBJECT_HAVE_OBJC_SUPPORT
.andReturn(inputMock); | ||
|
||
id sessionMock = OCMClassMock([AVCaptureSession class]); | ||
OCMStub([sessionMock alloc]).andReturn(sessionMock); |
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.
If you stub a class method you need to call -stopMock
explicitly or the mock will leak between tests, it won't stop mocking when the mock object deallocs.
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.
gotcha. good to know!
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.
Ah looks like stopMocking
is called on dealloc, from the doc: https://ocmock.org/reference/
The class can be returned to its original state by calling stopMocking. This is only necessary if the original state must be restored before the end of the test. The mock automatically calls stopMocking during its own deallocation.
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.
Wrote a dummy test and verified it.
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.
I didn't think it works stubbing class methods, alloc
in this case. Is that what you tested?
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.
Sounds good. Just manual tested the sample project and verified its still working. |
As discussed in the proposal:
Issue here: flutter/flutter#96429
Pre-launch Checklist
dart format
.)[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.md
to 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.