Clamp currentTime to buffered ranges for MSE GC

third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp

Index: third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp
diff --git a/third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp b/third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp
index 74465757dcef8d8363b23e01936e77b90286d736..c38af942142f22e7568350d7d13ff0aae1a1430b 100644
--- a/third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp
+++ b/third_party/WebKit/Source/modules/mediasource/SourceBuffer.cpp
@@ -584,6 +584,21 @@ bool SourceBuffer::evictCodedFrames(size_t newDataSize)
     ASSERT(m_source);
     ASSERT(m_source->mediaElement());
     double currentTime = m_source->mediaElement()->currentTime();
+
+    // Strictly speaking currentTime should always be in the buffered ranges,
+    // but media::Pipeline uses audio stream as the main time source. When audio

[wolenetz, 2015/12/02 23:11:00]

nit: (when there is an audio stream)

+    // and video streams have different buffered ranges, then the currentTime
+    // value might be slightly outside of the video stream buffered range. In
+    // those cases we need to clamp currentTime value to the current stream
+    // buffered ranges, to ensure the MSE garbage collection algorithm works
+    // correctly (see crbug.com/563292 for details).
+    Member<TimeRanges> bufferedRanges(buffered(ASSERT_NO_EXCEPTION));
+    if (!bufferedRanges->contain(currentTime)) {
+        double nearest = bufferedRanges->nearest(currentTime, currentTime);

[wolenetz, 2015/12/02 23:11:00]

Two strongly related comments:
1) This fix-up is specific to the Chromium (desktop) pipeline. Other content
embedders (Opera, etc) may have no need for this overhead here. ISTM this
clamping would be more appropriately done within Chromium SourceBufferStream.
2) TimeRanges::nearest(...,...) could return the *next* buffered range's start
time, rather than the previous buffered range's end time (which would introduce
issues due to potentially removing the currently playing video GOP, which could
be just a partial GOP so far). Specifically, the Chromium clamping scenario (to
workaround a currentTime that has slightly gone past the end of the
currently-playing video buffered range) needs to only be done to the end of the
currently playing buffered range, and be done iff a) no seek is in progress in
the demuxer, and b) no change to the end of the currently playing video buffered
range has occurred (such as an intervening explicit remove() operation).

IMHO, #2 further motivates #1. #1's also more easily testable at the
SourceBufferStream layer in a unit test, which is nice :)

+        WTF_LOG(Media, "SourceBuffer::evictCodedFrames %p currentTime %f outside of buffered ranges, adjusting currentTime to %f", this, currentTime, nearest);
+        currentTime = nearest;
+    }
+
     return m_webSourceBuffer->evictCodedFrames(currentTime, newDataSize);
 }