Tweak track buffer-related logic to switch to new data more quickly

This CL changes the SourceBufferStream logic to switch from the track
buffer data to the new underlying data as soon as it is possible.

BUG=151079


Review URL: https://chromiumcodereview.appspot.com/10982019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@158954 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 179 insertions, 20 deletions

diff --git a/media/filters/source_buffer_stream.cc b/media/filters/source_buffer_stream.cc
index ff96dbc..888934a 100644
--- a/media/filters/source_buffer_stream.cc
+++ b/media/filters/source_buffer_stream.cc
@@ -420,15 +420,23 @@ bool SourceBufferStream::Append(
   if (deleted_next_buffer) {
     DCHECK(!seek_pending_);
     SetSelectedRange(*range_for_new_buffers);
-    if (!deleted_buffers.empty()) {
-      // Seek the range to the keyframe at or after |next_buffer_timestamp|.
+    if (next_buffer_timestamp != kNoTimestamp()) {
+      // Seek ahead to the keyframe at or after |next_buffer_timestamp|, the
+      // timestamp of the deleted next buffer.
       selected_range_->SeekAheadTo(next_buffer_timestamp);
-      UpdateTrackBuffer(deleted_buffers);
+      // Update track buffer with non-keyframe buffers leading up to the current
+      // position of |selected_range_|.
+      PruneTrackBuffer();
+      if (!deleted_buffers.empty())
+        UpdateTrackBuffer(deleted_buffers);
     } else {
-      // If we've deleted the next buffer but |deleted_buffers| is empty,
-      // that means we need to seek past the timestamp of the last buffer in
-      // the range (i.e. the keyframe timestamp needs to be strictly greater
-      // than |end_buffer_timestamp|).
+      // If |next_buffer_timestamp| is kNoTimestamp(), it means the range
+      // we've overlapped didn't actually have the next buffer buffered, and it
+      // was waiting for the next buffer whose timestamp was greater than
+      // |end_buffer_timestamp|. Seek the |selected_range_| to the next keyframe
+      // after |end_buffer_timestamp|.
+      DCHECK(track_buffer_.empty());
+      DCHECK(deleted_buffers.empty());
       selected_range_->SeekAheadPast(end_buffer_timestamp);
     }
   }
@@ -699,24 +707,41 @@ void SourceBufferStream::ResolveEndOverlap(
   SetSelectedRange(NULL);
 }
 
+void SourceBufferStream::PruneTrackBuffer() {
+  DCHECK(selected_range_);
+  DCHECK(selected_range_->HasNextBufferPosition());
+  base::TimeDelta next_timestamp = selected_range_->GetNextTimestamp();
+
+  // If we don't have the next timestamp, we don't have anything to delete.
+  if (next_timestamp == kNoTimestamp())
+    return;
+
+  while (!track_buffer_.empty() &&
+         track_buffer_.back()->GetDecodeTimestamp() >= next_timestamp) {
+    track_buffer_.pop_back();
+  }
+}
+
 void SourceBufferStream::UpdateTrackBuffer(const BufferQueue& deleted_buffers) {
   DCHECK(!deleted_buffers.empty());
   DCHECK(selected_range_);
   DCHECK(selected_range_->HasNextBufferPosition());
 
   base::TimeDelta next_keyframe_timestamp = selected_range_->GetNextTimestamp();
+  base::TimeDelta start_of_deleted =
+      deleted_buffers.front()->GetDecodeTimestamp();
+
+  // |deleted_buffers| should always come after the buffers in |track_buffer|.
+  if (!track_buffer_.empty())
+    DCHECK(track_buffer_.back()->GetDecodeTimestamp() < start_of_deleted);
 
   // If there is no gap between what was deleted and what was added, nothing
   // should be added to the track buffer.
   if (selected_range_->HasNextBuffer() &&
-      (next_keyframe_timestamp ==
-       deleted_buffers.front()->GetDecodeTimestamp())) {
+      next_keyframe_timestamp <= start_of_deleted) {
     return;
   }
 
-  DCHECK(next_keyframe_timestamp >=
-         deleted_buffers.front()->GetDecodeTimestamp());
-
   // If the |selected_range_| is ready to return data, fill the track buffer
   // with all buffers that come before |next_keyframe_timestamp| and return.
   if (selected_range_->HasNextBuffer()) {
@@ -730,7 +755,8 @@ void SourceBufferStream::UpdateTrackBuffer(const BufferQueue& deleted_buffers) {
 
   // Otherwise, the |selected_range_| is not ready to return data, so add all
   // the deleted buffers into the |track_buffer_|.
-  track_buffer_ = deleted_buffers;
+  track_buffer_.insert(track_buffer_.end(),
+                       deleted_buffers.begin(), deleted_buffers.end());
 
   // See if the next range contains the keyframe after the end of the
   // |track_buffer_|, and if so, change |selected_range_|.
@@ -838,6 +864,7 @@ SourceBufferStream::Status SourceBufferStream::GetNextBuffer(
   CHECK(!config_change_pending_);
 
   if (!track_buffer_.empty()) {
+    DCHECK(selected_range_);
     if (track_buffer_.front()->GetConfigId() != current_config_index_) {
       config_change_pending_ = true;
       return kConfigChange;
@@ -861,10 +888,14 @@ SourceBufferStream::Status SourceBufferStream::GetNextBuffer(
 }
 
 base::TimeDelta SourceBufferStream::GetNextBufferTimestamp() {
-  if (!selected_range_)
+  if (!selected_range_) {
+    DCHECK(track_buffer_.empty());
     return kNoTimestamp();
+  }
 
   DCHECK(selected_range_->HasNextBufferPosition());
+  if (!track_buffer_.empty())
+    return track_buffer_.front()->GetDecodeTimestamp();
   return selected_range_->GetNextTimestamp();
 }
 
@@ -1054,7 +1085,7 @@ void SourceBufferRange::AppendBuffersToEnd(const BufferQueue& new_buffers) {
       if (waiting_for_keyframe_ &&
           (*itr)->GetDecodeTimestamp() >= next_keyframe_timestamp_) {
         next_buffer_index_ = buffers_.size() - 1;
-        next_keyframe_timestamp_ = base::TimeDelta();
+        next_keyframe_timestamp_ = kNoTimestamp();
         waiting_for_keyframe_ = false;
       }
     }
@@ -1065,7 +1096,7 @@ void SourceBufferRange::Seek(base::TimeDelta timestamp) {
   DCHECK(CanSeekTo(timestamp));
   DCHECK(!keyframe_map_.empty());
 
-  next_keyframe_timestamp_ = base::TimeDelta();
+  next_keyframe_timestamp_ = kNoTimestamp();
   waiting_for_keyframe_ = false;
 
   KeyframeMap::iterator result = GetFirstKeyframeBefore(timestamp);
@@ -1367,11 +1398,10 @@ base::TimeDelta SourceBufferRange::GetNextTimestamp() const {
   DCHECK(!buffers_.empty());
   DCHECK(HasNextBufferPosition());
 
-  if (waiting_for_keyframe_)
-    return next_keyframe_timestamp_;
-
-  if (next_buffer_index_ >= static_cast<int>(buffers_.size()))
+  if (waiting_for_keyframe_ ||
+      next_buffer_index_ >= static_cast<int>(buffers_.size())) {
     return kNoTimestamp();
+  }
 
   return buffers_.at(next_buffer_index_)->GetDecodeTimestamp();
 }
diff --git a/media/filters/source_buffer_stream.h b/media/filters/source_buffer_stream.h
index 803f877..1c3514b 100644
--- a/media/filters/source_buffer_stream.h
+++ b/media/filters/source_buffer_stream.h
@@ -150,6 +150,10 @@ class MEDIA_EXPORT SourceBufferStream {
   // TODO(vrk): This is a little crazy! Ideas for cleanup in crbug.com/129623.
   void UpdateTrackBuffer(const BufferQueue& deleted_buffers);
 
+  // Removes buffers that come before |selected_range_|'s next buffer from the
+  // |track_buffer_|.
+  void PruneTrackBuffer();
+
   // Checks to see if |range_with_new_buffers_itr| can be merged with the range
   // next to it, and merges them if so.
   void MergeWithAdjacentRangeIfNecessary(
diff --git a/media/filters/source_buffer_stream_unittest.cc b/media/filters/source_buffer_stream_unittest.cc
index 109ef5c..e46a703 100644
--- a/media/filters/source_buffer_stream_unittest.cc
+++ b/media/filters/source_buffer_stream_unittest.cc
@@ -1287,6 +1287,131 @@ TEST_F(SourceBufferStreamTest, Overlap_OneByOne_TrackBuffer) {
   CheckExpectedBuffers("0K 30 60 90 120K 130K");
 }
 
+// Overlap the next keyframe after the end of the track buffer with a new
+// keyframe.
+// old  :   10K  40  *70*  100K  125  130K
+// new  : 0K   30   60   90   120K
+// after: 0K   30   60   90  *120K*   130K
+// track:             70   100K
+// new  :                     110K    130
+// after: 0K   30   60   90  *110K*   130
+TEST_F(SourceBufferStreamTest, Overlap_OneByOne_TrackBuffer2) {
+  NewSegmentAppendOneByOne("10K 40 70 100K 125 130K");
+  CheckExpectedRangesByTimestamp("{ [10,160) }");
+
+  // Seek to 70ms.
+  SeekToTimestamp(base::TimeDelta::FromMilliseconds(70));
+  CheckExpectedBuffers("10K 40");
+
+  // Overlap with a new segment from 0 to 120ms; 70ms and 100ms go in track
+  // buffer.
+  NewSegmentAppendOneByOne("0K 30 60 90 120K");
+  CheckExpectedRangesByTimestamp("{ [0,160) }");
+
+  // Now overlap the keyframe at 120ms.
+  NewSegmentAppendOneByOne("110K 130");
+
+  // Should expect buffers 70ms and 100ms from the track buffer. Then it should
+  // return the keyframe after the track buffer, which is at 110ms.
+  CheckExpectedBuffers("70 100K 110K 130");
+}
+
+// Overlap the next keyframe after the end of the track buffer without a
+// new keyframe.
+// old  :   10K  40  *70*  100K  125  130K
+// new  : 0K   30   60   90   120K
+// after: 0K   30   60   90  *120K*   130K
+// track:             70   100K
+// new  :        50K   80   110          140
+// after: 0K   30   50K   80   110   140 * (waiting for keyframe)
+// track:               70   100K   120K   130K
+TEST_F(SourceBufferStreamTest, Overlap_OneByOne_TrackBuffer3) {
+  NewSegmentAppendOneByOne("10K 40 70 100K 125 130K");
+  CheckExpectedRangesByTimestamp("{ [10,160) }");
+
+  // Seek to 70ms.
+  SeekToTimestamp(base::TimeDelta::FromMilliseconds(70));
+  CheckExpectedBuffers("10K 40");
+
+  // Overlap with a new segment from 0 to 120ms; 70ms and 100ms go in track
+  // buffer.
+  NewSegmentAppendOneByOne("0K 30 60 90 120K");
+  CheckExpectedRangesByTimestamp("{ [0,160) }");
+
+  // Now overlap the keyframe at 120ms. There's no keyframe after 70ms, so 120ms
+  // and 130ms go into the track buffer.
+  NewSegmentAppendOneByOne("50K 80 110 140");
+
+  // Should have all the buffers from the track buffer, then stall.
+  CheckExpectedBuffers("70 100K 120K 130K");
+  CheckNoNextBuffer();
+
+  // Appending a keyframe should fulfill the read.
+  AppendBuffersOneByOne("150K");
+  CheckExpectedBuffers("150K");
+  CheckNoNextBuffer();
+}
+
+// Overlap the next keyframe after the end of the track buffer with a keyframe
+// that comes before the end of the track buffer.
+// old  :   10K  40  *70*  100K  125  130K
+// new  : 0K   30   60   90   120K
+// after: 0K   30   60   90  *120K*   130K
+// track:             70   100K
+// new  :              80K  110          140
+// after: 0K   30   60   *80K*  110   140
+// track:               70
+TEST_F(SourceBufferStreamTest, Overlap_OneByOne_TrackBuffer4) {
+  NewSegmentAppendOneByOne("10K 40 70 100K 125 130K");
+  CheckExpectedRangesByTimestamp("{ [10,160) }");
+
+  // Seek to 70ms.
+  SeekToTimestamp(base::TimeDelta::FromMilliseconds(70));
+  CheckExpectedBuffers("10K 40");
+
+  // Overlap with a new segment from 0 to 120ms; 70ms and 100ms go in track
+  // buffer.
+  NewSegmentAppendOneByOne("0K 30 60 90 120K");
+  CheckExpectedRangesByTimestamp("{ [0,160) }");
+
+  // Now append a keyframe at 80ms.
+  NewSegmentAppendOneByOne("80K 110 140");
+
+  CheckExpectedBuffers("70 80K 110 140");
+  CheckNoNextBuffer();
+}
+
+// Overlap the next keyframe after the end of the track buffer with a keyframe
+// that comes before the end of the track buffer, when the selected stream was
+// waiting for the next keyframe.
+// old  :   10K  40  *70*  100K
+// new  : 0K   30   60   90   120
+// after: 0K   30   60   90   120 * (waiting for keyframe)
+// track:             70   100K
+// new  :              80K  110          140
+// after: 0K   30   60   *80K*  110   140
+// track:               70
+TEST_F(SourceBufferStreamTest, Overlap_OneByOne_TrackBuffer5) {
+  NewSegmentAppendOneByOne("10K 40 70 100K");
+  CheckExpectedRangesByTimestamp("{ [10,130) }");
+
+  // Seek to 70ms.
+  SeekToTimestamp(base::TimeDelta::FromMilliseconds(70));
+  CheckExpectedBuffers("10K 40");
+
+  // Overlap with a new segment from 0 to 120ms; 70ms and 100ms go in track
+  // buffer.
+  NewSegmentAppendOneByOne("0K 30 60 90 120");
+  CheckExpectedRangesByTimestamp("{ [0,150) }");
+
+  // Now append a keyframe at 80ms. The buffer at 100ms should be deleted from
+  // the track buffer.
+  NewSegmentAppendOneByOne("80K 110 140");
+
+  CheckExpectedBuffers("70 80K 110 140");
+  CheckNoNextBuffer();
+}
+
 TEST_F(SourceBufferStreamTest, Seek_Keyframe) {
   // Append 6 buffers at positions 0 through 5.
   NewSegmentAppend(0, 6);