Add video decoding methods in Decryptor and add DecryptingVideoDecoder.

media/filters/decrypting_video_decoder.cc

Index: media/filters/decrypting_video_decoder.cc
diff --git a/media/filters/decrypting_video_decoder.cc b/media/filters/decrypting_video_decoder.cc
new file mode 100644
index 0000000000000000000000000000000000000000..dd1829d7ce07e95b0b29dcbd557da744865a9931
--- /dev/null
+++ b/media/filters/decrypting_video_decoder.cc
@@ -0,0 +1,279 @@
+// 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/decrypting_video_decoder.h"
+
+#include "base/bind.h"
+#include "base/callback_helpers.h"
+#include "base/location.h"
+#include "base/message_loop_proxy.h"
+#include "media/base/decoder_buffer.h"
+#include "media/base/decryptor.h"
+#include "media/base/demuxer_stream.h"
+#include "media/base/pipeline.h"
+#include "media/base/video_decoder_config.h"
+#include "media/base/video_frame.h"
+
+namespace media {
+
+DecryptingVideoDecoder::DecryptingVideoDecoder(
+    const MessageLoopFactoryCB& message_loop_factory_cb,
+    Decryptor* decryptor)
+    : message_loop_factory_cb_(message_loop_factory_cb),
+      message_loop_(NULL),
+      state_(kUninitialized),
+      decryptor_(decryptor) {
+}
+
+void DecryptingVideoDecoder::Initialize(
+    const scoped_refptr<DemuxerStream>& stream,
+    const PipelineStatusCB& status_cb,
+    const StatisticsCB& statistics_cb) {
+  if (!message_loop_) {

[ddorwin, 2012/09/28 17:36:40]

Initialize can be called multiple times? Or is this just for the self-post?
It would seem better to have a DoInitialize rather than calling itself with this
check.

[xhwang, 2012/09/30 19:58:51]

This is the same code we have in FFVD. But it's a good point to use DoInitialize
so that it's consistent w/ other function in this class.

By the VideoDecoder API, Initialize() should not be called multiple times
because when a VideoDecoder is stopped, it cannot be re-initialized.

Done.

+    message_loop_ = base::ResetAndReturn(&message_loop_factory_cb_).Run();
+    message_loop_->PostTask(FROM_HERE, base::Bind(
+        &DecryptingVideoDecoder::Initialize, this,
+        stream, status_cb, statistics_cb));
+    return;
+  }
+
+  DCHECK(!demuxer_stream_);
+  DCHECK(stream);
+  demuxer_stream_ = stream;

[ddorwin, 2012/09/28 17:36:40]

Can this class be reused after one of the following errors? If so, this should
probably be set at 56.

[xhwang, 2012/09/30 19:58:51]

Done.

+
+  const VideoDecoderConfig& config = demuxer_stream_->video_decoder_config();
+  if (!config.IsValidConfig()) {
+    DLOG(ERROR) << "Invalid video stream - " << config.AsHumanReadableString();
+    status_cb.Run(PIPELINE_ERROR_DECODE);
+    return;
+  }
+  // DecryptingVideoDecoder only accepts potentially encrypted stream.

[ddorwin, 2012/09/28 17:36:40]

Newline

[xhwang, 2012/09/30 19:58:51]

Done.

+  if (!config.is_encrypted()) {
+    status_cb.Run(DECODER_ERROR_NOT_SUPPORTED);
+    return;
+  }
+
+  status_cb_ = status_cb;
+  statistics_cb_ = statistics_cb;
+
+  DCHECK(decryptor_);
+  decryptor_->InitializeVideoDecoder(
+      config,
+      base::Bind(&DecryptingVideoDecoder::OnDecoderInitialized, this));
+}
+
+void DecryptingVideoDecoder::Read(const ReadCB& read_cb) {
+  // Complete operation asynchronously on different stack of execution as per
+  // the API contract of VideoDecoder::Read()
+  message_loop_->PostTask(FROM_HERE, base::Bind(
+      &DecryptingVideoDecoder::DoRead, this, read_cb));
+}
+
+void DecryptingVideoDecoder::Reset(const base::Closure& closure) {
+  if (!message_loop_->BelongsToCurrentThread()) {
+    message_loop_->PostTask(FROM_HERE, base::Bind(
+        &DecryptingVideoDecoder::Reset, this, closure));
+    return;
+  }
+
+  DCHECK(reset_cb_.is_null());
+  reset_cb_ = closure;
+
+  decryptor_->CancelDecryptAndDecodeVideo();
+
+  // Defer the reset if a read is pending.

[ddorwin, 2012/09/28 17:36:40]

Why? When will it be called then?

[xhwang, 2012/09/30 19:58:51]

We can't finish the reset process if we have pending callback not returned.

It will be called when the read callback passed to the demuxer stream is fired.
See DoDecryptAndDecodeBuffer().

+  if (!read_cb_.is_null())
+    return;
+
+  DoReset();
+}
+
+void DecryptingVideoDecoder::Stop(const base::Closure& closure) {
+  if (!message_loop_->BelongsToCurrentThread()) {
+    message_loop_->PostTask(FROM_HERE, base::Bind(
+        &DecryptingVideoDecoder::Stop, this, closure));
+    return;
+  }
+
+  DCHECK(stop_cb_.is_null());
+  stop_cb_ = closure;
+
+  decryptor_->StopVideoDecoder();
+
+  // Defer stopping if a read is pending.

[ddorwin, 2012/09/28 17:36:40]

Same.

[xhwang, 2012/09/30 19:58:51]

See above answer.

+  if (!read_cb_.is_null())
+    return;
+
+  DoStop();
+}
+
+DecryptingVideoDecoder::~DecryptingVideoDecoder() {

[ddorwin, 2012/09/28 17:36:40]

Do we need this? Presumably the parent is already virtual.

[xhwang, 2012/09/30 19:58:51]

Correct. I added a DCHECK in the dtor so now we need it.

+}
+
+void DecryptingVideoDecoder::OnDecoderInitialized(bool success) {
+  if (!message_loop_->BelongsToCurrentThread()) {
+    message_loop_->PostTask(FROM_HERE, base::Bind(
+        &DecryptingVideoDecoder::OnDecoderInitialized, this, success));
+    return;
+  }
+
+  DCHECK(!status_cb_.is_null());
+  if (!success) {

[ddorwin, 2012/09/28 17:36:40]

nit: newline since the above DCHECK is applicable to more than this line.

[xhwang, 2012/09/30 19:58:51]

Done.

+    base::ResetAndReturn(&status_cb_).Run(DECODER_ERROR_NOT_SUPPORTED);
+    return;
+  }
+
+  // Success!
+  state_ = kNormal;
+  base::ResetAndReturn(&status_cb_).Run(PIPELINE_OK);
+}
+
+void DecryptingVideoDecoder::DoRead(const ReadCB& read_cb) {
+  DCHECK(message_loop_->BelongsToCurrentThread());
+  DCHECK(!read_cb.is_null());
+  CHECK_NE(state_, kUninitialized);
+  CHECK(read_cb_.is_null()) << "Overlapping decodes are not supported.";
+
+  // Return empty frames if decoding has finished.
+  if (state_ == kDecodeFinished) {
+    read_cb.Run(kOk, VideoFrame::CreateEmptyFrame());
+    return;
+  }
+
+  read_cb_ = read_cb;
+  ReadFromDemuxerStream();
+}
+
+void DecryptingVideoDecoder::ReadFromDemuxerStream() {
+  DCHECK_NE(state_, kUninitialized);
+  DCHECK_NE(state_, kDecodeFinished);
+  DCHECK(!read_cb_.is_null());
+
+  demuxer_stream_->Read(
+      base::Bind(&DecryptingVideoDecoder::DecryptAndDecodeBuffer, this));
+}
+
+void DecryptingVideoDecoder::DecryptAndDecodeBuffer(
+    DemuxerStream::Status status,
+    const scoped_refptr<DecoderBuffer>& buffer) {
+  DCHECK_EQ(status != DemuxerStream::kOk, !buffer) << status;
+  // TODO(scherkus): fix FFmpegDemuxerStream::Read() to not execute our read
+  // callback on the same execution stack so we can get rid of forced task post.
+  message_loop_->PostTask(FROM_HERE, base::Bind(
+      &DecryptingVideoDecoder::DoDecryptAndDecodeBuffer, this, status, buffer));
+}
+
+void DecryptingVideoDecoder::DoDecryptAndDecodeBuffer(
+    DemuxerStream::Status status,
+    const scoped_refptr<DecoderBuffer>& buffer) {
+  DCHECK(message_loop_->BelongsToCurrentThread());
+  DCHECK_NE(state_, kUninitialized);

[ddorwin, 2012/09/28 17:36:40]

These two can be replaced with EQ kNormal.

[xhwang, 2012/09/30 19:58:51]

Done.

+  DCHECK_NE(state_, kDecodeFinished);
+  DCHECK(!read_cb_.is_null());
+
+  if (!stop_cb_.is_null()) {
+    base::ResetAndReturn(&read_cb_).Run(kOk, NULL);

[ddorwin, 2012/09/28 17:36:40]

Not related to this CL, but odd that we use the data in the second parameter as
an indication of whether a frame was successfully decoded.

[xhwang, 2012/09/30 19:58:51]

I agree. +scherkus/fischman: How about we add kAborted, kDecodeFinished into
VideoDecoder::Status?

+    DoStop();
+    return;
+  }
+
+  if (!reset_cb_.is_null()) {
+    base::ResetAndReturn(&read_cb_).Run(kOk, NULL);
+    DoReset();
+    return;
+  }
+
+  if (status == DemuxerStream::kAborted) {
+    base::ResetAndReturn(&read_cb_).Run(kOk, NULL);
+    return;
+  }
+
+  if (status == DemuxerStream::kConfigChanged) {
+    // TODO(xhwang): Add config change support.
+    base::ResetAndReturn(&read_cb_).Run(kDecodeError, NULL);
+    return;
+  }
+
+  DCHECK_EQ(DemuxerStream::kOk, status);

[ddorwin, 2012/09/28 17:36:40]

nit: Did we decide the other order for non-unittests?

[xhwang, 2012/09/30 19:58:51]

Yeah, as summarized by scherkus in another thread:
"So I'd sum up the rule as "Always prefer (foo == 0) for all comparisons except
for EXPECT_EQ and ASSERT_EQ which require the order to be reversed for prettier
error messages"

So thanks for pointing out again. 

Done.

+  DCHECK(buffer);
+
+  decryptor_->DecryptAndDecodeVideo(
+      buffer, base::Bind(&DecryptingVideoDecoder::DeliverFrame, this,
+                         buffer->GetDataSize()));
+}
+
+void DecryptingVideoDecoder::DeliverFrame(
+    int buffer_size,
+    Decryptor::Status status,
+    const scoped_refptr<VideoFrame>& frame) {
+  // We need to force task post here because the VideoDecodeCB can be executed
+  // synchronously or asynchronously by the decryptor.

[ddorwin, 2012/09/28 17:36:40]

Might be worth explaining why or pointing to an explanation so future readers
don't wonder why.

[xhwang, 2012/09/30 19:58:51]

Done.

+  message_loop_->PostTask(FROM_HERE, base::Bind(
+      &DecryptingVideoDecoder::DoDeliverFrame, this,
+      buffer_size, status, frame));
+}
+
+void DecryptingVideoDecoder::DoDeliverFrame(
+    int buffer_size,
+    Decryptor::Status status,
+    const scoped_refptr<VideoFrame>& frame) {
+  DCHECK(message_loop_->BelongsToCurrentThread());
+
+  DCHECK_NE(state_, kUninitialized);
+  DCHECK_NE(state_, kDecodeFinished);
+  DCHECK(!read_cb_.is_null());
+
+  if (!stop_cb_.is_null()) {
+    base::ResetAndReturn(&read_cb_).Run(kOk, NULL);
+    DoStop();
+    return;
+  }
+
+  if (!reset_cb_.is_null()) {
+    base::ResetAndReturn(&read_cb_).Run(kOk, NULL);
+    DoReset();
+    return;
+  }
+
+  if (status == Decryptor::kNoKey || status == Decryptor::kError) {
+    DCHECK(!frame);
+    state_ = kDecodeFinished;
+    base::ResetAndReturn(&read_cb_).Run(kDecodeError, NULL);

[ddorwin, 2012/09/28 17:36:40]

Why is kNoKey a decode error? Where is the wait and retry logic? Should we ever
get kNoKey here?

[xhwang, 2012/09/30 19:58:51]

The wait and retry logic is in ProxyDecryptor so it hides kNoKey intrenally. But
not all Decryptor implementations hides kNoKey, e.g. AesDecryptor. The idea of
DVD is that it can work with any Decryptor implementations, thus it needs to
handle kNoKey here.

[ddorwin, 2012/10/01 18:43:20]

But wouldn't this class always wrap the real Decryptor in ProxyDecryptor?

Should the kNoKey logic be here then? Thinking ahead to when we remove Decryptor
from FFmpegDecoder, this will be the central point for all kNoKey returns.

Regardless, it doesn't seem like we would expect kNoKey in this code, which
means we should at least DCHECK that it is not received at 238.

+    return;
+  }
+
+  // The buffer has been accepted by the decoder, let's report statistics.
+  if (buffer_size) {
+    PipelineStatistics statistics;
+    statistics.video_bytes_decoded = buffer_size;
+    statistics_cb_.Run(statistics);
+  }
+
+  if (status == Decryptor::kNeedMoreData) {
+    DCHECK(!frame);
+    ReadFromDemuxerStream();
+    return;
+  }
+
+  DCHECK_EQ(Decryptor::kSuccess, status);

[ddorwin, 2012/09/28 17:36:40]

same

[xhwang, 2012/09/30 19:58:51]

Done.

+  DCHECK(frame);
+  if (frame->IsEndOfStream())
+    state_ = kDecodeFinished;
+
+  base::ResetAndReturn(&read_cb_).Run(kOk, frame);
+}
+
+void DecryptingVideoDecoder::DoReset() {
+  DCHECK(read_cb_.is_null());
+  state_ = kNormal;

[ddorwin, 2012/09/28 17:36:40]

DCHECK(state_ != kUninitialized).

[xhwang, 2012/09/30 19:58:51]

Done.

+  base::ResetAndReturn(&reset_cb_).Run();
+}
+
+void DecryptingVideoDecoder::DoStop() {
+  DCHECK(read_cb_.is_null());
+  state_ = kUninitialized;
+  base::ResetAndReturn(&stop_cb_).Run();
+}
+
+}  // namespace media