[webkit-changes] [WebKit/WebKit] 080880: [MSE][GStreamer] Workaround basesink's lack of sup...

[Alicia Boya García]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 080880118a0748f903a6cdc0f69546f868533bf2
      https://github.com/WebKit/WebKit/commit/080880118a0748f903a6cdc0f69546f868533bf2
  Author: Alicia Boya Garcia <aboya at igalia.com>
  Date:   2023-05-24 (Wed, 24 May 2023)

  Changed paths:
    A LayoutTests/imported/w3c/web-platform-tests/media-source/mediasource-remove-preserves-currentTime-expected.txt
    A LayoutTests/imported/w3c/web-platform-tests/media-source/mediasource-remove-preserves-currentTime.html
    M Source/WebCore/platform/GStreamer.cmake
    R Source/WebCore/platform/graphics/gstreamer/AppSinkWorkaround.cpp
    R Source/WebCore/platform/graphics/gstreamer/AppSinkWorkaround.h
    M Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp
    M Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h
    M Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp
    A Source/WebCore/platform/graphics/gstreamer/GStreamerSinksWorkarounds.cpp
    A Source/WebCore/platform/graphics/gstreamer/GStreamerSinksWorkarounds.h
    M Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
    M Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp
    M Tools/Scripts/webkitpy/style/checker.py

  Log Message:
  -----------
  [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 https://github.com/WebKit/WebKit/pull/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