Utilize core-based synchronisation #258
Conversation
chezzdev
force-pushed
the
pad-telemetry-sync
branch
from
26a457e
to
3f51f1f
Compare
| @@ -89,6 +89,18 @@ class DeviceCheckerTests: XCTestCase { | |||
| // Given iPhone XR | |||
| // When // Then | |||
| XCTAssertTrue(UIDeviceIphoneXRStub().isHighPerformance) | |||
|
|
|||
There was a problem hiding this comment.
Is this a part of current PR?
There was a problem hiding this comment.
I was cleaning the tests, so for them to pass, I've added these cases.
| @@ -41,23 +36,28 @@ final class VideoTrimmer { | |||
| return | |||
There was a problem hiding this comment.
completion is not called for else-branch of guard statement here and line 36 above.
IMHO we should call it.
There was a problem hiding this comment.
Good catch. Will update.
| guard let exportSession = AVAssetExportSession(asset: composition, presetName: preferredPreset) else { return } | ||
| guard | ||
| let exportSession = AVAssetExportSession(asset: composition, presetName: AVAssetExportPresetPassthrough) | ||
| else { return } |
There was a problem hiding this comment.
same concern about completion which is not called on else-branch.
| @@ -50,7 +56,7 @@ final class VideoRecorder { | |||
| } | |||
|
|
|||
| func stopRecording(completion: (() -> Void)?) { | |||
| writerQueue.async { [weak self] in | |||
| writerQueue.sync { [weak self] in | |||
There was a problem hiding this comment.
Do we need to weakify self?
There was a problem hiding this comment.
Now that it's sync, no. Will update.
| @@ -2,62 +2,68 @@ import AVFoundation | |||
| import Foundation | |||
|
|
|||
| private let timeScale: CMTimeScale = 600 | |||
| private let fileType: AVFileType = .mp4 | |||
There was a problem hiding this comment.
Didn't we agree to store constants in caseless enums named Constants which is placed as private enum in classes, e.g. in MotionManager
| @@ -18,6 +18,12 @@ private extension VideoSettings { | |||
| } | |||
| } | |||
|
|
|||
| protocol FrameRecordable { | |||
There was a problem hiding this comment.
Maybe it will be better to call it FrameRecorder?
Checks:
devbranchLinked issues:
Another iOS part of #1739