Reland: MSE: Relax the 'media segment must begin with keyframe' requirement

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 19 matching lines @@
 // important for debugging splice generation.
 const int kMaxSpliceGenerationWarningLogs = 50;
 const int kMaxSpliceGenerationSuccessLogs = 20;
 
 // Limit the number of MEDIA_LOG() logs for track buffer time gaps.
 const int kMaxTrackBufferGapWarningLogs = 20;
 
 // Limit the number of MEDIA_LOG() logs for MSE GC algorithm warnings.
 const int kMaxGarbageCollectAlgorithmWarningLogs = 20;
 
+// Limit the number of MEDIA_LOG() logs for same DTS for non-keyframe followed
+// by keyframe. Prior to relaxing the "media segments must begin with a
+// keyframe" requirement, we issued decode error for this situation. That was
+// likely too strict, and now that the keyframe requirement is relaxed, we have
+// no knowledge of media segment boundaries here. Now, we log but don't trigger
+// decode error, since we allow these sequences which may cause extra decoder
+// work or other side-effects.
+const int kMaxStrangeSameTimestampsLogs = 20;
+
 // Helper method that returns true if |ranges| is sorted in increasing order,
 // false otherwise.
 bool IsRangeListSorted(const std::list<media::SourceBufferRange*>& ranges) {
   DecodeTimestamp prev = kNoDecodeTimestamp();
   for (std::list<SourceBufferRange*>::const_iterator itr =
        ranges.begin(); itr != ranges.end(); ++itr) {
     if (prev != kNoDecodeTimestamp() && prev >= (*itr)->GetStartTimestamp())
       return false;
     prev = (*itr)->GetEndTimestamp();
   }
@@ skipping 80 matching lines @@
   return SourceBufferRange::NO_GAPS_ALLOWED;
 }
 
 }  // namespace
 
 SourceBufferStream::SourceBufferStream(const AudioDecoderConfig& audio_config,
                                        const scoped_refptr<MediaLog>& media_log,
                                        bool splice_frames_enabled)
     : media_log_(media_log),
       seek_buffer_timestamp_(kNoTimestamp()),
-      media_segment_start_time_(kNoDecodeTimestamp()),
+      coded_frame_group_start_time_(kNoDecodeTimestamp()),
       range_for_next_append_(ranges_.end()),
       last_output_buffer_timestamp_(kNoDecodeTimestamp()),
       max_interbuffer_distance_(kNoTimestamp()),
       memory_limit_(kSourceBufferAudioMemoryLimit),
       splice_frames_enabled_(splice_frames_enabled) {
   DCHECK(audio_config.IsValidConfig());
   audio_configs_.push_back(audio_config);
 }
 
 SourceBufferStream::SourceBufferStream(const VideoDecoderConfig& video_config,
                                        const scoped_refptr<MediaLog>& media_log,
                                        bool splice_frames_enabled)
     : media_log_(media_log),
       seek_buffer_timestamp_(kNoTimestamp()),
-      media_segment_start_time_(kNoDecodeTimestamp()),
+      coded_frame_group_start_time_(kNoDecodeTimestamp()),
       range_for_next_append_(ranges_.end()),
       last_output_buffer_timestamp_(kNoDecodeTimestamp()),
       max_interbuffer_distance_(kNoTimestamp()),
       memory_limit_(kSourceBufferVideoMemoryLimit),
       splice_frames_enabled_(splice_frames_enabled) {
   DCHECK(video_config.IsValidConfig());
   video_configs_.push_back(video_config);
 }
 
 SourceBufferStream::SourceBufferStream(const TextTrackConfig& text_config,
                                        const scoped_refptr<MediaLog>& media_log,
                                        bool splice_frames_enabled)
     : media_log_(media_log),
       text_track_config_(text_config),
       seek_buffer_timestamp_(kNoTimestamp()),
-      media_segment_start_time_(kNoDecodeTimestamp()),
+      coded_frame_group_start_time_(kNoDecodeTimestamp()),
       range_for_next_append_(ranges_.end()),
       last_output_buffer_timestamp_(kNoDecodeTimestamp()),
       max_interbuffer_distance_(kNoTimestamp()),
       memory_limit_(kSourceBufferAudioMemoryLimit),
       splice_frames_enabled_(splice_frames_enabled) {}
 
 SourceBufferStream::~SourceBufferStream() {
   while (!ranges_.empty()) {
     delete ranges_.front();
     ranges_.pop_front();
   }
 }
 
