Skip to content

Commit 8774900

Browse files
eocanhaeocanha
committed
[MSE][GStreamer] Support markEndOfStream() before appendBuffer()
https://bugs.webkit.org/show_bug.cgi?id=278726 Reviewed by Xabier Rodriguez-Calvar. In the past, WebKit MSE couldn't complete playback when JavaScript performs MediaSource.markEndOfStream() before appending any samples with SourceBuffer.appendBuffer(). A previous patch added that support, but still the error keeps happening under some circumstances downstream (wpe-2.46, GStreamer 1.18) as well as upstream (where it was believed not to happen in GStreamer 1.24). This happens when the test case is exercised in a loop and the race condition for an error occurs while errors are ignored, but the GstMessage with the error takes a moment to propagate to the main thread (when the errors are no longer ignored). See: #1558 This patch now applies the fix to any GStreamer version (because the issue appears in all of them). When an EOS without buffers happens, the pipeline goes to READY, and errors are ignored. The m_ignoreErrors flag is kept enabled while the pipeline is being set to READY, but if any error has been detected by the sync message handler, it stays enabled until the last enqueued error is processed by the main thread. In any case m_ignoreErrors is only changed from the main thread (in different loop cycles in the worst case, but always from the main thread) and doesn't need concurrent access protection. As mentioned in the previous paragraph, a new sync error signal listener has been added. This listener doesn't do any actual error processing, just increases the m_queuedSyncErrors counter. The async message handler will end up getting the error message anyway. That's where the message will be ignored (if the m_ignoreErrors flag is set) and, ignored or not, the m_queuedSyncErrors counter will be decremented. Note that this counter is an Atomic, in order to support access from the (potentially) non-main thread that handles the sync error signal, from the main thread that ends up processing the error, and from the main thread that processes setEosWithNoBuffers(). If an error has happened, an EOS in the pipeline is simulated by calling didEnd(), the same method that would be called if such an EOS had happened. This call usually triggers a pipeline teardown and the further destruction of the player private. This may trigger a gst_bus_set_flushing() call and cause the error message to never be dispatched to the main thread. While this may mean that the m_queuedSyncErrors counter is never decremented, this isn't a real problem, since the player is being destructed anyway and nobody cares about the count anymore. * Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp: (WebCore::MediaPlayerPrivateGStreamer::handleSyncErrorMessage): Detect errors as they happen, in the same thread where they were generated (often a non-main thread). The count of m_queuedSyncErrors is atomically increased to indicate the main thread that there are error messages pending to process. There's no need to forward the processing to the main thread, since the bus will eventually dispatch it asynchronously. (WebCore::MediaPlayerPrivateGStreamer::handleMessage): If the last error is being processed while in a "ignore errors" state, reset the state to false. Also, exit early if we were in "ignore errors" state. In any case (early or standard exit), decrease the m_queuedSyncErrors count atomically. (WebCore::MediaPlayerPrivateGStreamer::createGSTPlayBin): Process error messages synchronously with the handleSyncErrorMessage() handler. * Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h: didEnd() is now protected, so it can be called by the subclass. Added the m_queuedSyncErrors atomic counter to signal how many errors are waiting to be processed by the main thread. * Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp: (WebCore::MediaPlayerPrivateGStreamerMSE::setEosWithNoBuffers): Ignore the errors on all GStreamer versions by declaring m_ignoreErrors as true. If pending error messages are enqueued, report EOS by calling didEnd() (m_ignoreErrors will be reset when the last error is processed). If no pending error messages appeared, just reset the m_ignoreErrors flag here. Canonical link: https://commits.webkit.org/300605@main
1 parent 9202c6f commit 8774900

File tree

3 files changed

+85
-45
lines changed

3 files changed

+85
-45
lines changed

‎Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp‎

Lines changed: 65 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1862,6 +1862,13 @@ bool MediaPlayerPrivateGStreamer::handleNeedContextMessage(GstMessage* message)
18621862
return false;
18631863
}
18641864

