MediaStream audio rendering: Bypass audio processing for non-WebRTC cases.

content/renderer/media/media_stream_renderer_factory_impl.cc

Index: content/renderer/media/media_stream_renderer_factory_impl.cc
diff --git a/content/renderer/media/media_stream_renderer_factory_impl.cc b/content/renderer/media/media_stream_renderer_factory_impl.cc
index 40463f58d3d69e5e51e003c992b8f711eb5e496e..00d45df2811e8fcecd19bdcc4d783c73b8c228e4 100644
--- a/content/renderer/media/media_stream_renderer_factory_impl.cc
+++ b/content/renderer/media/media_stream_renderer_factory_impl.cc
@@ -9,9 +9,9 @@
 #include "content/renderer/media/media_stream_audio_track.h"
 #include "content/renderer/media/media_stream_video_renderer_sink.h"
 #include "content/renderer/media/media_stream_video_track.h"
+#include "content/renderer/media/track_audio_renderer.h"
 #include "content/renderer/media/webrtc/peer_connection_dependency_factory.h"
 #include "content/renderer/media/webrtc_audio_renderer.h"
-#include "content/renderer/media/webrtc_local_audio_renderer.h"
 #include "content/renderer/render_thread_impl.h"
 #include "media/base/audio_hardware_config.h"
 #include "third_party/WebKit/public/platform/WebMediaStream.h"
@@ -27,59 +27,26 @@ PeerConnectionDependencyFactory* GetPeerConnectionDependencyFactory() {
   return RenderThreadImpl::current()->GetPeerConnectionDependencyFactory();
 }
 
-// Returns a valid session id if a single capture device is currently open
-// (and then the matching session_id), otherwise -1.
-// This is used to pass on a session id to a webrtc audio renderer (either
-// local or remote), so that audio will be rendered to a matching output
-// device, should one exist.
+// Returns a valid session id if a single WebRTC capture device is currently
+// open (and then the matching session_id), otherwise 0.

[miu, 2016/01/27 05:14:14]

Note: The comment here was mistaken.  The prior code was always returning 0, not
-1.

[tommi (sloooow) - chröme, 2016/02/01 20:32:24]

Acknowledged.

+// This is used to pass on a session id to an audio renderer, so that audio will
+// be rendered to a matching output device, should one exist.
 // Note that if there are more than one open capture devices the function
-// will not be able to pick an appropriate device and return false.
-bool GetSessionIdForAudioRenderer(int* session_id) {
+// will not be able to pick an appropriate device and return 0.
+int GetSessionIdForWebRtcAudioRenderer() {
   WebRtcAudioDeviceImpl* audio_device =
       GetPeerConnectionDependencyFactory()->GetWebRtcAudioDevice();
   if (!audio_device)
-    return false;
+    return 0;
 
+  int session_id = 0;
   int sample_rate;        // ignored, read from output device
   int frames_per_buffer;  // ignored, read from output device
-  return audio_device->GetAuthorizedDeviceInfoForAudioRenderer(
-      session_id, &sample_rate, &frames_per_buffer);
-}
-
-scoped_refptr<WebRtcAudioRenderer> CreateRemoteAudioRenderer(
-    const blink::WebMediaStream& stream,
-    int render_frame_id,
-    const std::string& device_id,
-    const url::Origin& security_origin) {
-  DVLOG(1) << "MediaStreamRendererFactoryImpl::CreateRemoteAudioRenderer id:"
-           << stream.id().utf8();
-  // |stream| will always contain at least one audio track.
-  // See MediaStreamRendererFactoryImpl::GetAudioRenderer.
-
-  // TODO(tommi): Change the default value of session_id to be
-  // StreamDeviceInfo::kNoId.  Also update AudioOutputDevice etc.
-  int session_id = 0;
-  GetSessionIdForAudioRenderer(&session_id);
-
-  return new WebRtcAudioRenderer(
-      GetPeerConnectionDependencyFactory()->GetWebRtcSignalingThread(), stream,
-      render_frame_id, session_id, device_id, security_origin);
-}
-
-scoped_refptr<WebRtcLocalAudioRenderer> CreateLocalAudioRenderer(
-    const blink::WebMediaStreamTrack& audio_track,
-    int render_frame_id,
-    const std::string& device_id,
-    const url::Origin& security_origin) {
-  DVLOG(1) << "MediaStreamRendererFactoryImpl::CreateLocalAudioRenderer";
-
-  int session_id = 0;
-  GetSessionIdForAudioRenderer(&session_id);
-
-  // Create a new WebRtcLocalAudioRenderer instance and connect it to the
-  // existing WebRtcAudioCapturer so that the renderer can use it as source.
-  return new WebRtcLocalAudioRenderer(audio_track, render_frame_id, session_id,
-                                      device_id, security_origin);
+  if (!audio_device->GetAuthorizedDeviceInfoForAudioRenderer(
+          &session_id, &sample_rate, &frames_per_buffer)) {
+    session_id = 0;
+  }
+  return session_id;
 }
 
 }  // namespace
