Skip to content

Commit

Permalink
[GStreamer] Set audio track ID to stream value in regular playback
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=260520

Reviewed by Xabier Rodriguez-Calvar.

This change only affects playbin2 implementations (regular playback),
where tracks are based on pads instead of in streams.

The proper track IDs are only actually available when the stream-start
bus message is received. The track creation is delayed until that moment.
At that point, all the audio and video pads handled by GStreamer are
processed and the corresponding audio/video tracks are created. The proper
track IDs are present at that moment. This means that the old
audio/videoChanged callbacks aren't needed anymore.

After that, we listen to changes in caps and tags in the pad and trigger
the needed updates, which will complete the information available for the
corresponding track.

Some approaches that were tried and resulted to be unsuccessful:

Trying to create the track the first time (on video-changed or
audio-changed), already with the definitive id isn't a valid approach,
since the sticky stream-start event isn't available at that moment.
Creating the track with a synthesized id and then updating it when the
stream-start event is available isn't a good idea, because the only way
to update the track is by removing it from audio/videoTracks and readding
it, and that triggers spureous events visible from JavaScript. Therefore,
the proper way was to delay track creation until stream-start comes.

Trying to install a probe on the audio/video pads and detecting the
stream-start event from there (instead of relying on the bus message),
but calling the track manipulation code from a non-main thread would be
unsafe and would require deferring the manipulation to the main thread.
Even in that case, there's no guarantee for the stream-start sticky event
to be available when the manipulation code runs in the main thread,
because that sticky event is stored after probe processing and there's
a race.

Some layout tests had to be changed to refer to the tracks by index
instead of by track id. The GStreamer based ports now return the native
id embedded in the stream container, while Apple ports still use synthesized
ids (A1, A2, V1...). That was the best way to deal with the differences
without duplicating the tests for different platforms.

Co-authored by Enrique Ocaña González <eocanha@igalia.com>

