Skip to content

Commit 7b9a46d

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 158900c commit 7b9a46d

4 files changed

Lines changed: 43 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
@@ -100,6 +100,20 @@ class MediaPlayerFactoryGStreamerMSE final : public MediaPlayerFactory {
100100
}
101101
};
102102

103+
static Vector<RefPtr<MediaSourceTrackGStreamer>> filterOutRepeatingTracks(const Vector<RefPtr<MediaSourceTrackGStreamer>>& tracks)
104+
{
105+
Vector<RefPtr<MediaSourceTrackGStreamer>> uniqueTracks;
106+
uniqueTracks.reserveInitialCapacity(tracks.size());
107+
108+
for (const auto& track : tracks) {
109+
if (!uniqueTracks.containsIf([&track](const auto& current) { return track->stringId() == current->stringId(); }))
110+
uniqueTracks.append(track);
111+
}
112+
113+
uniqueTracks.shrinkToFit();
114+
return uniqueTracks;
115+
}
116+
103117
void MediaPlayerPrivateGStreamerMSE::registerMediaEngine(MediaEngineRegistrar registrar)
104118
{
105119
GST_DEBUG_CATEGORY_INIT(webkit_mse_debug, "webkitmse", 0, "WebKit MSE media player");
@@ -318,8 +332,10 @@ void MediaPlayerPrivateGStreamerMSE::sourceSetup(GstElement* sourceElement)
318332
webKitMediaSrcSetPlayer(WEBKIT_MEDIA_SRC(sourceElement), WeakPtr { *this });
319333
m_source = sourceElement;
320334

321-
if (m_mediaSourcePrivate && m_mediaSourcePrivate->hasAllTracks())
335+
if (m_mediaSourcePrivate && m_mediaSourcePrivate->hasAllTracks()) {
336+
m_tracks = filterOutRepeatingTracks(m_tracks);
322337
webKitMediaSrcEmitStreams(WEBKIT_MEDIA_SRC(m_source.get()), m_tracks);
338+
}
323339
}
324340

325341
void MediaPlayerPrivateGStreamerMSE::updateStates()
@@ -384,8 +400,8 @@ void MediaPlayerPrivateGStreamerMSE::setInitialVideoSize(const FloatSize& videoS
384400

385401
void MediaPlayerPrivateGStreamerMSE::startSource(const Vector<RefPtr<MediaSourceTrackGStreamer>>& tracks)
386402
{
387-
m_tracks = tracks;
388-
webKitMediaSrcEmitStreams(WEBKIT_MEDIA_SRC(m_source.get()), tracks);
403+
m_tracks = filterOutRepeatingTracks(tracks);
404+
webKitMediaSrcEmitStreams(WEBKIT_MEDIA_SRC(m_source.get()), m_tracks);
389405
}
390406

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

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,10 @@ void SourceBufferPrivateGStreamer::notifyClientWhenReadyForMoreSamples(TrackID t
194194
ASSERT(isMainThread());
195195
ASSERT(m_tracks.contains(trackId));
196196
auto track = m_tracks[trackId];
197-
track->notifyWhenReadyForMoreSamples([protectedThis = Ref { *this }, this, trackId]() mutable {
198-
RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis), this, trackId]() {
197+
track->notifyWhenReadyForMoreSamples([weakPtr = WeakPtr { *this }, this, trackId]() mutable {
198+
RunLoop::main().dispatch([weakPtr = WTFMove(weakPtr), this, trackId]() {
199+
if (!weakPtr)
200+
return;
199201
if (!m_hasBeenRemovedFromMediaSource)
200202
provideMediaData(trackId);
201203
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ using TrackID = uint64_t;
5454
class AppendPipeline;
5555
class MediaSourcePrivateGStreamer;
5656

57-
class SourceBufferPrivateGStreamer final : public SourceBufferPrivate {
57+
class SourceBufferPrivateGStreamer final : public SourceBufferPrivate, public CanMakeWeakPtr<SourceBufferPrivateGStreamer> {
5858
public:
5959
static bool isContentTypeSupported(const ContentType&);
6060
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+
ThreadSafeWeakPtr<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 ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<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->stringId().string().utf8().data(), streamTypeToString(track->type()), track->initialCaps().get());
309+
if (source->priv->streams.contains(track->stringId())) {
310+
GST_ERROR_OBJECT(source, "stream with trackId '%s' already exists", track->stringId().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->stringId()).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->stringId().string().utf8().data(), track->initialCaps().get(), gstStreamType(track->type()), GST_STREAM_FLAG_SELECT))));
316-
pad->priv->stream = stream;
321+
pad->priv->stream = ThreadSafeWeakPtr { *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->stringId(), WTFMove(stream));
@@ -378,8 +383,11 @@ static gboolean webKitMediaSrcActivateMode(GstPad* pad, GstObject* source, GstPa
378383
if (active)
379384
gst_pad_start_task(pad, webKitMediaSrcLoop, pad, nullptr);
380385
else {
386+
RefPtr<Stream> stream(WEBKIT_MEDIA_SRC_PAD(pad)->priv->stream.get());
387+
if (!stream)
388+
return false;
389+
381390
// Unblock the streaming thread.
382-
RefPtr<Stream>& stream = WEBKIT_MEDIA_SRC_PAD(pad)->priv->stream;
383391
{
384392
DataMutexLocker streamingMembers { stream->streamingMembersDataMutex };
385393
streamingMembers->isFlushing = true;
@@ -400,7 +408,10 @@ static gboolean webKitMediaSrcActivateMode(GstPad* pad, GstObject* source, GstPa
400408

401409
static void webKitMediaSrcPadLinked(GstPad* pad, GstPad*, void*)
402410
{
403-
RefPtr<Stream>& stream = WEBKIT_MEDIA_SRC_PAD(pad)->priv->stream;
411+
RefPtr<Stream> stream(WEBKIT_MEDIA_SRC_PAD(pad)->priv->stream.get());
412+
if (!stream)
413+
return;
414+
404415
DataMutexLocker streamingMembers { stream->streamingMembersDataMutex };
405416
streamingMembers->padLinkedOrFlushedCondition.notifyOne();
406417
}
@@ -427,7 +438,9 @@ static void webKitMediaSrcWaitForPadLinkedOrFlush(GstPad* pad, DataMutexLocker<S
427438
static void webKitMediaSrcLoop(void* userData)
428439
{
429440
GstPad* pad = GST_PAD(userData);
430-
RefPtr<Stream>& stream = WEBKIT_MEDIA_SRC_PAD(pad)->priv->stream;
441+
RefPtr<Stream> stream(WEBKIT_MEDIA_SRC_PAD(pad)->priv->stream.get());
442+
if (!stream)
443+
return;
431444

432445
DataMutexLocker streamingMembers { stream->streamingMembersDataMutex };
433446
if (streamingMembers->isFlushing) {

0 commit comments

Comments
 (0)