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

Commit 6902ead

Browse files
hellohuanlinhellohuanlin
committed
address some comments, mostly remove the util class
1 parent 5bb5051 commit 6902ead

8 files changed

Lines changed: 23 additions & 31 deletions

File tree

‎packages/camera/camera/example/ios/Runner/main.m‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ int main(int argc, char *argv[]) {
1111
// The setup logic in `AppDelegate::didFinishLaunchingWithOptions:` eventually sends camera
1212
// operations on the background queue, which would run concurrently with the test cases during
1313
// unit tests, making the debugging process confusing. This setup is actually not necessary for
14-
// the unit tests, so here we want to skip the AppDelegate when running unit tests.
14+
// the unit tests, so it is better to skip the AppDelegate when running unit tests.
1515
BOOL isTesting = NSClassFromString(@"XCTestCase") != nil;
1616
return UIApplicationMain(argc, argv, nil,
1717
isTesting ? nil : NSStringFromClass([AppDelegate class]));

‎packages/camera/camera/example/ios/RunnerTests/CameraTestUtils.h‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,13 @@
66

77
NS_ASSUME_NONNULL_BEGIN
88

9-
@interface CameraTestUtils : NSObject
10-
119
/// Creates an `FLTCam` that runs its capture session operations on a given queue.
1210
/// @param captureSessionQueue the capture session queue
1311
/// @return an FLTCam object.
14-
+ (FLTCam *)createFLTCamWithCaptureSessionQueue:(dispatch_queue_t)captureSessionQueue;
12+
extern FLTCam *FLTCreateFLTCamWithCaptureSessionQueue(dispatch_queue_t captureSessionQueue);
1513

1614
/// Creates a test sample buffer.
1715
/// @return a test sample buffer.
18-
+ (CMSampleBufferRef)createTestSampleBuffer;
19-
20-
@end
16+
extern CMSampleBufferRef FLTCreateTestSampleBuffer(void);
2117

2218
NS_ASSUME_NONNULL_END

‎packages/camera/camera/example/ios/RunnerTests/CameraTestUtils.m‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,7 @@
66
#import <OCMock/OCMock.h>
77
@import AVFoundation;
88

9-
@implementation CameraTestUtils
10-
11-
+ (FLTCam *)createFLTCamWithCaptureSessionQueue:(dispatch_queue_t)captureSessionQueue {
9+
FLTCam *FLTCreateFLTCamWithCaptureSessionQueue(dispatch_queue_t captureSessionQueue) {
1210
id inputMock = OCMClassMock([AVCaptureDeviceInput class]);
1311
OCMStub([inputMock deviceInputWithDevice:[OCMArg any] error:[OCMArg setTo:nil]])
1412
.andReturn(inputMock);
@@ -26,7 +24,7 @@ + (FLTCam *)createFLTCamWithCaptureSessionQueue:(dispatch_queue_t)captureSession
2624
error:nil];
2725
}
2826

29-
+ (CMSampleBufferRef)createTestSampleBuffer {
27+
CMSampleBufferRef FLTCreateTestSampleBuffer(void) {
3028
CVPixelBufferRef pixelBuffer;
3129
CVPixelBufferCreate(kCFAllocatorDefault, 100, 100, kCVPixelFormatType_32BGRA, NULL, &pixelBuffer);
3230

@@ -44,5 +42,3 @@ + (CMSampleBufferRef)createTestSampleBuffer {
4442
CFRelease(formatDescription);
4543
return sampleBuffer;
4644
}
47-
48-
@end

‎packages/camera/camera/example/ios/RunnerTests/FLTCamPhotoCaptureTests.m‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ - (void)testCaptureToFile_mustReportErrorToResultIfSavePhotoDelegateCompletionsW
2424
dispatch_queue_t captureSessionQueue = dispatch_queue_create("capture_session_queue", NULL);
2525
dispatch_queue_set_specific(captureSessionQueue, FLTCaptureSessionQueueSpecific,
2626
(void *)FLTCaptureSessionQueueSpecific, NULL);
27-
FLTCam *cam = [CameraTestUtils createFLTCamWithCaptureSessionQueue:captureSessionQueue];
27+
FLTCam *cam = FLTCreateFLTCamWithCaptureSessionQueue(captureSessionQueue);
2828
AVCapturePhotoSettings *settings = [AVCapturePhotoSettings photoSettings];
2929
id mockSettings = OCMClassMock([AVCapturePhotoSettings class]);
3030
OCMStub([mockSettings photoSettings]).andReturn(settings);
@@ -63,7 +63,7 @@ - (void)testCaptureToFile_mustReportPathToResultIfSavePhotoDelegateCompletionsWi
6363
dispatch_queue_t captureSessionQueue = dispatch_queue_create("capture_session_queue", NULL);
6464
dispatch_queue_set_specific(captureSessionQueue, FLTCaptureSessionQueueSpecific,
6565
(void *)FLTCaptureSessionQueueSpecific, NULL);
66-
FLTCam *cam = [CameraTestUtils createFLTCamWithCaptureSessionQueue:captureSessionQueue];
66+
FLTCam *cam = FLTCreateFLTCamWithCaptureSessionQueue(captureSessionQueue);
6767

6868
AVCapturePhotoSettings *settings = [AVCapturePhotoSettings photoSettings];
6969
id mockSettings = OCMClassMock([AVCapturePhotoSettings class]);

‎packages/camera/camera/example/ios/RunnerTests/FLTCamSampleBufferTests.m‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,14 @@ @implementation FLTCamSampleBufferTests
1818

1919
- (void)testSampleBufferCallbackQueueMustBeCaptureSessionQueue {
2020
dispatch_queue_t captureSessionQueue = dispatch_queue_create("testing", NULL);
21-
FLTCam *cam = [CameraTestUtils createFLTCamWithCaptureSessionQueue:captureSessionQueue];
21+
FLTCam *cam = FLTCreateFLTCamWithCaptureSessionQueue(captureSessionQueue);
2222
XCTAssertEqual(captureSessionQueue, cam.captureVideoOutput.sampleBufferCallbackQueue,
2323
@"Sample buffer callback queue must be the capture session queue.");
2424
}
2525

2626
- (void)testCopyPixelBuffer {
27-
FLTCam *cam =
28-
[CameraTestUtils createFLTCamWithCaptureSessionQueue:dispatch_queue_create("test", NULL)];
29-
CMSampleBufferRef capturedSampleBuffer = [CameraTestUtils createTestSampleBuffer];
27+
FLTCam *cam = FLTCreateFLTCamWithCaptureSessionQueue(dispatch_queue_create("test", NULL));
28+
CMSampleBufferRef capturedSampleBuffer = FLTCreateTestSampleBuffer();
3029
CVPixelBufferRef capturedPixelBuffer = CMSampleBufferGetImageBuffer(capturedSampleBuffer);
3130
// Mimic sample buffer callback when captured a new video sample
3231
[cam captureOutput:cam.captureVideoOutput
@@ -36,6 +35,7 @@ - (void)testCopyPixelBuffer {
3635
XCTAssertEqual(deliveriedPixelBuffer, capturedPixelBuffer,
3736
@"FLTCam must deliver the latest captured pixel buffer to copyPixelBuffer API.");
3837
CFRelease(capturedSampleBuffer);
38+
CFRelease(deliveriedPixelBuffer);
3939
}
4040

4141
@end

‎packages/camera/camera/example/ios/RunnerTests/FLTSavePhotoDelegateTests.m‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ - (void)testHandlePhotoCaptureResult_mustCompleteWithErrorIfFailedToWrite {
5353
[completionExpectation fulfill];
5454
}];
5555

56-
// We can't use OCMClassMock for NSData because some XCTest APIs uses NSData (e.g.
56+
// Do not use OCMClassMock for NSData because some XCTest APIs uses NSData (e.g.
5757
// `XCTRunnerIDESession::logDebugMessage:`) on a private queue.
5858
id mockData = OCMPartialMock([NSData data]);
5959
OCMStub([mockData writeToFile:OCMOCK_ANY
@@ -82,7 +82,7 @@ - (void)testHandlePhotoCaptureResult_mustCompleteWithFilePathIfSuccessToWrite {
8282
[completionExpectation fulfill];
8383
}];
8484

85-
// We can't use OCMClassMock for NSData because some XCTest APIs uses NSData (e.g.
85+
// Do not use OCMClassMock for NSData because some XCTest APIs uses NSData (e.g.
8686
// `XCTRunnerIDESession::logDebugMessage:`) on a private queue.
8787
id mockData = OCMPartialMock([NSData data]);
8888
OCMStub([mockData writeToFile:filePath options:NSDataWritingAtomic error:[OCMArg setTo:nil]])
@@ -107,7 +107,7 @@ - (void)testHandlePhotoCaptureResult_bothProvideDataAndSaveFileMustRunOnIOQueue
107107
const char *ioQueueSpecific = "io_queue_specific";
108108
dispatch_queue_set_specific(ioQueue, ioQueueSpecific, (void *)ioQueueSpecific, NULL);
109109

110-
// We can't use OCMClassMock for NSData because some XCTest APIs uses NSData (e.g.
110+
// Do not use OCMClassMock for NSData because some XCTest APIs uses NSData (e.g.
111111
// `XCTRunnerIDESession::logDebugMessage:`) on a private queue.
112112
id mockData = OCMPartialMock([NSData data]);
113113
OCMStub([mockData writeToFile:OCMOCK_ANY options:NSDataWritingAtomic error:[OCMArg setTo:nil]])

‎packages/camera/camera/ios/Classes/FLTCam.m‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ @interface FLTCam () <AVCaptureVideoDataOutputSampleBufferDelegate,
7979
/// All FLTCam's state access and capture session related operations should be on run on this queue.
8080
@property(strong, nonatomic) dispatch_queue_t captureSessionQueue;
8181
/// The queue on which `latestPixelBuffer` property is accessed.
82-
/// To avoid unnecessary contention, we should not reuse the captureSessionQueue.
82+
/// To avoid unnecessary contention, do not access `latestPixelBuffer` on the `captureSessionQueue`.
8383
@property(strong, nonatomic) dispatch_queue_t pixelBufferSynchronizationQueue;
8484
/// The queue on which captured photos (not videos) are written to disk.
8585
/// Videos are written to disk by `videoAdaptor` on an internal queue managed by AVFoundation.
@@ -363,16 +363,16 @@ - (void)captureOutput:(AVCaptureOutput *)output
363363
CVPixelBufferRef newBuffer = CMSampleBufferGetImageBuffer(sampleBuffer);
364364
CFRetain(newBuffer);
365365

366-
__block CVPixelBufferRef old = nil;
366+
__block CVPixelBufferRef previousPixelBuffer = nil;
367367
// Use `dispatch_sync` to avoid unnecessary context switch under common non-contest scenarios;
368368
// Under rare contest scenarios, it will not block for too long since the critical section is
369369
// quite lightweight.
370370
dispatch_sync(self.pixelBufferSynchronizationQueue, ^{
371-
old = self.latestPixelBuffer;
371+
previousPixelBuffer = self.latestPixelBuffer;
372372
self.latestPixelBuffer = newBuffer;
373373
});
374-
if (old != nil) {
375-
CFRelease(old);
374+
if (previousPixelBuffer) {
375+
CFRelease(previousPixelBuffer);
376376
}
377377
if (_onFrameAvailable) {
378378
_onFrameAvailable();
@@ -432,7 +432,7 @@ - (void)captureOutput:(AVCaptureOutput *)output
432432

433433
[planes addObject:planeBuffer];
434434
}
435-
// Before accessing pixel data, we should lock the base address, and unlock it afterwards.
435+
// Lock the base address before accessing pixel data, and unlock it afterwards.
436436
// Done accessing the `pixelBuffer` at this point.
437437
CVPixelBufferUnlockBaseAddress(pixelBuffer, kCVPixelBufferLock_ReadOnly);
438438

@@ -588,7 +588,7 @@ - (void)dealloc {
588588

589589
- (CVPixelBufferRef)copyPixelBuffer {
590590
__block CVPixelBufferRef pixelBuffer = nil;
591-
// `copyPixelBuffer` API requires synchronous return, so we have to use `dispatch_sync`.
591+
// Use `dispatch_sync` because `copyPixelBuffer` API requires synchronous return.
592592
dispatch_sync(self.pixelBufferSynchronizationQueue, ^{
593593
pixelBuffer = self.latestPixelBuffer;
594594
self.latestPixelBuffer = nil;

‎packages/camera/camera/ios/Classes/FLTCam_Test.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,12 @@
1717
/// A dictionary to retain all in-progress FLTSavePhotoDelegates. The key of the dictionary is the
1818
/// AVCapturePhotoSettings's uniqueID for each photo capture operation, and the value is the
1919
/// FLTSavePhotoDelegate that handles the result of each photo capture operation. Note that photo
20-
/// capture operations may overlap, so we have to keep track of multiple delegates in progress,
20+
/// capture operations may overlap, so FLTCam has to keep track of multiple delegates in progress,
2121
/// instead of just a single delegate reference.
2222
@property(readonly, nonatomic)
2323
NSMutableDictionary<NSNumber *, FLTSavePhotoDelegate *> *inProgressSavePhotoDelegates;
2424

25-
/// Delegate callback when receive a new video or audio sample.
25+
/// Delegate callback when receiving a new video or audio sample.
2626
/// Exposed for unit tests.
2727
- (void)captureOutput:(AVCaptureOutput *)output
2828
didOutputSampleBuffer:(CMSampleBufferRef)sampleBuffer

0 commit comments

Comments
 (0)