Skip to content

Commit 237125c

Browse files
committed
[MSE][GStreamer] Avoid extra/circular refs between tracks/streams
https://bugs.webkit.org/show_bug.cgi?id=273488 Reviewed by Alicia Boya Garcia. 1. Avoid adding the same stream to the hash map. Also keeping the active/valid tracks in the player. 2. Avoid a hard reference in the source buffer when ready for more samples. 3. Break the indirect circular reference between the stream and track through the pad. * Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp: (WebCore::filterOutRepeatingTracks): (WebCore::MediaPlayerPrivateGStreamerMSE::sourceSetup): (WebCore::MediaPlayerPrivateGStreamerMSE::startSource): * Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp: (WebCore::SourceBufferPrivateGStreamer::notifyClientWhenReadyForMoreSamples): * Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.h: * Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp: (webKitMediaSrcEmitStreams): (webKitMediaSrcActivateMode): (webKitMediaSrcPadLinked): (webKitMediaSrcLoop): Canonical link: https://commits.webkit.org/278259@main
1 parent 7dfcfef commit 237125c

4 files changed

Lines changed: 44 additions & 12 deletions

File tree

Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,20 @@ class MediaPlayerFactoryGStreamerMSE final : public MediaPlayerFactory {
165165
}
166166
};
167167

168+
static Vector<RefPtr<MediaSourceTrackGStreamer>> filterOutRepeatingTracks(const Vector<RefPtr<MediaSourceTrackGStreamer>>& tracks)
169+
{
170+
Vector<RefPtr<MediaSourceTrackGStreamer>> uniqueTracks;
171+
uniqueTracks.reserveInitialCapacity(tracks.size());
172+
173+
for (const auto& track : tracks) {
174+
if (!uniqueTracks.containsIf([&track](const auto& current) { return track->trackId() == current->trackId(); }))
175+
uniqueTracks.append(track);
176+
}
177+
178+
uniqueTracks.shrinkToFit();
179+
return uniqueTracks;
180+
}
181+
168182
void MediaPlayerPrivateGStreamerMSE::registerMediaEngine(MediaEngineRegistrar registrar)
169183
{
170184
GST_DEBUG_CATEGORY_INIT(webkit_mse_debug, "webkitmse", 0, "WebKit MSE media player");
@@ -380,8 +394,10 @@ void MediaPlayerPrivateGStreamerMSE::sourceSetup(GstElement* sourceElement)
380394
webKitMediaSrcSetPlayer(WEBKIT_MEDIA_SRC(sourceElement), WeakPtr { *this });
381395
m_source = sourceElement;
382396

383-
if (m_mediaSourcePrivate->hasAllTracks())
397+
if (m_mediaSourcePrivate && m_mediaSourcePrivate->hasAllTracks()) {
398+
m_tracks = filterOutRepeatingTracks(m_tracks);
384399
webKitMediaSrcEmitStreams(WEBKIT_MEDIA_SRC(m_source.get()), m_tracks);
400+
}
385401
}
386402

