Bug 190076 – [MSE][GStreamer] Reset running time in PlaybackPipeline::flush()

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Alicia Boya García]

PlaybackPipeline::flush() is called when already enqueued frames are
appended again. This may be caused by a quality change or just a
redundant append. Either way, the pipeline has to be flushed and
playback begin again, but without changing the player position by
much.

There are two kinds of time to consider here: stream time (i.e. the
time of a frame as written in the file, e.g. a frame may have stream
time 0:01:00), and running time (i.e. how much time since playback
started should pass before the frame should be played, e.g. if we
started playing at 0:00:59 that same frame would have a running time
of just 1 second).

Notice how running time depends on where and when playback starts.
Running time can also be optionally resetted after a flush. (This is
indeed done currently by most demuxers after a seek.)

Instead of resetting running time, PlaybackPipeline used to modify the
first GstSegment emitted after the flush. A GstSegment declares the
mapping between stream time and running time for the following frames.
There, PlaybackPipeline used to set `base` (the running time at which
the segment starts) to the position reported by a position query
(which is stream time).

This, of course, only worked when playback (or the last seek) started
at stream time 0:00:00, since that's the only case where running time
equals stream time. In other cases delays as long as the difference
between these timelines would appear. This is demonstrated in the
attached test, where seeks and appends are made in such an order that
the difference is more than 5 minutes, making the playback stall for
>5 minutes before playing 1 second of audio.

This patch fixes the problem by resetting running time with the flush
and not modifying GstSegment.base anymore (it will be left as zero,
which is now correct since the running time has been reset).

[Alicia Boya García]

Created attachment 351083 [details]
Patch

[EWS Watchlist]

Comment on attachment 351083 [details]
Patch

Attachment 351083 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9382753

New failing tests:
media/media-source/only-bcp47-language-tags-accepted-as-valid.html

[EWS Watchlist]

Created attachment 351089 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 351083 [details]
Patch

Attachment 351083 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9382760

New failing tests:
media/media-source/only-bcp47-language-tags-accepted-as-valid.html

[EWS Watchlist]

Created attachment 351090 [details]
Archive of layout-test-results from ews105 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Alicia Boya García]

(In reply to Build Bot from comment #3)
> Created attachment 351089 [details]
> Archive of layout-test-results from ews103 for mac-sierra
> 
> The attached test failures were seen while running run-webkit-tests on the
> mac-ews.
> Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6

This is the failure... It's unfortunate that the expectations depend on line numbers. I assume somehow these numbers come from video-test.js, which I edited to add a utility function. I will move it to just the test.

--- /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/media/media-source/only-bcp47-language-tags-accepted-as-valid-expected.txt
+++ /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/media/media-source/only-bcp47-language-tags-accepted-as-valid-actual.txt
@@ -1,48 +1,48 @@
-CONSOLE MESSAGE: line 221: The language 'a' is not a valid BCP 47 language tag.
+CONSOLE MESSAGE: line 229: The language 'a' is not a valid BCP 47 language tag.
 CONSOLE MESSAGE: line 106: The language 'a' is not a valid BCP 47 language tag.
-CONSOLE MESSAGE: line 221: The language 'a' is not a valid BCP 47 language tag.
-CONSOLE MESSAGE: line 221: The language '1' is not a valid BCP 47 language tag.
+CONSOLE MESSAGE: line 229: The language 'a' is not a valid BCP 47 language tag.
+CONSOLE MESSAGE: line 229: The language '1' is not a valid BCP 47 language tag.
 CONSOLE MESSAGE: line 106: The language '1' is not a valid BCP 47 language tag.

[Alicia Boya García]

Created attachment 351094 [details]
Patch

[Philippe Normand]

Comment on attachment 351094 [details]
Patch

Very nice patch!

[WebKit Commit Bot]

Comment on attachment 351094 [details]
Patch

Clearing flags on attachment: 351094

Committed r236656: <https://trac.webkit.org/changeset/236656>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.