Bug 235362

Summary: AVAudioSessionCaptureDeviceManager is not always computing the right default input device
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebRTCAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, eric.carlson, ews-watchlist, ggaren, glenn, hta, jer.noble, philipj, sergio, tommyw, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description youenn fablet 2022-01-19 06:23:21 PST 
AVAudioSessionCaptureDeviceManager is not always computing the right default input device
Comment 1 youenn fablet 2022-01-19 06:25:23 PST 
<rdar://87707090>
Comment 2 youenn fablet 2022-01-19 06:31:07 PST 
Created attachment 449479 [details]
Patch
Comment 3 Darin Adler 2022-01-19 10:07:37 PST 
Comment on attachment 449479 [details]
Patch

What is our testing strategy for these kinds of things? I worry that we could easily break this and would not notice.
Comment 4 youenn fablet 2022-01-19 23:52:52 PST 
(In reply to Darin Adler from comment #3)
> Comment on attachment 449479 [details]
> Patch
> 
> What is our testing strategy for these kinds of things? I worry that we
> could easily break this and would not notice.

This whole area sadly resorts on manual testing.
It would be nice to automate more this area but this requires efforts outside WebKit.
I guess I can add some some manual test pages to help validating non regressions easily.
Comment 5 EWS 2022-01-20 00:37:36 PST 
Committed r288275 (246213@main): <https://commits.webkit.org/246213@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449479 [details].
Comment 6 Geoffrey Garen 2022-01-21 09:00:58 PST 
Comment on attachment 449479 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=449479&action=review

> Source/WebCore/platform/mediastream/ios/AVAudioSessionCaptureDeviceManager.mm:243
> +        RELEASE_LOG_INFO(WebRTC, "AVAudioSessionCaptureDeviceManager using previous default input");

Isn't it the other side of the if branch that uses m_lastDefaultMicrophone as defaultInput?
Comment 7 Geoffrey Garen 2022-01-21 09:01:41 PST 
Can we at least verify WebKit's internal behavior through mocking? That wouldn't prove that the mic actually worked, but it would prove that our code did what we thought it would.
Comment 8 youenn fablet 2022-01-24 07:22:30 PST 
Comment on attachment 449479 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=449479&action=review

>> Source/WebCore/platform/mediastream/ios/AVAudioSessionCaptureDeviceManager.mm:243
>> +        RELEASE_LOG_INFO(WebRTC, "AVAudioSessionCaptureDeviceManager using previous default input");
> 
> Isn't it the other side of the if branch that uses m_lastDefaultMicrophone as defaultInput?

Right, will fix it.
Comment 9 youenn fablet 2022-01-24 07:23:37 PST 
(In reply to Geoffrey Garen from comment #7)
> Can we at least verify WebKit's internal behavior through mocking? That
> wouldn't prove that the mic actually worked, but it would prove that our
> code did what we thought it would.

AVAudioSessionCaptureDeviceManager is not mocked at all like CoreAudioSharedUnit is.
But we should probably try to do so.
I'll think more about it.
Comment 10 youenn fablet 2022-01-24 07:31:16 PST 
Reopening to attach new patch.
Comment 11 youenn fablet 2022-01-24 07:31:20 PST 
Created attachment 449812 [details]
Patch
Comment 12 EWS 2022-01-24 08:05:00 PST 
Committed r288441 (246330@main): <https://commits.webkit.org/246330@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449812 [details].