Bug 211509 – Video capture does not get unmuted in case of tab switch on iOS

• Status: RESOLVED
• Resolution: FIXED
• Component: WebRTC

[youenn fablet]

Video capture does not get unmuted in case of tab switch on iOS.
This is probably a fall out of https://bugs.webkit.org/show_bug.cgi?id=208516.

[youenn fablet]

Created attachment 398615 [details]
Patch

[youenn fablet]

Created attachment 398620 [details]
Patch

[Eric Carlson]

Comment on attachment 398620 [details]
Patch

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

> Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:451
> +static bool isSourceCapturingForDocument(RealtimeMediaSource& activeSource, Document& document)

This name is slightly unclear about how it works. Maybe something like `isCapturingFromSameSourceAndDocument` or `isCapturingFromSourceAndDocument`?

> Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:464
> +void MediaStreamTrack::updateAccordingMutedState()

Nit: this name is also slightly unclear about what the method does. Maybe something like `syncToPageMutedState`, or `updateToPageMutedState`?

> Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:495
> +        bool muted = page->mutedState() & MediaProducer::AudioAndVideoCaptureIsMuted;

s/muted/pageMuted/

> Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:501
> +        bool muted = page->mutedState() & MediaProducer::AudioAndVideoCaptureIsMuted;

Ditto.

[youenn fablet]

Created attachment 398633 [details]
Patch for landing

[youenn fablet]

(In reply to youenn fablet from comment #4)
> Created attachment 398633 [details]
> Patch for landing

Wrong bug

[youenn fablet]

iOS test is failing as the ability to allow multiple cloned tracks breaks the ability to mute/unmute the active source.

I plan to temporarily disable this GPU process test and reenable it later on.
The tricky thing about fixing this test is that we do not want to resize video frames in WebProcess, so we cannot use RealtimeVideoSource/RealtimeVideoCaptureSource here.

[youenn fablet]

<rdar://problem/62537166>

[youenn fablet]

(In reply to Eric Carlson from comment #3)
> Comment on attachment 398620 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=398620&action=review
> 
> > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:451
> > +static bool isSourceCapturingForDocument(RealtimeMediaSource& activeSource, Document& document)
> 
> This name is slightly unclear about how it works. Maybe something like
> `isCapturingFromSameSourceAndDocument` or `isCapturingFromSourceAndDocument`?

I prefer the current name, we want to know whether the source is used by a track of the given document.

> > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:464
> > +void MediaStreamTrack::updateAccordingMutedState()
> 
> Nit: this name is also slightly unclear about what the method does. Maybe
> something like `syncToPageMutedState`, or `updateToPageMutedState`?

I'll go with updateToPageMutedState

> > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:495
> > +        bool muted = page->mutedState() & MediaProducer::AudioAndVideoCaptureIsMuted;
> 
> s/muted/pageMuted/

OK

> > Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:501
> > +        bool muted = page->mutedState() & MediaProducer::AudioAndVideoCaptureIsMuted;
> 
> Ditto.

OK

[youenn fablet]

Created attachment 398713 [details]
Patch

[youenn fablet]

Created attachment 398715 [details]
Patch

[youenn fablet]

Created attachment 398716 [details]
Patch

[youenn fablet]

Created attachment 398720 [details]
Patch

[Eric Carlson]

Comment on attachment 398620 [details]
Patch

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

>>> Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:451
>>> +static bool isSourceCapturingForDocument(RealtimeMediaSource& activeSource, Document& document)
>> 
>> This name is slightly unclear about how it works. Maybe something like `isCapturingFromSameSourceAndDocument` or `isCapturingFromSourceAndDocument`?
> 
> I prefer the current name, we want to know whether the source is used by a track of the given document.

`isSourceCapturingForTrackInDocument`?

[youenn fablet]

(In reply to Eric Carlson from comment #13)
> Comment on attachment 398620 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=398620&action=review
> 
> >>> Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:451
> >>> +static bool isSourceCapturingForDocument(RealtimeMediaSource& activeSource, Document& document)
> >> 
> >> This name is slightly unclear about how it works. Maybe something like `isCapturingFromSameSourceAndDocument` or `isCapturingFromSourceAndDocument`?
> > 
> > I prefer the current name, we want to know whether the source is used by a track of the given document.
> 
> `isSourceCapturingForTrackInDocument`?

OK, sounds good

[youenn fablet]

Created attachment 398843 [details]
Patch for landing

[EWS]

ChangeLog entry in Source/WebKit/ChangeLog is not at the top of the file.

[youenn fablet]

Created attachment 398852 [details]
Patch for landing

[EWS]

Committed r261373: <https://trac.webkit.org/changeset/261373>

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