Title: [172584] trunk/Source/WebCore
Revision
172584
Author
[email protected]
Date
2014-08-14 08:13:03 -0700 (Thu, 14 Aug 2014)

Log Message

[MSE] Altering the quality of a YouTube video will cause the video to distort and display an error message
https://bugs.webkit.org/show_bug.cgi?id=135931

Reviewed by Eric Carlson.

When removing samples from the TrackBuffer's sample map, also remove those samples from the
TrackBuffer's decode queue. Otherwise, removed samples may persist in the decode queue and
either break sync-sample dependencies or cause decoding artifacts.

Pull the code which removes samples from a track buffer into its own utility function, and
use this function both from removeCodedFrames(), and also when samples are removed due to
overlapping appends in sourceBufferPrivateDidReceiveSample(). In order to reference
TrackBuffers outside of SourceBuffer (and in the static removeSamplesFromTrackBuffer()
function), make TrackBuffer a public forward declaration.

* Modules/mediasource/SourceBuffer.cpp:
(WebCore::removeSamplesFromTrackBuffer):
(WebCore::SourceBuffer::removeCodedFrames):
(WebCore::SourceBuffer::sourceBufferPrivateDidReceiveSample):
* Modules/mediasource/SourceBuffer.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (172583 => 172584)


--- trunk/Source/WebCore/ChangeLog	2014-08-14 13:10:37 UTC (rev 172583)
+++ trunk/Source/WebCore/ChangeLog	2014-08-14 15:13:03 UTC (rev 172584)
@@ -1,3 +1,26 @@
+2014-08-14  Jer Noble  <[email protected]>
+
+        [MSE] Altering the quality of a YouTube video will cause the video to distort and display an error message
+        https://bugs.webkit.org/show_bug.cgi?id=135931
+
+        Reviewed by Eric Carlson.
+
+        When removing samples from the TrackBuffer's sample map, also remove those samples from the
+        TrackBuffer's decode queue. Otherwise, removed samples may persist in the decode queue and
+        either break sync-sample dependencies or cause decoding artifacts.
+
+        Pull the code which removes samples from a track buffer into its own utility function, and
+        use this function both from removeCodedFrames(), and also when samples are removed due to
+        overlapping appends in sourceBufferPrivateDidReceiveSample(). In order to reference
+        TrackBuffers outside of SourceBuffer (and in the static removeSamplesFromTrackBuffer()
+        function), make TrackBuffer a public forward declaration.
+
+        * Modules/mediasource/SourceBuffer.cpp:
+        (WebCore::removeSamplesFromTrackBuffer):
+        (WebCore::SourceBuffer::removeCodedFrames):
+        (WebCore::SourceBuffer::sourceBufferPrivateDidReceiveSample):
+        * Modules/mediasource/SourceBuffer.h:
+
 2014-08-14  Zan Dobersek  <[email protected]>
 
         ImageBufferDataCairo.h is missing header guards

Modified: trunk/Source/WebCore/Modules/mediasource/SourceBuffer.cpp (172583 => 172584)


--- trunk/Source/WebCore/Modules/mediasource/SourceBuffer.cpp	2014-08-14 13:10:37 UTC (rev 172583)
+++ trunk/Source/WebCore/Modules/mediasource/SourceBuffer.cpp	2014-08-14 15:13:03 UTC (rev 172584)
@@ -527,6 +527,27 @@
     return a.second->decodeTime() < b.second->decodeTime();
 }
 