* LayoutTests/media/track/audio/audio-track-mkv-vorbis-language-expected.txt: Changed expected output.
* LayoutTests/media/track/audio/audio-track-mkv-vorbis-language.html: Refer to the tracks by index instead of by id.
* LayoutTests/media/track/video/video-track-mkv-theora-language-expected.txt: Changed expected output.
* LayoutTests/media/track/video/video-track-mkv-theora-language.html: Refer to the tracks by index instead of by id.
* Source/WebCore/platform/graphics/gstreamer/AudioTrackPrivateGStreamer.cpp:
(WebCore::AudioTrackPrivateGStreamer::AudioTrackPrivateGStreamer): Install configuration change handlers. Use move semantics.
(WebCore::AudioTrackPrivateGStreamer::capsChanged): Updated signature, use move semantics.
(WebCore::AudioTrackPrivateGStreamer::updateConfigurationFromTags): Updated signature.
(WebCore::AudioTrackPrivateGStreamer::updateConfigurationFromCaps): Ditto.
* Source/WebCore/platform/graphics/gstreamer/AudioTrackPrivateGStreamer.h: Declare configuration change handlers as overridding the base implementation. Updated some signatures to use r-value references.
* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::notifyPlayerOfTrack): Set track IDs (now always valid and final at this point, unlike before) and only notify the media engine if actual changes have been made. This is because this method can now be called even if no tracks of the given
type have appeared.
(WebCore::MediaPlayerPrivateGStreamer::handleMessage): Listen to stream-start bus message and unconditionally notify for audio/video track creation. notifyPlayerOfTrack() will know if tracks of each type have been created or not.
(WebCore::MediaPlayerPrivateGStreamer::createGSTPlayBin): Don't listen audio/video/text-changed anymore. We now have everything we need when stream-start comes.
(WebCore::MediaPlayerPrivateGStreamer::audioChangedCallback): Deleted.
(WebCore::MediaPlayerPrivateGStreamer::textChangedCallback): Deleted.
(WebCore::MediaPlayerPrivateGStreamer::videoChangedCallback): Deleted.
* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h: Deleted audio/video/textChangedCallback().
* Source/WebCore/platform/graphics/gstreamer/TrackPrivateBaseGStreamer.cpp:
(WebCore::findBestUpstreamPad): Added protection against null pad.
(WebCore::TrackPrivateBaseGStreamer::TrackPrivateBaseGStreamer): Generate ID from pad stream-start or synthesize one. This factors in common code and avoids repetition.
(WebCore::TrackPrivateBaseGStreamer::setPad): Protect against null m_bestUpstreamPad and reuse preexistent streamId from track.
(WebCore::TrackPrivateBaseGStreamer::tagsChanged): Initialize tags list to avoid uninitialized pointer in case parsing fails. Use move semantics.
(WebCore::TrackPrivateBaseGStreamer::capsChanged): Use move semantics.
(WebCore::TrackPrivateBaseGStreamer::notifyTrackOfTagsChanged): Avoid leak. Use move semantics.
(WebCore::TrackPrivateBaseGStreamer::notifyTrackOfStreamChanged): Protect against null pad.
(WebCore::TrackPrivateBaseGStreamer::installUpdateConfigurationHandlers): Listen to caps and tags changes and update configuration accordingly by calling the abstract handlers (to be implemented by subclasses as needed, empty by default). This installation works for pad-based (playbin2) as well as stream-based (playbin3) variants.
(WebCore::TrackPrivateBaseGStreamer::updateConfigurationFromCaps): Use r-value references.
(WebCore::TrackPrivateBaseGStreamer::updateConfigurationFromTags): Ditto.
(WebCore::TrackPrivateBaseGStreamer::trackIdFromPadStreamStartOrUniqueID): Extract the track ID name from the pad, or synthesize an ID if it can't be extracted.
(WebCore::TrackPrivateBaseGStreamer::getAllTags): Utility function to merge multiple tags stored in a pad as a sticky tags event.
* Source/WebCore/platform/graphics/gstreamer/TrackPrivateBaseGStreamer.h: Factored in common code into trackIdFromPadStreamStartOrUniqueID(). Added default empty abstract implementations of configuration change handlers and common method to install them, for those subclasses that chose to do so. Use r-value references in some signatures.
* Source/WebCore/platform/graphics/gstreamer/VideoTrackPrivateGStreamer.h: Declare configuration change handlers as overridding the base implementation.
* Source/WebCore/platform/graphics/gstreamer/VideoTrackPrivateGStreamer.cpp:
(WebCore::VideoTrackPrivateGStreamer::VideoTrackPrivateGStreamer): Install configuration change handlers.
(WebCore::VideoTrackPrivateGStreamer::capsChanged): Use move semantics.
(WebCore::VideoTrackPrivateGStreamer::updateConfigurationFromTags): Ditto.
(WebCore::VideoTrackPrivateGStreamer::updateConfigurationFromCaps): Ditto.

Canonical link: https://commits.webkit.org/268194@main
  • Loading branch information
@vivienne-w @eocanha
vivienne-w authored and eocanha committed Sep 20, 2023
1 parent afbd4f0 commit b750030
Show file tree
Hide file tree
Showing 12 changed files with 153 additions and 103 deletions.
4 changes: 2 additions & 2 deletions LayoutTests/media/track/audio/audio-track-mkv-vorbis-language-expected.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ EVENT(canplaythrough)

** Check in-band kind attributes
EXPECTED (video.audioTracks.length == '2') OK
EXPECTED (video.audioTracks.getTrackById('A0').language == 'en') OK
EXPECTED (video.audioTracks.getTrackById('A1').language == 'la') OK
EXPECTED (video.audioTracks[0].language == 'en') OK
EXPECTED (video.audioTracks[1].language == 'la') OK

END OF TEST

2 changes: 1 addition & 1 deletion LayoutTests/media/track/audio/audio-track-mkv-vorbis-language.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<script src=../../video-test.js></script>
<script src=../../in-band-tracks.js></script>
</head>
<body onload="testAttribute('../../content/two-audio-and-video-tracks.mkv', 'audio', 'language', {'A0': 'en', 'A1': 'la'})">
<body onload="testAttribute('../../content/two-audio-and-video-tracks.mkv', 'audio', 'language', ['en', 'la'])">
<video controls></video>
<p>Check audio tracks' language attributes.</p>
</body>
Expand Down
4 changes: 2 additions & 2 deletions LayoutTests/media/track/video/video-track-mkv-theora-language-expected.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ EVENT(canplaythrough)

