Add a preference to mute video capture in case audio capture gets interrupted
<rdar://78421282>
Created attachment 453967 [details] Patch
<rdar://problem/89902939>
Created attachment 453983 [details] Patch
Created attachment 453985 [details] Patch
Created attachment 453987 [details] Patch
@philn, I am trying to remove the internals API to interrupt source and use testRunner interruption API which should be closer to how this actually works. The test currently fails in GTK, probably due to MockRealtimeAudioSource::setIsInterrupted being implemented for Cocoa right now. Any idea how this could be implemented for GTK?
(In reply to youenn fablet from comment #7) > @philn, I am trying to remove the internals API to interrupt source and use > testRunner interruption API which should be closer to how this actually > works. > > The test currently fails in GTK, probably due to > MockRealtimeAudioSource::setIsInterrupted being implemented for Cocoa right > now. > Any idea how this could be implemented for GTK? Humm maybe make it so that MockRealtimeAudioSourceGStreamer::render returns early without notifying samples?
We do implement MockRealtimeAudioSourceGStreamer::setInterruptedForTesting() though.
Comment on attachment 453987 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=453987&action=review > Source/WebCore/platform/mediastream/gstreamer/MockRealtimeAudioSourceGStreamer.cpp:-143 > -void MockRealtimeAudioSourceGStreamer::setInterruptedForTesting(bool isInterrupted) Sorry, didn't read the patch. :D
> Humm maybe make it so that MockRealtimeAudioSourceGStreamer::render returns > early without notifying samples? The main difference is that we are going from an instance method to a static method. Maybe a static variable that MockRealtimeAudioSourceGStreamer::render would use might do the trick?
(In reply to youenn fablet from comment #11) > > Humm maybe make it so that MockRealtimeAudioSourceGStreamer::render returns > > early without notifying samples? > > The main difference is that we are going from an instance method to a static > method. > Maybe a static variable that MockRealtimeAudioSourceGStreamer::render would > use might do the trick? Yes maybe. Otherwise mark the test as failing and I'll try to fix it later.
Comment on attachment 453987 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=453987&action=review r=me once the bots are happy > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:521 > + if (!source.isEnded() && source.type() == RealtimeMediaSource::Type::Video) > + source.setMuted(isMicrophoneInterrupted); Shouldn't we also mute screen and window capture during and interruption? > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:643 > + if (wasInterrupted != m_isInterrupted && m_private->source().type() == RealtimeMediaSource::Type::Audio && document->settings().muteCameraOnMicrophoneInterruptionEnabled()) > + updateVideoCaptureAccordingMicrophoneInterruption(*document, m_isInterrupted); Won't this run multiple times if we have more than one audio track?
(In reply to Eric Carlson from comment #13) > Comment on attachment 453987 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=453987&action=review > > r=me once the bots are happy > > > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:521 > > + if (!source.isEnded() && source.type() == RealtimeMediaSource::Type::Video) > > + source.setMuted(isMicrophoneInterrupted); > > Shouldn't we also mute screen and window capture during and interruption? I was not sure about that. Let's continue the discussion on this one. > > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:643 > > + if (wasInterrupted != m_isInterrupted && m_private->source().type() == RealtimeMediaSource::Type::Audio && document->settings().muteCameraOnMicrophoneInterruptionEnabled()) > > + updateVideoCaptureAccordingMicrophoneInterruption(*document, m_isInterrupted); > > Won't this run multiple times if we have more than one audio track? Yes, it will run once per live microphone track attached to the source. In general, probably only once but it could be more with cloning. That is the downside of using tracks but should have no functional impact visible to users.
Given gtk failures, I'll add back the internals API, we should remove it as a follow-up
Created attachment 454081 [details] Patch
Created attachment 454092 [details] Patch for landing
Committed r290985 (248163@main): <https://commits.webkit.org/248163@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 454092 [details].
> > Shouldn't we also mute screen and window capture during and interruption? > I was not sure about that. > Let's continue the discussion on this one. FWIW, I tend to agree with the general direction that, if we're going to interrupt, we should interrupt everything. I think that's a simpler mental model for the user, and it matches most UI's I've seen best.