REGRESSION (206001): Scrubbed video on Youtube stops playing
authorjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 Sep 2016 15:28:42 +0000 (15:28 +0000)
committerjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 Sep 2016 15:28:42 +0000 (15:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=162481
<rdar://problem/28436707>

Reviewed by Eric Carlson.

Source/WebCore:

Test: media/media-source/media-source-seek-back.html

When re-enqueing samples after a seek, modified the algorithm for finding the first sample
to re-enqueue. Instead of finding a sample which contained the requested presentation time,
we searched for the first sample on-or-after the presentation time. This meant that if the
last buffered sample in that range started before the seek time, and ended after, it would
get skipped during re-enquing, and a far, far future sample would be enqueued instead. Now
revert to the old behavior (find the sample containing the requested time), and only adopt
the new behavior (find the next sample on-or-after the requested time) if the first failed.
In addition, bail out if the second check resulted in a sample whose presentation time was
greater than a "fudge factor" away from the requested time.

To test this behavior, add a new method onto Internals that returns a list of the enqueued
samples from a SourceBuffer.

* Modules/mediasource/SourceBuffer.cpp:
(WebCore::SourceBuffer::reenqueueMediaForTime):
(WebCore::SourceBuffer::enqueuedSamplesForTrackID):
* Modules/mediasource/SourceBuffer.h:
* platform/mock/mediasource/MockMediaSourcePrivate.cpp:
(WebCore::MockMediaSourcePrivate::seekToTime):
* platform/mock/mediasource/MockSourceBufferPrivate.cpp:
(WebCore::MockSourceBufferPrivate::enqueuedSamplesForTrackID):
(WebCore::MockSourceBufferPrivate::enqueueSample):
* platform/mock/mediasource/MockSourceBufferPrivate.h:
* testing/Internals.cpp:
(WebCore::Internals::enqueuedSamplesForTrackID):
* testing/Internals.h:
* testing/Internals.idl:

LayoutTests:

* media/media-source/media-source-seek-back-expected.txt: Added.
* media/media-source/media-source-seek-back.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@206300 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/media/media-source/media-source-seek-back-expected.txt [new file with mode: 0644]
LayoutTests/media/media-source/media-source-seek-back.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediasource/SourceBuffer.cpp
Source/WebCore/Modules/mediasource/SourceBuffer.h
Source/WebCore/platform/mock/mediasource/MockMediaSourcePrivate.cpp
Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.cpp
Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.h
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl

index 14471df..fad1c90 100644 (file)
@@ -1,3 +1,14 @@
+2016-09-23  Jer Noble  <jer.noble@apple.com>
+
+        REGRESSION (206001): Scrubbed video on Youtube stops playing
+        https://bugs.webkit.org/show_bug.cgi?id=162481
+        <rdar://problem/28436707>
+
+        Reviewed by Eric Carlson.
+
+        * media/media-source/media-source-seek-back-expected.txt: Added.
+        * media/media-source/media-source-seek-back.html: Added.
+
 2016-09-23  Per Arne Vollan  <pvollan@apple.com>
 
         Skip failing html/syntax web platform test on Windows.
diff --git a/LayoutTests/media/media-source/media-source-seek-back-expected.txt b/LayoutTests/media/media-source/media-source-seek-back-expected.txt
new file mode 100644 (file)
index 0000000..aa4d986
--- /dev/null
@@ -0,0 +1,17 @@
+
+EVENT(sourceopen)
+RUN(sourceBuffer = source.addSourceBuffer("video/mock; codecs=mock"))
+RUN(sourceBuffer.appendBuffer(initSegment))
+EVENT(updateend)
+Append a sample from 10 -> 11
+RUN(sourceBuffer.appendBuffer(samples))
+EVENT(updateend)
+RUN(video.currentTime = 0.5)
+EVENT(seeking)
+Append a sample from 0 -> 1
+RUN(sourceBuffer.appendBuffer(samples))
+EVENT(seeked)
+EXPECTED (enqueuedSamples.length == '1') OK
+{PTS({0/1000, 0.000000}), DTS({0/1000, 0.000000}), duration({1000/1000, 1.000000}), flags(1), generation(0)}
+END OF TEST
+
diff --git a/LayoutTests/media/media-source/media-source-seek-back.html b/LayoutTests/media/media-source/media-source-seek-back.html
new file mode 100644 (file)
index 0000000..999a4ee
--- /dev/null
@@ -0,0 +1,74 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>mock-media-source</title>
+    <script src="mock-media-source.js"></script>
+    <script src="../video-test.js"></script>
+    <script>
+    var source;
+    var sourceBuffer;
+    var initSegment;
+    var samples;
+
+    if (window.internals)
+        internals.initializeMockMediaSource();
+
+    function runTest()
+    {
+        findMediaElement();
+
+        source = new MediaSource();
+        waitForEventOn(source, 'sourceopen', sourceOpen);
+        var videoSource = document.createElement('source');
+        videoSource.type = 'video/mock; codecs=mock';
+        videoSource.src = URL.createObjectURL(source);
+        video.appendChild(videoSource);
+    }
+
+    function sourceOpen()
+    {
+        run('sourceBuffer = source.addSourceBuffer("video/mock; codecs=mock")');
+        waitForEventOn(sourceBuffer, 'updateend', loadSamples, false, true);
+        initSegment = makeAInit(8, [makeATrack(1, 'mock', TRACK_KIND.VIDEO)]);
+        run('sourceBuffer.appendBuffer(initSegment)');
+    }
+
+    function loadSamples()
+    {
+        samples = concatenateSamples([
+            makeASample(10, 10, 1, 1, SAMPLE_FLAG.SYNC),
+        ]);
+        consoleWrite('Append a sample from 10 -> 11')
+        waitForEventOn(sourceBuffer, 'updateend', seek, false, true);
+        run('sourceBuffer.appendBuffer(samples)');
+    }
+
+    function seek()
+    {
+        waitForEvent('seeking', seeking);
+        run('video.currentTime = 0.5');
+    }
+
+    function seeking()
+    {
+        samples = concatenateSamples([
+            makeASample(0, 0, 1, 1, SAMPLE_FLAG.SYNC),
+        ]);
+        consoleWrite('Append a sample from 0 -> 1')
+        waitForEvent('seeked', seeked);
+        run('sourceBuffer.appendBuffer(samples)');
+    }
+
+    function seeked()
+    {
+        enqueuedSamples = internals.enqueuedSamplesForTrackID(sourceBuffer, 1);
+        testExpected("enqueuedSamples.length", 1);
+        enqueuedSamples.forEach(consoleWrite);
+        endTest();
+    }
+    </script>
+</head>
+<body onload="runTest()">
+    <video></video>
+</body>
+</html>
index ee294a1..20e6d9a 100644 (file)
@@ -1,3 +1,41 @@
+2016-09-23  Jer Noble  <jer.noble@apple.com>
+
+        REGRESSION (206001): Scrubbed video on Youtube stops playing
+        https://bugs.webkit.org/show_bug.cgi?id=162481
+        <rdar://problem/28436707>
+
+        Reviewed by Eric Carlson.
+
+        Test: media/media-source/media-source-seek-back.html
+
+        When re-enqueing samples after a seek, modified the algorithm for finding the first sample
+        to re-enqueue. Instead of finding a sample which contained the requested presentation time,
+        we searched for the first sample on-or-after the presentation time. This meant that if the
+        last buffered sample in that range started before the seek time, and ended after, it would
+        get skipped during re-enquing, and a far, far future sample would be enqueued instead. Now
+        revert to the old behavior (find the sample containing the requested time), and only adopt
+        the new behavior (find the next sample on-or-after the requested time) if the first failed.
+        In addition, bail out if the second check resulted in a sample whose presentation time was
+        greater than a "fudge factor" away from the requested time.
+
+        To test this behavior, add a new method onto Internals that returns a list of the enqueued
+        samples from a SourceBuffer.
+
+        * Modules/mediasource/SourceBuffer.cpp:
+        (WebCore::SourceBuffer::reenqueueMediaForTime):
+        (WebCore::SourceBuffer::enqueuedSamplesForTrackID):
+        * Modules/mediasource/SourceBuffer.h:
+        * platform/mock/mediasource/MockMediaSourcePrivate.cpp:
+        (WebCore::MockMediaSourcePrivate::seekToTime):
+        * platform/mock/mediasource/MockSourceBufferPrivate.cpp:
+        (WebCore::MockSourceBufferPrivate::enqueuedSamplesForTrackID):
+        (WebCore::MockSourceBufferPrivate::enqueueSample):
+        * platform/mock/mediasource/MockSourceBufferPrivate.h:
+        * testing/Internals.cpp:
+        (WebCore::Internals::enqueuedSamplesForTrackID):
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2016-09-22  Zalan Bujtas  <zalan@apple.com>
 
         Replace redundant prepareForDestruction() call with RELEASE_ASSERT in Document::removedLastRef.
index 812b5c6..7ae67fb 100644 (file)
@@ -1856,9 +1856,13 @@ void SourceBuffer::provideMediaData(TrackBuffer& trackBuffer, AtomicString track
 void SourceBuffer::reenqueueMediaForTime(TrackBuffer& trackBuffer, AtomicString trackID, const MediaTime& time)
 {
     // Find the sample which contains the current presentation time.
-    auto currentSamplePTSIterator = trackBuffer.samples.presentationOrder().findSampleOnOrAfterPresentationTime(time);
+    auto currentSamplePTSIterator = trackBuffer.samples.presentationOrder().findSampleContainingPresentationTime(time);
 
-    if (currentSamplePTSIterator == trackBuffer.samples.presentationOrder().end()) {
+    if (currentSamplePTSIterator == trackBuffer.samples.presentationOrder().end())
+        currentSamplePTSIterator = trackBuffer.samples.presentationOrder().findSampleOnOrAfterPresentationTime(time);
+
+    if (currentSamplePTSIterator == trackBuffer.samples.presentationOrder().end()
+        || (currentSamplePTSIterator->first - time) > MediaSource::currentTimeFudgeFactor()) {
         trackBuffer.decodeQueue.clear();
         m_private->flushAndEnqueueNonDisplayingSamples(Vector<RefPtr<MediaSample>>(), trackID);
         return;
@@ -2028,6 +2032,11 @@ Vector<String> SourceBuffer::bufferedSamplesForTrackID(const AtomicString& track
     return sampleDescriptions;
 }
 
+Vector<String> SourceBuffer::enqueuedSamplesForTrackID(const AtomicString& trackID)
+{
+    return m_private->enqueuedSamplesForTrackID(trackID);
+}
+
 Document& SourceBuffer::document() const
 {
     ASSERT(scriptExecutionContext());
index c9fad6e..14b2c33 100644 (file)
@@ -195,6 +195,7 @@ private:
     // Internals
     friend class Internals;
     WEBCORE_EXPORT Vector<String> bufferedSamplesForTrackID(const AtomicString&);
+    WEBCORE_EXPORT Vector<String> enqueuedSamplesForTrackID(const AtomicString&);
 
     Ref<SourceBufferPrivate> m_private;
     MediaSource* m_source;
index 45aecfa..a6bc458 100644 (file)
@@ -167,8 +167,7 @@ bool MockMediaSourcePrivate::hasVideo() const
 
 void MockMediaSourcePrivate::seekToTime(const MediaTime& time)
 {
-    for (auto it = m_activeSourceBuffers.begin(), end = m_activeSourceBuffers.end(); it != end; ++it)
-        (*it)->seekToTime(time);
+    m_client->seekToTime(time);
 }
 
 MediaTime MockMediaSourcePrivate::seekToTime(const MediaTime& targetTime, const MediaTime& negativeThreshold, const MediaTime& positiveThreshold)
index 5740d7f..b760d2a 100644 (file)
@@ -225,8 +225,14 @@ void MockSourceBufferPrivate::setActive(bool isActive)
         m_mediaSource->sourceBufferPrivateDidChangeActiveState(this, isActive);
 }
 
-void MockSourceBufferPrivate::enqueueSample(PassRefPtr<MediaSample> sample, AtomicString)
+Vector<String> MockSourceBufferPrivate::enqueuedSamplesForTrackID(AtomicString)
 {
+    return m_enqueuedSamples;
+}
+
+void MockSourceBufferPrivate::enqueueSample(PassRefPtr<MediaSample> prpSample, AtomicString)
+{
+    RefPtr<MediaSample> sample = prpSample;
     if (!m_mediaSource || !sample)
         return;
 
@@ -245,6 +251,8 @@ void MockSourceBufferPrivate::enqueueSample(PassRefPtr<MediaSample> sample, Atom
         m_mediaSource->incrementDroppedFrames();
     if (box->isDelayed())
         m_mediaSource->incrementTotalFrameDelayBy(MediaTime(1, 1));
+
+    m_enqueuedSamples.append(toString(sample));
 }
 
 bool MockSourceBufferPrivate::hasVideo() const
index 1feecb1..ae0c66c 100644 (file)
@@ -67,16 +67,19 @@ private:
     MediaPlayer::ReadyState readyState() const override;
     void setReadyState(MediaPlayer::ReadyState) override;
 
-    void flushAndEnqueueNonDisplayingSamples(Vector<RefPtr<MediaSample>>, AtomicString) override { }
+    void flushAndEnqueueNonDisplayingSamples(Vector<RefPtr<MediaSample>>, AtomicString) override { m_enqueuedSamples.clear(); }
     void enqueueSample(PassRefPtr<MediaSample>, AtomicString) override;
     bool isReadyForMoreSamples(AtomicString) override { return true; }
     void setActive(bool) override;
 
+    Vector<String> enqueuedSamplesForTrackID(AtomicString) override;
+
     void didReceiveInitializationSegment(const MockInitializationBox&);
     void didReceiveSample(const MockSampleBox&);
 
     MockMediaSourcePrivate* m_mediaSource;
     SourceBufferPrivateClient* m_client;
+    Vector<String> m_enqueuedSamples;
 
     Vector<char> m_inputBuffer;
 };
index 662be1a..cad6b40 100644 (file)
@@ -2764,6 +2764,11 @@ Vector<String> Internals::bufferedSamplesForTrackID(SourceBuffer& buffer, const
     return buffer.bufferedSamplesForTrackID(trackID);
 }
     
+Vector<String> Internals::enqueuedSamplesForTrackID(SourceBuffer& buffer, const AtomicString& trackID)
+{
+    return buffer.enqueuedSamplesForTrackID(trackID);
+}
+
 void Internals::setShouldGenerateTimestamps(SourceBuffer& buffer, bool flag)
 {
     buffer.setShouldGenerateTimestamps(flag);
index c3ed9c7..00c2137 100644 (file)
@@ -401,6 +401,7 @@ public:
 #if ENABLE(MEDIA_SOURCE)
     WEBCORE_TESTSUPPORT_EXPORT void initializeMockMediaSource();
     Vector<String> bufferedSamplesForTrackID(SourceBuffer&, const AtomicString&);
+    Vector<String> enqueuedSamplesForTrackID(SourceBuffer&, const AtomicString&);
     void setShouldGenerateTimestamps(SourceBuffer&, bool);
 #endif
 
index 34e3507..7a11e70 100644 (file)
@@ -392,6 +392,7 @@ enum UserInterfaceLayoutDirection {
 
     [Conditional=MEDIA_SOURCE] void initializeMockMediaSource();
     [Conditional=MEDIA_SOURCE] sequence<DOMString> bufferedSamplesForTrackID(SourceBuffer buffer, DOMString trackID);
+    [Conditional=MEDIA_SOURCE] sequence<DOMString> enqueuedSamplesForTrackID(SourceBuffer buffer, DOMString trackID);
     [Conditional=MEDIA_SOURCE] void setShouldGenerateTimestamps(SourceBuffer buffer, boolean flag);
 
     [Conditional=VIDEO, RaisesException] void beginMediaSessionInterruption(DOMString interruptionType);