Clamp currentTime to buffered ranges for MSE GC

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

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 566 matching lines @@
 
     TRACE_EVENT_ASYNC_END0("media", "SourceBuffer::prepareAppend", this);
     return true;
 }
 
 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);
 }
 
 void SourceBuffer::appendBufferInternal(const unsigned char* data, unsigned size, ExceptionState& exceptionState)
 {
     TRACE_EVENT_ASYNC_BEGIN1("media", "SourceBuffer::appendBuffer", this, "size", size);
     // Section 3.2 appendBuffer()
     // https://dvcs.w3.org/hg/html-media/raw-file/default/media-source/media-source.html#widl-SourceBuffer-appendBuffer-void-ArrayBufferView-data
 
     // 1. Run the prepare append algorithm.
@@ skipping 260 matching lines @@
 {
     visitor->trace(m_source);
     visitor->trace(m_stream);
     visitor->trace(m_trackDefaults);
     visitor->trace(m_asyncEventQueue);
     RefCountedGarbageCollectedEventTargetWithInlineData<SourceBuffer>::trace(visitor);
     ActiveDOMObject::trace(visitor);
 }
 
 } // namespace blink