Implement MojoAudioDecoder

media/mojo/services/mojo_audio_decoder.cc

Index: media/mojo/services/mojo_audio_decoder.cc
diff --git a/media/mojo/services/mojo_audio_decoder.cc b/media/mojo/services/mojo_audio_decoder.cc
index 1c6ad0c616e1de1574d7d57681d9082b28487ef3..f81e32c20f6d4cef9a3ce31a41d234dfaaaee863 100644
--- a/media/mojo/services/mojo_audio_decoder.cc
+++ b/media/mojo/services/mojo_audio_decoder.cc
@@ -5,17 +5,25 @@
 #include "media/mojo/services/mojo_audio_decoder.h"
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/callback_helpers.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/single_thread_task_runner.h"
 #include "base/thread_task_runner_handle.h"
+#include "media/base/cdm_context.h"
+#include "media/mojo/common/media_type_converters.h"
 
 namespace media {
 
 MojoAudioDecoder::MojoAudioDecoder(
     scoped_refptr<base::SingleThreadTaskRunner> task_runner,
     interfaces::AudioDecoderPtr remote_decoder)
-    : task_runner_(task_runner), remote_decoder_(std::move(remote_decoder)) {
+    : task_runner_(task_runner),
+      remote_decoder_(std::move(remote_decoder)),
+      binding_(this),
+      has_connection_error_(false),
+      needs_bitstream_conversion_(false) {
   DVLOG(1) << __FUNCTION__;
 }
 
@@ -34,10 +42,36 @@ void MojoAudioDecoder::Initialize(const AudioDecoderConfig& config,
   DVLOG(1) << __FUNCTION__;
   DCHECK(task_runner_->BelongsToCurrentThread());
 
-  NOTIMPLEMENTED();
-
-  // Fail initialization while not implemented.
-  task_runner_->PostTask(FROM_HERE, base::Bind(init_cb, false));
+  // Fail immediately if the stream is encrypted but |cdm_context| is invalid.
+  int cdm_id = (config.is_encrypted() && cdm_context)
+                   ? cdm_context->GetCdmId()
+                   : CdmContext::kInvalidCdmId;
+
+  if (config.is_encrypted() && CdmContext::kInvalidCdmId == cdm_id) {
+    task_runner_->PostTask(FROM_HERE, base::Bind(init_cb, false));
+    return;
+  }
+
+  // If connection error has happened, fail immediately.
+  if (remote_decoder_.encountered_error()) {
+    task_runner_->PostTask(FROM_HERE, base::Bind(init_cb, false));
+    return;
+  }
+
+  // Otherwise, set an error handler to catch the connection error.
+  // Using base::Unretained(this) is safe because |this| owns |remote_decoder_|,
+  // and the error handler can't be invoked once |remote_decoder_| is destroyed.
+  remote_decoder_.set_connection_error_handler(
+      base::Bind(&MojoAudioDecoder::OnConnectionError, base::Unretained(this)));
+
+  init_cb_ = init_cb;
+
+  // Using base::Unretained(this) is safe because |this| owns |remote_decoder_|,
+  // and the callback won't be dispatched if |remote_decoder_| is destroyed.
+  remote_decoder_->Initialize(
+      binding_.CreateInterfacePtrAndBind(),
+      interfaces::AudioDecoderConfig::From(config), cdm_id,
+      base::Bind(&MojoAudioDecoder::OnInitialized, base::Unretained(this)));
 }
 
 void MojoAudioDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer,
@@ -45,25 +79,108 @@ void MojoAudioDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer,
   DVLOG(3) << __FUNCTION__;
   DCHECK(task_runner_->BelongsToCurrentThread());
 
-  NOTIMPLEMENTED();
+  if (has_connection_error_) {
+    task_runner_->PostTask(FROM_HERE, base::Bind(decode_cb, kDecodeError));
+    return;
+  }
 
-  // Actually we can't decode anything.
-  task_runner_->PostTask(FROM_HERE, base::Bind(decode_cb, kDecodeError));
+  DCHECK(decode_cb_.is_null());
+  decode_cb_ = decode_cb;
+
+  remote_decoder_->Decode(

[xhwang, 2016/03/18 02:19:43]

Add a comment that this will not work. We need a DataPipe to send the real data.

[Tima Vaisburd, 2016/03/18 19:05:49]

In the doc you said we can use MessagePipe instead of DataPipe as a first step.
Please explain.

[xhwang, 2016/03/18 19:17:17]

There are two places we need to use DataPipe, one is to send DecoderBuffer over.
One is to send AudioBuffer back.

For DecoderBuffer, the data is not serialized into the message pipe at all, so
this code will not work (and worth a comment). Later you can add DataPipe so
it'll work.

For AudioBuffer, today we are using MessagePipe to serialize the data. This
should work for now. In the future, we may still want to use DataPipe for better
performance.

[Tima Vaisburd, 2016/03/18 19:35:03]

Added comment.

+      interfaces::DecoderBuffer::From(buffer),
+      base::Bind(&MojoAudioDecoder::OnDecodeStarted, base::Unretained(this)));
 }
 
 void MojoAudioDecoder::Reset(const base::Closure& closure) {
   DVLOG(2) << __FUNCTION__;
   DCHECK(task_runner_->BelongsToCurrentThread());
 
-  NOTIMPLEMENTED();
+  if (has_connection_error_) {
+    if (!decode_cb_.is_null()) {
+      task_runner_->PostTask(
+          FROM_HERE,
+          base::Bind(base::ResetAndReturn(&decode_cb_), kDecodeError));
+    }
+
+    task_runner_->PostTask(FROM_HERE, closure);
+    return;
+  }
+
+  DCHECK(reset_cb_.is_null());
+  reset_cb_ = closure;

[xhwang, 2016/03/18 02:19:43]

Do you want to call remote_decoder_->Reset() here?

Note that you'll want to wait for the decode_cb to be fired to fire the
reset_cb:
https://code.google.com/p/chromium/codesearch#chromium/src/media/base/audio_d...

See example here:
https://code.google.com/p/chromium/codesearch#chromium/src/media/filters/decr...

[Tima Vaisburd, 2016/03/18 19:05:49]

Yes, I forgot to call remote decoder. Thank you.

I do not quite understand why do we have to do any pending treatment that you
have referenced. If the order of the calls is preserved across IPC, the Decode()
should arrive to the service before Reset(), and then MCAD will take care to
send OnDecodeStatus() before OnReset()?
What do I miss?

 }
 
 bool MojoAudioDecoder::NeedsBitstreamConversion() const {
   DVLOG(1) << __FUNCTION__;
   DCHECK(task_runner_->BelongsToCurrentThread());
 
+  return needs_bitstream_conversion_;
+}
+
+void MojoAudioDecoder::OnBufferDecoded(interfaces::AudioBufferPtr buffer) {
+  DVLOG(1) << __FUNCTION__;
+  DCHECK(task_runner_->BelongsToCurrentThread());
+
   NOTIMPLEMENTED();
-  return false;
+}
+
+void MojoAudioDecoder::OnConnectionError() {
+  DVLOG(1) << __FUNCTION__;
+  DCHECK(task_runner_->BelongsToCurrentThread());
+
+  has_connection_error_ = true;
+
+  if (!init_cb_.is_null()) {
+    base::ResetAndReturn(&init_cb_).Run(false);
+    return;
+  }
+
+  if (!decode_cb_.is_null())
+    base::ResetAndReturn(&decode_cb_).Run(kDecodeError);
+  if (!reset_cb_.is_null())
+    base::ResetAndReturn(&reset_cb_).Run();
+}
+
+void MojoAudioDecoder::OnInitialized(bool status,
+                                     bool needs_bitstream_conversion) {
+  DVLOG(1) << __FUNCTION__ << ": status:" << status;
+  DCHECK(task_runner_->BelongsToCurrentThread());
+
+  needs_bitstream_conversion_ = needs_bitstream_conversion;
+
+  task_runner_->PostTask(FROM_HERE, base::Bind(init_cb_, status));
+}
+
+static media::AudioDecoder::Status ConvertDecodeStatus(

[xhwang, 2016/03/18 02:19:43]

FYI, we have a bug to track the work to avoid this conversion. But this is fine
now.

https://bugs.chromium.org/p/chromium/issues/detail?id=581405

[Tima Vaisburd, 2016/03/18 19:05:49]

I've seen static_cast<>, maybe just cast?

[xhwang, 2016/03/18 19:17:17]

The bug will fix the issue completely. If you do cast, you need static_assert.
The currently conversion sgtm.

[Tima Vaisburd, 2016/03/18 19:35:03]

Acknowledged.

+    interfaces::AudioDecoder::DecodeStatus status) {
+  switch (status) {
+    case interfaces::AudioDecoder::DecodeStatus::OK:
+      return media::AudioDecoder::kOk;
+    case interfaces::AudioDecoder::DecodeStatus::ABORTED:
+      return media::AudioDecoder::kAborted;
+    case interfaces::AudioDecoder::DecodeStatus::DECODE_ERROR:
+      return media::AudioDecoder::kDecodeError;
+  }
+  NOTREACHED();
+  return media::AudioDecoder::kDecodeError;
+}
+
+void MojoAudioDecoder::OnDecodeStarted(
+    interfaces::AudioDecoder::DecodeStatus status) {
+  DVLOG(1) << __FUNCTION__ << ": status:" << status;
+  DCHECK(task_runner_->BelongsToCurrentThread());
+
+  if (!decode_cb_.is_null())

[xhwang, 2016/03/18 02:19:43]

When would this happen? Typically we assume this should not happen. 

Note that if connection error happened OnDecodeStarted or OnResetDone should not
be dispatched.

[Tima Vaisburd, 2016/03/18 19:05:49]

Yes, I assumed these can be called after connection error.
Changed to DCHECK().

+    base::ResetAndReturn(&decode_cb_).Run(ConvertDecodeStatus(status));
+}
+
+void MojoAudioDecoder::OnResetDone() {
+  DVLOG(1) << __FUNCTION__;
+  DCHECK(task_runner_->BelongsToCurrentThread());
+
+  if (!reset_cb_.is_null())

[xhwang, 2016/03/18 02:19:43]

ditto

[Tima Vaisburd, 2016/03/18 19:05:49]

Changed to DCHECK()

+    base::ResetAndReturn(&reset_cb_).Run();
 }
 
 }  // namespace media