Fix seeking back in the new MSE GC algorithm

media/filters/source_buffer_stream.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/filters/source_buffer_stream.h"
 
 #include <algorithm>
 #include <map>
 #include <sstream>
 
@@ skipping 641 matching lines @@
     // If the last append happened before the current playback position
     // |media_time|, then JS player is probably preparing to seek back and we
     // should try to preserve all most recently appended data (which is in
     // range_for_next_append_) from being removed by GC (see crbug.com/440173)
     if (range_for_next_append_ != ranges_.end()) {
       DCHECK((*range_for_next_append_)->GetStartTimestamp() <= media_time);
       media_time = (*range_for_next_append_)->GetStartTimestamp();
     }
   }
 
-  // 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"
            << " bytes_to_free=" << bytes_to_free
            << " bytes_freed=" << bytes_freed
            << " ranges_=" << RangesToString(ranges_);
 
   return bytes_freed >= bytes_to_free;
 }
 
@@ skipping 1043 matching lines @@
     return false;
 
   DCHECK_NE(have_splice_buffers, have_preroll_buffer);
   splice_buffers_index_ = 0;
   pending_buffer_.swap(*out_buffer);
   pending_buffers_complete_ = false;
   return true;
 }
 
 }  // namespace media