[GStreamer] Refactor track handling to use stream-ids in playbin3 mode

• Component: WebKitGTK
• Status: RESOLVED WONTFIX

[Alicia Boya García]

Currently the player private uses track indexes to select tracks in
all cases. While using track indices is OK in playbin2, it shouldn't
be done in playbin3, where tracks are identified with string
stream-ids instead.

This patch reworks the track handling to use the right method for each
playbin.

[Alicia Boya García]

Created attachment 380870 [details]
Patch

[Alicia Boya García]

Created attachment 380876 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 380876 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/AudioTrackPrivateGStreamer.cpp:96
> +    else if (enabled) // playbin2: support for exactly one track of each kind.
>          m_player->enableTrack(TrackPrivateBaseGStreamer::TrackType::Audio, m_index);

Not a strong opinion but for coherence with the pb3 function that asserts on it, here I would do something like:

I'd do something like:
else {
    ASSERT(m_isLegacyPlaybin);
    if (enabled)
...

> Source/WebCore/platform/graphics/gstreamer/VideoTrackPrivateGStreamer.cpp:95
> +    else if (selected) // playbin2: support for exactly one track of each kind.
> +        m_player->enableTrack(TrackPrivateBaseGStreamer::TrackType::Audio, m_index);

Ditto

[Xabier Rodríguez Calvar]

Comment on attachment 380876 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:779
> +    ASSERT(!m_isLegacyPlaybin);

ASSERT(isMainThread()) cause m_selectedStreamIds is not thread safe.

[Alicia Boya García]

I'm deferring this patch because I think it needs more changes.

[Philippe Normand]

Can the bug remains open though?

[Alicia Boya García]

(In reply to Philippe Normand from comment #6)
> Can the bug remains open though?

That's fine.

[Philippe Normand]

So what happens here now? Is this a duplicate of bug 211623 ?

[Alicia Boya García]

(In reply to Philippe Normand from comment #8)
> So what happens here now? Is this a duplicate of bug 211623 ?

Bug 211623 obsoletes this. This patch assumed we wanted to get rid of track ids completely to allow for tracks to change dynamically, taking MSE as a case where that would occur. This is no longer the goal: I found dynamically adding and removing tracks, as opposed to emitting them all in one go, is not stable enough in playbin3, nor necessary for a working MSE implementation. Removed that requirement, there is no benefit from removing track indices.