** Check in-band kind attributes
EXPECTED (video.videoTracks.length == '2') OK
EXPECTED (video.videoTracks.getTrackById('V0').language == 'zh') OK
EXPECTED (video.videoTracks.getTrackById('V1').language == 'ru') OK
EXPECTED (video.videoTracks[0].language == 'zh') OK
EXPECTED (video.videoTracks[1].language == 'ru') OK

END OF TEST

2 changes: 1 addition & 1 deletion LayoutTests/media/track/video/video-track-mkv-theora-language.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<script src=../../video-test.js></script>
<script src=../../in-band-tracks.js></script>
</head>
<body onload="testAttribute('../../content/two-audio-and-video-tracks.mkv', 'video', 'language', {'V0':'zh', 'V1':'ru'})">
<body onload="testAttribute('../../content/two-audio-and-video-tracks.mkv', 'video', 'language', ['zh', 'ru'])">
<video controls></video>
<p>Check video tracks' language attributes.</p>
</body>
Expand Down
26 changes: 8 additions & 18 deletions Source/WebCore/platform/graphics/gstreamer/AudioTrackPrivateGStreamer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,37 +51,27 @@ AudioTrackPrivateGStreamer::AudioTrackPrivateGStreamer(WeakPtr<MediaPlayerPrivat
, m_player(player)
{
ensureDebugCategoryInitialized();
installUpdateConfigurationHandlers();
}

AudioTrackPrivateGStreamer::AudioTrackPrivateGStreamer(WeakPtr<MediaPlayerPrivateGStreamer> player, unsigned index, GstStream* stream)
: TrackPrivateBaseGStreamer(TrackPrivateBaseGStreamer::TrackType::Audio, this, index, stream)
, m_player(player)
{
ensureDebugCategoryInitialized();
g_signal_connect_swapped(m_stream.get(), "notify::caps", G_CALLBACK(+[](AudioTrackPrivateGStreamer* track) {
track->m_taskQueue.enqueueTask([track]() {
auto caps = adoptGRef(gst_stream_get_caps(track->m_stream.get()));
track->capsChanged(String::fromLatin1(gst_stream_get_stream_id(track->m_stream.get())), caps);
});
}), this);
g_signal_connect_swapped(m_stream.get(), "notify::tags", G_CALLBACK(+[](AudioTrackPrivateGStreamer* track) {
track->m_taskQueue.enqueueTask([track]() {
auto tags = adoptGRef(gst_stream_get_tags(track->m_stream.get()));
track->updateConfigurationFromTags(tags);
});
}), this);
installUpdateConfigurationHandlers();

auto caps = adoptGRef(gst_stream_get_caps(m_stream.get()));
updateConfigurationFromCaps(caps);
updateConfigurationFromCaps(WTFMove(caps));

auto tags = adoptGRef(gst_stream_get_tags(m_stream.get()));
updateConfigurationFromTags(tags);
updateConfigurationFromTags(WTFMove(tags));
}

void AudioTrackPrivateGStreamer::capsChanged(const String& streamId, const GRefPtr<GstCaps>& caps)
void AudioTrackPrivateGStreamer::capsChanged(const String& streamId, GRefPtr<GstCaps>&& caps)
{
ASSERT(isMainThread());
updateConfigurationFromCaps(caps);
updateConfigurationFromCaps(WTFMove(caps));

if (!m_player)
return;
Expand All @@ -96,7 +86,7 @@ void AudioTrackPrivateGStreamer::capsChanged(const String& streamId, const GRefP
setConfiguration(WTFMove(configuration));
}

void AudioTrackPrivateGStreamer::updateConfigurationFromTags(const GRefPtr<GstTagList>& tags)
void AudioTrackPrivateGStreamer::updateConfigurationFromTags(const GRefPtr<GstTagList>&& tags)
{
ASSERT(isMainThread());
GST_DEBUG_OBJECT(objectForLogging(), "Updating audio configuration from %" GST_PTR_FORMAT, tags.get());
Expand All @@ -113,7 +103,7 @@ void AudioTrackPrivateGStreamer::updateConfigurationFromTags(const GRefPtr<GstTa
setConfiguration(WTFMove(configuration));
}

void AudioTrackPrivateGStreamer::updateConfigurationFromCaps(const GRefPtr<GstCaps>& caps)
void AudioTrackPrivateGStreamer::updateConfigurationFromCaps(const GRefPtr<GstCaps>&& caps)
{
ASSERT(isMainThread());
if (!caps || !gst_caps_is_fixed(caps.get()))
Expand Down
8 changes: 4 additions & 4 deletions Source/WebCore/platform/graphics/gstreamer/AudioTrackPrivateGStreamer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,11 @@ class AudioTrackPrivateGStreamer final : public AudioTrackPrivate, public TrackP
AtomString language() const final { return m_language; }

protected:
void updateConfigurationFromCaps(const GRefPtr<GstCaps>&);
void updateConfigurationFromTags(const GRefPtr<GstTagList>&);
void updateConfigurationFromCaps(const GRefPtr<GstCaps>&&) override;
void updateConfigurationFromTags(const GRefPtr<GstTagList>&&) override;

void tagsChanged(const GRefPtr<GstTagList>& tags) final { updateConfigurationFromTags(tags); }
void capsChanged(const String& streamId, const GRefPtr<GstCaps>&) final;
void tagsChanged(GRefPtr<GstTagList>&& tags) final { updateConfigurationFromTags(WTFMove(tags)); }
void capsChanged(const String& streamId, GRefPtr<GstCaps>&&) final;

private:
AudioTrackPrivateGStreamer(WeakPtr<MediaPlayerPrivateGStreamer>, unsigned index, GRefPtr<GstPad>&&, bool shouldHandleStreamStartEvent);
Expand Down
59 changes: 22 additions & 37 deletions Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1055,15 +1055,15 @@ void MediaPlayerPrivateGStreamer::notifyPlayerOfTrack()
StringPrintStream getPadProperty;
getPadProperty.printf("get-%s-pad", typeName);

bool changed = false;
for (unsigned i = 0; i < numberOfTracks; ++i) {
GRefPtr<GstPad> pad;
g_signal_emit_by_name(m_pipeline.get(), getPadProperty.toCString().data(), i, &pad.outPtr(), nullptr);
ASSERT(pad);
if (!pad)
continue;

// The pad might not have a sticky stream-start event yet, so we can't use
// gst_pad_get_stream_id() here.
auto streamId = TrackPrivateBaseGStreamer::generateUniquePlaybin2StreamID(type, i);
validStreams.append(streamId);
AtomString streamId(TrackPrivateBaseGStreamer::trackIdFromPadStreamStartOrUniqueID(type, i, pad));

if (i < tracks.size()) {
RefPtr<TrackPrivateType> existingTrack = tracks.get(streamId);
Expand All @@ -1077,10 +1077,11 @@ void MediaPlayerPrivateGStreamer::notifyPlayerOfTrack()
}
}

auto track = TrackPrivateType::create(*this, i, WTFMove(pad));
auto track = TrackPrivateType::create(*this, i, GRefPtr(pad));
ASSERT(track->id() == streamId);
validStreams.append(track->id());
if (!track->trackIndex() && (type == TrackType::Audio || type == TrackType::Video))
track->setActive(true);
ASSERT(streamId == track->id());

std::variant<AudioTrackPrivate*, VideoTrackPrivate*, InbandTextTrackPrivate*> variantTrack(&track.get());
switch (variantTrack.index()) {
Expand All @@ -1094,15 +1095,17 @@ void MediaPlayerPrivateGStreamer::notifyPlayerOfTrack()
player->addTextTrack(*std::get<InbandTextTrackPrivate*>(variantTrack));
break;
}
tracks.add(streamId, WTFMove(track));
tracks.add(track->id(), WTFMove(track));
changed = true;
}

// Purge invalid tracks
tracks.removeIf([validStreams](auto& keyAndValue) {
changed = changed || tracks.removeIf([validStreams](auto& keyAndValue) {
return !validStreams.contains(keyAndValue.key);
});

player->mediaEngineUpdated();
if (changed)
player->mediaEngineUpdated();
}

bool MediaPlayerPrivateGStreamer::hasFirstVideoSampleReachedSink() const
Expand Down Expand Up @@ -1136,20 +1139,6 @@ void MediaPlayerPrivateGStreamer::videoSinkCapsChanged(GstPad* videoSinkPad)
});
}

void MediaPlayerPrivateGStreamer::audioChangedCallback(MediaPlayerPrivateGStreamer* player)
{
player->m_notifier->notify(MainThreadNotification::AudioChanged, [player] {
player->notifyPlayerOfTrack<AudioTrackPrivateGStreamer>();
});
}

void MediaPlayerPrivateGStreamer::textChangedCallback(MediaPlayerPrivateGStreamer* player)
{
player->m_notifier->notify(MainThreadNotification::TextChanged, [player] {
player->notifyPlayerOfTrack<InbandTextTrackPrivateGStreamer>();
});
}

void MediaPlayerPrivateGStreamer::handleTextSample(GstSample* sample, const char* streamId)
{
for (auto& track : m_textTracks.values()) {
Expand Down Expand Up @@ -1560,13 +1549,6 @@ void MediaPlayerPrivateGStreamer::updateTracks(const GRefPtr<GstObject>& collect
#undef CREATE_OR_SELECT_TRACK
}

void MediaPlayerPrivateGStreamer::videoChangedCallback(MediaPlayerPrivateGStreamer* player)
{
player->m_notifier->notify(MainThreadNotification::VideoChanged, [player] {
player->notifyPlayerOfTrack<VideoTrackPrivateGStreamer>();
});
}

void MediaPlayerPrivateGStreamer::handleStreamCollectionMessage(GstMessage* message)
{
if (m_isLegacyPlaybin)
Expand Down Expand Up @@ -2063,6 +2045,16 @@ void MediaPlayerPrivateGStreamer::handleMessage(GstMessage* message)
playbin3SendSelectStreamsIfAppropriate();
break;
}
case GST_MESSAGE_STREAM_START: {
// Real track id configuration in MSE is managed by AppendPipeline. In MediaStream we don't support native stream ids.
if (!m_isLegacyPlaybin)
break;

notifyPlayerOfTrack<VideoTrackPrivateGStreamer>();
notifyPlayerOfTrack<AudioTrackPrivateGStreamer>();
notifyPlayerOfTrack<InbandTextTrackPrivateGStreamer>();
break;
}
default:
GST_DEBUG_OBJECT(pipeline(), "Unhandled GStreamer message type: %s", GST_MESSAGE_TYPE_NAME(message));
break;
Expand Down Expand Up @@ -2985,13 +2977,6 @@ void MediaPlayerPrivateGStreamer::createGSTPlayBin(const URL& url)
}), this);