@@ -151,29 +118,40 @@ MediaStreamRendererFactoryImpl::GetAudioRenderer(
     return nullptr;
   }
 
-  if (audio_track->is_local_track()) {
+  // If the track has a local source, or is a remote track that does not use the
+  // WebRTC audio pipeline, return a new TrackAudioRenderer instance.
+  if (audio_track->is_local_track() || !audio_track->GetAudioAdapter()) {

[miu, 2016/01/27 05:14:14]

Note: This is needed in a follow-up change that addresses changes to
MediaStreamAudioSource/Tracks.  It basically means that if the remote source is
not via WebRTC/PeerConnection (e.g., a Cast Streaming Receiver), then
TrackAudioRenderer can be used as well.

[tommi (sloooow) - chröme, 2016/02/01 20:32:23]

I'd like to get rid of the GetAudioAdapter method.  If we get rid of it, is
there another way to distinguish remote cast tracks from webrtc's?  E.g via the
source?

[miu, 2016/02/03 03:48:45]

Agreed.  Yes, I am working on this in a follow-up change:
https://codereview.chromium.org/1647773002/.  However, note that that change is
not ready to continue with review yet (hubbe@ made a suggestion that caused me
to discover a much cleaner refactoring).

For now, I've added a TODO comment to track this for the follow-up change.  :-)

     // TODO(xians): Add support for the case where the media stream contains
     // multiple audio tracks.
-    return CreateLocalAudioRenderer(audio_tracks[0], render_frame_id, device_id,
-                                    security_origin);
+    DVLOG(1) << "Creating TrackAudioRenderer for "
+             << (audio_track->is_local_track() ? "local" : "remote")
+             << " track.";
+    return new TrackAudioRenderer(audio_tracks[0], render_frame_id,
+                                  0 /* no session_id */, device_id,
+                                  security_origin);
   }
 
   // This is a remote WebRTC media stream.
   WebRtcAudioDeviceImpl* audio_device =
       GetPeerConnectionDependencyFactory()->GetWebRtcAudioDevice();
+  DCHECK(audio_device);
 
   // Share the existing renderer if any, otherwise create a new one.
   scoped_refptr<WebRtcAudioRenderer> renderer(audio_device->renderer());
-  if (!renderer.get()) {
-    renderer = CreateRemoteAudioRenderer(web_stream, render_frame_id,
-                                         device_id, security_origin);
-
-    if (renderer.get() && !audio_device->SetAudioRenderer(renderer.get()))
-      renderer = NULL;
+  if (renderer) {
+    DVLOG(1) << "Using existing WebRtcAudioRenderer for remote WebRTC track.";
+  } else {
+    DVLOG(1) << "Creating WebRtcAudioRenderer for remote WebRTC track.";
+    renderer = new WebRtcAudioRenderer(
+        GetPeerConnectionDependencyFactory()->GetWebRtcSignalingThread(),
+        web_stream, render_frame_id, GetSessionIdForWebRtcAudioRenderer(),
+        device_id, security_origin);
+
+    if (!audio_device->SetAudioRenderer(renderer.get()))
+      return nullptr;
   }
 
-  return renderer.get() ? renderer->CreateSharedAudioRendererProxy(web_stream)
-                        : NULL;
+  return renderer->CreateSharedAudioRendererProxy(web_stream);
 }
 
 }  // namespace content