Allow StreamParsers to request automatic timestampOffset updates.

media/filters/chunk_demuxer.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/chunk_demuxer.h"
 
 #include <algorithm>
 #include <limits>
 #include <list>
 
@@ skipping 159 matching lines @@
   // Called by the |stream_parser_| when new buffers have been parsed.
   // It processes the new buffers using |frame_processor_|, which includes
   // appending the processed frames to associated demuxer streams for each
   // frame's track.
   // Returns true on a successful call. Returns false if an error occurred while
   // processing the buffers.
   bool OnNewBuffers(const StreamParser::BufferQueue& audio_buffers,
                     const StreamParser::BufferQueue& video_buffers,
                     const StreamParser::TextBufferQueueMap& text_map);
 
+  void OnSourceInitDone(bool success,
+                        TimeDelta duration,
+                        bool auto_update_timestamp_offset);
+
   CreateDemuxerStreamCB create_demuxer_stream_cb_;
   NewTextTrackCB new_text_track_cb_;
 
   // During Append(), if OnNewBuffers() coded frame processing updates the
   // timestamp offset then |*timestamp_offset_during_append_| is also updated
   // so Append()'s caller can know the new offset. This pointer is only non-NULL
   // during the lifetime of an Append() call.
   TimeDelta* timestamp_offset_during_append_;
 
   // During Append(), coded frame processing triggered by OnNewBuffers()
@@ skipping 19 matching lines @@
   scoped_ptr<StreamParser> stream_parser_;
 
   ChunkDemuxerStream* audio_;  // Not owned by |this|.
   ChunkDemuxerStream* video_;  // Not owned by |this|.
 
   typedef std::map<StreamParser::TrackId, ChunkDemuxerStream*> TextStreamMap;
   TextStreamMap text_stream_map_;  // |this| owns the map's stream pointers.
 
   scoped_ptr<FrameProcessorBase> frame_processor_;
   LogCB log_cb_;
+  StreamParser::InitCB init_cb_;
+
+  // Indicates that timestampOffset should be updated automatically during
+  // OnNewBuffers() based on the earliest end timestamp of the buffers provided.
+  bool auto_update_timestamp_offset_;
 
   DISALLOW_COPY_AND_ASSIGN(SourceState);
 };
 
 SourceState::SourceState(
     scoped_ptr<StreamParser> stream_parser,
     scoped_ptr<FrameProcessorBase> frame_processor,
     const LogCB& log_cb,
     const CreateDemuxerStreamCB& create_demuxer_stream_cb)
     : create_demuxer_stream_cb_(create_demuxer_stream_cb),
       timestamp_offset_during_append_(NULL),
       new_media_segment_(false),
       parsing_media_segment_(false),
       stream_parser_(stream_parser.release()),
       audio_(NULL),
       video_(NULL),
       frame_processor_(frame_processor.release()),
