Render inband text tracks in the media pipeline

media/base/text_decoder.h

Index: media/base/text_decoder.h
diff --git a/media/base/text_decoder.h b/media/base/text_decoder.h
new file mode 100644
index 0000000000000000000000000000000000000000..6a2d6220ff5a843b4d3795320367a62829dcbee5
--- /dev/null
+++ b/media/base/text_decoder.h
@@ -0,0 +1,45 @@
+// Copyright (c) 2013 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.
+
+#ifndef MEDIA_BASE_TEXT_DECODER_H_
+#define MEDIA_BASE_TEXT_DECODER_H_
+
+#include "base/callback.h"
+#include "base/memory/ref_counted.h"
+#include "media/base/media_export.h"
+
+namespace media {
+
+class Demuxer;
+class TextBuffer;
+
+class MEDIA_EXPORT TextDecoder {
+ public:
+  TextDecoder();
+  virtual ~TextDecoder();
+
+  // Initialize a TextDecoder from the text streams from the given demuxer.
+  virtual void Initialize(Demuxer* demuxer) = 0;

[acolwell GONE FROM CHROMIUM, 2013/09/12 00:15:15]

ISTM that this should be taking a DemuxerStream and not a Demuxer.

[Matthew Heaney (Chromium), 2013/09/13 19:51:54]

The pipeline model is that there is one decoder per renderer.  There is only a
single text renderer (it's a state machine, basically), hence there is only a
single text decoder.

[acolwell GONE FROM CHROMIUM, 2013/09/13 20:57:30]

Text is blazing a new trail here since it is the first datatype to actually
support multiple tracks. When we actually implement multiple audio & video
tracks, I believe we will have a model where 1 renderer will have multiple
decoders. This is because those renderers may consume multiple tracks, but it is
responsible for combining them into a single output. 

In the case of text tracks, it isn't clear to me that you'll actually need a
TextDecoder once you have a DemuxerTextStream that actually hands out parsed
queues. It seems like the renderer will just be responsible for pulling cues
from the DemuxerTextStream and forwarding them to the media::TextTrack that is
created by WebMediaPlayerImpl.

It is also here in the TextRenderer that I think you'll be able to handle your
"dropped cue" problem in the other CL. I think the renderer shouldn't hand out
cues until play is called on the renderer. That way you should be able to enable
the text track before playback begins. That is just my current hunch based on
how this CL is shaping up. I'm fine if that fix is deferred to a followup CL.

[Matthew Heaney (Chromium), 2013/09/20 23:53:54]

I haven't done anything yet with a DemuxerTextStream, so this patch set still
contains a separate TextDecoder.  I wanted to get the big issues worked out
first.

+
+  // Request frame from the demuxer stream having the given |index|, to be
+  // decoded and returned as a text track cue via the provided callback. Only
+  // one read per demuxer stream may be in flight at any given time.
+  //
+  // Implementations guarantee that the callback will not be called from within
+  // this method.
+  //
+  // A non-NULL text buffer pointer will contain a decoded text track cue.
+  // A NULL buffer pointer indicates end-of-stream, or error.
+  typedef base::Callback<void(int index,
+                              const scoped_refptr<TextBuffer>& text_buffer)>
+      ReadCB;
+  virtual void Read(int index, const ReadCB& read_cb) = 0;

[acolwell GONE FROM CHROMIUM, 2013/09/12 00:15:15]

Why do you need an index here? It seems like there should be a 1:1 relationship
between TextDecoders & DemuxerStreams.

[Matthew Heaney (Chromium), 2013/09/13 19:51:54]

It doesn't work that way.  There is only a single text decoder.  It uses the
index to associate read requests with a specific stream.

[acolwell GONE FROM CHROMIUM, 2013/09/13 20:57:30]

Right. I was trying to say that I think there should be 1 decoder instance per
stream since that is likely what would be the case for audio & video when
multi-track support is added to those track types. Since I don't think you'll
need this decoder class anymore, this is likely moot.

[Matthew Heaney (Chromium), 2013/09/20 23:53:54]

I did change this from int to DemuxerStream, but the TextDecoder is still there.
 You're right that the TextDecoder doesn't do much, and it might be possible to
get rid of it.  I just needed to make sure the threading issues got resolved
first.

+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TextDecoder);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_TEXT_DECODER_H_