-void SourceBufferStream::OnNewMediaSegment(
-    DecodeTimestamp media_segment_start_time) {
-  DVLOG(1) << __FUNCTION__ << " " << GetStreamTypeName()
-           << " (" << media_segment_start_time.InSecondsF() << ")";
+void SourceBufferStream::OnStartOfCodedFrameGroup(
+    DecodeTimestamp coded_frame_group_start_time) {
+  DVLOG(1) << __FUNCTION__ << " " << GetStreamTypeName() << " ("
+           << coded_frame_group_start_time.InSecondsF() << ")";
   DCHECK(!end_of_stream_);
-  media_segment_start_time_ = media_segment_start_time;
-  new_media_segment_ = true;
+  coded_frame_group_start_time_ = coded_frame_group_start_time;
+  new_coded_frame_group_ = true;
 
   RangeList::iterator last_range = range_for_next_append_;
-  range_for_next_append_ = FindExistingRangeFor(media_segment_start_time);
+  range_for_next_append_ = FindExistingRangeFor(coded_frame_group_start_time);
 
-  // Only reset |last_appended_buffer_timestamp_| if this new media segment is
-  // not adjacent to the previous media segment appended to the stream.
+  // Only reset |last_appended_buffer_timestamp_| if this new coded frame group
+  // is not adjacent to the previous coded frame group appended to the stream.
   if (range_for_next_append_ == ranges_.end() ||
       !AreAdjacentInSequence(last_appended_buffer_timestamp_,
-                             media_segment_start_time)) {
+                             coded_frame_group_start_time)) {
     last_appended_buffer_timestamp_ = kNoDecodeTimestamp();
     last_appended_buffer_duration_ = kNoTimestamp();
     last_appended_buffer_is_keyframe_ = false;
-    DVLOG(3) << __FUNCTION__ << " next appended buffers will be in a new range";
+    DVLOG(3) << __FUNCTION__ << " next appended buffers will "
+             << (range_for_next_append_ == ranges_.end()
+                     ? "be in a new range"
+                     : "overlap an existing range");
   } else if (last_range != ranges_.end()) {
     DCHECK(last_range == range_for_next_append_);
     DVLOG(3) << __FUNCTION__ << " next appended buffers will continue range "
              << "unless intervening remove makes discontinuity";
   }
 }
 
 bool SourceBufferStream::Append(const BufferQueue& buffers) {
   TRACE_EVENT2("media", "SourceBufferStream::Append",
                "stream type", GetStreamTypeName(),
                "buffers to append", buffers.size());
 
   DCHECK(!buffers.empty());
-  DCHECK(media_segment_start_time_ != kNoDecodeTimestamp());
-  DCHECK(media_segment_start_time_ <= buffers.front()->GetDecodeTimestamp());
+  DCHECK(coded_frame_group_start_time_ != kNoDecodeTimestamp());
+  DCHECK(coded_frame_group_start_time_ <=
+         buffers.front()->GetDecodeTimestamp());
   DCHECK(!end_of_stream_);
 
   DVLOG(1) << __FUNCTION__ << " " << GetStreamTypeName()
            << ": buffers " << BufferQueueToLogString(buffers);
 
-  // New media segments must begin with a keyframe.
-  // TODO(wolenetz): Relax this requirement. See http://crbug.com/229412.
-  if (new_media_segment_ && !buffers.front()->is_key_frame()) {
-    MEDIA_LOG(ERROR, media_log_) << "Media segment did not begin with key "
-                                    "frame. Support for such segments will be "
-                                    "available in a future version. Please see "
-                                    "https://crbug.com/229412.";
+  // New coded frame groups emitted by the coded frame processor must begin with
+  // a keyframe. TODO(wolenetz): Change this to [DCHECK + MEDIA_LOG(ERROR...) +
+  // return false] once the CHECK has baked in a stable release. See
+  // https://crbug.com/580621.
+  CHECK(!new_coded_frame_group_ || buffers.front()->is_key_frame());
+
+  // Buffers within a coded frame group should be monotonically increasing.
+  if (!IsMonotonicallyIncreasing(buffers)) {
     return false;
   }
 
-  // Buffers within a media segment should be monotonically increasing.
-  if (!IsMonotonicallyIncreasing(buffers))
-    return false;
-
-  if (media_segment_start_time_ < DecodeTimestamp() ||
+  if (coded_frame_group_start_time_ < DecodeTimestamp() ||
       buffers.front()->GetDecodeTimestamp() < DecodeTimestamp()) {
     MEDIA_LOG(ERROR, media_log_)
-        << "Cannot append a media segment with negative timestamps.";
+        << "Cannot append a coded frame group with negative timestamps.";
     return false;
   }
 
-  if (!IsNextTimestampValid(buffers.front()->GetDecodeTimestamp(),
-                            buffers.front()->is_key_frame())) {
-    const DecodeTimestamp& dts = buffers.front()->GetDecodeTimestamp();
-    MEDIA_LOG(ERROR, media_log_)
-        << "Invalid same timestamp construct detected at"
-        << " time " << dts.InSecondsF();
-
-    return false;
-  }
-
   UpdateMaxInterbufferDistance(buffers);
   SetConfigIds(buffers);
 
   // Save a snapshot of stream state before range modifications are made.
   DecodeTimestamp next_buffer_timestamp = GetNextBufferTimestamp();
   BufferQueue deleted_buffers;
 
   PrepareRangesForNextAppend(buffers, &deleted_buffers);
 
   // If there's a range for |buffers|, insert |buffers| accordingly. Otherwise,
   // create a new range with |buffers|.
   if (range_for_next_append_ != ranges_.end()) {
-    (*range_for_next_append_)->AppendBuffersToEnd(buffers);
+    if (new_coded_frame_group_ && (!splice_frames_enabled_ ||
+                                   buffers.front()->splice_buffers().empty())) {
+      // If the first append to this stream in a new coded frame group continues
+      // a previous range, use the new group's start time instead of the first
+      // new buffer's timestamp as the proof of adjacency to the existing range.
+      // A large gap (larger than our normal buffer adjacency test) can occur in
+      // a muxed set of streams (which share a common coded frame group start
+      // time) with a significantly jagged start across the streams.
+      // Don't do this logic if there was a splice frame generated for the first
+      // new buffer, since splices are guaranteed to be in the same range and
+      // adjacent, and since the splice frame's timestamp can be less than
+      // |coded_frame_group_start_time_| due to the splicing.
+      (*range_for_next_append_)
+          ->AppendBuffersToEnd(buffers, coded_frame_group_start_time_);
+    } else {
+      // Otherwise, use the first new buffer's timestamp as the proof of
+      // adjacency.
+      (*range_for_next_append_)
+          ->AppendBuffersToEnd(buffers, kNoDecodeTimestamp());
+    }
+
     last_appended_buffer_timestamp_ = buffers.back()->GetDecodeTimestamp();
     last_appended_buffer_duration_ = buffers.back()->duration();
     last_appended_buffer_is_keyframe_ = buffers.back()->is_key_frame();
   } else {
     DecodeTimestamp new_range_start_time = std::min(
-        media_segment_start_time_, buffers.front()->GetDecodeTimestamp());
+        coded_frame_group_start_time_, buffers.front()->GetDecodeTimestamp());
     const BufferQueue* buffers_for_new_range = &buffers;
     BufferQueue trimmed_buffers;
 
-    // If the new range is not being created because of a new media
-    // segment, then we must make sure that we start with a key frame.
-    // This can happen if the GOP in the previous append gets destroyed
-    // by a Remove() call.
-    if (!new_media_segment_) {
+    // If the new range is not being created because of a new coded frame group,
+    // then we must make sure that we start with a key frame.  This can happen
+    // if the GOP in the previous append gets destroyed by a Remove() call.
+    if (!new_coded_frame_group_) {
       BufferQueue::const_iterator itr = buffers.begin();
 
       // Scan past all the non-key-frames.
       while (itr != buffers.end() && !(*itr)->is_key_frame()) {
         ++itr;
       }
 
       // If we didn't find a key frame, then update the last appended
       // buffer state and return.
       if (itr == buffers.end()) {
         last_appended_buffer_timestamp_ = buffers.back()->GetDecodeTimestamp();
         last_appended_buffer_duration_ = buffers.back()->duration();
         last_appended_buffer_is_keyframe_ = buffers.back()->is_key_frame();
         DVLOG(1) << __FUNCTION__ << " " << GetStreamTypeName()
-                 << ": new buffers in the middle of media segment depend on"
+                 << ": new buffers in the middle of coded frame group depend on"
                     "keyframe that has been removed, and contain no keyframes."
                     "Skipping further processing.";
         DVLOG(1) << __FUNCTION__ << " " << GetStreamTypeName()
                  << ": done. ranges_=" << RangesToString(ranges_);
         return true;
       } else if (itr != buffers.begin()) {
         // Copy the first key frame and everything after it into
         // |trimmed_buffers|.
         trimmed_buffers.assign(itr, buffers.end());
         buffers_for_new_range = &trimmed_buffers;
       }
 
       new_range_start_time =
           buffers_for_new_range->front()->GetDecodeTimestamp();
     }
 
     range_for_next_append_ =
         AddToRanges(new SourceBufferRange(
             TypeToGapPolicy(GetType()),
             *buffers_for_new_range, new_range_start_time,
             base::Bind(&SourceBufferStream::GetMaxInterbufferDistance,
                        base::Unretained(this))));
     last_appended_buffer_timestamp_ =
         buffers_for_new_range->back()->GetDecodeTimestamp();
     last_appended_buffer_duration_ = buffers_for_new_range->back()->duration();
     last_appended_buffer_is_keyframe_ =
         buffers_for_new_range->back()->is_key_frame();
   }
 
-  new_media_segment_ = false;
+  new_coded_frame_group_ = false;
 
   MergeWithAdjacentRangeIfNecessary(range_for_next_append_);
 
   // Seek to try to fulfill a previous call to Seek().
   if (seek_pending_) {
     DCHECK(!selected_range_);
     DCHECK(deleted_buffers.empty());
     Seek(seek_buffer_timestamp_);
   }
 
@@ skipping 68 matching lines @@
       // stream, yet we also had output no buffer since the last Seek.
       // Re-seek to prevent stall.
       DVLOG(1) << __FUNCTION__ << " " << GetStreamTypeName()
                << ": re-seeking to " << seek_buffer_timestamp_
                << " to prevent stall if this time becomes buffered again";
       Seek(seek_buffer_timestamp_);
     }
   }
 }
 
