Bug 211980

Summary: [GStreamer][MediaStream] Remove orphaned tracks in updateTracks()
Product: WebKit Reporter: Alicia Boya García <aboya>
Component: WebKitGTKAssignee: Alicia Boya García <aboya>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, calvaris, cgarcia, eric.carlson, ews-watchlist, glenn, gustavo, jer.noble, menard, philipj, pnormand, sergio, vjaquez
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=211946
Attachments:
Description Flags
Patch
none
Patch none

Description Alicia Boya García 2020-05-16 03:01:25 PDT 
This patch ensures tracks missing from a subsequent updateTracks()
calls are removed from the player.

This fixes the following tests regressed by the track switch rework:

imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-removetrack.https.html
imported/w3c/web-platform-tests/mediacapture-streams/MediaStreamTrack-applyConstraints.https.html
Comment 1 Alicia Boya García 2020-05-16 05:26:31 PDT 
Created attachment 399548 [details]
Patch
Comment 2 Alicia Boya García 2020-05-16 09:17:18 PDT 
Created attachment 399555 [details]
Patch
Comment 3 Philippe Normand 2020-05-18 01:15:46 PDT 
Comment on attachment 399555 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1512
> +template<typename K, typename V>
> +HashSet<K> hashSetFromHashMapKeys(const HashMap<K, V>& hashMap)
> +{
> +    HashSet<K> keys;
> +    for (auto& key : hashMap.keys())
> +        keys.add(key);
> +    return keys;
> +}

Would this be worth moving to HashMap as a new method?
Comment 4 Alicia Boya García 2020-05-18 01:52:04 PDT 
Comment on attachment 399555 [details]
Patch

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

>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:1512
>> +}
> 
> Would this be worth moving to HashMap as a new method?

I don't know, all HashMap methods are pretty basic, so maybe someone would oppose this in WTF, or maybe a function already exists and I just didn't find it. I wanted a self-contained fix that didn't need a potentially stalling WTF review, so this was a reasonable solution.

I've asked in the WebKit slack, if it turns out there was an existing function or there wasn't but people really want it in WTF another patch can be made that moves it there. For now I'm using this.
Comment 5 EWS 2020-05-18 02:09:34 PDT 
Committed r261803: <https://trac.webkit.org/changeset/261803>

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