Add Chromium-side support for SourceBuffer.appendWindowStart and SourceBuffer.appendWindowEnd.

media/filters/chunk_demuxer.cc

Index: media/filters/chunk_demuxer.cc
diff --git a/media/filters/chunk_demuxer.cc b/media/filters/chunk_demuxer.cc
index 29bfc6fb5577860040173e8d7fff5362ded679b7..edd9dd469e5f093a0c5a5b6e7c905c20de428c09 100644
--- a/media/filters/chunk_demuxer.cc
+++ b/media/filters/chunk_demuxer.cc
@@ -37,15 +37,15 @@ class SourceState {
 
   SourceState(scoped_ptr<StreamParser> stream_parser, const LogCB& log_cb,
               const CreateDemuxerStreamCB& create_demuxer_stream_cb,
-              const IncreaseDurationCB& increase_duration_cb);
+              const IncreaseDurationCB& increase_duration_cb,
+              const StreamParser::NewMediaSegmentCB& new_segment_cb);
 
   void Init(const StreamParser::InitCB& init_cb,
             bool allow_audio,
             bool allow_video,
             const StreamParser::NewTextBuffersCB& text_cb,
             const StreamParser::NeedKeyCB& need_key_cb,
-            const AddTextTrackCB& add_text_track_cb,
-            const StreamParser::NewMediaSegmentCB& new_segment_cb);
+            const AddTextTrackCB& add_text_track_cb);
 
   // Appends new data to the StreamParser.
   // Returns true if the data was successfully appended. Returns false if an
