Bug 224235

Summary: [GStreamer] Only seek to change the rate un updatePlaybackRate() when needed
Product: WebKit Reporter: Enrique Ocaña <eocanha>
Component: WebKitGTKAssignee: Enrique Ocaña <eocanha>
Status: RESOLVED FIXED    
Severity: Normal CC: aboya, bugs-noreply, calvaris, cgarcia, eric.carlson, ews-watchlist, glenn, gustavo, jer.noble, menard, philipj, pnormand, sergio, vjaquez
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Enrique Ocaña 2021-04-06 06:45:10 PDT
A seek is used by MediaPlayerPrivateGStreamer to update the playback rate used in the pipeline. There's a special case when the rate is 0.0. This actually means "paused" and, when set, the pipeline is changed to PAUSED and no updatePlaybackRate() call is made because of an early return (and therefore, no seek). When the rate is restored to a non-zero value, the pipeline is set again to PLAYING, but an unconditional call to updatePlaybackRate() is done, leading to a seek.

It should be possible to call updatePlaybackRate() only if the non-zero rate changed with respect to the original one, instead of always doing it. This would avoid unneeded seeks, which are complex operations that can trigger collateral issues such as the one addressed in https://bugs.webkit.org/show_bug.cgi?id=223742.
Comment 1 Enrique Ocaña 2021-04-06 08:45:17 PDT
Created attachment 425284 [details]
Patch
Comment 2 Alicia Boya García 2021-04-06 09:16:51 PDT
Comment on attachment 425284 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=425284&action=review

LGTM

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:539
> +            m_lastPlaybackRate = m_playbackRate;

Add a documentation comment in the header file explaining this means the last playback rate sent through a GStreamer seek.
Comment 3 Enrique Ocaña 2021-04-06 09:39:18 PDT
Created attachment 425288 [details]
Patch
Comment 4 EWS 2021-04-07 02:10:16 PDT
Committed r275596: <https://commits.webkit.org/r275596>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 425288 [details].