Fix crash caused by removing the gap at the beginning of a media segment.

Some media segments have a start time that is slightly 
earlier than the timestamp of the first buffer in that
segment. If SourceBufferStream::Remove() is called with a
range that only covered part of this gap, then the
SourceBufferStream code could end up crashing when it was
trying to handle the remove. This change fixes the crash by
properly removing the empty range that gets created when
such a remove occurs.

BUG=382815
TEST=SourceBufferStreamTest.Remove_GapAtBeginningOfMediaSegment

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

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

media/filters/source_buffer_stream.cc

@@ -168,7 +168,9 @@ class SourceBufferRange {
 
   // Gets the timestamp for the keyframe that is after |timestamp|. If
   // there isn't a keyframe in the range after |timestamp| then kNoTimestamp()
-  // is returned.
+  // is returned. If |timestamp| is in the "gap" between the value  returned by
+  // GetStartTimestamp() and the timestamp on the first buffer in |buffers_|,
+  // then |timestamp| is returned.
   base::TimeDelta NextKeyframeTimestamp(base::TimeDelta timestamp);
 
   // Gets the timestamp for the closest keyframe that is <= |timestamp|. If
@@ -1707,7 +1709,7 @@ SourceBufferRange::SourceBufferRange(
       media_segment_start_time_(media_segment_start_time),
       interbuffer_distance_cb_(interbuffer_distance_cb),
       size_in_bytes_(0) {
-  DCHECK(!new_buffers.empty());
+  CHECK(!new_buffers.empty());
   DCHECK(new_buffers.front()->IsKeyframe());
   DCHECK(!interbuffer_distance_cb.is_null());
   AppendBuffersToEnd(new_buffers);
@@ -1774,6 +1776,8 @@ void SourceBufferRange::SeekToStart() {
 
 SourceBufferRange* SourceBufferRange::SplitRange(
     base::TimeDelta timestamp, bool is_exclusive) {
+  CHECK(!buffers_.empty());
+
   // Find the first keyframe after |timestamp|. If |is_exclusive|, do not
   // include keyframes at |timestamp|.
   KeyframeMap::iterator new_beginning_keyframe =
@@ -1790,13 +1794,25 @@ SourceBufferRange* SourceBufferRange::SplitRange(
   DCHECK_LT(keyframe_index, static_cast<int>(buffers_.size()));
   BufferQueue::iterator starting_point = buffers_.begin() + keyframe_index;
   BufferQueue removed_buffers(starting_point, buffers_.end());
+
+  base::TimeDelta new_range_start_timestamp = kNoTimestamp();
+  if (GetStartTimestamp() < buffers_.front()->GetDecodeTimestamp() &&
+      timestamp < removed_buffers.front()->GetDecodeTimestamp()) {
+    // The split is in the gap between |media_segment_start_time_| and
+    // the first buffer of the new range so we should set the start
+    // time of the new range to |timestamp| so we preserve part of the
+    // gap in the new range.
+    new_range_start_timestamp = timestamp;
+  }
+
   keyframe_map_.erase(new_beginning_keyframe, keyframe_map_.end());
   FreeBufferRange(starting_point, buffers_.end());
 
   // Create a new range with |removed_buffers|.
   SourceBufferRange* split_range =
       new SourceBufferRange(
-          type_, removed_buffers, kNoTimestamp(), interbuffer_distance_cb_);
+          type_, removed_buffers, new_range_start_timestamp,
+          interbuffer_distance_cb_);
 
   // If the next buffer position is now in |split_range|, update the state of
   // this range and |split_range| accordingly.
@@ -2015,7 +2031,7 @@ bool SourceBufferRange::TruncateAt(
 
   // Return if we're not deleting anything.
   if (starting_point == buffers_.end())
-    return false;
+    return buffers_.empty();
 
   // Reset the next buffer index if we will be deleting the buffer that's next
   // in sequence.
@@ -2164,6 +2180,16 @@ base::TimeDelta SourceBufferRange::NextKeyframeTimestamp(
   KeyframeMap::iterator itr = GetFirstKeyframeAt(timestamp, false);
   if (itr == keyframe_map_.end())
     return kNoTimestamp();
+
+  // If the timestamp is inside the gap between the start of the media
+  // segment and the first buffer, then just pretend there is a
+  // keyframe at the specified timestamp.
+  if (itr == keyframe_map_.begin() &&
+      timestamp > media_segment_start_time_ &&
+      timestamp < itr->first) {
+    return timestamp;
+  }
+
   return itr->first;
 }
 

media/filters/source_buffer_stream_unittest.cc

@@ -3562,6 +3562,36 @@ TEST_F(SourceBufferStreamTest,
   CheckExpectedBuffers("90K 121 151");
 }
 
+TEST_F(SourceBufferStreamTest, Remove_GapAtBeginningOfMediaSegment) {
+  Seek(0);
+
+  // Append a media segment that has a gap at the beginning of it.
+  NewSegmentAppend(base::TimeDelta::FromMilliseconds(0),
+                   "30K 60 90 120K 150");
+  CheckExpectedRangesByTimestamp("{ [0,180) }");
+
+  // Remove the gap that doesn't contain any buffers.
+  RemoveInMs(0, 10, 180);
+  CheckExpectedRangesByTimestamp("{ [10,180) }");
+
+  // Verify we still get the first buffer still since only part of
+  // the gap was removed.
+  // TODO(acolwell/wolenetz): Consider not returning a buffer at this
+  // point since the current seek position has been explicitly
+  // removed but didn't happen to remove any buffers.
+  // http://crbug.com/384016
+  CheckExpectedBuffers("30K");
+
+  // Remove a range that includes the first GOP.
+  RemoveInMs(0, 60, 180);
+
+  // Verify that no buffer is returned because the current buffer
+  // position has been removed.
+  CheckNoNextBuffer();
+
+  CheckExpectedRangesByTimestamp("{ [120,180) }");
+}
+
 TEST_F(SourceBufferStreamTest, Text_Append_SingleRange) {
   SetTextStream();
   NewSegmentAppend("0K 500K 1000K");