Render inband text tracks in the media pipeline

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 dbf1b031bec9d75e5d5cb2b82521d7c81b707366..0602cc52489e85416870d6419967788b79fe0f3c 100644
--- a/media/filters/chunk_demuxer.cc
+++ b/media/filters/chunk_demuxer.cc
@@ -17,7 +17,6 @@
 #include "media/base/stream_parser_buffer.h"
 #include "media/base/video_decoder_config.h"
 #include "media/filters/stream_parser_factory.h"
-#include "media/webm/webm_webvtt_parser.h"
 
 using base::TimeDelta;
 
@@ -39,12 +38,18 @@ class SourceState {
               const CreateDemuxerStreamCB& create_demuxer_stream_cb,
               const IncreaseDurationCB& increase_duration_cb);
 
+  // Called when a new text track has been parsed, so the demuxer can alert
+  // the demuxer host.
+  typedef base::Callback<void(ChunkDemuxerStream* stream,
+                              TextKind kind,
+                              const std::string& name,
+                              const std::string& language)> NewTextTrackCB;
+
   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 NewTextTrackCB& new_text_track_cb);
 
   // Appends new data to the StreamParser.
   // Returns true if the data was successfully appended. Returns false if an
@@ -90,13 +95,18 @@ class SourceState {
   bool OnNewBuffers(const StreamParser::BufferQueue& audio_buffers,
                     const StreamParser::BufferQueue& video_buffers);
 
+  void OnNewTextTrack(int text_track_num,
+                      TextKind kind,
+                      const std::string& name,
+                      const std::string& language);
+
   // Called by the |stream_parser_| when new text buffers have been parsed. It
-  // applies |timestamp_offset_| to all buffers in |buffers| and then calls
-  // |new_buffers_cb| with the modified buffers.
+  // applies |timestamp_offset_| to all buffers in |buffers| and then appends
+  // the (modified) buffers to the demuxer stream associated with
+  // the track having |text_track_number|.
   // Returns true on a successful call. Returns false if an error occured while
   // processing the buffers.
-  bool OnTextBuffers(const StreamParser::NewTextBuffersCB& new_buffers_cb,
-                     TextTrack* text_track,
+  bool OnTextBuffers(int text_track_number,
                      const StreamParser::BufferQueue& buffers);
 
   // Helper function that adds |timestamp_offset_| to each buffer in |buffers|.
@@ -142,6 +152,11 @@ class SourceState {
   ChunkDemuxerStream* video_;
   bool video_needs_keyframe_;
 
+  NewTextTrackCB new_text_track_cb_;
+
+  typedef std::map<int, ChunkDemuxerStream*> TextStreamMap;
+  TextStreamMap text_stream_map_;
+
   LogCB log_cb_;
 
   DISALLOW_COPY_AND_ASSIGN(SourceState);
@@ -193,6 +208,7 @@ class ChunkDemuxerStream : public DemuxerStream {
   // Returns false if the new config should trigger an error.
   bool UpdateAudioConfig(const AudioDecoderConfig& config, const LogCB& log_cb);
   bool UpdateVideoConfig(const VideoDecoderConfig& config, const LogCB& log_cb);
+  void UpdateTextConfig(const LogCB& log_cb);
 
   void MarkEndOfStream();
   void UnmarkEndOfStream();
@@ -261,10 +277,10 @@ SourceState::SourceState(scoped_ptr<StreamParser> stream_parser,
 void SourceState::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 NewTextTrackCB& new_text_track_cb) {
   StreamParser::NewBuffersCB audio_cb;
+  new_text_track_cb_ = new_text_track_cb;
 
   stream_parser_->Init(init_cb,
                        base::Bind(&SourceState::OnNewConfigs,
@@ -274,9 +290,10 @@ void SourceState::Init(const StreamParser::InitCB& init_cb,
                        base::Bind(&SourceState::OnNewBuffers,
                                   base::Unretained(this)),
                        base::Bind(&SourceState::OnTextBuffers,
-                                  base::Unretained(this), text_cb),
+                                  base::Unretained(this)),
                        need_key_cb,
-                       add_text_track_cb,
+                       base::Bind(&SourceState::OnNewTextTrack,
+                                  base::Unretained(this)),
                        base::Bind(&SourceState::OnNewMediaSegment,
                                   base::Unretained(this)),
                        base::Bind(&SourceState::OnEndOfMediaSegment,
@@ -432,6 +449,11 @@ bool SourceState::OnNewBuffers(const StreamParser::BufferQueue& audio_buffers,
 
     if (video_)
       video_->OnNewMediaSegment(segment_timestamp);
+
+    for (TextStreamMap::iterator itr = text_stream_map_.begin();
+         itr != text_stream_map_.end(); ++itr) {
+      itr->second->OnNewMediaSegment(segment_timestamp);
+    }
   }
 
   if (!filtered_audio.empty()) {
@@ -450,15 +472,33 @@ bool SourceState::OnNewBuffers(const StreamParser::BufferQueue& audio_buffers,
 }
 
 bool SourceState::OnTextBuffers(
-    const StreamParser::NewTextBuffersCB& new_buffers_cb,
-    TextTrack* text_track,
+    int text_track_number,
     const StreamParser::BufferQueue& buffers) {
-  if (new_buffers_cb.is_null())
-    return false;
+  if (buffers.empty())

[acolwell GONE FROM CHROMIUM, 2013/10/14 20:42:24]

nit: DCHECK(!buffers.empty()) instead.

[Matthew Heaney (Chromium), 2013/10/17 05:46:44]

Done.

[Matthew Heaney (Chromium), 2013/10/17 05:46:44]

Done.

+    return true;
+
+  TextStreamMap::iterator itr = text_stream_map_.find(text_track_number);
+  if (itr == text_stream_map_.end())
+    return true;

[acolwell GONE FROM CHROMIUM, 2013/10/14 20:42:24]

nit: I think this should return false. Doesn't this mean that we have received
cues for a track that we don't know about? This situation seems like it should
trigger an error.

[Matthew Heaney (Chromium), 2013/10/17 05:46:44]

Done.

 
   AdjustBufferTimestamps(buffers);
 
-  return new_buffers_cb.Run(text_track, buffers);
+  return itr->second->Append(buffers);
+}
+
+void SourceState::OnNewTextTrack(int text_track_number,

[acolwell GONE FROM CHROMIUM, 2013/10/14 20:42:24]

I think this may need to be folded into OnNewConfigs(). You need a global
picture of all the text tracks provided in the headers so that you can determine
whether the track numbers, kind, and language change. If there is one text track
then the track number is allowed to change, but the kind and language are not.
If there are more than one text track then the kind, language, and track numbers
must all be the same. The number of tracks must also stay constant throughout
the life of a SourceState object.

You might want to consider creating a TextTrackConfig object similar to the
AudioDecoderConfig that contains language, kind, and codec (i.e., WebVTT) so
that it is easy to transport these values together and enables Matches() and
IsValid() operations.

[Matthew Heaney (Chromium), 2013/10/17 05:46:44]

I did an incomplete implementation.  You can check it out and make sure I'm
headed in the right direction.

+                                 TextKind kind,
+                                 const std::string& name,
+                                 const std::string& language) {
+  ChunkDemuxerStream* const text_stream =
+      create_demuxer_stream_cb_.Run(DemuxerStream::TEXT);
+
+  // Interpret NULL as meaning that inband text tracks are disabled.
+  if (text_stream) {
+    text_stream->UpdateTextConfig(log_cb_);
+    text_stream_map_[text_track_number] = text_stream;
+    new_text_track_cb_.Run(text_stream, kind, name, language);
+  }
 }
 
 void SourceState::FilterWithAppendWindow(
@@ -649,6 +689,16 @@ bool ChunkDemuxerStream::UpdateVideoConfig(const VideoDecoderConfig& config,
   return stream_->UpdateVideoConfig(config);
 }
 
+void ChunkDemuxerStream::UpdateTextConfig(const LogCB& log_cb) {
+  DCHECK_EQ(type_, TEXT);
+  base::AutoLock auto_lock(lock_);
+
+  if (!stream_) {
+    DCHECK_EQ(state_, UNINITIALIZED);
+    stream_.reset(new SourceBufferStream(log_cb));
+  }

[acolwell GONE FROM CHROMIUM, 2013/10/14 20:42:24]

This needs to verify that kind and language haven't changed just like
UpdateVideoConfig() and UpdateAudioConfig() verify that the codec and parameters
like sample rate can't change from config to config.

[Matthew Heaney (Chromium), 2013/10/17 05:46:44]

See if I got OnNewConfigs right, then I can adjust this as necessary.

I am a little unclear about going from no streams (as when the source state
object is first created) to non-zero streams (e.g. the very first parse of a
webm stream). It's one thing to change the configs, but I'm talking about
first-time initialization.

+}
+
 void ChunkDemuxerStream::MarkEndOfStream() {
   base::AutoLock auto_lock(lock_);
   stream_->MarkEndOfStream();
@@ -744,14 +794,14 @@ void ChunkDemuxerStream::CompletePendingReadIfPossible_Locked() {
 
 ChunkDemuxer::ChunkDemuxer(const base::Closure& open_cb,
                            const NeedKeyCB& need_key_cb,
-                           const AddTextTrackCB& add_text_track_cb,
+                           bool enable_text,
                            const LogCB& log_cb)
     : state_(WAITING_FOR_INIT),
       cancel_next_seek_(false),
       host_(NULL),
       open_cb_(open_cb),
       need_key_cb_(need_key_cb),
-      add_text_track_cb_(add_text_track_cb),
+      enable_text_(enable_text),
       log_cb_(log_cb),
       duration_(kNoTimestamp()),
       user_specified_duration_(-1) {
@@ -828,7 +878,11 @@ DemuxerStream* ChunkDemuxer::GetStream(DemuxerStream::Type type) {
   if (type == DemuxerStream::AUDIO)
     return audio_.get();
 
-  return NULL;
+  if (!enable_text_ || text_stream_set_.empty())

[acolwell GONE FROM CHROMIUM, 2013/10/14 20:42:24]

nit: I'd prefer to not return text tracks through this interface since it
doesn't allow all tracks to be accessed and it represents the old way. Please
add a DCHECK for the TEXT type and make sure the DemuxerStream interface has a
comment indicating that |type| is not allowed to be TEXT. 

[Matthew Heaney (Chromium), 2013/10/17 05:46:44]

Done.

+    return NULL;
+
+  TextStreamSet::iterator itr = text_stream_set_.begin();
+  return *itr;
 }
 
 TimeDelta ChunkDemuxer::GetStartTime() const {
@@ -910,9 +964,8 @@ ChunkDemuxer::Status ChunkDemuxer::AddId(const std::string& id,
       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::OnNewTextTrack, base::Unretained(this)));
 
   source_state_map_[id] = source_state.release();
   return kOk;
@@ -936,6 +989,8 @@ void ChunkDemuxer::RemoveId(const std::string& id) {
       video_->Shutdown();
     source_id_video_.clear();
   }
+
+  // TODO(matthewjheaney): delete the text streams associated with this id

[acolwell GONE FROM CHROMIUM, 2013/10/14 20:42:24]

This needs to be addressed in this CL. If you let the SourceState object own the
text DemuxerStreams instead of text_stream_set_, this should be pretty straight
forward.

You will also need a way to notify pipeline that these streams are being removed
from the presentation. Adding the following signature to DemuxerHost and adding
the necessary plumbing to Pipeline & TextRenderer should work.

void RemoveTextStream(DemuxerStream* text_stream)

[Matthew Heaney (Chromium), 2013/10/17 05:46:44]

I did a partial implementation.  You can see if I'm headed in the right
direction.

 }
 
 Ranges<TimeDelta> ChunkDemuxer::GetBufferedRanges(const std::string& id) const {
@@ -1252,6 +1307,13 @@ ChunkDemuxer::~ChunkDemuxer() {
     delete it->second;
   }
   source_state_map_.clear();
+
+  TextStreamSet::iterator it = text_stream_set_.begin();

[acolwell GONE FROM CHROMIUM, 2013/10/14 20:42:24]

This would go away if you let SourceState own the text DemuxerStreams.

[Matthew Heaney (Chromium), 2013/10/17 05:46:44]

Done.

+  while (it != text_stream_set_.end()) {
+    ChunkDemuxerStream* text_stream = *it;
+    text_stream_set_.erase(it++);
+    delete text_stream;
+  }
 }
 
 void ChunkDemuxer::ReportError_Locked(PipelineStatus error) {
@@ -1342,6 +1404,14 @@ ChunkDemuxer::CreateDemuxerStream(DemuxerStream::Type type) {
       video_.reset(new ChunkDemuxerStream(DemuxerStream::VIDEO));
       return video_.get();
       break;
+    case DemuxerStream::TEXT: {
+      // TODO(matthewjheaney): we need to destroy this text stream
+      ChunkDemuxerStream* text_stream =
+          new ChunkDemuxerStream(DemuxerStream::TEXT);
+      text_stream_set_.insert(text_stream);
+      return text_stream;
+      break;
+    }
     case DemuxerStream::UNKNOWN:
     case DemuxerStream::NUM_TYPES:
       NOTREACHED();
@@ -1351,30 +1421,14 @@ ChunkDemuxer::CreateDemuxerStream(DemuxerStream::Type type) {
   return NULL;
 }
 
-bool ChunkDemuxer::OnTextBuffers(
-  TextTrack* text_track,
-  const StreamParser::BufferQueue& buffers) {
+void ChunkDemuxer::OnNewTextTrack(ChunkDemuxerStream* text_stream,
+                                  TextKind kind,
+                                  const std::string& name,
+                                  const std::string& language) {
   lock_.AssertAcquired();
   DCHECK_NE(state_, SHUTDOWN);
-
-  // TODO(matthewjheaney): IncreaseDurationIfNecessary
-
-  for (StreamParser::BufferQueue::const_iterator itr = buffers.begin();
-       itr != buffers.end(); ++itr) {
-    const StreamParserBuffer* const buffer = itr->get();
-    const TimeDelta start = buffer->timestamp();
-    const TimeDelta end = start + buffer->duration();
-
-    std::string id, settings, content;
-
-    WebMWebVTTParser::Parse(buffer->data(),
-                            buffer->data_size(),
-                            &id, &settings, &content);
-
-    text_track->addWebVTTCue(start, end, id, content, settings);
-  }
-
-  return true;
+  DCHECK(text_stream_set_.find(text_stream) != text_stream_set_.end());
+  host_->AddTextStream(text_stream, kind, name, language);
 }
 
 bool ChunkDemuxer::IsValidId(const std::string& source_id) const {
@@ -1428,6 +1482,11 @@ void ChunkDemuxer::StartReturningData() {
 
   if (video_)
     video_->StartReturningData();
+
+  for (TextStreamSet::iterator itr = text_stream_set_.begin();

[acolwell GONE FROM CHROMIUM, 2013/10/14 20:42:24]

nit: Consider iterating over the SourceState objects instead and have them each
iterate over their DemuxerStreams. You'd likely be able to move the calls on
audio_ & video_ into SourceState as well.

[Matthew Heaney (Chromium), 2013/10/17 05:46:44]

Done.

+       itr != text_stream_set_.end(); ++itr) {
+    (*itr)->StartReturningData();
+  }
 }
 
 void ChunkDemuxer::AbortPendingReads() {
@@ -1436,6 +1495,11 @@ void ChunkDemuxer::AbortPendingReads() {
 
   if (video_)
     video_->AbortReads();
+
+  for (TextStreamSet::iterator itr = text_stream_set_.begin();

[acolwell GONE FROM CHROMIUM, 2013/10/14 20:42:24]

ditto for this & the other 2 methods below.

[Matthew Heaney (Chromium), 2013/10/17 05:46:44]

Done.

+       itr != text_stream_set_.end(); ++itr) {
+    (*itr)->AbortReads();
+  }
 }
 
 void ChunkDemuxer::SeekAllSources(TimeDelta seek_time) {
@@ -1444,6 +1508,11 @@ void ChunkDemuxer::SeekAllSources(TimeDelta seek_time) {
 
   if (video_)
     video_->Seek(seek_time);
+
+  for (TextStreamSet::iterator itr = text_stream_set_.begin();
+       itr != text_stream_set_.end(); ++itr) {
+    (*itr)->Seek(seek_time);
+  }
 }
 
 void ChunkDemuxer::CompletePendingReadsIfPossible() {
@@ -1452,6 +1521,11 @@ void ChunkDemuxer::CompletePendingReadsIfPossible() {
 
   if (video_)
     video_->CompletePendingReadIfPossible();
+
+  for (TextStreamSet::iterator itr = text_stream_set_.begin();
+       itr != text_stream_set_.end(); ++itr) {
+    (*itr)->CompletePendingReadIfPossible();
+  }
 }
 
 }  // namespace media