[MSE][GStreamer] Workaround basesink's lack of support for position queries between a non-resetting flush and a pre-roll

https://bugs.webkit.org/show_bug.cgi?id=248683

Reviewed by Xabier Rodriguez-Calvar.

Workaround for upstream GStreamer patch projected to land in GStreamer
1.24.0: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/3471
basesink: Support position queries after non-resetting flushes

Briefly explained, before the fix GStreamer sinks will fail to answer
position queries after a FLUSH_STOP(reset_time=FALSE) until they receive
a new segment, even though the sinks in this situation will preserve the
previous segment.

This is particularly a problem for streams with quality changes, such as
MSE, but potentially also native adaptive streaming implementations, as
the currentTime will suddenly reset to zero, and unlike seeks, quality
change flushes generally occur automatically without user intervention.

The bug means that currentTime can reset to zero seemingly randomly when
a stream is flushed for reason other than a seek, notably in MSE, and
most notably after WebKit/WebKit#3966 finally
gets landed. This has been seen in failures for the test
media/media-source/media-source-seek-unbuffered.html -- although that
test has several simultaneous unrelated issues that need work on.

To demonstrate the issue in a more isolated case, a new test case has
been added:

imported/w3c/web-platform-tests/media-source/mediasource-remove-preserves-currentTime.html

As of writing there are no plans currently for the patch to be
officially backported into the GStreamer 1.22.x series due to concerns
over making changes on critical paths of critical elements.

Therefore, in order to not suffer this problem in WebKit with any
currently released versions of GStreamer, we need a workaround.

This is a difficult bug to work around: you can add probes that capture
queries flowing through a pipeline, but you cannot add a probe that
captures a query that is sent directly to an element without going
through any pads (e.g. with gst_element_query_position()). Unfortunately
this is more often than not how position queries are made.

Since we cannot monkey-patch or probe the position queries for basesink
directly, the workaround is instead based on analysis and manipulation
of the basesink internals so that gst_base_sink_get_position() won't
fail in the desired case, by tweaking basesink->have_newsegment inside a
probe for the FLUSH_STOP event.

I went through lots of different approaches for this workaround over
weeks, and the workaround method proposed was the final one that was
able to fix the issue without introducing any regressions in
LayoutTests.

This is the second workaround for an issue in GStreamer sinks that takes
advantage of re-registering elements. To keep things clean, both
workarounds have been grouped in a new file
GStreamerSinksWorkarounds.cpp, and refactored into classes with similar
interfaces.

Also, now the workarounds can be enabled and disabled independently with
new environment variables:

* WEBKIT_GST_WORKAROUND_APP_SINK_FLUSH_CAPS
* WEBKIT_GST_WORKAROUND_BASE_SINK_POSITION_FLUSH

Allowed values are: ForceEnable, ForceDisabled and UseIfNeeded
(case-insensitive). The latter enables each workaround only for
GStreamer versions known to need it.

UseIfNeeded is the default value for both workarounds, but this can be
changed at build time if desired by setting the following macros:

* WEBKIT_GST_WORKAROUND_APP_SINK_FLUSH_CAPS_DEFAULT_MODE
* WEBKIT_GST_WORKAROUND_BASE_SINK_POSITION_FLUSH_DEFAULT_MODE

