Fix unit test failures with estimated durations and splice frames.

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 "base/bind.h"
@@ skipping 1630 matching lines @@
       const scoped_refptr<StreamParserBuffer>& buffer = *it;
       if (buffer->timestamp() <= max_splice_end_timestamp &&
           (buffer->timestamp() < overlapping_buffer->timestamp() ||
            buffer == overlapping_buffer)) {
         // Add the buffer and adjust the index forward to compensate.
         pre_splice_buffers.insert(pre_splice_buffers.begin() + ++i, buffer);
       }
     }
   }
 
+  if (new_buffers.front()->timestamp() == media_segment_start_time_)
+    media_segment_start_time_ = pre_splice_buffers.front()->timestamp();

[acolwell GONE FROM CHROMIUM, 2014/04/01 00:39:11]

I believe this makes the crash go away, but it isn't really addressing the
underlying problem. media_segment_start_time_ is intended to represent the start
of the media segment being appended. You should not be overriding it here since
doing so erases that information and replaces it with something else completely
unrelated to that. I believe the fix should be in Append() around where
media_segment_start_time_ is assigned to new_range_start_time. What you really
want is the min of the first buffer or the media_segment_start_time_.

I think the actual problem is that there is a mismatch between the range start
time and the buffers you are passing in. SourceBufferRange should probably have
the following DCHECK to make sure it isn't passed bad data.

DCHECK(media_segment_start_time == kNoTimestamp() ||
         media_segment_start_time <= new_buffers.front()->GetDecodeTimestamp());

Append() should probably also have a DCHECK at the top that makes sure that the
buffers coming in have a timestamp that is >= media_segment_start_time_.

[DaleCurtis, 2014/04/01 21:33:57]

Your suggestion also fixes the issue.  However adding the recommended DCHECK()
to SourceBufferStream::Append() causes many tests to fail.  Here are two:

[ RUN      ] ChunkDemuxerTest.AppendWindow_Text
[24692:24692:0401/143118:329900817718:FATAL:source_buffer_stream.cc(450)] Check
failed: media_segment_start_time_ <= buffers.front()->timestamp(). 120000 vs.
100000

[ RUN      ] SourceBufferStreamTest.Append_DoesNotBeginWithKeyframe
[24760:24760:0401/143201:329943721497:FATAL:source_buffer_stream.cc(450)] Check
failed: media_segment_start_time_ <= buffers.front()->timestamp(). 99999 vs.
66666

Bad behavior?

+
   new_buffers.front()->ConvertToSpliceBuffer(pre_splice_buffers);
 }
 
 SourceBufferRange::SourceBufferRange(
     SourceBufferStream::Type type, const BufferQueue& new_buffers,
     base::TimeDelta media_segment_start_time,
     const InterbufferDistanceCB& interbuffer_distance_cb)
     : type_(type),
       keyframe_map_index_base_(0),
       next_buffer_index_(-1),
@@ skipping 511 matching lines @@
     if (buffer->end_of_stream() || buffer->timestamp() >= end)
       break;
     if (buffer->timestamp() + buffer->duration() <= start)
       continue;
     buffers->push_back(buffer);
   }
   return previous_size < buffers->size();
 }
 
 }  // namespace media