Register MediaPlayerRenderer service

media/base/pipeline_impl.cc

Index: media/base/pipeline_impl.cc
diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc
index 0a08497ef248beeb82db6e420dffefd585f82809..e70ca30ca3155a8703a54d28f5c09a0d111c0650 100644
--- a/media/base/pipeline_impl.cc
+++ b/media/base/pipeline_impl.cc
@@ -823,10 +823,22 @@ void PipelineImpl::RendererWrapper::InitializeRenderer(
     const PipelineStatusCB& done_cb) {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
 
-  if (!demuxer_->GetStream(DemuxerStream::AUDIO) &&
-      !demuxer_->GetStream(DemuxerStream::VIDEO)) {
-    done_cb.Run(PIPELINE_ERROR_COULD_NOT_RENDER);
-    return;
+  switch (demuxer_->GetType()) {
+    case DemuxerStreamProvider::Type::STREAM:
+      if (!demuxer_->GetStream(DemuxerStream::AUDIO) &&
+          !demuxer_->GetStream(DemuxerStream::VIDEO)) {
+        done_cb.Run(PIPELINE_ERROR_COULD_NOT_RENDER);

[xhwang, 2016/10/12 20:14:15]

Could you please add a DVLOG since it's a failure case?

[tguilbert, 2016/10/12 21:46:25]

Done.

+        return;
+      }
+      break;
+
+    case DemuxerStreamProvider::Type::URL:
+      // N.B: Empty GURL are not valid.

[xhwang, 2016/10/12 20:14:15]

nit: N.B.?

[tguilbert, 2016/10/12 21:46:25]

Woops...
N.B. is short for the latin "nota bene", which means "pay close attention". It's
common in french textbooks and legal documents. It seems like "NOTE:" is the way
to go in Chrome.

+      if (!demuxer_->GetUrl().is_valid()) {
+        done_cb.Run(PIPELINE_ERROR_COULD_NOT_RENDER);
+        return;

[xhwang, 2016/10/12 20:14:15]

Could this happen in reality? Wondering whether we should dcheck.

[xhwang, 2016/10/12 20:14:15]

ditto for DVLOG

[tguilbert, 2016/10/12 21:46:24]

I'm not sure TBH. Leaving it as is for the moment.

[tguilbert, 2016/10/12 21:46:25]

Done.

+      }
+      break;
   }
 
   if (cdm_context_)
@@ -852,15 +864,28 @@ void PipelineImpl::RendererWrapper::ReportMetadata() {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
 
   PipelineMetadata metadata;
-  metadata.timeline_offset = demuxer_->GetTimelineOffset();
-  DemuxerStream* stream = demuxer_->GetStream(DemuxerStream::VIDEO);
-  if (stream) {
-    metadata.has_video = true;
-    metadata.natural_size = stream->video_decoder_config().natural_size();
-    metadata.video_rotation = stream->video_rotation();
-  }
-  if (demuxer_->GetStream(DemuxerStream::AUDIO)) {
-    metadata.has_audio = true;
+  DemuxerStream* stream;
+
+  switch (demuxer_->GetType()) {
+    case DemuxerStreamProvider::Type::STREAM:
+      metadata.timeline_offset = demuxer_->GetTimelineOffset();
+      stream = demuxer_->GetStream(DemuxerStream::VIDEO);
+      if (stream) {
+        metadata.has_video = true;
+        metadata.natural_size = stream->video_decoder_config().natural_size();
+        metadata.video_rotation = stream->video_rotation();
+      }
+      if (demuxer_->GetStream(DemuxerStream::AUDIO)) {
+        metadata.has_audio = true;

[xhwang, 2016/10/12 20:14:15]

In https://chromiumcodereview.appspot.com/2389473002/, we also report
video/audio config here. And the assumption was that if has_audio is true, the
audio config is not null. Now for URL type this is not true, we should add
comments in PipelineMetadata about this.

[tguilbert, 2016/10/12 21:46:25]

Done.

+      }
+      break;
+
+    case DemuxerStreamProvider::Type::URL:
+      // We don't know if the MediaPlayerRender has Audio/Video until we start
+      // playing. Conservatively assume that they do.
+      metadata.has_video = true;
+      metadata.has_audio = true;
+      break;
   }
 
   main_task_runner_->PostTask(FROM_HERE, base::Bind(&PipelineImpl::OnMetadata,