Fix MSE GC, make it less aggressive, more spec-compliant

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 383 matching lines @@
   // are appended to the range covered by |track_buffer_|.
   if (!track_buffer_.empty()) {
     DecodeTimestamp keyframe_timestamp =
         FindKeyframeAfterTimestamp(track_buffer_.front()->GetDecodeTimestamp());
     if (keyframe_timestamp != kNoDecodeTimestamp())
       PruneTrackBuffer(keyframe_timestamp);
   }
 
   SetSelectedRangeIfNeeded(next_buffer_timestamp);
 
-  GarbageCollectIfNeeded();
-
   DVLOG(1) << __FUNCTION__ << " " << GetStreamTypeName()
            << ": done. ranges_=" << RangesToString(ranges_);
   DCHECK(IsRangeListSorted(ranges_));
   DCHECK(OnlySelectedRangeIsSeeked());
   return true;
 }
 
 void SourceBufferStream::Remove(base::TimeDelta start, base::TimeDelta end,
                                 base::TimeDelta duration) {
   DVLOG(1) << __FUNCTION__ << " " << GetStreamTypeName()
@@ skipping 228 matching lines @@
   }
 }
 
 void SourceBufferStream::SetConfigIds(const BufferQueue& buffers) {
   for (BufferQueue::const_iterator itr = buffers.begin();
        itr != buffers.end(); ++itr) {
     (*itr)->SetConfigId(append_config_index_);
   }
 }
 
