- Revision
- 236696
- Author
- ape...@igalia.com
- Date
- 2018-10-01 16:12:55 -0700 (Mon, 01 Oct 2018)
Log Message
[MSE][GStreamer] Reset running time in PlaybackPipeline::flush()
https://bugs.webkit.org/show_bug.cgi?id=190076
Reviewed by Philippe Normand.
Source/WebCore:
Test: media/media-source/media-source-seek-redundant-append.html
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).
* platform/graphics/gstreamer/mse/PlaybackPipeline.cpp:
(WebCore::PlaybackPipeline::flush):
(WebCore::segmentFixerProbe): Deleted.
LayoutTests:
A test where a seek is followed by a redundant append is added. This
test timed out in the GStreamer MSE implementation before the
accompanying patch fixed it.
The MIME type declared in test-48khz-manifest.json has also been
changed, from non-standard `audio/x-m4a` to `audio/mp4;
codecs="mp4a.40.2"`, as implied by the MSE specs. This should not
affect other tests because no other tests were reading this type
string before.
* media/media-source/content/test-48khz-manifest.json:
* media/media-source/media-source-seek-redundant-append-expected.txt: Added.
* media/media-source/media-source-seek-redundant-append.html: Added.
* media/video-test.js:
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-2.22/LayoutTests/ChangeLog (236695 => 236696)
--- releases/WebKitGTK/webkit-2.22/LayoutTests/ChangeLog 2018-10-01 23:12:44 UTC (rev 236695)
+++ releases/WebKitGTK/webkit-2.22/LayoutTests/ChangeLog 2018-10-01 23:12:55 UTC (rev 236696)
@@ -1,5 +1,27 @@
2018-10-01 Alicia Boya García <ab...@igalia.com>
+ [MSE][GStreamer] Reset running time in PlaybackPipeline::flush()
+ https://bugs.webkit.org/show_bug.cgi?id=190076
+
+ Reviewed by Philippe Normand.
+
+ A test where a seek is followed by a redundant append is added. This
+ test timed out in the GStreamer MSE implementation before the
+ accompanying patch fixed it.
+
+ The MIME type declared in test-48khz-manifest.json has also been
+ changed, from non-standard `audio/x-m4a` to `audio/mp4;
+ codecs="mp4a.40.2"`, as implied by the MSE specs. This should not
+ affect other tests because no other tests were reading this type
+ string before.
+
+ * media/media-source/content/test-48khz-manifest.json:
+ * media/media-source/media-source-seek-redundant-append-expected.txt: Added.
+ * media/media-source/media-source-seek-redundant-append.html: Added.
+ * media/video-test.js:
+
+2018-10-01 Alicia Boya García <ab...@igalia.com>
+
[MSE] Use tolerance when growing the coded frame group
https://bugs.webkit.org/show_bug.cgi?id=190085
Modified: releases/WebKitGTK/webkit-2.22/LayoutTests/media/media-source/content/test-48khz-manifest.json (236695 => 236696)
--- releases/WebKitGTK/webkit-2.22/LayoutTests/media/media-source/content/test-48khz-manifest.json 2018-10-01 23:12:44 UTC (rev 236695)
+++ releases/WebKitGTK/webkit-2.22/LayoutTests/media/media-source/content/test-48khz-manifest.json 2018-10-01 23:12:55 UTC (rev 236696)
@@ -1,6 +1,6 @@
{
"url": "content/test-48kHz.m4a",
- "type": "audio/x-m4a",
+ "type": "audio/mp4; codecs=\"mp4a.40.2\"",
"init": { "offset": 0, "size": 624 },
"duration": 10.0906,
"media": [
Added: releases/WebKitGTK/webkit-2.22/LayoutTests/media/media-source/media-source-seek-redundant-append-expected.txt (0 => 236696)
--- releases/WebKitGTK/webkit-2.22/LayoutTests/media/media-source/media-source-seek-redundant-append-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.22/LayoutTests/media/media-source/media-source-seek-redundant-append-expected.txt 2018-10-01 23:12:55 UTC (rev 236696)
@@ -0,0 +1,36 @@
+
+RUN(video.src = ""
+EVENT(sourceopen)
+RUN(sourceBuffer = source.addSourceBuffer(loader.type()))
+RUN(sourceBuffer.appendBuffer(loader.initSegment()))
+EVENT(update)
+RUN(source.duration = 310)
+RUN(sourceBuffer.timestampOffset = 300)
+RUN(video.play())
+START seekAndAppend: time=300, segments=[0, 3)
+RUN(sourceBuffer.appendBuffer(loader.mediaSegment(0)))
+Successful append
+RUN(sourceBuffer.appendBuffer(loader.mediaSegment(1)))
+Successful append
+RUN(sourceBuffer.appendBuffer(loader.mediaSegment(2)))
+Successful append
+SUCCESS seekAndAppend: time=300, segments=[0, 3)
+START waitForVideoToReach(300.2)
+SUCCESS waitForVideoToReach(300.2)
+START seekAndAppend: time=309.2, segments=[9, 10)
+RUN(sourceBuffer.appendBuffer(loader.mediaSegment(9)))
+Successful append
+SUCCESS seekAndAppend: time=309.2, segments=[9, 10)
+START waitForVideoToReach(309.7)
+SUCCESS waitForVideoToReach(309.7)
+RUN(video.currentTime = 308.9)
+START seekAndAppend: time=308.9, segments=[8, 10)
+RUN(sourceBuffer.appendBuffer(loader.mediaSegment(8)))
+Successful append
+RUN(sourceBuffer.appendBuffer(loader.mediaSegment(9)))
+Successful append
+SUCCESS seekAndAppend: time=308.9, segments=[8, 10)
+RUN(source.endOfStream())
+EVENT(ended)
+END OF TEST
+
Added: releases/WebKitGTK/webkit-2.22/LayoutTests/media/media-source/media-source-seek-redundant-append.html (0 => 236696)
--- releases/WebKitGTK/webkit-2.22/LayoutTests/media/media-source/media-source-seek-redundant-append.html (rev 0)
+++ releases/WebKitGTK/webkit-2.22/LayoutTests/media/media-source/media-source-seek-redundant-append.html 2018-10-01 23:12:55 UTC (rev 236696)
@@ -0,0 +1,95 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <title>media-source-seek-redundant-append</title>
+ <script src=""
+ <script src=""
+ <script>
+ var loader;
+ var source;
+ var sourceBuffer;
+
+ function waitSilentlyFor(element, type) {
+ return new Promise(resolve => {
+ element.addEventListener(type, event => {
+ resolve(event);
+ }, { once: true });
+ });
+ }
+
+ function waitForVideoToReach(time) {
+ return new Promise(resolve => {
+ consoleWrite(`START waitForVideoToReach(${time})`);
+ function timeupdateHandler() {
+ if (video.currentTime >= time) {
+ video.removeEventListener("timeupdate", timeupdateHandler);
+ consoleWrite(`SUCCESS waitForVideoToReach(${time})`);
+ resolve();
+ }
+ }
+ video.addEventListener("timeupdate", timeupdateHandler);
+ });
+ }
+
+ async function seekAndAppend(time, startSegment, endSegment) {
+ // Note: "seeked" and "update" can occur in any order because we may have noticed enough data to complete the
+ // seek before the whole append has been demuxed (demuxing occurs in a background thread and the main thread
+ // may be notified of new samples before the demuxer has finished the append).
+ // Therefore, we need to wait silently for these events in order for the test to have a reliable (non-flaky)
+ // output.
+
+ consoleWrite(`START seekAndAppend: time=${time}, segments=[${startSegment}, ${endSegment})`);
+ const seeked = waitSilentlyFor(mediaElement, "seeked");
+ mediaElement.currentTime = time;
+ for (let segmentNumber = startSegment; segmentNumber < endSegment; segmentNumber++) {
+ run(`sourceBuffer.appendBuffer(loader.mediaSegment(${segmentNumber}))`);
+ await waitSilentlyFor(sourceBuffer, 'update');
+ consoleWrite("Successful append");
+ }
+ await seeked;
+ consoleWrite(`SUCCESS seekAndAppend: time=${time}, segments=[${startSegment}, ${endSegment})`);
+ }
+
+ window.addEventListener('load', () => {
+ findMediaElement();
+
+ loader = new MediaSourceLoader('content/test-48khz-manifest.json');
+ loader._onload_ = async () => {
+ source = new MediaSource();
+ run('video.src = ""
+ await waitFor(source, 'sourceopen');
+
+ const offset = 300;
+
+ run('sourceBuffer = source.addSourceBuffer(loader.type())');
+ run('sourceBuffer.appendBuffer(loader.initSegment())');
+ await waitFor(sourceBuffer, 'update');
+ run(`source.duration = ${offset + 10}`);
+ run(`sourceBuffer.timestampOffset = ${offset}`);
+
+ run('video.play()');
+ await seekAndAppend(offset, 0, 3); // Segment A
+ await waitForVideoToReach(offset + 0.2);
+
+ await seekAndAppend(offset + 9.2, 9, 10); // Segment C
+ await waitForVideoToReach(offset + 9.7);
+
+ run(`video.currentTime = ${offset + 8.9}`);
+ // Appending C again is redundant, but it should work.
+ await seekAndAppend(offset + 8.9, 8, 10); // Segment B+C
+ run('source.endOfStream()');
+ // After this point playback should finish in ~1.1 seconds (well under the timeout), not get stuck.
+ await waitFor(video, 'ended');
+
+ endTest();
+ };
+ loader._onerror_ = () => {
+ failTest('Media data loading failed');
+ };
+ });
+ </script>
+</head>
+<body>
+ <video controls></video>
+</body>
+</html>
\ No newline at end of file
Modified: releases/WebKitGTK/webkit-2.22/Source/WebCore/ChangeLog (236695 => 236696)
--- releases/WebKitGTK/webkit-2.22/Source/WebCore/ChangeLog 2018-10-01 23:12:44 UTC (rev 236695)
+++ releases/WebKitGTK/webkit-2.22/Source/WebCore/ChangeLog 2018-10-01 23:12:55 UTC (rev 236696)
@@ -1,5 +1,54 @@
2018-10-01 Alicia Boya García <ab...@igalia.com>
+ [MSE][GStreamer] Reset running time in PlaybackPipeline::flush()
+ https://bugs.webkit.org/show_bug.cgi?id=190076
+
+ Reviewed by Philippe Normand.
+
+ Test: media/media-source/media-source-seek-redundant-append.html
+
+ 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).
+
+ * platform/graphics/gstreamer/mse/PlaybackPipeline.cpp:
+ (WebCore::PlaybackPipeline::flush):
+ (WebCore::segmentFixerProbe): Deleted.
+
+2018-10-01 Alicia Boya García <ab...@igalia.com>
+
[MSE] Use tolerance when growing the coded frame group
https://bugs.webkit.org/show_bug.cgi?id=190085
Modified: releases/WebKitGTK/webkit-2.22/Source/WebCore/platform/graphics/gstreamer/mse/PlaybackPipeline.cpp (236695 => 236696)
--- releases/WebKitGTK/webkit-2.22/Source/WebCore/platform/graphics/gstreamer/mse/PlaybackPipeline.cpp 2018-10-01 23:12:44 UTC (rev 236695)
+++ releases/WebKitGTK/webkit-2.22/Source/WebCore/platform/graphics/gstreamer/mse/PlaybackPipeline.cpp 2018-10-01 23:12:55 UTC (rev 236696)
@@ -289,25 +289,6 @@
gst_app_src_end_of_stream(appsrc);
}
-GstPadProbeReturn segmentFixerProbe(GstPad*, GstPadProbeInfo* info, gpointer)
-{
- GstEvent* event = GST_EVENT(info->data);
-
- if (GST_EVENT_TYPE(event) != GST_EVENT_SEGMENT)
- return GST_PAD_PROBE_OK;
-
- GstSegment* segment = nullptr;
- gst_event_parse_segment(event, const_cast<const GstSegment**>(&segment));
-
- GST_TRACE("Fixed segment base time from %" GST_TIME_FORMAT " to %" GST_TIME_FORMAT,
- GST_TIME_ARGS(segment->base), GST_TIME_ARGS(segment->start));
-
- segment->base = segment->start;
- segment->flags = static_cast<GstSegmentFlags>(0);
-
- return GST_PAD_PROBE_REMOVE;
-}
-
void PlaybackPipeline::flush(AtomicString trackId)
{
ASSERT(WTF::isMainThread());
@@ -358,7 +339,7 @@
return;
}
- if (!gst_element_send_event(GST_ELEMENT(appsrc), gst_event_new_flush_stop(false))) {
+ if (!gst_element_send_event(GST_ELEMENT(appsrc), gst_event_new_flush_stop(true))) {
GST_WARNING("Failed to send flush-stop event for trackId=%s", trackId.string().utf8().data());
return;
}
@@ -371,11 +352,6 @@
gst_segment_do_seek(segment.get(), rate, GST_FORMAT_TIME, GST_SEEK_FLAG_NONE,
GST_SEEK_TYPE_SET, position, GST_SEEK_TYPE_SET, stop, nullptr);
- GRefPtr<GstPad> sinkPad = adoptGRef(gst_element_get_static_pad(appsrc, "src"));
- GRefPtr<GstPad> srcPad = sinkPad ? adoptGRef(gst_pad_get_peer(sinkPad.get())) : nullptr;
- if (srcPad)
- gst_pad_add_probe(srcPad.get(), GST_PAD_PROBE_TYPE_EVENT_DOWNSTREAM, segmentFixerProbe, nullptr, nullptr);
-
GST_TRACE("Sending new seamless segment: [%" GST_TIME_FORMAT ", %" GST_TIME_FORMAT "], rate: %f",
GST_TIME_ARGS(segment->start), GST_TIME_ARGS(segment->stop), segment->rate);