-      log_cb_(log_cb) {
+      log_cb_(log_cb),
+      auto_update_timestamp_offset_(false) {
   DCHECK(!create_demuxer_stream_cb_.is_null());
   DCHECK(frame_processor_);
 }
 
 SourceState::~SourceState() {
   Shutdown();
 
   STLDeleteValues(&text_stream_map_);
 }
 
 void SourceState::Init(const StreamParser::InitCB& init_cb,
                        bool allow_audio,
                        bool allow_video,
                        const StreamParser::NeedKeyCB& need_key_cb,
                        const NewTextTrackCB& new_text_track_cb) {
   new_text_track_cb_ = new_text_track_cb;
+  init_cb_ = init_cb;
 
-  stream_parser_->Init(init_cb,
+  stream_parser_->Init(base::Bind(&SourceState::OnSourceInitDone,
+                                  base::Unretained(this)),
                        base::Bind(&SourceState::OnNewConfigs,
                                   base::Unretained(this),
                                   allow_audio,
                                   allow_video),
                        base::Bind(&SourceState::OnNewBuffers,
                                   base::Unretained(this)),
                        new_text_track_cb_.is_null(),
                        need_key_cb,
                        base::Bind(&SourceState::OnNewMediaSegment,
                                   base::Unretained(this)),
@@ skipping 366 matching lines @@
   new_media_segment_ = false;
 }
 
 bool SourceState::OnNewBuffers(
     const StreamParser::BufferQueue& audio_buffers,
     const StreamParser::BufferQueue& video_buffers,
     const StreamParser::TextBufferQueueMap& text_map) {
   DVLOG(2) << "OnNewBuffers()";
   DCHECK(timestamp_offset_during_append_);
 
-  return frame_processor_->ProcessFrames(
-      audio_buffers, video_buffers, text_map,
-      append_window_start_during_append_, append_window_end_during_append_,
-      &new_media_segment_, timestamp_offset_during_append_);
+  const bool success =
+      frame_processor_->ProcessFrames(audio_buffers,
+                                      video_buffers,
+                                      text_map,
+                                      append_window_start_during_append_,
+                                      append_window_end_during_append_,
+                                      &new_media_segment_,
+                                      timestamp_offset_during_append_);
+
+  if (auto_update_timestamp_offset_) {
+    const bool have_video_buffers = !video_buffers.empty();
+    base::TimeDelta new_timestamp_offset =

[acolwell GONE FROM CHROMIUM, 2014/03/17 22:52:17]

nit: Drop base:: since we've got a using directive above.

[DaleCurtis, 2014/03/18 00:14:02]

Done.

+        have_video_buffers ? video_buffers.back()->timestamp() +
+                                 video_buffers.back()->duration()
+                           : audio_buffers.back()->timestamp() +

[acolwell GONE FROM CHROMIUM, 2014/03/17 22:52:17]

You need an empty guard here if we only happened to get text track data. How
about something like this?

TimeDelta audio_end = audio_buffers.empty() ? kNoTimestamp() :
audio_buffers.back()->timestamp() + audio_buffers.back()->duration();
TimeDelta video_end = video_buffers.empty() ? kNoTimestamp() :
video_buffers.back()->timestamp() + video_buffers.back()->duration();

TimeDelta new_timestamp_offset = audio_end;
if (new_timestamp_offset == kNoTimestamp()) {
  new_timestamp_offset = video_end;
} else if (video_end != kNoTimestamp() && video_end < audio_end)
  new_timestamp_offset = video_end;
}

[DaleCurtis, 2014/03/18 00:14:02]

Reflowed to a multi-if statement + helper function which I think reads a lot
more clearly.

+                                 audio_buffers.back()->duration();
+    if (have_video_buffers && !audio_buffers.empty()) {
+      new_timestamp_offset = std::min(
+          new_timestamp_offset,
+          audio_buffers.back()->timestamp() + audio_buffers.back()->duration());
+    }
+
+    *timestamp_offset_during_append_ = new_timestamp_offset;

[acolwell GONE FROM CHROMIUM, 2014/03/17 22:52:17]

You might need to snapshot *timestamp_offset_during_append_ before the
ProcessFrames() call and only update it if the *timestamp_offset_during_append_
matches the snapshot. You might want to check w/ Matt about how this logic
should interact with his new coded frame processing algorithm that can also
update the offset.

[DaleCurtis, 2014/03/18 00:14:02]

+wolenetz, what do you think? Probably we both shouldn't be updating the
timestamp offset.  For now I'll guard by checking prev==curr.

+  }
+
+  return success;
+}
+
+void SourceState::OnSourceInitDone(bool success,
+                                   TimeDelta duration,
+                                   bool auto_update_timestamp_offset) {
+  auto_update_timestamp_offset_ = auto_update_timestamp_offset;
+  init_cb_.Run(success, duration, auto_update_timestamp_offset);
 }
 
 ChunkDemuxerStream::ChunkDemuxerStream(Type type)
     : type_(type),
       state_(UNINITIALIZED) {
 }
 
 void ChunkDemuxerStream::StartReturningData() {
   DVLOG(1) << "ChunkDemuxerStream::StartReturningData()";
   base::AutoLock auto_lock(lock_);
@@ skipping 754 matching lines @@
   lock_.AssertAcquired();
   for (SourceStateMap::const_iterator itr = source_state_map_.begin();
        itr != source_state_map_.end(); ++itr) {
     if (itr->second->IsSeekWaitingForData())
       return true;
   }
 
   return false;
 }
 
-void ChunkDemuxer::OnSourceInitDone(bool success, TimeDelta duration) {
-  DVLOG(1) << "OnSourceInitDone(" << success << ", "
-           << duration.InSecondsF() << ")";
+void ChunkDemuxer::OnSourceInitDone(bool success,
+                                    TimeDelta duration,
+                                    bool auto_update_timestamp_offset) {

[acolwell GONE FROM CHROMIUM, 2014/03/17 22:52:17]

nit: No need to expose this detail to ChunkDemuxer. Just leave this signature as
is and have SourceState provide a typedef for its own InitCB instead of using
the StreamParser::InitCB.

[DaleCurtis, 2014/03/18 00:14:02]

Done.

+  DVLOG(1) << "OnSourceInitDone(" << success << ", " << duration.InSecondsF()
+           << ", " << auto_update_timestamp_offset << ")";
   lock_.AssertAcquired();
   DCHECK_EQ(state_, INITIALIZING);
   if (!success || (!audio_ && !video_)) {
     ReportError_Locked(DEMUXER_ERROR_COULD_NOT_OPEN);
     return;
   }
 
   if (duration != TimeDelta() && duration_ == kNoTimestamp())
     UpdateDuration(duration);
 
@@ skipping 141 matching lines @@
 }
 
 void ChunkDemuxer::ShutdownAllStreams() {
   for (SourceStateMap::iterator itr = source_state_map_.begin();
        itr != source_state_map_.end(); ++itr) {
     itr->second->Shutdown();
   }
 }
 
 }  // namespace media