387403
void MediaPlayerPrivateGStreamerMSE::updateStates()
@@ -443,8 +459,8 @@ void MediaPlayerPrivateGStreamerMSE::setInitialVideoSize(const FloatSize& videoS
443459

444460
void MediaPlayerPrivateGStreamerMSE::startSource(const Vector<RefPtr<MediaSourceTrackGStreamer>>& tracks)
445461
{
446-
m_tracks = tracks;
447-
webKitMediaSrcEmitStreams(WEBKIT_MEDIA_SRC(m_source.get()), tracks);
462+
m_tracks = filterOutRepeatingTracks(tracks);
463+
webKitMediaSrcEmitStreams(WEBKIT_MEDIA_SRC(m_source.get()), m_tracks);
448464
}
449465

450466
void MediaPlayerPrivateGStreamerMSE::getSupportedTypes(HashSet<String, ASCIICaseInsensitiveHash>& types)

Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,9 +183,12 @@ bool SourceBufferPrivateGStreamer::isReadyForMoreSamples(const AtomString& track
183183
void SourceBufferPrivateGStreamer::notifyClientWhenReadyForMoreSamples(const AtomString& trackId)
184184
{
185185
ASSERT(isMainThread());
186+
ASSERT(m_tracks.contains(trackId));
186187
MediaSourceTrackGStreamer* track = m_tracks.get(trackId);
187-
track->notifyWhenReadyForMoreSamples([protector = Ref { *this }, this, trackId]() mutable {
188-
RunLoop::main().dispatch([protector = WTFMove(protector), this, trackId]() {
188+
track->notifyWhenReadyForMoreSamples([weakPtr = WeakPtr { *this }, this, trackId]() mutable {
189+
RunLoop::main().dispatch([weakPtr = WTFMove(weakPtr), this, trackId]() {
190+
if (!weakPtr)
191+
return;
189192
if (!m_hasBeenRemovedFromMediaSource)
190193
provideMediaData(trackId);
191194
});

Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class AppendPipeline;
5050
class MediaSourcePrivateGStreamer;
5151
class MediaSourceTrackGStreamer;
5252

53-
class SourceBufferPrivateGStreamer final : public SourceBufferPrivate {
53+
class SourceBufferPrivateGStreamer final : public SourceBufferPrivate, public CanMakeWeakPtr<SourceBufferPrivateGStreamer> {
5454
public:
5555
static bool isContentTypeSupported(const ContentType&);
5656
static Ref<SourceBufferPrivateGStreamer> create(MediaSourcePrivateGStreamer*, const ContentType&, MediaPlayerPrivateGStreamerMSE&);

Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ static MediaPlayerPrivateGStreamerMSE* webKitMediaSrcPlayer(WebKitMediaSrc*);
105105
#define webkit_media_src_parent_class parent_class
106106

107107
struct WebKitMediaSrcPadPrivate {
108-
RefPtr<Stream> stream;
108+
WeakPtr<Stream> stream;
109109
};
110110

111111
struct WebKitMediaSrcPad {
@@ -154,7 +154,7 @@ WEBKIT_DEFINE_TYPE_WITH_CODE(WebKitMediaSrc, webkit_media_src, GST_TYPE_ELEMENT,
154154
G_IMPLEMENT_INTERFACE(GST_TYPE_URI_HANDLER, webKitMediaSrcUriHandlerInit);
155155
GST_DEBUG_CATEGORY_INIT(webkit_media_src_debug, "webkitmediasrc", 0, "WebKit MSE source element"));
156156

157-
struct Stream : public ThreadSafeRefCounted<Stream> {
157+
struct Stream : public ThreadSafeRefCounted<Stream>, CanMakeWeakPtr<Stream> {
158158
Stream(WebKitMediaSrc* source, GRefPtr<GstPad>&& pad, Ref<MediaSourceTrackGStreamer>&& track, GRefPtr<GstStream>&& streamInfo)
159159
: source(source)
160160
, pad(WTFMove(pad))
@@ -306,14 +306,19 @@ void webKitMediaSrcEmitStreams(WebKitMediaSrc* source, const Vector<RefPtr<Media
306306
source->priv->collection = adoptGRef(gst_stream_collection_new("WebKitMediaSrc"));
307307
for (const auto& track : tracks) {
308308
GST_DEBUG_OBJECT(source, "Adding stream with trackId '%s' of type %s with caps %" GST_PTR_FORMAT, track->trackId().string().utf8().data(), streamTypeToString(track->type()), track->initialCaps().get());
309+
if (source->priv->streams.contains(track->trackId())) {
310+
GST_ERROR_OBJECT(source, "stream with trackId '%s' already exists", track->trackId().string().utf8().data());
311+
ASSERT_NOT_REACHED();
312+
continue;
313+
}
309314

310315
GRefPtr<WebKitMediaSrcPad> pad = WEBKIT_MEDIA_SRC_PAD(g_object_new(webkit_media_src_pad_get_type(), "name", makeString("src_", track->trackId()).utf8().data(), "direction", GST_PAD_SRC, NULL));
311316
gst_pad_set_activatemode_function(GST_PAD(pad.get()), webKitMediaSrcActivateMode);
312317

313318
ASSERT(track->initialCaps());
314319
auto stream = adoptRef(new Stream(source, GRefPtr<GstPad>(GST_PAD(pad.get())), *track,
315320
adoptGRef(gst_stream_new(track->trackId().string().utf8().data(), track->initialCaps().get(), gstStreamType(track->type()), GST_STREAM_FLAG_SELECT))));
316-
pad->priv->stream = stream;
321+
pad->priv->stream = WeakPtr { *stream.get() };
317322

318323
gst_stream_collection_add_stream(source->priv->collection.get(), GRefPtr<GstStream>(stream->streamInfo.get()).leakRef());
319324
source->priv->streams.set(track->trackId(), WTFMove(stream));
@@ -375,8 +380,11 @@ static gboolean webKitMediaSrcActivateMode(GstPad* pad, GstObject* source, GstPa
375380
if (active)
376381
gst_pad_start_task(pad, webKitMediaSrcLoop, pad, nullptr);
377382
else {
383+
RefPtr<Stream> stream(WEBKIT_MEDIA_SRC_PAD(pad)->priv->stream.get());
384+
if (!stream)
385+
return false;
386+
378387
// Unblock the streaming thread.
379-
RefPtr<Stream>& stream = WEBKIT_MEDIA_SRC_PAD(pad)->priv->stream;
380388
{
381389
DataMutexLocker streamingMembers { stream->streamingMembersDataMutex };
382390
streamingMembers->isFlushing = true;
@@ -397,7 +405,10 @@ static gboolean webKitMediaSrcActivateMode(GstPad* pad, GstObject* source, GstPa
397405

398406
static void webKitMediaSrcPadLinked(GstPad* pad, GstPad*, void*)
399407
{
400-
RefPtr<Stream>& stream = WEBKIT_MEDIA_SRC_PAD(pad)->priv->stream;
408+
RefPtr<Stream> stream(WEBKIT_MEDIA_SRC_PAD(pad)->priv->stream.get());
409+
if (!stream)
410+
return;
411+
401412
DataMutexLocker streamingMembers { stream->streamingMembersDataMutex };
402413
streamingMembers->padLinkedOrFlushedCondition.notifyOne();
403414
}
@@ -424,7 +435,9 @@ static void webKitMediaSrcWaitForPadLinkedOrFlush(GstPad* pad, DataMutexLocker<S
424435
static void webKitMediaSrcLoop(void* userData)
425436
{
426437
GstPad* pad = GST_PAD(userData);
427-
RefPtr<Stream>& stream = WEBKIT_MEDIA_SRC_PAD(pad)->priv->stream;
438+
RefPtr<Stream> stream(WEBKIT_MEDIA_SRC_PAD(pad)->priv->stream.get());
439+
if (!stream)
440+
return;
428441

429442
DataMutexLocker streamingMembers { stream->streamingMembersDataMutex };
430443
if (streamingMembers->isFlushing) {

0 commit comments

Comments
 (0)