1865+
void MediaPlayerPrivateGStreamer::handleSyncErrorMessage(GstMessage*)
1866+
{
1867+
GST_DEBUG_OBJECT(pipeline(), "Accounting queued sync error");
1868+
// This attribute is decremented later from handleMessage() in the main thread.
1869+
m_queuedSyncErrors.exchangeAdd(1);
1870+
}
1871+
18651872
// Returns the size of the video.
18661873
FloatSize MediaPlayerPrivateGStreamer::naturalSize() const
18671874
{
@@ -2016,51 +2023,67 @@ void MediaPlayerPrivateGStreamer::handleMessage(GstMessage* message)
20162023
GST_LOG_OBJECT(pipeline(), "Message %s received from element %s", GST_MESSAGE_TYPE_NAME(message), GST_MESSAGE_SRC_NAME(message));
20172024
switch (GST_MESSAGE_TYPE(message)) {
20182025
case GST_MESSAGE_ERROR:
2019-
gst_message_parse_error(message, &err.outPtr(), &debug.outPtr());
2020-
GST_ERROR_OBJECT(pipeline(), "%s (url=%s) (code=%d)", err->message, m_url.string().utf8().data(), err->code);
2026+
{
2027+
auto onScopeExit = makeScopeExit([this] {
2028+
GST_DEBUG_OBJECT(pipeline(), "Decreasing m_queuedSyncErrors");
2029+
m_queuedSyncErrors.exchangeSub(1);
2030+
});
2031+
2032+
gst_message_parse_error(message, &err.outPtr(), &debug.outPtr());
2033+
2034+
if (m_shouldResetPipeline || m_didErrorOccur || m_ignoreErrors) {
2035+
GST_WARNING_OBJECT(pipeline(), "Ignoring error: %s (url=%s) (code=%d)", err->message, m_url.string().utf8().data(), err->code);
2036+
// Deferred reset of m_ignoreErrors because there were queued sync errors not yet processed and
2037+
// we're processing the last one now.
2038+
if (m_ignoreErrors && m_queuedSyncErrors.loadFullyFenced() == 1) {
2039+
m_ignoreErrors = false;
2040+
GST_DEBUG_OBJECT(pipeline(), "Last queued error processed while on ignoring state. Not ignoring errors anymore.");
2041+
}
2042+
break;
2043+
}
20212044

2022-
if (m_shouldResetPipeline || m_didErrorOccur || m_ignoreErrors)
2023-
break;
2045+
GST_ERROR_OBJECT(pipeline(), "%s (url=%s) (code=%d)", err->message, m_url.string().utf8().data(), err->code);
20242046

2025-
m_errorMessage = String::fromLatin1(err->message);
2047+
m_errorMessage = String::fromLatin1(err->message);
20262048
#if ENABLE(MEDIA_TELEMETRY)
2027-
MediaTelemetryReport::singleton().reportPlaybackState(MediaTelemetryReport::AVPipelineState::PlaybackError,
2028-
m_errorMessage);
2049+
MediaTelemetryReport::singleton().reportPlaybackState(MediaTelemetryReport::AVPipelineState::PlaybackError,
2050+
m_errorMessage);
20292051
#endif
20302052

2031-
error = MediaPlayer::NetworkState::Empty;
2032-
if (g_error_matches(err.get(), GST_STREAM_ERROR, GST_STREAM_ERROR_CODEC_NOT_FOUND)
2033-
|| g_error_matches(err.get(), GST_STREAM_ERROR, GST_STREAM_ERROR_DECRYPT)
2034-
|| g_error_matches(err.get(), GST_STREAM_ERROR, GST_STREAM_ERROR_DECRYPT_NOKEY)
2035-
|| g_error_matches(err.get(), GST_STREAM_ERROR, GST_STREAM_ERROR_WRONG_TYPE)
2036-
|| g_error_matches(err.get(), GST_STREAM_ERROR, GST_STREAM_ERROR_FAILED)
2037-
|| g_error_matches(err.get(), GST_CORE_ERROR, GST_CORE_ERROR_MISSING_PLUGIN)
2038-
|| g_error_matches(err.get(), GST_CORE_ERROR, GST_CORE_ERROR_PAD)
2039-
|| g_error_matches(err.get(), GST_RESOURCE_ERROR, GST_RESOURCE_ERROR_NOT_FOUND))
2040-
error = MediaPlayer::NetworkState::FormatError;
2041-
else if (g_error_matches(err.get(), GST_STREAM_ERROR, GST_STREAM_ERROR_TYPE_NOT_FOUND)) {
2042-
GST_ERROR_OBJECT(pipeline(), "Decode error, let the Media element emit a stalled event.");
2043-
m_loadingStalled = true;
2044-
error = MediaPlayer::NetworkState::DecodeError;
2045-
attemptNextLocation = true;
2046-
} else if (err->domain == GST_STREAM_ERROR
2047-
|| g_error_matches(err.get(), GST_STREAM_ERROR, GST_STREAM_ERROR_DECODE)) {
2048-
error = MediaPlayer::NetworkState::DecodeError;
2049-
attemptNextLocation = true;
2050-
} else if (err->domain == GST_RESOURCE_ERROR)
2051-
error = MediaPlayer::NetworkState::NetworkError;
2052-
2053-
if (attemptNextLocation)
2054-
issueError = !loadNextLocation();
2055-
if (issueError) {
2056-
m_didErrorOccur = true;
2057-
if (m_networkState != error) {
2058-
m_networkState = error;
2059-
if (player)
2060-
player->networkStateChanged();
2053+
error = MediaPlayer::NetworkState::Empty;
2054+
if (g_error_matches(err.get(), GST_STREAM_ERROR, GST_STREAM_ERROR_CODEC_NOT_FOUND)
2055+
|| g_error_matches(err.get(), GST_STREAM_ERROR, GST_STREAM_ERROR_DECRYPT)
2056+
|| g_error_matches(err.get(), GST_STREAM_ERROR, GST_STREAM_ERROR_DECRYPT_NOKEY)
2057+
|| g_error_matches(err.get(), GST_STREAM_ERROR, GST_STREAM_ERROR_WRONG_TYPE)
2058+
|| g_error_matches(err.get(), GST_STREAM_ERROR, GST_STREAM_ERROR_FAILED)
2059+
|| g_error_matches(err.get(), GST_CORE_ERROR, GST_CORE_ERROR_MISSING_PLUGIN)
2060+
|| g_error_matches(err.get(), GST_CORE_ERROR, GST_CORE_ERROR_PAD)
2061+
|| g_error_matches(err.get(), GST_RESOURCE_ERROR, GST_RESOURCE_ERROR_NOT_FOUND))
2062+
error = MediaPlayer::NetworkState::FormatError;
2063+
else if (g_error_matches(err.get(), GST_STREAM_ERROR, GST_STREAM_ERROR_TYPE_NOT_FOUND)) {
2064+
GST_ERROR_OBJECT(pipeline(), "Decode error, let the Media element emit a stalled event.");
2065+
m_loadingStalled = true;
2066+
error = MediaPlayer::NetworkState::DecodeError;
2067+
attemptNextLocation = true;
2068+
} else if (err->domain == GST_STREAM_ERROR
2069+
|| g_error_matches(err.get(), GST_STREAM_ERROR, GST_STREAM_ERROR_DECODE)) {
2070+
error = MediaPlayer::NetworkState::DecodeError;
2071+
attemptNextLocation = true;
2072+
} else if (err->domain == GST_RESOURCE_ERROR)
2073+
error = MediaPlayer::NetworkState::NetworkError;
2074+
2075+
if (attemptNextLocation)
2076+
issueError = !loadNextLocation();
2077+
if (issueError) {
2078+
m_didErrorOccur = true;
2079+
if (m_networkState != error) {
2080+
m_networkState = error;
2081+
if (player)
2082+
player->networkStateChanged();
2083+
}
20612084
}
2085+
break;
20622086
}
2063-
break;
20642087
case GST_MESSAGE_WARNING:
20652088
gst_message_parse_warning(message, &err.outPtr(), &debug.outPtr());
20662089
GST_WARNING_OBJECT(pipeline(), "%s (url=%s) (code=%d)", err->message, m_url.string().utf8().data(), err->code);
@@ -3363,6 +3386,10 @@ void MediaPlayerPrivateGStreamer::createGSTPlayBin(const URL& url)
33633386
player->handleStreamCollectionMessage(message);
33643387
}), this);
33653388

3389+
g_signal_connect_swapped(bus.get(), "sync-message::error", G_CALLBACK(+[](MediaPlayerPrivateGStreamer* player, GstMessage* message) {
3390+
player->handleSyncErrorMessage(message);
3391+
}), this);
3392+
33663393
g_object_set(m_pipeline.get(), "mute", static_cast<gboolean>(player->muted()), nullptr);
33673394

33683395
// From GStreamer 1.22.0, uridecodebin3 is created in playbin3's _init(), so "element-setup" isn't called with it.

‎Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,7 @@ class MediaPlayerPrivateGStreamer
229229
bool handleNeedContextMessage(GstMessage*);
230230

231231
void handleStreamCollectionMessage(GstMessage*);
232+
void handleSyncErrorMessage(GstMessage*);
232233
void handleMessage(GstMessage*);
233234

234235
void triggerRepaint(GRefPtr<GstSample>&&);
@@ -407,6 +408,8 @@ class MediaPlayerPrivateGStreamer
407408
bool isPipelineWaitingPreroll(GstState current, GstState pending, GstStateChangeReturn) const;
408409
bool isPipelineWaitingPreroll() const;
409410

411+
void didEnd();
412+
410413
Ref<MainThreadNotifier<MainThreadNotification>> m_notifier;
411414
ThreadSafeWeakPtr<MediaPlayer> m_player;
412415
String m_referrer;
@@ -489,6 +492,7 @@ class MediaPlayerPrivateGStreamer
489492
std::optional<GstVideoDecoderPlatform> m_videoDecoderPlatform;
490493
GstSeekFlags m_seekFlags;
491494
bool m_ignoreErrors { false };
495+
Atomic<unsigned> m_queuedSyncErrors { 0 };
492496

493497
TrackIDHashMap<Ref<AudioTrackPrivateGStreamer>> m_audioTracks;
494498
TrackIDHashMap<Ref<VideoTrackPrivateGStreamer>> m_videoTracks;
@@ -541,7 +545,6 @@ class MediaPlayerPrivateGStreamer
541545
bool isMuted() const;
542546
void commitLoad();
543547
void fillTimerFired();
544-
void didEnd();
545548
void setPlaybackFlags(bool isMediaStream);
546549

547550
GstElement* createVideoSink();

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

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -535,15 +535,25 @@ void MediaPlayerPrivateGStreamerMSE::setEosWithNoBuffers(bool eosWithNoBuffers)
535535
m_isEosWithNoBuffers = eosWithNoBuffers;
536536
// Parsebin will trigger an error, instruct MediaPlayerPrivateGStreamer to ignore it.
537537
if (eosWithNoBuffers) {
538-
// On GStreamer 1.18.6, EOS with no buffers causes a parsebin error here:
538+
// On GStreamer, EOS with no buffers causes a parsebin error here:
539539
// https://github.com/GStreamer/gst-plugins-base/blob/1.18.6/gst/playback/gstparsebin.c#L3495
540-
// On GStreamer 1.24 (at least) that doesn't happen. Let's play safe and protect against the
541-
// error in lower versions.
542-
if (!webkitGstCheckVersion(1, 24, 0))
543-
m_ignoreErrors = true;
540+
541+
m_ignoreErrors = true;
542+
544543
GST_DEBUG_OBJECT(pipeline(), "EOS with no buffers, setting pipeline to READY state.");
545544
changePipelineState(GST_STATE_READY);
546-
if (!webkitGstCheckVersion(1, 24, 0))
545+
546+
unsigned errorCount = m_queuedSyncErrors.loadFullyFenced();
547+
if (errorCount) {
548+
GST_WARNING_OBJECT(pipeline(), "%u errors pending to process happened while processing EOS with"
549+
" no buffers and should be ignored, manually reporting EOS", errorCount);
550+
didEnd();
551+
// In the best case, m_ignoreErrors will be cleaned up (in the main thread) in MediaPlayerPrivateGStreamer::handleMessage()
552+
// in the future, after the last pending error is processed on the main thread. In the worst case, didEnd() will trigger
553+
// a pipeline teardown, which will trigger gst_bus_set_flushing() and the pending error message won't ever be processed
554+
// by the main thread. That's not a problem, since the player itself will be destructed and nobody will care about the
555+
// error count integrity anymore.
556+
} else
547557
m_ignoreErrors = false;
548558
}
549559
}

0 commit comments

Comments
 (0)