Render inband text tracks in the media pipeline

media/filters/text_renderer_impl.cc

Index: media/filters/text_renderer_impl.cc
diff --git a/media/filters/text_renderer_impl.cc b/media/filters/text_renderer_impl.cc
new file mode 100644
index 0000000000000000000000000000000000000000..905b8640b130a51a14264cf9e180900b092d55f9
--- /dev/null
+++ b/media/filters/text_renderer_impl.cc
@@ -0,0 +1,229 @@
+// 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.
+
+#include "media/filters/text_renderer_impl.h"
+
+#include "base/bind.h"
+#include "base/callback_helpers.h"
+#include "base/logging.h"
+#include "base/message_loop/message_loop_proxy.h"
+#include "media/base/demuxer.h"
+#include "media/base/demuxer_stream.h"
+#include "media/base/text_buffer.h"
+#include "media/base/text_decoder.h"
+
+namespace media {
+
+TextRendererImpl::TextRendererImpl(
+    const scoped_refptr<base::MessageLoopProxy>& message_loop,
+    scoped_ptr<TextDecoder> decoder,
+    const CueReadyCB& cue_ready_cb)
+    : message_loop_(message_loop),
+      weak_factory_(this),
+      decoder_(decoder.Pass()),
+      cue_ready_cb_(cue_ready_cb),
+      state_(kUninitialized),
+      text_stream_count_(0),
+      pending_read_count_(0),
+      eos_count_(0) {
+}
+
+TextRendererImpl::~TextRendererImpl() {
+  DCHECK(state_ == kUninitialized || state_ == kStopped);
+  DCHECK_EQ(pending_read_count_, 0);
+}
+
+void TextRendererImpl::Initialize(Demuxer* demuxer,
+                                  const PipelineStatusCB& init_cb,
+                                  const base::Closure& ended_cb) {
+  DCHECK(message_loop_->BelongsToCurrentThread());
+  DCHECK_NE(demuxer, static_cast<Demuxer*>(NULL));
+  DCHECK(!init_cb.is_null());
+  DCHECK(!ended_cb.is_null());
+  DCHECK_EQ(kUninitialized, state_);
+  DCHECK_EQ(text_stream_count_, 0);
+  DCHECK_EQ(pending_read_count_, 0);
+  DCHECK_EQ(eos_count_, 0);
+  DCHECK(init_cb_.is_null());
+  DCHECK(ended_cb_.is_null());
+
+  weak_this_ = weak_factory_.GetWeakPtr();
+  init_cb_ = init_cb;
+  ended_cb_ = ended_cb;
+
+  decoder_->Initialize(demuxer);
+
+  const int stream_count = demuxer->GetStreamCount();
+  read_state_.reserve(stream_count);
+
+  for (int idx = 0; idx < stream_count; ++idx) {
+    DemuxerStream* stream = demuxer->GetStreamByIndex(idx);
+    if (stream == NULL || stream->type() != DemuxerStream::TEXT) {
+      read_state_.push_back(kReadInactive);
+    } else {
+      read_state_.push_back(kReadIdle);
+      ++text_stream_count_;
+    }
+  }
+
+  // Start the machine in the stopped state. We transition out
+  // of the stopped state via a Pause() or Play() request.
+  state_ = kStopped;

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

It seems like state_ = kPaused and eos_count_ = text_stream_count_ should be set
here instead.

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

Done.

+
+  base::ResetAndReturn(&init_cb_).Run(PIPELINE_OK);

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

This method doesn't look like it actually needs to be asynchronous so there is
no need for the init_cb parameter or init_cb_ member variable.

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

Done.

+}
+
+void TextRendererImpl::Play(const base::Closure& callback) {
+  DCHECK(message_loop_->BelongsToCurrentThread());
+  base::AutoLock auto_lock(lock_);
+  DCHECK_NE(state_, kUninitialized);

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

nit: Remove since the DCHECK below is more specific and would fail if the
condition here was true.

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

Done.

+  DCHECK(state_ == kPaused || state_ == kStopped);
+
+  if (state_ == kStopped)
+    eos_count_ = text_stream_count_;
+
+  state_ = kPlaying;
+  callback.Run();
+
+  for (std::vector<ReadState>::size_type idx = 0;
+       idx < read_state_.size(); ++idx) {
+    if (read_state_[idx] == kReadInactive)
+      continue;
+
+    if (read_state_[idx] == kReadPending) {
+      DCHECK_GT(pending_read_count_, 0);
+      continue;
+    }
+
+    read_state_[idx] = kReadPending;
+    ++pending_read_count_;
+    decoder_->Read(idx, base::Bind(&TextRendererImpl::CueReady, weak_this_));
+  }
+}
+
+void TextRendererImpl::Pause(const base::Closure& callback) {
+  DCHECK(message_loop_->BelongsToCurrentThread());
+  base::AutoLock auto_lock(lock_);
+  DCHECK_NE(state_, kUninitialized);
+  DCHECK_NE(state_, kStopPending);
+  pause_cb_ = callback;
+
+  if (state_ == kStopped) {
+    eos_count_ = text_stream_count_;

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

Why is this line here? This doesn't feel right to me.

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

Done.

+    state_ = kPaused;
+    base::ResetAndReturn(&pause_cb_).Run();
+    return;
+  }
+
+  if (state_ == kPaused) {

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

You shouldn't get a Pause() call if you are already paused. I think you should
DCHECK above

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

Done.

+    DCHECK_EQ(pending_read_count_, 0);
+    base::ResetAndReturn(&pause_cb_).Run();
+    return;
+  }
+
+  if (state_ == kPausePending)

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

You shouldn't get another Pause() call if the current one is still pending.
Update DCHECKs

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

Done.

+    return;
+
+  if (pending_read_count_ <= 0) {
+    state_ = kPaused;
+    base::ResetAndReturn(&pause_cb_).Run();
+    return;
+  }
+
+  state_ = kPausePending;
+}
+

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

It seems like you would need a Flush() method so that you can properly set
eos_count_ = text_stream_count_ on a seek.

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

Done.

+void TextRendererImpl::Stop(const base::Closure& cb) {
+  DCHECK(message_loop_->BelongsToCurrentThread());
+  DCHECK(!cb.is_null());
+  base::AutoLock auto_lock(lock_);
+  DCHECK_NE(state_, kUninitialized);
+
+  stop_cb_ = cb;
+
+  if (state_ == kStopped) {

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

You shound't get a stop if you have already received a Stop() call.

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

Done.

+    DCHECK_EQ(pending_read_count_, 0);
+    base::ResetAndReturn(&stop_cb_).Run();
+    return;
+  }
+
+  if (state_ == kStopPending) {

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

You should not get a Stop() if the previous one is still pending.

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

Done.

+    DCHECK_GT(pending_read_count_, 0);
+    stop_cb_ = cb;
+    return;
+  }
+
+  if (pending_read_count_ <= 0) {
+    state_ = kStopped;
+    base::ResetAndReturn(&stop_cb_).Run();
+    return;
+  }
+
+  state_ = kStopPending;
+}
+
+void TextRendererImpl::CueReady(
+    int index,
+    const scoped_refptr<TextBuffer>& buffer) {
+  base::AutoLock auto_lock(lock_);
+  DCHECK_NE(state_, kUninitialized);

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

can't you DCHECK(message_loop_->BelongsToCurrentThread()) here? If not then you
should BindToCurrentLoop() when you make the Read() calls so that you don't need
the lock.

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

Done.

+  DCHECK_NE(state_, kStopped);
+  DCHECK_GT(pending_read_count_, 0);
+  DCHECK_GT(eos_count_, 0);
+  DCHECK_EQ(read_state_[index], kReadPending);
+
+  --pending_read_count_;
+  read_state_[index] = kReadIdle;
+
+  switch (state_) {
+    case kPlaying:
+      if (buffer.get())

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

nit: You shouldn't need the get()

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

Done.

+        break;
+
+      // A NULL buffer means that we have reached EOS (or there's an error).
+
+      --eos_count_;
+
+      if (pending_read_count_ > 0)

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

DCHECK

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

Multiple read requests can be pending simultaneously -- one for each text
stream.  

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

Ok. Right. I don't know what I was thinking here.

+        return;
+
+      if (eos_count_ <= 0) {
+        state_ = kStopped;

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

it seems like an kEnded state would be more appropriate to differentiate this
state from the "stop has been called and we have run the stop callback."

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

Done.

+        ended_cb_.Run();
+      }
+
+      return;
+
+    case kPausePending:
+      if (pending_read_count_ <= 0) {
+        state_ = kPaused;
+        pause_cb_.Run();
+      }
+
+      return;
+
+    case kStopPending:
+      if (pending_read_count_ <= 0) {
+        state_ = kStopped;
+        stop_cb_.Run();
+      }
+
+      return;
+
+    case kPaused:
+    case kStopped:
+    case kUninitialized:
+    default:
+      NOTREACHED();
+      return;
+  }
+
+  cue_ready_cb_.Run(index, buffer);
+
+  read_state_[index] = kReadPending;
+  ++pending_read_count_;
+  decoder_->Read(index, base::Bind(&TextRendererImpl::CueReady, weak_this_));
+}
+
+}  // namespace media