+static PassRefPtr<TimeRanges> removeSamplesFromTrackBuffer(const DecodeOrderSampleMap::MapType& samples, SourceBuffer::TrackBuffer& trackBuffer)
+{
+    RefPtr<TimeRanges> erasedRanges = TimeRanges::create();
+    MediaTime microsecond(1, 1000000);
+    for (auto sampleIt : samples) {
+        const DecodeOrderSampleMap::KeyType& decodeKey = sampleIt.first;
+        RefPtr<MediaSample>& sample = sampleIt.second;
+
+        // Remove the erased samples from the TrackBuffer sample map.
+        trackBuffer.samples.removeSample(sample.get());
+
+        // Also remove the erased samples from the TrackBuffer decodeQueue.
+        trackBuffer.decodeQueue.erase(decodeKey);
+
+        double startTime = sample->presentationTime().toDouble();
+        double endTime = startTime + (sample->duration() + microsecond).toDouble();
+        erasedRanges->add(startTime, endTime);
+    }
+    return erasedRanges.release();
+}
+
 void SourceBuffer::removeCodedFrames(const MediaTime& start, const MediaTime& end)
 {
     LOG(MediaSource, "SourceBuffer::removeCodedFrames(%p) - start(%s), end(%s)", this, toString(start).utf8().data(), toString(end).utf8().data());
@@ -569,17 +590,8 @@
         DecodeOrderSampleMap::iterator removeDecodeStart = trackBuffer.samples.decodeOrder().findSampleWithDecodeKey(decodeKey);
 
         DecodeOrderSampleMap::MapType erasedSamples(removeDecodeStart, removeDecodeEnd);
+        RefPtr<TimeRanges> erasedRanges = removeSamplesFromTrackBuffer(erasedSamples, trackBuffer);
 
-        RefPtr<TimeRanges> erasedRanges = TimeRanges::create();
-        MediaTime microsecond(1, 1000000);
-        for (auto erasedIt : erasedSamples) {
-            RefPtr<MediaSample>& sample = erasedIt.second;
-            trackBuffer.samples.removeSample(sample.get());
-            double startTime = sample->presentationTime().toDouble();
-            double endTime = startTime + (sample->duration() + microsecond).toDouble();
-            erasedRanges->add(startTime, endTime);
-        }
-
         // Only force the TrackBuffer to re-enqueue if the removed ranges overlap with enqueued and possibly
         // not yet displayed samples.
         PlatformTimeRanges possiblyEnqueuedRanges(currentMediaTime, trackBuffer.lastEnqueuedPresentationTime);
@@ -1186,15 +1198,11 @@
             auto nextSyncIter = trackBuffer.samples.decodeOrder().findSyncSampleAfterDecodeIterator(lastDecodeIter);
             dependentSamples.insert(firstDecodeIter, nextSyncIter);
 
-
-            RefPtr<TimeRanges> erasedRanges = TimeRanges::create();
-            for (auto& samplePair : dependentSamples) {
-                MediaTime startTime = samplePair.second->presentationTime();
-                MediaTime endTime = startTime + samplePair.second->duration() + microsecond;
-                erasedRanges->add(startTime.toDouble(), endTime.toDouble());
+            RefPtr<TimeRanges> erasedRanges = removeSamplesFromTrackBuffer(dependentSamples, trackBuffer);
+#if !LOG_DISABLED
+            for (auto& samplePair : dependentSamples)
                 LOG(MediaSource, "SourceBuffer::sourceBufferPrivateDidReceiveSample(%p) - removing sample(%s)", this, toString(*samplePair.second).utf8().data());
-                trackBuffer.samples.removeSample(samplePair.second.get());
-            }
+#endif
 
             // Only force the TrackBuffer to re-enqueue if the removed ranges overlap with enqueued and possibly
             // not yet displayed samples.

Modified: trunk/Source/WebCore/Modules/mediasource/SourceBuffer.h (172583 => 172584)


--- trunk/Source/WebCore/Modules/mediasource/SourceBuffer.h	2014-08-14 13:10:37 UTC (rev 172583)
+++ trunk/Source/WebCore/Modules/mediasource/SourceBuffer.h	2014-08-14 15:13:03 UTC (rev 172584)
@@ -103,6 +103,8 @@
     using RefCounted<SourceBuffer>::ref;
     using RefCounted<SourceBuffer>::deref;
 
+    struct TrackBuffer;
+
 protected:
     // EventTarget interface
     virtual void refEventTarget() override { ref(); }
@@ -149,7 +151,6 @@
 
     bool validateInitializationSegment(const InitializationSegment&);
 
-    struct TrackBuffer;
     void reenqueueMediaForTime(TrackBuffer&, AtomicString trackID, const MediaTime&);
     void provideMediaData(TrackBuffer&, AtomicString trackID);
     void didDropSample();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to