[Mac] Audio capture fails when shouldCaptureAudioInUIProcess is set.
rdar://problem/31848689
Created attachment 309137 [details] Patch
Created attachment 309141 [details] Patch
Created attachment 309145 [details] Patch
Comment on attachment 309145 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=309145&action=review > Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp:115 > + if (!audioDeviceID.isEmpty() && audioFactory()) { Can we have cases where we have some device id but no audio factory? Shouldn't we assert that audioFactory is not null if device is not empty? > Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp:117 > if (audioSource) If audioSource is null, shouldn't we reject the getUserMedia promise? Or do we want to be able to resolve with only the video track if we are able to get it?
Comment on attachment 309145 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=309145&action=review >> Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp:117 >> if (audioSource) > > If audioSource is null, shouldn't we reject the getUserMedia promise? > Or do we want to be able to resolve with only the video track if we are able to get it? If audioSource is NULL when audioDeviceID is not empty is an internal failure because we shouldn't end up in createMediaStream if validateRequestConstraints failed, so an ASSERT is appropriate.
(In reply to Eric Carlson from comment #6) > Comment on attachment 309145 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=309145&action=review > > >> Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp:117 > >> if (audioSource) > > > > If audioSource is null, shouldn't we reject the getUserMedia promise? > > Or do we want to be able to resolve with only the video track if we are able to get it? > > If audioSource is NULL when audioDeviceID is not empty is an internal > failure because we shouldn't end up in createMediaStream if > validateRequestConstraints failed, so an ASSERT is appropriate. OK, so we should add an ASSERT for audio, an ASSERT for video. Do we also want to return early if audio is failing so that we reject the promise. Currently if audio fails and video succeeds, we are resolving the promise.
(In reply to youenn fablet from comment #5) > Comment on attachment 309145 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=309145&action=review > > > Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp:115 > > + if (!audioDeviceID.isEmpty() && audioFactory()) { > > Can we have cases where we have some device id but no audio factory? > Shouldn't we assert that audioFactory is not null if device is not empty? Honestly, if we do anything it should be to require subclasses of RealtimeMediaSourceCenter to return a non-NULL value from defaultAudioFactory() and defaultVideoFactory() and then change the return value of audioFactory() and videoFactory() to a reference instead of a pointer. Then an ASSERT won't be necessary. > > Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.cpp:117 > > if (audioSource) > > If audioSource is null, shouldn't we reject the getUserMedia promise? > Or do we want to be able to resolve with only the video track if we are able > to get it? We're not changing this behavior in this patch. Please file a bug to do this in a follow up.
Let's land it, I'll fix the clean-up things in a follow-up.
Comment on attachment 309145 [details] Patch Clearing flags on attachment: 309145 Committed r216446: <http://trac.webkit.org/changeset/216446>
All reviewed patches have been landed. Closing bug.