* LayoutTests/imported/w3c/web-platform-tests/media-source/mediasource-remove-preserves-currentTime.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/media-source/mediasource-remove-preserves-currentTime-expected.txt: Added.
* Source/WebCore/platform/GStreamer.cmake:
* Source/WebCore/platform/graphics/gstreamer/AppSinkWorkaround.cpp: Removed.
* Source/WebCore/platform/graphics/gstreamer/AppSinkWorkaround.h: Removed.
* Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp:
(WTF::adoptGRef):
(WTF::refGPtr<GstBaseSink>):
(WTF::derefGPtr<GstBaseSink>):
* Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h:
* Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:
(WebCore::ensureGStreamerInitialized):
* Source/WebCore/platform/graphics/gstreamer/GStreamerSinksWorkarounds.cpp: Added.
(WebCore::getWorkAroundModeFromEnvironment):
(WebCore::BaseSinkPositionFlushWorkaroundProbe::isNeeded):
(WebCore::BaseSinkPositionFlushWorkaroundProbe::installIfNeeded):
(WebCore::BaseSinkPositionFlushWorkaroundProbe::checkIsNeeded):
(WebCore::BaseSinkPositionFlushWorkaroundProbe::initializeIsNeeded):
(WebCore::BaseSinkPositionFlushWorkaroundProbe::flushIsNonResetting):
(WebCore::BaseSinkPositionFlushWorkaroundProbe::probe):
(WebCore::BaseSinkPositionFlushWorkaroundProbe::newUserData):
(WebCore::BaseSinkPositionFlushWorkaroundProbe::deleteUserData):
(WebCore::AppSinkFlushCapsWorkaroundProbe::isNeeded):
(WebCore::AppSinkFlushCapsWorkaroundProbe::installIfNeeded):
(WebCore::AppSinkFlushCapsWorkaroundProbe::checkIsNeeded):
(WebCore::AppSinkFlushCapsWorkaroundProbe::initializeIsNeeded):
(WebCore::AppSinkFlushCapsWorkaroundProbe::probe):
(WebCore::AppSinkFlushCapsWorkaroundProbe::newUserData):
(WebCore::AppSinkFlushCapsWorkaroundProbe::deleteUserData):
(WebCore::registerAppsinkWithWorkaroundsIfNeededCallOnce):
(WebCore::registerAppsinkWithWorkaroundsIfNeeded):
(WebCore::installBaseSinkPositionFlushWorkaroundIfNeeded):
(webkitAppSinkWithWorkAroundsConstructed):
(webkit_app_sink_with_workarounds_class_init):
* Source/WebCore/platform/graphics/gstreamer/GStreamerSinksWorkarounds.h: Added.
* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::gstreamerPositionFromSinks const):
* Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp:
(webKitMediaSrcStreamFlush):
* Tools/Scripts/webkitpy/style/checker.py:

Canonical link: https://commits.webkit.org/264476@main

Author: ntrrgc

LayoutTests/imported/w3c/web-platform-tests/media-source/mediasource-remove-preserves-currentTime-expected.txt

@@ -0,0 +1,3 @@
+
+PASS Test removing coded frames during playback doesn't regress currentTime
+

LayoutTests/imported/w3c/web-platform-tests/media-source/mediasource-remove-preserves-currentTime.html

@@ -0,0 +1,69 @@
+<!DOCTYPE html>
+<!-- Copyright © 2023 Igalia S.L. -->
+<!-- Copyright © 2023 Metrological -->
+<html>
+    <head>
+        <meta name="charset" content="UTF-8">
+        <title>Test currentTime doesn't regress after erasing the current playback position.</title>
+        <meta name="timeout" content="long">
+        <link rel="author" title="Alicia Boya García" href="mailto:aboya@igalia.com">
+        <script src="/resources/testharness.js"></script>
+        <script src="/resources/testharnessreport.js"></script>
+        <script src="mediasource-util.js"></script>
+    </head>
+    <body>
+        <div id="log"></div>
+        <script>
+
+function waitForCurrentTimeToReach(test, mediaElement, targetTime, callback)
+{
+    let lastTime = mediaElement.currentTime;
+    const timeUpdateHandler = test.step_func(function _timeUpdateHandler() {
+        let newTime = mediaElement.currentTime;
+        assert_greater_than_equal(newTime, lastTime);
+        lastTime = newTime;
+        if (newTime >= targetTime) {
+            mediaElement.removeEventListener('timeupdate', timeUpdateHandler);
+            callback();
+        }
+    });
+    mediaElement.addEventListener('timeupdate', timeUpdateHandler);
+}
+
+function testCurrentTimeDoesNotRegress(test, mediaElement, initialTime, callback)
+{
+    let numChecksLeft = 10; // 0.5 seconds
+    let lastTime = initialTime;
+    const intervalId = setInterval(test.step_func(function() {
+        let newTime = mediaElement.currentTime;
+        assert_greater_than_equal(newTime, lastTime);
+        lastTime = newTime;
+        if (numChecksLeft-- <= 0) {
+            clearInterval(intervalId);
+            callback();
+        }
+    }, 50));
+}
+
+mediasource_testafterdataloaded(function(test, mediaElement, mediaSource, segmentInfo, sourceBuffer, mediaData)
+{
+    test.expectEvent(sourceBuffer, 'updateend', 'sourceBuffer');
+    sourceBuffer.appendBuffer(mediaData)
+
+    test.waitForExpectedEvents(function() {
+        assert_equals(mediaElement.currentTime, 0);
+        test.expectEvent(mediaElement, 'playing', 'Playing triggered');
+        mediaElement.play();
+        waitForCurrentTimeToReach(test, mediaElement, 0.1, test.step_func(whenPlaybackIsMoving));
+    })
+
+    function whenPlaybackIsMoving()
+    {
+        testCurrentTimeDoesNotRegress(test, mediaElement, mediaElement.currentTime, () => test.done())
+        sourceBuffer.remove(0, 10);
+    }
+},  "Test removing coded frames during playback doesn't regress currentTime");
+
+        </script>
+    </body>
+</html>

