Fix SourceBufferStream to handle range truncations that happen between the segment start and the first buffer.

BUG=334394
TEST=SourceBufferStreamTest.SetExplicitDuration_AfterSegmentTimestampAndBeforeFirstBufferTimestamp,
 SourceBufferStreamTest.Complete_Overlap_AfterSegmentTimestampAndBeforeFirstBufferTimestamp

Review URL: https://codereview.chromium.org/135813004

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

2 files changed, 95 insertions, 26 deletions

diff --git a/media/filters/source_buffer_stream.cc b/media/filters/source_buffer_stream.cc
index 331a0dc..785643b 100644
--- a/media/filters/source_buffer_stream.cc
+++ b/media/filters/source_buffer_stream.cc
@@ -95,7 +95,9 @@ class SourceBufferRange {
   // were removed.
   // |deleted_buffers| contains the buffers that were deleted from this range,
   // starting at the buffer that had been at |next_buffer_index_|.
-  void TruncateAt(base::TimeDelta timestamp,
+  // Returns true if everything in the range was deleted. Otherwise
+  // returns false.
+  bool TruncateAt(base::TimeDelta timestamp,
                   BufferQueue* deleted_buffers, bool is_exclusive);
   // Deletes all buffers in range.
   void DeleteAll(BufferQueue* deleted_buffers);
@@ -214,7 +216,9 @@ class SourceBufferRange {
 
   // Helper method to delete buffers in |buffers_| starting at
   // |starting_point|, an iterator in |buffers_|.
-  void TruncateAt(const BufferQueue::iterator& starting_point,
+  // Returns true if everything in the range was removed. Returns
+  // false if the range still contains buffers.
+  bool TruncateAt(const BufferQueue::iterator& starting_point,
                   BufferQueue* deleted_buffers);
 
   // Frees the buffers in |buffers_| from [|start_point|,|ending_point|) and
@@ -610,15 +614,10 @@ void SourceBufferStream::RemoveInternal(
         SetSelectedRange(new_range);
     }
 
-    // If the current range now is completely covered by the removal
-    // range then we want to delete it.
-    bool delete_range = start < range->GetStartTimestamp() ||
-        (!is_exclusive && start == range->GetStartTimestamp());
-
     // Truncate the current range so that it only contains data before
     // the removal range.
     BufferQueue saved_buffers;
-    range->TruncateAt(start, &saved_buffers, is_exclusive);
+    bool delete_range = range->TruncateAt(start, &saved_buffers, is_exclusive);
 
     // Check to see if the current playback position was removed and
     // update the selected range appropriately.
@@ -1075,10 +1074,15 @@ void SourceBufferStream::OnSetDuration(base::TimeDelta duration) {
 
   // Need to partially truncate this range.
   if ((*itr)->GetStartTimestamp() < duration) {
-    (*itr)->TruncateAt(duration, NULL, false);
+    bool delete_range = (*itr)->TruncateAt(duration, NULL, false);
     if ((*itr == selected_range_) && !selected_range_->HasNextBufferPosition())
       SetSelectedRange(NULL);
-    ++itr;
+
+    if (delete_range) {
+      DeleteAndRemoveRange(&itr);
+    } else {
+      ++itr;
+    }
   }
 
   // Delete all ranges that begin after |duration|.
@@ -1658,13 +1662,13 @@ void SourceBufferRange::DeleteAll(BufferQueue* removed_buffers) {
   TruncateAt(buffers_.begin(), removed_buffers);
 }
 
-void SourceBufferRange::TruncateAt(
+bool SourceBufferRange::TruncateAt(
     base::TimeDelta timestamp, BufferQueue* removed_buffers,
     bool is_exclusive) {
   // Find the place in |buffers_| where we will begin deleting data.
   BufferQueue::iterator starting_point =
       GetBufferItrAt(timestamp, is_exclusive);
-  TruncateAt(starting_point, removed_buffers);
+  return TruncateAt(starting_point, removed_buffers);
 }
 
 int SourceBufferRange::DeleteGOPFromFront(BufferQueue* deleted_buffers) {
@@ -1820,13 +1824,13 @@ void SourceBufferRange::FreeBufferRange(
   buffers_.erase(starting_point, ending_point);
 }
 
-void SourceBufferRange::TruncateAt(
+bool SourceBufferRange::TruncateAt(
     const BufferQueue::iterator& starting_point, BufferQueue* removed_buffers) {
   DCHECK(!removed_buffers || removed_buffers->empty());
 
   // Return if we're not deleting anything.
   if (starting_point == buffers_.end())
-    return;
+    return false;
 
   // Reset the next buffer index if we will be deleting the buffer that's next
   // in sequence.
@@ -1852,6 +1856,7 @@ void SourceBufferRange::TruncateAt(
 
   // Remove everything from |starting_point| onward.
   FreeBufferRange(starting_point, buffers_.end());
+  return buffers_.empty();
 }
 
 bool SourceBufferRange::GetNextBuffer(
diff --git a/media/filters/source_buffer_stream_unittest.cc b/media/filters/source_buffer_stream_unittest.cc
index c9a193a..03345fe 100644
--- a/media/filters/source_buffer_stream_unittest.cc
+++ b/media/filters/source_buffer_stream_unittest.cc
@@ -86,27 +86,32 @@ class SourceBufferStreamTest : public testing::Test {
   }
 
   void NewSegmentAppend(const std::string& buffers_to_append) {
-    AppendBuffers(buffers_to_append, true, false, true);
+    AppendBuffers(buffers_to_append, true, kNoTimestamp(), false, true);
+  }
+
+  void NewSegmentAppend(base::TimeDelta start_timestamp,
+                        const std::string& buffers_to_append) {
+    AppendBuffers(buffers_to_append, true, start_timestamp, false, true);
   }
 
   void AppendBuffers(const std::string& buffers_to_append) {
-    AppendBuffers(buffers_to_append, false, false, true);
+    AppendBuffers(buffers_to_append, false, kNoTimestamp(), false, true);
   }
 
   void NewSegmentAppendOneByOne(const std::string& buffers_to_append) {
-    AppendBuffers(buffers_to_append, true, true, true);
+    AppendBuffers(buffers_to_append, true, kNoTimestamp(), true, true);
   }
 
   void AppendBuffersOneByOne(const std::string& buffers_to_append) {
-    AppendBuffers(buffers_to_append, false, true, true);
+    AppendBuffers(buffers_to_append, false, kNoTimestamp(), true, true);
   }
 
   void NewSegmentAppend_ExpectFailure(const std::string& buffers_to_append) {
-    AppendBuffers(buffers_to_append, true, false, false);
+    AppendBuffers(buffers_to_append, true, kNoTimestamp(), false, false);
   }
 
   void AppendBuffers_ExpectFailure(const std::string& buffers_to_append) {
-    AppendBuffers(buffers_to_append, false, false, false);
+    AppendBuffers(buffers_to_append, false, kNoTimestamp(), false, false);
   }
 
   void Seek(int position) {
@@ -313,9 +318,8 @@ class SourceBufferStreamTest : public testing::Test {
       EXPECT_EQ(expect_success, stream_->Append(queue));
   }
 
-  void AppendBuffers(const std::string& buffers_to_append,
-                     bool start_new_segment, bool one_by_one,
-                     bool expect_success) {
+  SourceBufferStream::BufferQueue StringToBufferQueue(
+      const std::string& buffers_to_append) {
     std::vector<std::string> timestamps;
     base::SplitString(buffers_to_append, ' ', &timestamps);
 
@@ -337,13 +341,31 @@ class SourceBufferStreamTest : public testing::Test {
           StreamParserBuffer::CopyFrom(&kDataA, kDataSize, is_keyframe);
       base::TimeDelta timestamp =
           base::TimeDelta::FromMilliseconds(time_in_ms);
+      buffer->set_timestamp(timestamp);
       buffer->SetDecodeTimestamp(timestamp);
 
-      if (i == 0u && start_new_segment)
-        stream_->OnNewMediaSegment(timestamp);
-
       buffers.push_back(buffer);
     }
+    return buffers;
+  }
+
+  void AppendBuffers(const std::string& buffers_to_append,
+                     bool start_new_segment,
+                     base::TimeDelta segment_start_timestamp,
+                     bool one_by_one,
+                     bool expect_success) {
+    SourceBufferStream::BufferQueue buffers =
+        StringToBufferQueue(buffers_to_append);
+
+    if (start_new_segment) {
+      base::TimeDelta start_timestamp = segment_start_timestamp;
+      if (start_timestamp == kNoTimestamp())
+        start_timestamp = buffers[0]->GetDecodeTimestamp();
+
+      ASSERT_TRUE(start_timestamp <= buffers[0]->GetDecodeTimestamp());
+
+      stream_->OnNewMediaSegment(start_timestamp);
+    }
 
     if (!one_by_one) {
       EXPECT_EQ(expect_success, stream_->Append(buffers));
@@ -471,6 +493,27 @@ TEST_F(SourceBufferStreamTest, Complete_Overlap) {
   CheckExpectedBuffers(0, 14);
 }
 
+TEST_F(SourceBufferStreamTest,
+       Complete_Overlap_AfterSegmentTimestampAndBeforeFirstBufferTimestamp) {
+  // Append a segment with a start timestamp of 0, but the first
+  // buffer starts at 30ms. This can happen in muxed content where the
+  // audio starts before the first frame.
+  NewSegmentAppend(base::TimeDelta::FromMilliseconds(0), "30K 60K 90K 120K");
+
+  CheckExpectedRangesByTimestamp("{ [0,150) }");
+
+  // Completely overlap the old buffers, with a segment that starts
+  // after the old segment start timestamp, but before the timestamp
+  // of the first buffer in the segment.
+  NewSegmentAppend("20K 50K 80K 110K");
+
+  // Verify that the buffered ranges are updated properly and we don't crash.
+  CheckExpectedRangesByTimestamp("{ [20,150) }");
+
+  SeekToTimestamp(base::TimeDelta::FromMilliseconds(20));
+  CheckExpectedBuffers("20K 50K 80K 110K 120K");
+}
+
 TEST_F(SourceBufferStreamTest, Complete_Overlap_EdgeCase) {
   // Make each frame a keyframe so that it's okay to overlap frames at any point
   // (instead of needing to respect keyframe boundaries).
@@ -2898,6 +2941,27 @@ TEST_F(SourceBufferStreamTest, SetExplicitDuration_UpdateSelectedRange) {
   CheckExpectedRangesByTimestamp("{ [0,60) [120,180) }");
 }
 
+TEST_F(SourceBufferStreamTest,
+       SetExplicitDuration_AfterSegmentTimestampAndBeforeFirstBufferTimestamp) {
+
+  NewSegmentAppend("0K 30K 60K");
+
+  // Append a segment with a start timestamp of 200, but the first
+  // buffer starts at 230ms. This can happen in muxed content where the
+  // audio starts before the first frame.
+  NewSegmentAppend(base::TimeDelta::FromMilliseconds(200),
+                   "230K 260K 290K 320K");
+
+  NewSegmentAppend("400K 430K 460K");
+
+  CheckExpectedRangesByTimestamp("{ [0,90) [200,350) [400,490) }");
+
+  stream_->OnSetDuration(base::TimeDelta::FromMilliseconds(120));
+
+  // Verify that the buffered ranges are updated properly and we don't crash.
+  CheckExpectedRangesByTimestamp("{ [0,90) }");
+}
+
 // Test the case were the current playback position is at the end of the
 // buffered data and several overlaps occur that causes the selected
 // range to get split and then merged back into a single range.