g_signal_connect_swapped(m_pipeline.get(), "source-setup", G_CALLBACK(sourceSetupCallback), this);
if (m_isLegacyPlaybin) {
g_signal_connect_swapped(m_pipeline.get(), "video-changed", G_CALLBACK(videoChangedCallback), this);
g_signal_connect_swapped(m_pipeline.get(), "audio-changed", G_CALLBACK(audioChangedCallback), this);
}

if (m_isLegacyPlaybin)
g_signal_connect_swapped(m_pipeline.get(), "text-changed", G_CALLBACK(textChangedCallback), this);

if (auto* textCombiner = webkitTextCombinerNew())
g_object_set(m_pipeline.get(), "text-stream-combiner", textCombiner, nullptr);
Expand Down
3 changes: 0 additions & 3 deletions Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,9 +318,6 @@ class MediaPlayerPrivateGStreamer : public MediaPlayerPrivateInterface
void ensureSeekFlags();

static void sourceSetupCallback(MediaPlayerPrivateGStreamer*, GstElement*);
static void videoChangedCallback(MediaPlayerPrivateGStreamer*);
static void audioChangedCallback(MediaPlayerPrivateGStreamer*);
static void textChangedCallback(MediaPlayerPrivateGStreamer*);

void timeChanged(const MediaTime&); // If MediaTime is valid, indicates that a seek has completed.
void loadingFailed(MediaPlayer::NetworkState, MediaPlayer::ReadyState = MediaPlayer::ReadyState::HaveNothing, bool forceNotifications = false);
Expand Down
Loading

0 comments on commit b750030

Please sign in to comment.