+DecodeTimestamp SourceBufferStream::PotentialNextAppendTimestamp() const {
+  // The next potential append will either be just at or after
+  // |last_appended_buffer_timestamp_| (if known), or if unknown and we are
+  // still at the beginning of a new coded frame group, then will be into the
+  // range (if any) to which |coded_frame_group_start_time_| belongs.
+  if (last_appended_buffer_timestamp_ != kNoDecodeTimestamp()) {
+    return last_appended_buffer_timestamp_ +
+           base::TimeDelta::FromInternalValue(1);

[chcunningham, 2016/02/12 22:46:40]

What if the next timestamp is the same as the last timestamp? Should we not do
the +1 micro

[chcunningham, 2016/02/12 23:25:36]

We chatted. Please make a todo && crbug. This is at least no worse than it was
previously and doesn't seem to be impacted by key frame relaxation.

[wolenetz, 2016/02/24 00:34:46]

I think this +1us was useful when we previously had more strict disallowance of
Same Timestamp sequences of various kinds (now more lenient) along with
uncertainty about all buffers having nonzero duration (crbug 312836). Since
SourceBufferRange::BelongsToRange() is now more lenient w.r.t. same timestamp
sequences, I think we can relax this particular +1us. I'm on the fence about
doing that in this CL at this point, so I've gone with our previous decision to
TODO + crbug for this.

[chcunningham, 2016/02/24 19:06:26]

Acknowledged.

+  }
+
+  if (new_coded_frame_group_)
+    return coded_frame_group_start_time_;
+
+  // If we still don't know a potential next append timestamp, then we have
+  // removed the ranged to which it previously belonged and have not completed a
+  // subsequent append or received a subsequent OnStartOfCodedFrameGroup()
+  // signal.
+  return kNoDecodeTimestamp();
+}
+
 void SourceBufferStream::RemoveInternal(DecodeTimestamp start,
                                         DecodeTimestamp end,
                                         bool exclude_start,
                                         BufferQueue* deleted_buffers) {
   DVLOG(2) << __FUNCTION__ << " " << GetStreamTypeName() << " ("
            << start.InSecondsF() << ", " << end.InSecondsF() << ", "
            << exclude_start << ")";
   DVLOG(3) << __FUNCTION__ << " " << GetStreamTypeName()
            << ": before remove ranges_=" << RangesToString(ranges_);
 
   DCHECK(start >= DecodeTimestamp());
   DCHECK(start < end) << "start " << start.InSecondsF()
                       << " end " << end.InSecondsF();
   DCHECK(deleted_buffers);
 
   RangeList::iterator itr = ranges_.begin();
 
   while (itr != ranges_.end()) {
     SourceBufferRange* range = *itr;
     if (range->GetStartTimestamp() >= end)
       break;
 
     // Split off any remaining GOPs starting at or after |end| and add it to
     // |ranges_|.
     SourceBufferRange* new_range = range->SplitRange(end);
     if (new_range) {
       itr = ranges_.insert(++itr, new_range);
+
+      // Update |range_for_next_append_| if it was previously |range| and should
+      // be |new_range| now.
+      if (range_for_next_append_ != ranges_.end() &&
+          *range_for_next_append_ == range) {
+        DecodeTimestamp potential_next_append_timestamp =
+            PotentialNextAppendTimestamp();
+        if (potential_next_append_timestamp != kNoDecodeTimestamp() &&
+            new_range->BelongsToRange(potential_next_append_timestamp)) {
+          range_for_next_append_ = itr;
+        }
+      }
+
       --itr;
 
       // Update the selected range if the next buffer position was transferred
       // to |new_range|.
       if (new_range->HasNextBufferPosition())
         SetSelectedRange(new_range);
     }
 
     // Truncate the current range so that it only contains data before
     // the removal range.
@@ skipping 16 matching lines @@
     // range then delete it and move on.
     if (delete_range) {
       DeleteAndRemoveRange(&itr);
       continue;
     }
 
     // Clear |range_for_next_append_| if we determine that the removal
     // operation makes it impossible for the next append to be added
     // to the current range.
     if (range_for_next_append_ != ranges_.end() &&
-        *range_for_next_append_ == range &&
-        last_appended_buffer_timestamp_ != kNoDecodeTimestamp()) {
+        *range_for_next_append_ == range) {
       DecodeTimestamp potential_next_append_timestamp =
-          last_appended_buffer_timestamp_ +
-          base::TimeDelta::FromInternalValue(1);
+          PotentialNextAppendTimestamp();
 
       if (!range->BelongsToRange(potential_next_append_timestamp)) {
         DVLOG(1) << "Resetting range_for_next_append_ since the next append"
                  <<  " can't add to the current range.";
         range_for_next_append_ =
             FindExistingRangeFor(potential_next_append_timestamp);
       }
     }
 
     // Move on to the next range.
@@ skipping 21 matching lines @@
 bool SourceBufferStream::ShouldSeekToStartOfBuffered(
     base::TimeDelta seek_timestamp) const {
   if (ranges_.empty())
     return false;
   base::TimeDelta beginning_of_buffered =
       ranges_.front()->GetStartTimestamp().ToPresentationTime();
   return (seek_timestamp <= beginning_of_buffered &&
           beginning_of_buffered < kSeekToStartFudgeRoom());
 }
 
-bool SourceBufferStream::IsMonotonicallyIncreasing(
-    const BufferQueue& buffers) const {
+bool SourceBufferStream::IsMonotonicallyIncreasing(const BufferQueue& buffers) {
   DCHECK(!buffers.empty());
   DecodeTimestamp prev_timestamp = last_appended_buffer_timestamp_;
   bool prev_is_keyframe = last_appended_buffer_is_keyframe_;
   for (BufferQueue::const_iterator itr = buffers.begin();
        itr != buffers.end(); ++itr) {
     DecodeTimestamp current_timestamp = (*itr)->GetDecodeTimestamp();
     bool current_is_keyframe = (*itr)->is_key_frame();
     DCHECK(current_timestamp != kNoDecodeTimestamp());
     DCHECK((*itr)->duration() >= base::TimeDelta())
         << "Packet with invalid duration."
         << " pts " << (*itr)->timestamp().InSecondsF()
         << " dts " << (*itr)->GetDecodeTimestamp().InSecondsF()
         << " dur " << (*itr)->duration().InSecondsF();
 
     if (prev_timestamp != kNoDecodeTimestamp()) {
       if (current_timestamp < prev_timestamp) {
         MEDIA_LOG(ERROR, media_log_)
             << "Buffers did not monotonically increase.";
         return false;
       }
 
       if (current_timestamp == prev_timestamp &&
-          !SourceBufferRange::AllowSameTimestamp(prev_is_keyframe,
-                                                 current_is_keyframe)) {
-        MEDIA_LOG(ERROR, media_log_) << "Unexpected combination of buffers with"
-                                     << " the same timestamp detected at "
-                                     << current_timestamp.InSecondsF();
-        return false;
+          SourceBufferRange::IsUncommonSameTimestampSequence(
+              prev_is_keyframe, current_is_keyframe)) {
+        LIMITED_MEDIA_LOG(DEBUG, media_log_, num_strange_same_timestamps_logs_,
+                          kMaxStrangeSameTimestampsLogs)
+            << "Detected an append sequence with keyframe following a "
+               "non-keyframe, both with the same decode timestamp of "
+            << current_timestamp.InSecondsF();
       }
     }
 
     prev_timestamp = current_timestamp;
     prev_is_keyframe = current_is_keyframe;
   }
   return true;
 }
 
-bool SourceBufferStream::IsNextTimestampValid(
-    DecodeTimestamp next_timestamp, bool next_is_keyframe) const {
-  return (last_appended_buffer_timestamp_ != next_timestamp) ||
-      new_media_segment_ ||
-      SourceBufferRange::AllowSameTimestamp(last_appended_buffer_is_keyframe_,
-                                            next_is_keyframe);
-}
-
-
 bool SourceBufferStream::OnlySelectedRangeIsSeeked() const {
   for (RangeList::const_iterator itr = ranges_.begin();
        itr != ranges_.end(); ++itr) {
     if ((*itr)->HasNextBufferPosition() && (*itr) != selected_range_)
       return false;
   }
   return !selected_range_ || selected_range_->HasNextBufferPosition();
 }
 
 void SourceBufferStream::UpdateMaxInterbufferDistance(
@@ skipping 337 matching lines @@
     const BufferQueue& new_buffers, BufferQueue* deleted_buffers) {
   DCHECK(deleted_buffers);
 
   // Handle splices between the existing buffers and the new buffers.  If a
   // splice is generated the timestamp and duration of the first buffer in
   // |new_buffers| will be modified.
   if (splice_frames_enabled_)
     GenerateSpliceFrame(new_buffers);
 
   DecodeTimestamp prev_timestamp = last_appended_buffer_timestamp_;
-  bool prev_is_keyframe = last_appended_buffer_is_keyframe_;
   DecodeTimestamp next_timestamp = new_buffers.front()->GetDecodeTimestamp();
-  bool next_is_keyframe = new_buffers.front()->is_key_frame();
 
   if (prev_timestamp != kNoDecodeTimestamp() &&
       prev_timestamp != next_timestamp) {
     // Clean up the old buffers between the last appended buffer and the
     // beginning of |new_buffers|.
     RemoveInternal(prev_timestamp, next_timestamp, true, deleted_buffers);
   }
 
-  // Make the delete range exclusive if we are dealing with an allowed same
-  // timestamp situation. This prevents the first buffer in the current append
-  // from deleting the last buffer in the previous append if both buffers
-  // have the same timestamp.
-  //
-  // The delete range should never be exclusive if a splice frame was generated
-  // because we don't generate splice frames for same timestamp situations.
+  // Always make the start of the delete range exclusive for same timestamp
+  // across the last buffer in the previous append and the first buffer in the
+  // current append. Never be exclusive if a splice frame was generated because
+  // we don't generate splice frames for same timestamp situations.
   DCHECK(new_buffers.front()->splice_timestamp() !=
          new_buffers.front()->timestamp());
-  const bool exclude_start =
-      new_buffers.front()->splice_buffers().empty() &&
-      prev_timestamp == next_timestamp &&
-      SourceBufferRange::AllowSameTimestamp(prev_is_keyframe, next_is_keyframe);
+  const bool exclude_start = new_buffers.front()->splice_buffers().empty() &&
+                             prev_timestamp == next_timestamp;
 
   // Delete the buffers that |new_buffers| overlaps.
   DecodeTimestamp start = new_buffers.front()->GetDecodeTimestamp();
+  if (new_coded_frame_group_) {
+    // Extend the deletion range earlier to the coded frame group start time if
+    // this is the first append in a new coded frame group. Note that |start|
+    // could already be less than |coded_frame_group_start_time_| if a splice
+    // was generated.
+    DCHECK(coded_frame_group_start_time_ != kNoDecodeTimestamp());
+    start = std::min(coded_frame_group_start_time_, start);
+  }
   DecodeTimestamp end = new_buffers.back()->GetDecodeTimestamp();
   base::TimeDelta duration = new_buffers.back()->duration();
 
   // Set end time for remove to include the duration of last buffer. If the
   // duration is estimated, use 1 microsecond instead to ensure frames are not
   // accidentally removed due to over-estimation.
   if (duration != kNoTimestamp() && duration > base::TimeDelta() &&
       !new_buffers.back()->is_duration_estimated()) {
     end += duration;
   } else {
@@ skipping 815 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