fallback to null sink in WebAudioSourceProvider::Initialize() + UMA stats for device status

media/blink/webmediaplayer_impl.cc

Index: media/blink/webmediaplayer_impl.cc
diff --git a/media/blink/webmediaplayer_impl.cc b/media/blink/webmediaplayer_impl.cc
index cd6091ddd3ccfcfc26d03a634d700428c75ac598..8f03ae3df43cfbf0cbfd77559ffd7e3f6584a3f9 100644
--- a/media/blink/webmediaplayer_impl.cc
+++ b/media/blink/webmediaplayer_impl.cc
@@ -265,14 +265,22 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
   // TODO(xhwang): When we use an external Renderer, many methods won't work,
   // e.g. GetCurrentFrameFromCompositor(). See http://crbug.com/434861
 
+  if (params.audio_renderer_sink().get()) {

[Guido Urdaneta, 2016/11/24 17:30:09]

nit: Should a stat be collected if params.audio_renderer_sink() is null?

+    media::OutputDeviceStatus device_status =
+        params.audio_renderer_sink()->GetOutputDeviceInfo().device_status();

[DaleCurtis, 2016/11/28 18:58:17]

In retrospect we probably shouldn't do this since it blocks the main render
thread. Instead we shouldn't construct the audio_source_provider_ until we know
if the sink is any good and that determination should be made on another thread.

This is no worse than we've had in the past, but guidou@ did a lot of work to
avoid us having this code run on the main thread, it's a shame to put it back.

[o1ka, 2016/11/29 13:55:58]

It's not new in this CL, we have already been doing it at l.272 (It's a change
Guido introduced recently to fallback to null sink)

[DaleCurtis, 2016/11/29 20:32:02]

I know, sorry I wasn't clear, I was saying it's not great that we added it in
the previous CL.

[o1ka, 2016/11/30 16:13:47]

Ah I see. We can probably defer fallback until
WebAudioSourceProviderImpl::Initialize, but I'm not sure how it all will work if
WebAudioSourceProviderImpl::GetOutputDeviceInfo() is called prior to
initialization.

We can remove it now, or figure out how to make it happen on media thread and
address it in a separate CL. WDYT?

+    UMA_HISTOGRAM_ENUMERATION("Media.AudioSinkStatus", device_status,

[DaleCurtis, 2016/11/28 18:58:17]

Maybe Media.WebMediaPlayer.SinkStatus?

[o1ka, 2016/11/30 16:13:48]

Done.

+                              OUTPUT_DEVICE_STATUS_MAX + 1);
+
+    if (device_status == OUTPUT_DEVICE_STATUS_OK) {
+      audio_source_provider_ =
+          new WebAudioSourceProviderImpl(params.audio_renderer_sink());
+      return;
+    }
+    LOG(WARNING) << "Output device error, falling back to null sink";

[DaleCurtis, 2016/11/28 18:58:17]

This should be a MEDIA_LOG() so it shows up in chrome://media-internals

[o1ka, 2016/11/30 16:13:47]

Done.

+  }
   // Use the null sink if no valid sink was provided.
-  audio_source_provider_ = new WebAudioSourceProviderImpl(
-      params.audio_renderer_sink().get() &&
-              params.audio_renderer_sink()
-                      ->GetOutputDeviceInfo()
-                      .device_status() == OUTPUT_DEVICE_STATUS_OK
-          ? params.audio_renderer_sink()
-          : new NullAudioSink(media_task_runner_));
+  audio_source_provider_ =
+      new WebAudioSourceProviderImpl(new NullAudioSink(media_task_runner_));
 }
 
 WebMediaPlayerImpl::~WebMediaPlayerImpl() {