Source/WebCore/platform/GStreamer.cmake

@@ -26,7 +26,7 @@ if (ENABLE_VIDEO OR ENABLE_WEB_AUDIO)
 
         Modules/webaudio/MediaStreamAudioSourceGStreamer.cpp
 
-        platform/graphics/gstreamer/AppSinkWorkaround.cpp
+        platform/graphics/gstreamer/GStreamerSinksWorkarounds.cpp
         platform/graphics/gstreamer/AudioTrackPrivateGStreamer.cpp
         platform/graphics/gstreamer/DMABufVideoSinkGStreamer.cpp
         platform/graphics/gstreamer/GLVideoSinkGStreamer.cpp

Source/WebCore/platform/graphics/gstreamer/AppSinkWorkaround.cpp

@@ -506,6 +506,26 @@ template <> void derefGPtr<WebKitVideoSink>(WebKitVideoSink* ptr)
         gst_object_unref(GST_OBJECT(ptr));
 }
 
+template <> GRefPtr<GstBaseSink> adoptGRef(GstBaseSink* ptr)
+{
+    ASSERT(!ptr || !g_object_is_floating(ptr));
+    return GRefPtr<GstBaseSink>(ptr, GRefPtrAdopt);
+}
+
+template <> GstBaseSink* refGPtr<GstBaseSink>(GstBaseSink* ptr)
+{
+    if (ptr)
+        gst_object_ref_sink(GST_OBJECT(ptr));
+
+    return ptr;
+}
+
+template <> void derefGPtr<GstBaseSink>(GstBaseSink* ptr)
+{
+    if (ptr)
+        gst_object_unref(GST_OBJECT(ptr));
+}
+
 template <> GRefPtr<WebKitWebSrc> adoptGRef(WebKitWebSrc* ptr)
 {
     ASSERT(!ptr || !g_object_is_floating(ptr));

Source/WebCore/platform/graphics/gstreamer/AppSinkWorkaround.h

@@ -27,6 +27,7 @@
 
 typedef struct _WebKitVideoSink WebKitVideoSink;
 struct WebKitWebSrc;
+typedef struct _GstBaseSink GstBaseSink;
 
 #if USE(GSTREAMER_GL)
 typedef struct _GstGLDisplay GstGLDisplay;
@@ -59,6 +60,10 @@ template<> GRefPtr<GstElement> adoptGRef(GstElement* ptr);
 template<> GstElement* refGPtr<GstElement>(GstElement* ptr);
 template<> void derefGPtr<GstElement>(GstElement* ptr);
 
+template<> GRefPtr<GstBaseSink> adoptGRef(GstBaseSink* ptr);
+template<> GstBaseSink* refGPtr<GstBaseSink>(GstBaseSink* ptr);
+template<> void derefGPtr<GstBaseSink>(GstBaseSink* ptr);
+
 template<> GRefPtr<GstPad> adoptGRef(GstPad* ptr);
 template<> GstPad* refGPtr<GstPad>(GstPad* ptr);
 template<> void derefGPtr<GstPad>(GstPad* ptr);

Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp

@@ -23,12 +23,12 @@
 
 #if USE(GSTREAMER)
 
-#include "AppSinkWorkaround.h"
 #include "ApplicationGLib.h"
 #include "DMABufVideoSinkGStreamer.h"
 #include "GLVideoSinkGStreamer.h"
 #include "GStreamerAudioMixer.h"
 #include "GStreamerRegistryScanner.h"
+#include "GStreamerSinksWorkarounds.h"
 #include "GUniquePtrGStreamer.h"
 #include "GstAllocatorFastMalloc.h"
 #include "IntSize.h"
@@ -307,8 +307,7 @@ bool ensureGStreamerInitialized()
             gst_mpegts_initialize();
 #endif
 
-        // Workaround for https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/2413
-        registerAppsinkWorkaroundIfNeeded();
+        registerAppsinkWithWorkaroundsIfNeeded();
 #endif
     });
     return isGStreamerInitialized;