-void SourceBufferStream::GarbageCollectIfNeeded() {
+bool SourceBufferStream::GarbageCollectIfNeeded(DecodeTimestamp media_time,
+                                                size_t newDataSize) {
+  DCHECK(media_time != kNoDecodeTimestamp());
   // Compute size of |ranges_|.
   size_t ranges_size = 0;
   for (RangeList::iterator itr = ranges_.begin(); itr != ranges_.end(); ++itr)

[wolenetz, 2015/08/20 23:23:37]

instead, use GetBufferedSize() ?

[servolk, 2015/08/21 01:32:08]

Done.

     ranges_size += (*itr)->size_in_bytes();
 
   // Return if we're under or at the memory limit.
-  if (ranges_size <= memory_limit_)
-    return;
+  if (ranges_size + newDataSize <= memory_limit_)

[wolenetz, 2015/08/20 23:23:37]

this can overflow

[servolk, 2015/08/21 01:32:08]

Done, added a sanity and overflow check

+    return true;
 
-  size_t bytes_to_free = ranges_size - memory_limit_;
+  size_t bytes_to_free = ranges_size + newDataSize - memory_limit_;

[wolenetz, 2015/08/20 23:23:37]

If newDataSize > memory_limit, trivially return false ? Or remove everything and
then return false ?

[servolk, 2015/08/21 01:32:08]

Done.

 
   DVLOG(2) << __FUNCTION__ << " " << GetStreamTypeName() << ": Before GC"
+           << " media_time=" << media_time.InSecondsF()
+           << " ranges_=" << RangesToString(ranges_)
            << " ranges_size=" << ranges_size
-           << " ranges_=" << RangesToString(ranges_)
-           << " memory_limit_=" << memory_limit_;
+           << " newDataSize=" << newDataSize
+           << " memory_limit_=" << memory_limit_
+           << " last_appended_buffer_timestamp_="
+           << last_appended_buffer_timestamp_.InSecondsF();
 
-  // Begin deleting after the last appended buffer.
-  size_t bytes_freed = FreeBuffersAfterLastAppended(bytes_to_free);
+  size_t bytes_freed = 0;
 
-  // Begin deleting from the front.
-  if (bytes_freed < bytes_to_free)
-    bytes_freed += FreeBuffers(bytes_to_free - bytes_freed, false);
+  // If last appended buffer position was earlier than the current playback time
+  // then try deleting data between last append and current media_time.
+  if (last_appended_buffer_timestamp_ != kNoDecodeTimestamp() &&
+      last_appended_buffer_timestamp_ < media_time) {
+    size_t between = FreeBuffersAfterLastAppended(bytes_to_free, media_time);
+    DVLOG(3) << __FUNCTION__ << " FreeBuffersAfterLastAppended "
+             << " released " << between << " bytes"
+             << " ranges_=" << RangesToString(ranges_);
+    bytes_freed += between;
 
-  // Begin deleting from the back.
-  if (bytes_freed < bytes_to_free)
-    bytes_freed += FreeBuffers(bytes_to_free - bytes_freed, true);
+    // 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;
+  }
+
+  // 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;
 }
 
 size_t SourceBufferStream::FreeBuffersAfterLastAppended(
-    size_t total_bytes_to_free) {
-  DecodeTimestamp next_buffer_timestamp = GetNextBufferTimestamp();
-  if (last_appended_buffer_timestamp_ == kNoDecodeTimestamp() ||
-      next_buffer_timestamp == kNoDecodeTimestamp() ||
-      last_appended_buffer_timestamp_ >= next_buffer_timestamp) {
-    return 0;
-  }
+    size_t total_bytes_to_free, DecodeTimestamp media_time) {
+  DVLOG(4) << __FUNCTION__ << " last_appended_buffer_timestamp_="
+            << last_appended_buffer_timestamp_.InSecondsF()
+            << " media_time=" << media_time.InSecondsF();
 
   DecodeTimestamp remove_range_start = last_appended_buffer_timestamp_;
   if (last_appended_buffer_is_keyframe_)
     remove_range_start += GetMaxInterbufferDistance();
 
   DecodeTimestamp remove_range_start_keyframe = FindKeyframeAfterTimestamp(
       remove_range_start);
   if (remove_range_start_keyframe != kNoDecodeTimestamp())
     remove_range_start = remove_range_start_keyframe;
-  if (remove_range_start >= next_buffer_timestamp)
+  if (remove_range_start >= media_time)
     return 0;
 
   DecodeTimestamp remove_range_end;
   size_t bytes_freed = GetRemovalRange(remove_range_start,
-                                       next_buffer_timestamp,
+                                       media_time,
                                        total_bytes_to_free,
                                        &remove_range_end);
   if (bytes_freed > 0) {
+    DVLOG(4) << __FUNCTION__ << " removing ["
+             << remove_range_start.ToPresentationTime().InSecondsF() << ";"
+             << remove_range_end.ToPresentationTime().InSecondsF() << "]";
     Remove(remove_range_start.ToPresentationTime(),
            remove_range_end.ToPresentationTime(),
-           next_buffer_timestamp.ToPresentationTime());
+           media_time.ToPresentationTime());
   }
 
   return bytes_freed;
 }
 
 size_t SourceBufferStream::GetRemovalRange(
     DecodeTimestamp start_timestamp, DecodeTimestamp end_timestamp,
     size_t total_bytes_to_free, DecodeTimestamp* removal_end_timestamp) {
   DCHECK(start_timestamp >= DecodeTimestamp()) << start_timestamp.InSecondsF();
   DCHECK(start_timestamp < end_timestamp)
@@ skipping 12 matching lines @@
 
     size_t bytes_to_free = total_bytes_to_free - bytes_freed;
     size_t bytes_removed = range->GetRemovalGOP(
         start_timestamp, end_timestamp, bytes_to_free, removal_end_timestamp);
     bytes_freed += bytes_removed;
   }
   return bytes_freed;
 }
 
 size_t SourceBufferStream::FreeBuffers(size_t total_bytes_to_free,
+                                       DecodeTimestamp media_time,
                                        bool reverse_direction) {
   TRACE_EVENT2("media", "SourceBufferStream::FreeBuffers",
                "total bytes to free", total_bytes_to_free,
                "reverse direction", reverse_direction);
 
   DCHECK_GT(total_bytes_to_free, 0u);
   size_t bytes_freed = 0;
 
   // This range will save the last GOP appended to |range_for_next_append_|
   // if the buffers surrounding it get deleted during garbage collection.
   SourceBufferRange* new_range_for_append = NULL;
 
   while (!ranges_.empty() && bytes_freed < total_bytes_to_free) {
     SourceBufferRange* current_range = NULL;
     BufferQueue buffers;
     size_t bytes_deleted = 0;
 
     if (reverse_direction) {
       current_range = ranges_.back();
+      DVLOG(5) << "current_range=" << RangeToString(*current_range);
       if (current_range->LastGOPContainsNextBufferPosition()) {
         DCHECK_EQ(current_range, selected_range_);
+        DVLOG(5) << "current_range contains next read position, stopping GC";
         break;
       }
+      DVLOG(5) << "Deleting GOP from back: " << RangeToString(*current_range);
       bytes_deleted = current_range->DeleteGOPFromBack(&buffers);
     } else {
       current_range = ranges_.front();
-      if (current_range->FirstGOPContainsNextBufferPosition()) {
-        DCHECK_EQ(current_range, selected_range_);
+      DVLOG(5) << "current_range=" << RangeToString(*current_range);
+      if (!current_range->FirstGOPEarlierThanMediaTime(media_time)) {
+        // We have removed all data up to the GOP that contains current playback
+        // position, we can't delete any further.
+        DVLOG(5) << "current_range contains playback position, stopping GC";
         break;
       }
+      DVLOG(4) << "Deleting GOP from front: " << RangeToString(*current_range);
       bytes_deleted = current_range->DeleteGOPFromFront(&buffers);
     }
 
     // Check to see if we've just deleted the GOP that was last appended.
     DecodeTimestamp end_timestamp = buffers.back()->GetDecodeTimestamp();
     if (end_timestamp == last_appended_buffer_timestamp_) {
       DCHECK(last_appended_buffer_timestamp_ != kNoDecodeTimestamp());
       DCHECK(!new_range_for_append);
+
       // Create a new range containing these buffers.
       new_range_for_append = new SourceBufferRange(
           TypeToGapPolicy(GetType()),
           buffers, kNoDecodeTimestamp(),
           base::Bind(&SourceBufferStream::GetMaxInterbufferDistance,
                      base::Unretained(this)));
       range_for_next_append_ = ranges_.end();
     } else {
       bytes_freed += bytes_deleted;
     }
 
     if (current_range->size_in_bytes() == 0) {
       DCHECK_NE(current_range, selected_range_);
       DCHECK(range_for_next_append_ == ranges_.end() ||
              *range_for_next_append_ != current_range);
       delete current_range;
       reverse_direction ? ranges_.pop_back() : ranges_.pop_front();
     }
+
+    if (reverse_direction && new_range_for_append) {
+      // We don't want to delete any further, or we'll be creating gaps
+      break;
+    }
   }
 
   // Insert |new_range_for_append| into |ranges_|, if applicable.
   if (new_range_for_append) {
     range_for_next_append_ = AddToRanges(new_range_for_append);
     DCHECK(range_for_next_append_ != ranges_.end());
 
     // Check to see if we need to merge |new_range_for_append| with the range
     // before or after it. |new_range_for_append| is created whenever the last
     // GOP appended is encountered, regardless of whether any buffers after it
@@ skipping 439 matching lines @@
 }
 
 void SourceBufferStream::SeekAndSetSelectedRange(
     SourceBufferRange* range, DecodeTimestamp seek_timestamp) {
   if (range)
     range->Seek(seek_timestamp);
   SetSelectedRange(range);
 }
 
 void SourceBufferStream::SetSelectedRange(SourceBufferRange* range) {
-  DVLOG(1) << __FUNCTION__ << " " << GetStreamTypeName()
-           << ": " << selected_range_ << " -> " << range;
+  DVLOG(1) << __FUNCTION__ << " " << GetStreamTypeName() << ": "
+           << selected_range_ << " "
+           << (selected_range_ ? RangeToString(*selected_range_) : "")
+           << " -> " << range << " " << (range ? RangeToString(*range) : "");
   if (selected_range_)
     selected_range_->ResetNextBufferPosition();
   DCHECK(!range || range->HasNextBufferPosition());
   selected_range_ = range;
 }
 
 Ranges<base::TimeDelta> SourceBufferStream::GetBufferedTime() const {
   Ranges<base::TimeDelta> ranges;
   for (RangeList::const_iterator itr = ranges_.begin();
        itr != ranges_.end(); ++itr) {
     ranges.Add((*itr)->GetStartTimestamp().ToPresentationTime(),
                (*itr)->GetBufferedEndTimestamp().ToPresentationTime());
   }
   return ranges;
 }
 
 base::TimeDelta SourceBufferStream::GetBufferedDuration() const {
   if (ranges_.empty())
     return base::TimeDelta();
 
   return ranges_.back()->GetBufferedEndTimestamp().ToPresentationTime();
 }
 
+size_t SourceBufferStream::GetBufferedSize() const {
+  size_t ranges_size = 0;
+  for (const auto& range : ranges_)
+    ranges_size += range->size_in_bytes();
+  return ranges_size;
+}
+
 void SourceBufferStream::MarkEndOfStream() {
   DCHECK(!end_of_stream_);
   end_of_stream_ = true;
 }
 
 void SourceBufferStream::UnmarkEndOfStream() {
   DCHECK(end_of_stream_);
   end_of_stream_ = false;
 }
 
@@ skipping 409 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