-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[camera_avfoundation] iOS: Fix crash when enableAudio == false
by correcting guard condition
#9949
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
0635db4
b48b43f
559e563
092a790
b6ab67e
e4656fe
d44fe6f
c7375b8
ef0e572
3d3b031
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,6 @@ private let testResolutionPreset = FCPPlatformResolutionPreset.medium | |
private let testFramesPerSecond = 15 | ||
private let testVideoBitrate = 200000 | ||
private let testAudioBitrate = 32000 | ||
private let testEnableAudio = true | ||
|
||
private final class TestMediaSettingsAVWrapper: FLTCamMediaSettingsAVWrapper { | ||
let lockExpectation: XCTestExpectation | ||
|
@@ -28,14 +27,15 @@ private final class TestMediaSettingsAVWrapper: FLTCamMediaSettingsAVWrapper { | |
let audioSettingsExpectation: XCTestExpectation | ||
let videoSettingsExpectation: XCTestExpectation | ||
|
||
init(test: XCTestCase) { | ||
init(test: XCTestCase, expectAudio: Bool) { | ||
lockExpectation = test.expectation(description: "lockExpectation") | ||
unlockExpectation = test.expectation(description: "unlockExpectation") | ||
minFrameDurationExpectation = test.expectation(description: "minFrameDurationExpectation") | ||
maxFrameDurationExpectation = test.expectation(description: "maxFrameDurationExpectation") | ||
beginConfigurationExpectation = test.expectation(description: "beginConfigurationExpectation") | ||
commitConfigurationExpectation = test.expectation(description: "commitConfigurationExpectation") | ||
audioSettingsExpectation = test.expectation(description: "audioSettingsExpectation") | ||
audioSettingsExpectation.isInverted = !expectAudio | ||
videoSettingsExpectation = test.expectation(description: "videoSettingsExpectation") | ||
} | ||
|
||
|
@@ -114,14 +114,15 @@ private final class TestMediaSettingsAVWrapper: FLTCamMediaSettingsAVWrapper { | |
|
||
final class CameraSettingsTests: XCTestCase { | ||
func testSettings_shouldPassConfigurationToCameraDeviceAndWriter() { | ||
let enableAudio: Bool = true | ||
let settings = FCPPlatformMediaSettings.make( | ||
with: testResolutionPreset, | ||
framesPerSecond: NSNumber(value: testFramesPerSecond), | ||
videoBitrate: NSNumber(value: testVideoBitrate), | ||
audioBitrate: NSNumber(value: testAudioBitrate), | ||
enableAudio: testEnableAudio | ||
enableAudio: enableAudio | ||
) | ||
let injectedWrapper = TestMediaSettingsAVWrapper(test: self) | ||
let injectedWrapper = TestMediaSettingsAVWrapper(test: self, expectAudio: enableAudio) | ||
|
||
let configuration = CameraTestUtils.createTestCameraConfiguration() | ||
configuration.mediaSettingsWrapper = injectedWrapper | ||
|
@@ -173,7 +174,7 @@ final class CameraSettingsTests: XCTestCase { | |
framesPerSecond: NSNumber(value: testFramesPerSecond), | ||
videoBitrate: NSNumber(value: testVideoBitrate), | ||
audioBitrate: NSNumber(value: testAudioBitrate), | ||
enableAudio: testEnableAudio | ||
enableAudio: false | ||
) | ||
var resultValue: NSNumber? | ||
camera.createCameraOnSessionQueue( | ||
|
@@ -195,7 +196,7 @@ final class CameraSettingsTests: XCTestCase { | |
framesPerSecond: NSNumber(value: 60), | ||
videoBitrate: NSNumber(value: testVideoBitrate), | ||
audioBitrate: NSNumber(value: testAudioBitrate), | ||
enableAudio: testEnableAudio | ||
enableAudio: false | ||
) | ||
|
||
let configuration = CameraTestUtils.createTestCameraConfiguration() | ||
|
@@ -206,4 +207,97 @@ final class CameraSettingsTests: XCTestCase { | |
XCTAssertLessThanOrEqual(range.minFrameRate, 60) | ||
XCTAssertGreaterThanOrEqual(range.maxFrameRate, 60) | ||
} | ||
func test_setUpCaptureSessionForAudioIfNeeded_skipsAudioSession_whenAudioDisabled() { | ||
let settings = FCPPlatformMediaSettings.make( | ||
with: testResolutionPreset, | ||
framesPerSecond: NSNumber(value: testFramesPerSecond), | ||
videoBitrate: NSNumber(value: testVideoBitrate), | ||
audioBitrate: NSNumber(value: testAudioBitrate), | ||
enableAudio: false | ||
) | ||
|
||
let wrapper = TestMediaSettingsAVWrapper(test: self, expectAudio: false) | ||
let mockAudioSession = MockCaptureSession() | ||
|
||
let configuration = CameraTestUtils.createTestCameraConfiguration() | ||
configuration.mediaSettingsWrapper = wrapper | ||
configuration.mediaSettings = settings | ||
configuration.audioCaptureSession = mockAudioSession | ||
let camera = CameraTestUtils.createTestCamera(configuration) | ||
|
||
wait( | ||
for: [ | ||
wrapper.lockExpectation, | ||
wrapper.beginConfigurationExpectation, | ||
wrapper.minFrameDurationExpectation, | ||
wrapper.maxFrameDurationExpectation, | ||
wrapper.commitConfigurationExpectation, | ||
wrapper.unlockExpectation, | ||
], | ||
timeout: 1, | ||
enforceOrder: true | ||
) | ||
|
||
camera.startVideoRecording(completion: { _ in }, messengerForStreaming: nil) | ||
|
||
wait( | ||
for: [ | ||
wrapper.audioSettingsExpectation, | ||
wrapper.videoSettingsExpectation, | ||
], | ||
timeout: 1 | ||
) | ||
|
||
XCTAssertEqual( | ||
mockAudioSession.addedAudioOutputCount, 0, | ||
"Audio session should not receive AVCaptureAudioDataOutput when enableAudio is false" | ||
) | ||
} | ||
|
||
func test_setUpCaptureSessionForAudioIfNeeded_addsAudioSession_whenAudioEnabled() { | ||
let settings = FCPPlatformMediaSettings.make( | ||
with: testResolutionPreset, | ||
framesPerSecond: NSNumber(value: testFramesPerSecond), | ||
videoBitrate: NSNumber(value: testVideoBitrate), | ||
audioBitrate: NSNumber(value: testAudioBitrate), | ||
enableAudio: true | ||
) | ||
|
||
let wrapper = TestMediaSettingsAVWrapper(test: self, expectAudio: true) | ||
let mockAudioSession = MockCaptureSession() | ||
|
||
let configuration = CameraTestUtils.createTestCameraConfiguration() | ||
configuration.mediaSettingsWrapper = wrapper | ||
configuration.mediaSettings = settings | ||
configuration.audioCaptureSession = mockAudioSession | ||
let camera = CameraTestUtils.createTestCamera(configuration) | ||
|
||
wait( | ||
for: [ | ||
wrapper.lockExpectation, | ||
wrapper.beginConfigurationExpectation, | ||
wrapper.minFrameDurationExpectation, | ||
wrapper.maxFrameDurationExpectation, | ||
wrapper.commitConfigurationExpectation, | ||
wrapper.unlockExpectation, | ||
], | ||
timeout: 1, | ||
enforceOrder: true | ||
) | ||
|
||
camera.startVideoRecording(completion: { _ in }, messengerForStreaming: nil) | ||
|
||
wait( | ||
for: [ | ||
wrapper.audioSettingsExpectation, | ||
wrapper.videoSettingsExpectation, | ||
], | ||
timeout: 1 | ||
) | ||
|
||
XCTAssertGreaterThan( | ||
mockAudioSession.addedAudioOutputCount, 0, | ||
"Audio session should receive AVCaptureAudioDataOutput when enableAudio is true" | ||
) | ||
} | ||
} | ||
Comment on lines
+210
to
303
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The two new tests, This function could take Here is an example of how this could be structured: private func performAudioSetupTest(enableAudio: Bool) -> MockCaptureSession {
// ... common setup for settings, wrapper, mockAudioSession, configuration, and camera ...
// ... first wait block for configuration ...
camera.startVideoRecording(completion: { _ in }, messengerForStreaming: nil)
// ... second wait block for recording start ...
return mockAudioSession
}
func test_setUpCaptureSessionForAudioIfNeeded_skipsAudioSession_whenAudioDisabled() {
let mockAudioSession = performAudioSetupTest(enableAudio: false)
XCTAssertEqual(
mockAudioSession.addedAudioOutputCount, 0,
"Audio session should not receive AVCaptureAudioDataOutput when enableAudio is false"
)
}
func test_setUpCaptureSessionForAudioIfNeeded_addsAudioSession_whenAudioEnabled() {
let mockAudioSession = performAudioSetupTest(enableAudio: true)
XCTAssertGreaterThan(
mockAudioSession.addedAudioOutputCount, 0,
"Audio session should receive AVCaptureAudioDataOutput when enableAudio is true"
)
} This refactoring would make the tests more concise and easier to maintain. |
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.
can remove
by correcting audio setup guard
since it's implementation details that audience doesn't care