[webkit-changes] [WebKit/WebKit] 29bf1a: Cherry-pick 284072 at main (aededeb7ddd9). https://bu...

[Enrique Ocaña González]

  Branch: refs/heads/webkitglib/2.46
  Home:   https://github.com/WebKit/WebKit
  Commit: 29bf1a418f6324883025ac830607896d4e92ff57
      https://github.com/WebKit/WebKit/commit/29bf1a418f6324883025ac830607896d4e92ff57
  Author: Enrique Ocaña González <eocanha at igalia.com>
  Date:   2024-09-23 (Mon, 23 Sep 2024)

  Changed paths:
    M LayoutTests/platform/glib/TestExpectations
    M Source/WebCore/platform/SourcesGStreamer.txt
    M Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp
    M Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h
    M Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp
    M Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h
    M Source/WebCore/platform/gstreamer/GStreamerHolePunchQuirkBcmNexus.h
    M Source/WebCore/platform/gstreamer/GStreamerHolePunchQuirkFake.h
    M Source/WebCore/platform/gstreamer/GStreamerHolePunchQuirkRialto.h
    M Source/WebCore/platform/gstreamer/GStreamerHolePunchQuirkWesteros.h
    M Source/WebCore/platform/gstreamer/GStreamerQuirkAmLogic.h
    M Source/WebCore/platform/gstreamer/GStreamerQuirkBcmNexus.h
    M Source/WebCore/platform/gstreamer/GStreamerQuirkBroadcom.h
    A Source/WebCore/platform/gstreamer/GStreamerQuirkBroadcomBase.cpp
    A Source/WebCore/platform/gstreamer/GStreamerQuirkBroadcomBase.h
    M Source/WebCore/platform/gstreamer/GStreamerQuirkRealtek.h
    M Source/WebCore/platform/gstreamer/GStreamerQuirkRialto.h
    M Source/WebCore/platform/gstreamer/GStreamerQuirkWesteros.h
    M Source/WebCore/platform/gstreamer/GStreamerQuirks.cpp
    M Source/WebCore/platform/gstreamer/GStreamerQuirks.h

  Log Message:
  -----------
  Cherry-pick 284072 at main (aededeb7ddd9). https://bugs.webkit.org/show_bug.cgi?id=275683

    [GStreamer] Buffering hysteresis and buffering improvements for Broadcom
    https://bugs.webkit.org/show_bug.cgi?id=275683

    Reviewed by Xabier Rodriguez-Calvar.

    The usage of a fixed buffering level to evaluate the need for buffering
    results in a too unstable behaviour, with quick changes between states
    that may create stuttering.

    This commit adds hysteresis to the buffering level, with low and high
    watermark levels that trigger "buffering needed" (when below the low
    watermark), "buffering completed" (when above high watermark) and "same
    state as before" when the level is between both marks.

    In addition to that, the pipeline is automatically paused on a seek when
    working on stream mode (using GstQueue2 for partial in-memory buffering
    instead of using GstDownload for full on-disk download of the media).
    This is because, when seeking, rebuffering must occur from scratch to
    get the data for the new target position, and buffering level starts
    from 0% in that case, which means that buffering is needed and the
    pipeline must be paused. Both the m_isBuffering flag and
    m_bufferingPercentage are immediately forced to true and 0%, respectively.
    The previous values (see explanation below) are also set to the same
    values by using the new resetHistory parameter in updateBufferingStatus().
    This makes sure that the "end of buffering" condition can be properly
    detected.

    The buffering status and buffering level computation has been
    centralized in updateBufferingStatus(). Both the status and the level
    now store the last value, so it's possible to check if there's been any
    difference on them and to decide here on the hysteresis status
    evolution.

    updateStates() is where the consequences of buffering changes happen.
    This method now relies in the status and level updates explained in the
    previous paragraph. In the specific case of an async change, the current
    values for m_isBuffering and m_bufferingPercentage are ignored (previous
    ones are used). This is to be able to detect significant changes in the
    hysteresis (watermark level crossing) after the async change has
    completed. Otherwise, we would be detecting those crossings in a moment
    where nothing can be done (pipeline state changes are ignored can can't
    be enforced during async state changes).

    Some extra improvements have been added to adapt to the special
    circumstances of Broadcom devices, where the PlayPump Nexus component
    can store a lot of data which is hidden to the vanilla buffering
    algorithm. Now that data is had into account to compute the realistic
    buffering level. Also, more agile low/high watermarks of 20% and 80% are
    used for buffering in the case of playbin2 in stream buffering mode on
    Broadcom.

    20-80% levels for low/high watermark are now used on all devices
    (Broadcom and not Broadcom), since usage tests proved that this makes
    the buffering more agile also on Raspberry Pi, for instance.

    All these changes are compatible with the Quirks framework. A new
    needsBufferingPercentageCorrection() method has been added to enable the
    extra buffering logic (which corrects the queue2 buffering level with the
    buffer levels of PlayPump and multiqueue) on the platforms that use Nexus.
    All the code to implement that logic has been hidden in the
    GStreamerQuirkBroadcomBase class, from which both GStreamerQuirkBcmNexus
    and GStreamerQuirkBroadcom inherit.

    That code needs some elements that in principle would be very coupled with
    MediaPlayerPrivateGStreamer, such as vidfilter, queue2, multiqueue and a
    MovingAverage to smooth buffering levels. To avoid that coupling, they
    are stored in an opaque GStreamerQuirkState, stored by the player private
    but implemented by a GStreamerQuirkBroadcomBaseState subclass that only
    GStreamerQuirkBroadcomBase knows how to interpret (by casting after
    ensuring that the casting is legal).

    There's also the problem that multiple competing quirks can be installing
    at the same time, but all this buffering percentage correction code must
    only be run by one (only one!) of them. To allow the theoretical coexistence
    of multiple quirk state, each quirk instance has one associated state, and
    the player private maintains that correspondence using a HashMap indexed by
    quirk. To ensure that only one quirk is run at each time, the family of methods
    in GStreamerQuirksManager only forward the call to the first configured
    GStreamerQuirk subclass that declares the need to apply corrections. Any other
    configured subclass is ignored.

    Finally, some drive-by improvements are present in this commit. For instance,
    converting some const char* to ASCIILiteral here and there.

    * Source/WebCore/platform/SourcesGStreamer.txt: Added GStreamerQuirkBroadcomBase.
    * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h:
    (WebCore::gstStructureGetArray): Added.
    * Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:
    (WebCore::gstStructureGetArray): Method to get a GstArray GstValue stored into a GstStructure. Returns a Vector of the specified type, which includes basic types and "const GstStructure*".
    * Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
    (WebCore::MediaPlayerPrivateGStreamer::play): Make code compatible with InitiallyPaused.
    (WebCore::MediaPlayerPrivateGStreamer::doSeek): Pause pipeline and reset buffering level to 0% in case we're operating in stream mode.
    (WebCore::MediaPlayerPrivateGStreamer::queryBufferingPercentage): Get the buffering percentage from the most relevant element and delegate to the quirks.
    (WebCore::MediaPlayerPrivateGStreamer::fillTimerFired): Clarify that this method is only used in on-disk buffering. Rely on queryBufferingPercentage() now, instead of manually getting the value from here.
    (WebCore::MediaPlayerPrivateGStreamer::handleMessage): Delegate to the quirks to store/delete references to specific elements in the quirks state.
    (WebCore::MediaPlayerPrivateGStreamer::processBufferingStats): Delegate to the quierks to apply buffering percentage corrections on platforms that need them.
    (WebCore::MediaPlayerPrivateGStreamer::updateBufferingStatus): Update the values of m_wasBuffering, m_isBuffering, m_previousBufferingPercentage, m_bufferingPercentage and m_didDownloadFinish. The first 4 attributes should *ONLY* be modified from this method, in order to guarantee coherency. The only *EXCEPTION* to this rule is the delay which can happen in updateStates(). The new resetHistory parameter sets the value both in the previous value family of attributes and in the current value family of variables. Also, restart the fillTimer if download has restarted (it was never restarted before this patch!)
    (WebCore::MediaPlayerPrivateGStreamer::updateStates): Assume that m_wasBuffering, etc. have been precomputed by updateBufferingStatus() before updateStates() is called. In the case of an async state change, delay m_isBuffering and m_bufferingPercentage "to the past" (m_wasBuffering and m_previousBufferingPercentage), so that any change on them can be detected on the next iteration.
    * Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h: Added new m_wasBuffering and m_previousBufferingPercentage attributes to store previous values of m_isBuffering and m_bufferingPercentage and ease detection of changes. Added quirks state.
    (WebCore::MediaPlayerPrivateGStreamer::shouldDownload): Needed by quirks.
    (WebCore::MediaPlayerPrivateGStreamer::setQuirkState): Configure a GStreamerQuirkState for a specific GStreamerQuirk on the player private (each quirk has its own state, and in theory there can be many quirk-state stored).
    (WebCore::MediaPlayerPrivateGStreamer::quirkState): Get the quirks state associated to a specific quirk from the player private, or nullptr if there's no configured state for that quirk.
    * Source/WebCore/platform/gstreamer/GStreamerHolePunchQuirkBcmNexus.h: identifier() is now const ASCIILiteral.
    * Source/WebCore/platform/gstreamer/GStreamerHolePunchQuirkFake.h: Ditto.
    * Source/WebCore/platform/gstreamer/GStreamerHolePunchQuirkRialto.h: Ditto.
    * Source/WebCore/platform/gstreamer/GStreamerHolePunchQuirkWesteros.h: Ditto.
    * Source/WebCore/platform/gstreamer/GStreamerQuirkAmLogic.h: Ditto.
    * Source/WebCore/platform/gstreamer/GStreamerQuirkBcmNexus.h: Ditto. Inherit from base class.
    * Source/WebCore/platform/gstreamer/GStreamerQuirkBroadcom.h: Ditto. Inherit from base class.
    * Source/WebCore/platform/gstreamer/GStreamerQuirkBroadcomBase.cpp: Added. Base class for GStreamerQuirkBcmNexus and GStreamerQuirkBroadcom.
    (WebCore::GStreamerQuirkBroadcomBase::GStreamerQuirkBroadcomBase): Constructor.
    (WebCore::GStreamerQuirkBroadcomBase::queryBufferingPercentage const): Get the buffering percentage from Queue2 if present and the player shouldn't download (streaming mode). Returns the name of the element finally used to query the percentage to, or nullptr if none was queried.
    (WebCore::GStreamerQuirkBroadcomBase::correctBufferingPercentage const): On systems using PlayPump (eg: Broadcom Nexus), correct the buffering percentage with the data stored in multiqueue and PlayPump in order to make the buffering percentage more realistic. Uses the MovingAverage. Queries vidfilter and audfilter if needed, so it also works in audio or video-only pipelines.
    (WebCore::GStreamerQuirkBroadcomBase::resetBufferingPercentage const): Sets the percentage to a given value in the MovingAverage.
    (WebCore::GStreamerQuirkBroadcomBase::setupBufferingPercentageCorrection const): Gets/deletes references to vidfilter, multiqueue and queue2 in the quirks state.
    (WebCore::GStreamerQuirkBroadcomBase::ensureState const): Retrieves GStreamerQuirkState associated to this quirk, creating one and storing it on MediaPlayerPrivateGStreamer if not already present.
    * Source/WebCore/platform/gstreamer/GStreamerQuirkBroadcomBase.h: Added.
    (WebCore::GStreamerQuirkBroadcomBase::needsBufferingPercentageCorrection const): True for this class and derived ones.
    (WebCore::GStreamerQuirkBroadcomBase::MovingAverage::MovingAverage): Initialize an array of "length" number of elements, asserting if length is too big that the values would create overflow when computing the average.
    (WebCore::GStreamerQuirkBroadcomBase::MovingAverage::reset): Reset all the values to a given value (usually zero).
    (WebCore::GStreamerQuirkBroadcomBase::MovingAverage::accumulate): Insert a new value in the array and recompute the moving average.
    * Source/WebCore/platform/gstreamer/GStreamerQuirkRealtek.h: identifier() is now const ASCIILiteral.
    * Source/WebCore/platform/gstreamer/GStreamerQuirkRialto.h: Ditto.
    * Source/WebCore/platform/gstreamer/GStreamerQuirkWesteros.h: Ditto.
    * Source/WebCore/platform/gstreamer/GStreamerQuirks.cpp: Added new methods.
    (WebCore::GStreamerQuirksManager::GStreamerQuirksManager): Adapt to ASCIILiteral quirk identifier.
    (WebCore::GStreamerQuirksManager::createAudioSink): Ditto.
    (WebCore::GStreamerQuirksManager::createWebAudioSink): Ditto.
    (WebCore::GStreamerQuirksManager::createHolePunchVideoSink): Ditto.
    (WebCore::GStreamerQuirksManager::isHardwareAccelerated const): Ditto.
    (WebCore::GStreamerQuirksManager::audioVideoDecoderFactoryListType const): Ditto.
    (WebCore::GStreamerQuirksManager::getAdditionalPlaybinFlags const): Ditto.
    (WebCore::GStreamerQuirksManager::needsBufferingPercentageCorrection const): True if any configured quirk needs it. The first configured quirk that needs it will be the one responsible to fulfill all the requests for this family of methods. The other ones will be ignored, even if they also need percentage correction. This is to avoid more than one implementation doing the corrections, as we don't want them to accumulate and over-correct.
    (WebCore::GStreamerQuirksManager::queryBufferingPercentage const): Forward query request to the first configured quirk needing correction.
    (WebCore::GStreamerQuirksManager::correctBufferingPercentage const): Forward correction request to the first configured quirk needing correction.
    (WebCore::GStreamerQuirksManager::resetBufferingPercentage const): Ditto, but for reset.
    (WebCore::GStreamerQuirksManager::setupBufferingPercentageCorrection const): Ditto, but for setup.
    * Source/WebCore/platform/gstreamer/GStreamerQuirks.h:
    (WebCore::GStreamerQuirkBase::GStreamerQuirkState::GStreamerQuirkState): Constructor.
    (WebCore::GStreamerQuirk::needsBufferingPercentageCorrection const): Default empty implementation.
    (WebCore::GStreamerQuirk::queryBufferingPercentage const): Ditto.
    (WebCore::GStreamerQuirk::correctBufferingPercentage const): Ditto.
    (WebCore::GStreamerQuirk::resetBufferingPercentage const): Ditto.
    (WebCore::GStreamerQuirk::setupBufferingPercentageCorrection const): Ditto.
    * LayoutTests/platform/glib/TestExpectations: Reported new failing test as https://bugs.webkit.org/show_bug.cgi?id=280069, but IMHO it's not a regression. The test was passing before by sheer luck.

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

Canonical link: https://commits.webkit.org/282416.128@webkitglib/2.46



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications