Fix seeking back in the new MSE GC algorithm

media/filters/source_buffer_stream.cc

Index: media/filters/source_buffer_stream.cc
diff --git a/media/filters/source_buffer_stream.cc b/media/filters/source_buffer_stream.cc
index e17a4f1c91952b64cf280a9777404e4adeec71d2..b47b09cba24715c1fa445163e1ea15e79a7fb1a7 100644
--- a/media/filters/source_buffer_stream.cc
+++ b/media/filters/source_buffer_stream.cc
@@ -659,22 +659,54 @@ bool SourceBufferStream::GarbageCollectIfNeeded(DecodeTimestamp media_time,
     }
   }
 
-  // Try removing data from the front of the SourceBuffer up to |media_time|
-  // position.
-  if (bytes_freed < bytes_to_free) {
-    size_t front = FreeBuffers(bytes_to_free - bytes_freed, media_time, false);
-    DVLOG(3) << __FUNCTION__ << " Removed " << front << " bytes from the front"
-             << " ranges_=" << RangesToString(ranges_);
-    bytes_freed += front;
-  }
+  // If |media_time| is earlier than any of the buffered data, that means we are

[wolenetz, 2015/09/16 19:25:27]

I wonder if we should make this even more aggressive:

Suppose buffered:
[0,10] [20,30] [40,50]

Last append: 50
Seek==media time: 15
GC requested such that bytes to free implies maybe just 5 seconds buffered time
could remain after successful eviction (for simplicity; I know we talk bytes,
but in this example I'm talking seconds).

In this PS, [20,30] and [40,50] would survive because the 'else' clause won't
collect between media_time and last_append (where media_time < last_append), and
GC would fail. Yet I think we could be greedy here instead.

WDYT? Proposed change to l.669 in this PS would be something like
if (!ranges_.empty() &&
    (media time is not in any buffered range)) { be greedy }else{ be
conservative }

+  // seeking back in the stream. In that case we should delete as much data as
+  // possible from the back of the buffered range without evicting the most
+  // recently appended chunks, and then, if that's not enough, we can delete
+  // from the front of the buffered ranges greedily, i.e. without trying to save
+  // the previous media playback position, since the player is going to append
+  // new data to the new seek target next.
+  if (!ranges_.empty() &&
+      media_time < ranges_.front()->GetStartTimestamp()) {
+    DVLOG(3) << __FUNCTION__ << " Detected seeking backwards (media_time="
+             << media_time.InSecondsF() << "; buffered_start="
+             << ranges_.front()->GetStartTimestamp().InSecondsF()
+             << "), removing data from the back of buffered ranges first";
+    if (bytes_freed < bytes_to_free) {
+      // Remove data from the back, until we reach most recently appended GOP.
+      size_t back = FreeBuffers(bytes_to_free - bytes_freed, media_time, true);
+      DVLOG(3) << __FUNCTION__ << " Removed " << back << " bytes from the back"
+               << " ranges_=" << RangesToString(ranges_);
+      bytes_freed += back;
+    }
 
-  // Try removing data from the back of the SourceBuffer, until we reach the
-  // most recent append position.
-  if (bytes_freed < bytes_to_free) {
-    size_t back = FreeBuffers(bytes_to_free - bytes_freed, media_time, true);
-    DVLOG(3) << __FUNCTION__ << " Removed " << back << " bytes from the back"
-             << " ranges_=" << RangesToString(ranges_);
-    bytes_freed += back;
+    if (bytes_freed < bytes_to_free) {
+      // Remove data from the front, removing as much as necessary.
+      size_t back = FreeBuffers(bytes_to_free - bytes_freed,
+                                ranges_.back()->GetEndTimestamp(), false);
+      DVLOG(3) << __FUNCTION__ << " Removed " << back << " bytes from the back"
+               << " ranges_=" << RangesToString(ranges_);
+      bytes_freed += back;
+    }
+  } else {
+    // Try removing data from the front of the SourceBuffer up to |media_time|
+    // position.
+    if (bytes_freed < bytes_to_free) {
+      size_t front =
+          FreeBuffers(bytes_to_free - bytes_freed, media_time, false);
+      DVLOG(3) << __FUNCTION__ << " Removed " << front << " bytes from the"
+               << " front. ranges_=" << RangesToString(ranges_);
+      bytes_freed += front;
+    }
+
+    // Try removing data from the back of the SourceBuffer, until we reach the
+    // most recent append position.
+    if (bytes_freed < bytes_to_free) {
+      size_t back = FreeBuffers(bytes_to_free - bytes_freed, media_time, true);
+      DVLOG(3) << __FUNCTION__ << " Removed " << back << " bytes from the back."
+               << " ranges_=" << RangesToString(ranges_);
+      bytes_freed += back;
+    }
   }
 
   DVLOG(2) << __FUNCTION__ << " " << GetStreamTypeName() << ": After GC"