media: scaffolding and plumbing for MojoRenderer{Impl, Service}

media/mojo/services/mojo_demuxer_stream_adapter.cc

+// Copyright 2014 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/mojo/services/mojo_demuxer_stream_adapter.h"
+
+#include "base/bind.h"
+#include "base/callback_helpers.h"
+#include "media/base/decoder_buffer.h"
+#include "media/mojo/services/media_type_converters.h"
+
+namespace media {
+
+MojoDemuxerStreamAdapter::MojoDemuxerStreamAdapter(

[scherkus (not reviewing), 2014/09/09 20:35:32]

media/ loves us some clang-format ... we just haven't written a PRESUBMIT.py for
it yet

can you run "git cl format" the next time you upload a CL?

+    mojo::DemuxerStreamPtr stream,
+    const base::Closure& stream_ready) : demuxer_stream_(stream.Pass()),
+                                         stream_ready_callback_(stream_ready),
+                                         weak_factory_(this) {
+  demuxer_stream_.set_client(this);
+}
+
+MojoDemuxerStreamAdapter::~MojoDemuxerStreamAdapter() {
+}
+
+void MojoDemuxerStreamAdapter::Read(const DemuxerStream::ReadCB& read_cb) {
+  // We shouldn't be holding on to a previous callback if a new Read() came in.
+  DCHECK(on_config_change_cb_.is_null());
+  demuxer_stream_->Read(base::Bind(&MojoDemuxerStreamAdapter::OnBufferReady,
+                                   weak_factory_.GetWeakPtr(),
+                                   read_cb));

[xhwang, 2014/09/10 00:00:52]

Based on the DemuxerStream API, we should have at most one pending read_cb at
any time. So we can have a permanent |read_cb_| defined in this class. That way,
we don't need to bind the |read_cb| here.

Also, in case of flushing, we'll need to return the pending read_cb. Not sure
how we'll implement that yet, but by binding the callback in a callback, we
can't access the original callback easily...

[tim (not reviewing), 2014/09/10 23:08:28]

I see.  I was going to say why bother adding state to |this| if we can tuck it
into the callback and forget about it, but if we need to return it on flush...
then I can add a member I guess.  Hmm, actually this isn't clear to me - if you
get kConfigChanged OnBufferReady, is the |buffer| always null, or can it be
provided?

[xhwang, 2014/09/15 04:57:28]

For kConfigChanged, the buffer is always NULL.

[tim (not reviewing), 2014/09/15 21:52:51]

Acknowledged.

+}
+
+AudioDecoderConfig MojoDemuxerStreamAdapter::audio_decoder_config() {
+  DCHECK(!config_queue_.empty());
+  return config_queue_.front();
+}
+
+VideoDecoderConfig MojoDemuxerStreamAdapter::video_decoder_config() {
+  DCHECK(false);  // TODO(tim): Video unsupported.

[scherkus (not reviewing), 2014/09/09 20:35:32]

NOTREACHED()?

[xhwang, 2014/09/10 00:00:52]

NOTREACHED() given l.44.

[tim (not reviewing), 2014/09/10 23:08:28]

Done.

[tim (not reviewing), 2014/09/10 23:08:29]

Done.

+  return VideoDecoderConfig();
+}
+
+media::DemuxerStream::Type MojoDemuxerStreamAdapter::type() {
+  return media::DemuxerStream::AUDIO;
+}
+
+void MojoDemuxerStreamAdapter::EnableBitstreamConverter() {
+  NOTIMPLEMENTED();
+}

[xhwang, 2014/09/10 00:00:52]

In media pipeline this is only used by GpuVideoDecoder. I guess the decoder we
are targeting won't need this. How about just NOTREACHED()?

[tim (not reviewing), 2014/09/10 23:08:28]

Done.

+
+bool MojoDemuxerStreamAdapter::SupportsConfigChanges() {
+  return true;
+}

[xhwang, 2014/09/10 00:00:52]

This is only used by audio_renderer_impl.cc. So if we are not using ARI we can
NOTREACHED() this. If we want to have a complete implementation, we need to
plumb the value from the original DemuxerStream over here.

[tim (not reviewing), 2014/09/10 23:08:28]

For now I'm using ARI.  In fact the test I added instantiates and initializes
one and does plumb the value over from the dx stream (See
OnAudioDecoderConfigChanged).

+
+VideoRotation MojoDemuxerStreamAdapter::video_rotation() {
+  NOTIMPLEMENTED();
+  return VIDEO_ROTATION_0;
+}
+
+void MojoDemuxerStreamAdapter::OnStreamReady(
+    mojo::ScopedDataPipeConsumerHandle pipe) {
+  // TODO(tim): We don't support pipe streaming yet.
+  DCHECK(!pipe.is_valid());
+  DCHECK(!config_queue_.empty());
+  stream_ready_callback_.Run();
+}
+
+void MojoDemuxerStreamAdapter::OnAudioDecoderConfigChanged(
+    mojo::AudioDecoderConfigPtr config) {
+  config_queue_.push(config.To<media::AudioDecoderConfig>());
+
+  if (!on_config_change_cb_.is_null())
+    base::ResetAndReturn(&on_config_change_cb_).Run();
+}
+
+void MojoDemuxerStreamAdapter::OnBufferReady(
+    const ReadCB& read_cb,
+    mojo::DemuxerStream::Status status,
+    mojo::MediaDecoderBufferPtr buffer) {
+  DCHECK(on_config_change_cb_.is_null());
+  DCHECK(!config_queue_.empty());
+
+  media::DemuxerStream::Status media_status(
+      static_cast<media::DemuxerStream::Status>(status));
+  scoped_refptr<media::DecoderBuffer> media_buffer(
+      buffer.To<scoped_refptr<media::DecoderBuffer> >());
+
+  if (status == mojo::DemuxerStream::STATUS_CONFIG_CHANGED) {
+    DCHECK_GE(config_queue_.size(), 1U);

[scherkus (not reviewing), 2014/09/09 20:35:32]

DCHECK(!config_queue_.empty()) ?

[tim (not reviewing), 2014/09/10 23:08:28]

There should be at least 1 element in the queue at this point; the current
config.
Or am I missing something?

[xhwang, 2014/09/15 04:57:28]

scherkus was recommending to check !empty(), which is same as >=1 ?

[tim (not reviewing), 2014/09/15 21:52:51]

Done.

+    config_queue_.pop();
+
+    // If the |config_queue_| is empty we need to wait for
+    // OnAudioDecoderConfigChanged before invoking |read_cb|.
+    if (config_queue_.empty()) {
+      on_config_change_cb_ = base::Bind(read_cb, media_status, media_buffer);

[xhwang, 2014/09/10 00:00:52]

If we keep a class member read_cb_, we don't need on_config_change_cb_... 

[tim (not reviewing), 2014/09/10 23:08:28]

See my question at 30, I might be misunderstanding OnBufferReady semantics
(because I'd like to avoid having to add multiple member variables to also
provide temp storage for the returned buffer as well as the callback)

[xhwang, 2014/09/15 04:57:28]

See my reply above.

[tim (not reviewing), 2014/09/15 21:52:51]

Acknowledged.

+      return;
+    }
+  }
+  read_cb.Run(media_status, media_buffer);
+}
+
+}  // namespace media