@@ -62,6 +62,9 @@ class SourceState {
 
   TimeDelta timestamp_offset() const { return timestamp_offset_; }
 
+  void SetAppendWindowStart(TimeDelta start) { append_window_start_ = start; }

[scherkus (not reviewing), 2013/07/24 22:47:41]

these should be unix_hacker style as they're inlined simple setters

[acolwell GONE FROM CHROMIUM, 2013/07/25 20:39:33]

Done.

+  void SetAppendWindowEnd(TimeDelta end) { append_window_end_ = end; }
+
  private:
   // Called by the |stream_parser_| when a new initialization segment is
   // encountered.
@@ -73,22 +76,19 @@ class SourceState {
 
   // Called by the |stream_parser_| at the beginning of a new media segment.
   // |timestamp| is the timestamp on the first buffer in the segment.
-  // It modifies the state of this object and then calls |new_segment_cb| with
-  // modified version of |timestamp|.
-  void OnNewMediaSegment(const StreamParser::NewMediaSegmentCB& new_segment_cb,
-                         TimeDelta timestamp);
+  void OnNewMediaSegment(TimeDelta timestamp);
 
   // Called by the |stream_parser_| at the end of a media segment.
   void OnEndOfMediaSegment();
 
   // Called by the |stream_parser_| when new buffers have been parsed. It
-  // applies |timestamp_offset_| to all buffers in |buffers| and then calls
-  // Append() on either |audio_| or |video_| with the modified buffers based on
-  // the value of |type|.
+  // applies |timestamp_offset_| to all buffers in |audio_buffers| and
+  // |video_buffers| and then calls Append() on |audio_| and/or
+  // |video_| with the modified buffers.
   // Returns true on a successful call. Returns false if an error occured while
   // processing the buffers.
-  bool OnBuffers(DemuxerStream::Type type,
-                 const StreamParser::BufferQueue& buffers);
+  bool OnNewBuffers(const StreamParser::BufferQueue& audio_buffers,
+                    const StreamParser::BufferQueue& video_buffers);
 
   // Called by the |stream_parser_| when new text buffers have been parsed. It
   // applies |timestamp_offset_| to all buffers in |buffers| and then calls
@@ -102,12 +102,33 @@ class SourceState {
   // Helper function that adds |timestamp_offset_| to each buffer in |buffers|.
   void AdjustBufferTimestamps(const StreamParser::BufferQueue& buffers);
 
+  // Filters out buffers that are outside of the append window
+  // [|append_window_start_|, |append_window_end_|).
+  // |needs_keyframe| is a pointer to the |xxx_need_keyframe_| flag
+  // associated with the |buffers|. Its state is read an updated as
+  // this method filters |buffers|.
+  // Returns a filtered queue of buffers that are inside the append window.
+  StreamParser::BufferQueue FilterWithAppendWindow(
+      const StreamParser::BufferQueue& buffers, bool* needs_keyframe);
+
   CreateDemuxerStreamCB create_demuxer_stream_cb_;
   IncreaseDurationCB increase_duration_cb_;
+  StreamParser::NewMediaSegmentCB new_segment_cb_;
 
   // The offset to apply to media segment timestamps.
   TimeDelta timestamp_offset_;
 
+  TimeDelta append_window_start_;
+  TimeDelta append_window_end_;
+
+  // Set to true if the next buffers appended within the append window
+  // represent the start of a new media segment. This flag being set
+  // triggers a call to |new_segment_cb_| when the new buffers are
+  // appended. The flag is set on actual media segment boundaries and
+  // when the "append window" filtering causes discontinuities in the
+  // appended data.
+  bool new_media_segment_;
+
   // Keeps track of whether |timestamp_offset_| can be modified.
   bool can_update_offset_;
 
@@ -115,7 +136,10 @@ class SourceState {
   scoped_ptr<StreamParser> stream_parser_;
 
   ChunkDemuxerStream* audio_;
+  bool audio_needs_keyframe_;
+
   ChunkDemuxerStream* video_;
+  bool video_needs_keyframe_;
 
   LogCB log_cb_;
 
@@ -217,16 +241,24 @@ class ChunkDemuxerStream : public DemuxerStream {
 SourceState::SourceState(scoped_ptr<StreamParser> stream_parser,
                          const LogCB& log_cb,
                          const CreateDemuxerStreamCB& create_demuxer_stream_cb,
-                         const IncreaseDurationCB& increase_duration_cb)
+                         const IncreaseDurationCB& increase_duration_cb,
+                         const StreamParser::NewMediaSegmentCB& new_segment_cb)
     : create_demuxer_stream_cb_(create_demuxer_stream_cb),
       increase_duration_cb_(increase_duration_cb),
+      new_segment_cb_(new_segment_cb),
+      append_window_start_(TimeDelta()),

[scherkus (not reviewing), 2013/07/24 22:47:41]

do you have this here to make it more explicit?

it's technically not needed as the default ctor is used anyway 

[acolwell GONE FROM CHROMIUM, 2013/07/25 20:39:33]

Yeah that was the original idea, but it's a pretty lame reason. Removed.

+      append_window_end_(kInfiniteDuration()),
+      new_media_segment_(false),
       can_update_offset_(true),
       stream_parser_(stream_parser.release()),
       audio_(NULL),
+      audio_needs_keyframe_(true),
       video_(NULL),
+      video_needs_keyframe_(true),
       log_cb_(log_cb) {
   DCHECK(!create_demuxer_stream_cb_.is_null());
   DCHECK(!increase_duration_cb_.is_null());
+  DCHECK(!new_segment_cb_.is_null());
 }
 
 void SourceState::Init(const StreamParser::InitCB& init_cb,
@@ -234,34 +266,22 @@ void SourceState::Init(const StreamParser::InitCB& init_cb,
                        bool allow_video,
                        const StreamParser::NewTextBuffersCB& text_cb,
                        const StreamParser::NeedKeyCB& need_key_cb,
-                       const AddTextTrackCB& add_text_track_cb,
-                       const StreamParser::NewMediaSegmentCB& new_segment_cb) {
+                       const AddTextTrackCB& add_text_track_cb) {
   StreamParser::NewBuffersCB audio_cb;
-  StreamParser::NewBuffersCB video_cb;
-
-  if (allow_audio) {
-    audio_cb = base::Bind(&SourceState::OnBuffers,
-                          base::Unretained(this), DemuxerStream::AUDIO);
-  }
-
-  if (allow_video) {
-    video_cb = base::Bind(&SourceState::OnBuffers,
-                          base::Unretained(this), DemuxerStream::VIDEO);
-  }
 
   stream_parser_->Init(init_cb,
                        base::Bind(&SourceState::OnNewConfigs,
                                   base::Unretained(this),
                                   allow_audio,
                                   allow_video),
-                       audio_cb,
-                       video_cb,
+                       base::Bind(&SourceState::OnNewBuffers,
+                                  base::Unretained(this)),
                        base::Bind(&SourceState::OnTextBuffers,
                                   base::Unretained(this), text_cb),
                        need_key_cb,
                        add_text_track_cb,
                        base::Bind(&SourceState::OnNewMediaSegment,
-                                  base::Unretained(this), new_segment_cb),
+                                  base::Unretained(this)),
                        base::Bind(&SourceState::OnEndOfMediaSegment,
                                   base::Unretained(this)),
                        log_cb_);
@@ -281,6 +301,8 @@ bool SourceState::Append(const uint8* data, size_t length) {
 
 void SourceState::Abort() {
   stream_parser_->Flush();
+  audio_needs_keyframe_ = true;
+  video_needs_keyframe_ = true;
   can_update_offset_ = true;

[scherkus (not reviewing), 2013/07/24 22:47:41]

sanity check: does new_media_segment_ need to be updated here?

[acolwell GONE FROM CHROMIUM, 2013/07/25 20:39:33]

No. We will always get an OnNewMediaSegment() call before any OnNewBuffers()
call that occurs after this function.

 }
 
@@ -362,43 +384,65 @@ bool SourceState::OnNewConfigs(bool allow_audio, bool allow_video,
   return success;
 }
 
-void SourceState::OnNewMediaSegment(
-    const StreamParser::NewMediaSegmentCB& new_segment_cb,
-    TimeDelta timestamp) {
+void SourceState::OnNewMediaSegment(TimeDelta timestamp) {

[scherkus (not reviewing), 2013/07/24 22:47:41]

|timestamp| isn't used anymore other than for DCHECK/DVLOG

time to remove?

[acolwell GONE FROM CHROMIUM, 2013/07/25 20:39:33]

Done.

   DCHECK(timestamp != kNoTimestamp());
-  DVLOG(2) << "OnNewMediaSegment(" << timestamp.InSecondsF() << ")";
-
+  DVLOG(2) << "SourceState::OnNewMediaSegment("
+           << timestamp.InSecondsF() << ")";
   can_update_offset_ = false;
-  new_segment_cb.Run(timestamp + timestamp_offset_);
+  new_media_segment_ = true;
 }
 
 void SourceState::OnEndOfMediaSegment() {
   DVLOG(2) << "OnEndOfMediaSegment()";
   can_update_offset_ = true;
+  new_media_segment_ = false;
 }
 
-bool SourceState::OnBuffers(DemuxerStream::Type type,
-                            const StreamParser::BufferQueue& buffers) {
-  DCHECK(!buffers.empty());
-  AdjustBufferTimestamps(buffers);
+bool SourceState::OnNewBuffers(const StreamParser::BufferQueue& audio_buffers,
+                               const StreamParser::BufferQueue& video_buffers) {
+  DCHECK(!audio_buffers.empty() || !video_buffers.empty());
+  AdjustBufferTimestamps(audio_buffers);
+  AdjustBufferTimestamps(video_buffers);
 
-  ChunkDemuxerStream* stream = NULL;
-  switch (type) {
-    case DemuxerStream::AUDIO:
-      stream = audio_;
-      break;
-    case DemuxerStream::VIDEO:
-      stream = video_;
-      break;
-    case DemuxerStream::UNKNOWN:
-    case DemuxerStream::NUM_TYPES:
-      NOTREACHED();
+  StreamParser::BufferQueue filtered_audio =
+      FilterWithAppendWindow(audio_buffers, &audio_needs_keyframe_);
+
+  StreamParser::BufferQueue filtered_video =
+      FilterWithAppendWindow(video_buffers, &video_needs_keyframe_);
+
+  if (filtered_audio.empty() && filtered_video.empty())
+    return true;
+
+  if (new_media_segment_) {
+    // Find the earliest timestamp in the filtered buffers and use

[scherkus (not reviewing), 2013/07/24 22:47:41]

nit: you can fit a few more words on this line

[acolwell GONE FROM CHROMIUM, 2013/07/25 20:39:33]

Done.

+    // that for the segment start timestamp.
+    TimeDelta segment_timestamp = kNoTimestamp();
+
+    if (!filtered_audio.empty())
+      segment_timestamp = filtered_audio.front()->GetDecodeTimestamp();
+
+    if (!filtered_video.empty() &&
+        (segment_timestamp == kNoTimestamp() ||
+         filtered_video.front()->GetDecodeTimestamp() < segment_timestamp)) {
+      segment_timestamp = filtered_video.front()->GetDecodeTimestamp();
+    }
+
+    new_media_segment_ = false;

[scherkus (not reviewing), 2013/07/24 22:47:41]

sanity check: does can_update_offset_ need to be updated here?

[acolwell GONE FROM CHROMIUM, 2013/07/25 20:39:33]

No. can_update_offset_ tracks the actual media segments being appended. The spec
says to reject timestamp offset changes when the application is in the middle of
appending a media segment.

new_media_segment_ essentially keeps track of the beginning of bytestream media
segments as well as what the spec calls a "coded frame group". I'm just using
"media segment" here because historically there wasn't a difference between
"media segments" and "coded frame groups." Now because of the append window,
multiple "coded frame groups" can be created within a single "media segment". 

If it is ok, I'd prefer to untangle the meanings of "media segment" and "coded
frame group" here and in SourceBufferStream in a followup CL. SourceState should
care about media segments, but SourceBufferStream should only care about "coded
frame groups". I've mixed the concepts here so that SourceBufferStream is
getting all the calls it was getting before, plus a few additional ones caused
by the append window.

+    new_segment_cb_.Run(segment_timestamp);
+  }
+
+  if (!filtered_audio.empty()) {
+    if (!audio_ || !audio_->Append(filtered_audio))
       return false;
+    increase_duration_cb_.Run(filtered_audio.back()->timestamp(), audio_);
+  }
+
+  if (!filtered_video.empty()) {
+    if (!video_ || !video_->Append(filtered_video))
+      return false;
+    increase_duration_cb_.Run(filtered_video.back()->timestamp(), video_);
   }
 
-  if (!stream->Append(buffers))
-    return false;
-  increase_duration_cb_.Run(buffers.back()->timestamp(), stream);
   return true;
 }
 
@@ -414,6 +458,60 @@ bool SourceState::OnTextBuffers(
   return new_buffers_cb.Run(text_track, buffers);
 }
 
+StreamParser::BufferQueue SourceState::FilterWithAppendWindow(

[scherkus (not reviewing), 2013/07/24 22:47:41]

do we know how large this queue can get?

any reason not to use an out-param to avoid copying the std::deque and
ref+deref?


(I was intrigued by this question and did a quick benchmark... on release build
copying a std::deque of scoped_refptr<T*> vs T* is ~3x slower, but I was using
very large std::deques... but still!)

[acolwell GONE FROM CHROMIUM, 2013/07/25 20:39:33]

Nope. Done.

+    const StreamParser::BufferQueue& buffers, bool* needs_keyframe) {
+  StreamParser::BufferQueue filtered_buffers;
+  DCHECK(needs_keyframe);
+
+  // This loop implements steps 1.9, 1.10, & 1.11 of the "Coded frame
+  // processing loop" in the Media Source Extensions spec.
+  // https://dvcs.w3.org/hg/html-media/raw-file/default/media-source/media-source.html#sourcebuffer-coded-frame-processing
+  // These steps filter out buffers that are not within the "append
+  // window" and handles resyncing on the next random access point
+  // (i.e., next keyframe) if a buffer gets dropped.
+  for (StreamParser::BufferQueue::const_iterator itr = buffers.begin();
+       itr != buffers.end(); ++itr) {
+    // Filter out buffers that are outside the append window. Anytime
+    // a buffer gets dropped we need to set |*needs_keyframe| to true
+    // because we can only resume decoding at keyframes.
+    TimeDelta presentation_timestamp = (*itr)->timestamp();
+
+    // TODO(acolwell): Change |frame_end_timestamp| value to
+    // |presentation_timestamp + (*itr)->duration()|, like the spec
+    // requires, once frame durations are actually present in all buffers.
+    TimeDelta frame_end_timestamp = presentation_timestamp;
+    if (presentation_timestamp < append_window_start_ ||
+        frame_end_timestamp > append_window_end_) {
+      DVLOG(1) << "Dropping buffer outside append window."
+               << " presentation_timestamp "
+               << presentation_timestamp.InSecondsF();
+      *needs_keyframe = true;
+
+      // This triggers a discontinuity so we need to treat the next frames
+      // appended within the append window as if they were the beginning of a
+      // new segment.
+      new_media_segment_ = true;

[scherkus (not reviewing), 2013/07/24 22:47:41]

sanity check: does can_update_offset_ need to be updated here?

[acolwell GONE FROM CHROMIUM, 2013/07/25 20:39:33]

No. This is a "virtual" media segment (i.e., a new coded frame group) within the
current media segment. can_update_offset_ should be and remain false here
because we are still within the bytestream media segment.

+      continue;
+    }
+
+    // If |*needs_keyframe| is true then filter out buffers until we
+    // encounter the next keyframe.
+    if (*needs_keyframe) {
+      if (!(*itr)->IsKeyframe()) {
+        DVLOG(1) << "Dropping non-keyframe. presentation_timestamp "
+                 << presentation_timestamp.InSecondsF();
+        continue;
+      }
+
+      *needs_keyframe = false;
+    }
+
+    filtered_buffers.push_back(*itr);
+  }
+
+  return filtered_buffers;
+}
+
 ChunkDemuxerStream::ChunkDemuxerStream(Type type)
     : type_(type),
       state_(UNINITIALIZED) {
@@ -514,6 +612,8 @@ Ranges<TimeDelta> ChunkDemuxerStream::GetBufferedRanges(
 }
 
 void ChunkDemuxerStream::OnNewMediaSegment(TimeDelta start_timestamp) {
+  DVLOG(2) << "ChunkDemuxerStream::OnNewMediaSegment("
+           << start_timestamp.InSecondsF() << ")";
   base::AutoLock auto_lock(lock_);
   stream_->OnNewMediaSegment(start_timestamp);
 }
@@ -802,15 +902,17 @@ ChunkDemuxer::Status ChunkDemuxer::AddId(const std::string& id,
                       base::Bind(&ChunkDemuxer::CreateDemuxerStream,
                                  base::Unretained(this)),
                       base::Bind(&ChunkDemuxer::IncreaseDurationIfNecessary,
-                                 base::Unretained(this))));
+                                 base::Unretained(this)),
+                      base::Bind(&ChunkDemuxer::OnNewMediaSegment,
+                                 base::Unretained(this), id)));
+
   source_state->Init(
       base::Bind(&ChunkDemuxer::OnSourceInitDone, base::Unretained(this)),
       has_audio,
       has_video,
       base::Bind(&ChunkDemuxer::OnTextBuffers, base::Unretained(this)),
       need_key_cb_,
-      add_text_track_cb_,
-      base::Bind(&ChunkDemuxer::OnNewMediaSegment, base::Unretained(this), id));
+      add_text_track_cb_);
 
   source_state_map_[id] = source_state.release();
   return kOk;
@@ -1093,6 +1195,22 @@ void ChunkDemuxer::UnmarkEndOfStream() {
     video_->UnmarkEndOfStream();
 }
 
+void ChunkDemuxer::SetAppendWindowStart(const std::string& id,
+                                        TimeDelta start) {
+  base::AutoLock auto_lock(lock_);
+  DVLOG(1) << "SetAppendWindowStart(" << id << ", "
+           << start.InSecondsF() << ")";
+  CHECK(IsValidId(id));
+  source_state_map_[id]->SetAppendWindowStart(start);
+}
+
+void ChunkDemuxer::SetAppendWindowEnd(const std::string& id, TimeDelta end) {
+  base::AutoLock auto_lock(lock_);
+  DVLOG(1) << "SetAppendWindowEnd(" << id << ", " << end.InSecondsF() << ")";
+  CHECK(IsValidId(id));
+  source_state_map_[id]->SetAppendWindowEnd(end);
+}
+
 void ChunkDemuxer::Shutdown() {
   DVLOG(1) << "Shutdown()";
   base::AutoLock auto_lock(lock_);
@@ -1262,7 +1380,7 @@ bool ChunkDemuxer::OnTextBuffers(
 void ChunkDemuxer::OnNewMediaSegment(const std::string& source_id,
                                      TimeDelta timestamp) {
   DCHECK(timestamp != kNoTimestamp());
-  DVLOG(2) << "OnNewMediaSegment(" << source_id << ", "
+  DVLOG(2) << "ChunkDemuxer::OnNewMediaSegment(" << source_id << ", "

[scherkus (not reviewing), 2013/07/24 22:47:41]

FYI the DVLOGs in this class are inconsistent re: class name (e.g., Shutdown())

was this an intentional change? should we remove all the CD:: or add CD::?

[acolwell GONE FROM CHROMIUM, 2013/07/25 20:39:33]

Actually this method doesn't need to exist anymore. I've merged the body into
SourceState::OnNewBuffers() since that is the only place this method was being
called from via the new_segment_cb_. Removed all new_segment_cb_ code as well.

            << timestamp.InSecondsF() << ")";
   lock_.AssertAcquired();