Skip to content

Commit c3e9799

Browse files
ntrrgcvivienne-w
authored andcommitted
[MSE][GStreamer] Actually do flushes, unify m_hasAllTracks
https://bugs.webkit.org/show_bug.cgi?id=257035 Reviewed by Xabier Rodriguez-Calvar. Before this patch, there were two classes having a m_hasAllTracks field: MediaSourcePrivateGStreamer and MediaPlayerPrivateGStreamerMSE. This redundancy lead to non-synchronization, and in consequence this was making flushes not occur on MSE, as SourceBufferPrivateGStreamer::flush() checked for the one in MediaPlayerPrivateGStreamerMSE which no code ever set to true. A visible consequence of this was video was being repeated on quality changes, due to the lack of flush of old frames. This patch fixes the issue by keeping the `hasAllTracks` state in one single place, in MediaSourcePrivateGStreamer. Note: The triggering of MSE flushes exposed several bugs on the handling of flushes in other parts of the code. It's important these are addressed to avoid regressions in behavior when incorporating this patch: (1) WebKit/WebKit#3802 [GStreamer] MediaPlayerPrivateGStreamer: Abort stale tasks on flushes Without this patch, MSE flushes can deadlock. (2) https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/2413 appsink: Fix race condition on caps handling appsink used to have a race condition on the handling of caps after a flush that can cause crashes. A fix is present in GStreamer 1.20.3+. A workaround in the WebKit side is possible and desirable to avoid headaches in Linux distros, and has been uploaded as: WebKit/WebKit#3965 [GStreamer] Introduce workaround for race condition in appsink (3) https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/3471 basesink: Support position queries after non-resetting flushes basesink doesn't answer position queries between a FLUSH_STOP with reset_time=FALSE and a SEGMENT event until GStreamer 1.24.0, which incorporates that patch. For backwards-compatibility -- and present-compatibility for that matter since GStreamer 1.24.0 has not been released yet, a workaround has been Proposed for WebKit: WebKit/WebKit#14082 [MSE][GStreamer] Workaround basesink's lack of support for position queries between a non-resetting flush and a pre-roll * Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp: (WebCore::MediaPlayerPrivateGStreamerMSE::sourceSetup): * Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.h: (WebCore::MediaPlayerPrivateGStreamerMSE::hasAllTracks const): Deleted. * Source/WebCore/platform/graphics/gstreamer/mse/MediaSourcePrivateGStreamer.cpp: (WebCore::MediaSourcePrivateGStreamer::addSourceBuffer): * Source/WebCore/platform/graphics/gstreamer/mse/MediaSourcePrivateGStreamer.h: * Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp: (WebCore::SourceBufferPrivateGStreamer::flush): Canonical link: https://commits.webkit.org/264510@main
1 parent 11db264 commit c3e9799

5 files changed

Lines changed: 4 additions & 5 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ void MediaPlayerPrivateGStreamerMSE::sourceSetup(GstElement* sourceElement)
348348
GST_DEBUG_OBJECT(pipeline(), "Source %p setup (old was: %p)", sourceElement, m_source.get());
349349
m_source = sourceElement;
350350

351-
if (m_hasAllTracks)
351+
if (m_mediaSourcePrivate->hasAllTracks())
352352
webKitMediaSrcEmitStreams(WEBKIT_MEDIA_SRC(m_source.get()), m_tracks);
353353
}
354354

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@ class MediaPlayerPrivateGStreamerMSE : public MediaPlayerPrivateGStreamer {
8181

8282
void asyncStateChangeDone() override;
8383

84-
bool hasAllTracks() const { return m_hasAllTracks; }
8584
void startSource(const Vector<RefPtr<MediaSourceTrackGStreamer>>& tracks);
8685
WebKitMediaSrc* webKitMediaSrc() { return reinterpret_cast<WebKitMediaSrc*>(m_source.get()); }
8786

@@ -111,7 +110,6 @@ class MediaPlayerPrivateGStreamerMSE : public MediaPlayerPrivateGStreamer {
111110
RefPtr<MediaSourcePrivateGStreamer> m_mediaSourcePrivate;
112111
MediaTime m_mediaTimeDuration { MediaTime::invalidTime() };
113112
bool m_isPipelinePlaying = true;
114-
bool m_hasAllTracks = false;
115113
Vector<RefPtr<MediaSourceTrackGStreamer>> m_tracks;
116114

117115
bool m_isWaitingForPreroll = true;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ MediaSourcePrivateGStreamer::AddStatus MediaSourcePrivateGStreamer::addSourceBuf
8484
DEBUG_LOG(LOGIDENTIFIER, contentType);
8585

8686
// Once every SourceBuffer has had an initialization segment appended playback starts and it's too late to add new SourceBuffers.
87-
if (m_playerPrivate.hasAllTracks())
87+
if (m_hasAllTracks)
8888
return MediaSourcePrivateGStreamer::AddStatus::ReachedIdLimit;
8989

9090
if (!SourceBufferPrivateGStreamer::isContentTypeSupported(contentType))

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ class MediaSourcePrivateGStreamer final : public MediaSourcePrivate
7575

7676
void sourceBufferPrivateDidChangeActiveState(SourceBufferPrivateGStreamer*, bool);
7777
void startPlaybackIfHasAllTracks();
78+
bool hasAllTracks() const { return m_hasAllTracks; }
7879

7980
std::unique_ptr<PlatformTimeRanges> buffered();
8081

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ void SourceBufferPrivateGStreamer::flush(const AtomString& trackId)
143143

144144
// This is only for on-the-fly reenqueues after appends. When seeking, the seek will do its own flush.
145145

146-
if (!m_playerPrivate.hasAllTracks()) {
146+
if (!m_mediaSource->hasAllTracks()) {
147147
GST_DEBUG_OBJECT(m_playerPrivate.pipeline(), "Source element has not emitted tracks yet, so we only need to clear the queue. trackId = '%s'", trackId.string().utf8().data());
148148
MediaSourceTrackGStreamer* track = m_tracks.get(trackId);
149149
track->clearQueue();

0 commit comments

Comments
 (0)