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 119 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";
   } 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.
+  CHECK(!new_coded_frame_group_ || buffers.front()->is_key_frame());

[chcunningham, 2016/01/22 01:26:38]

Just to confirm/document: this is a CHECK because FrameProcessor should always
prevent such frames from making it to SourceBufferStream. Users will never hit
this if they simply append bad media. 

[wolenetz, 2016/01/22 01:50:03]

Correct. Normally I'd use a DCHECK for something like this, but the consequences
of this condition not being met merit us getting data earlier, faster. I don't
think it would be a security vuln, but per the style guide, CHECK can also be
used (temporarily): "you can also temporarily use CHECK() instead of DCHECK()
when trying to force crashes in release builds to sniff out which of your
assertions is failing."

I think this also merits me adding a TODO + crbug to change this to a
MEDIA_LOG(ERROR) (+ return false to cause decode error in release builds, or + a
DCHECK otherwise) once the CHECK has been confirmed not to be triggered by this
change initially in the wild in release builds.

wdyt?

[wolenetz, 2016/01/22 18:59:31]

From offline chat w/chcunningham@, temporary CHECK + TODO+[crbug to change to
DCHECK+MEDIA_LOG(ERROR..)+decode error] is best.
(Done)

[chcunningham, 2016/01/22 19:26:02]

Acknowledged.

+
+  // 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();
 
@@ skipping 11 matching lines @@
 
   // 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);
     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()) {
@@ skipping 24 matching lines @@
             *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 219 matching lines @@
 
     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);
+         new_coded_frame_group_ ||
+         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();
@@ skipping 1202 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