Storing raw audio sink for default audio track.

talk/media/webrtc/webrtcvoiceengine.cc

Index: talk/media/webrtc/webrtcvoiceengine.cc
diff --git a/talk/media/webrtc/webrtcvoiceengine.cc b/talk/media/webrtc/webrtcvoiceengine.cc
index 9742564985bca6c0437865dafbe76803608e384a..79c807564a504e71939687582447e8fb749ded89 100644
--- a/talk/media/webrtc/webrtcvoiceengine.cc
+++ b/talk/media/webrtc/webrtcvoiceengine.cc
@@ -132,6 +132,16 @@ const int kMaxTelephoneEventCode = 255;
 const int kMinTelephoneEventDuration = 100;
 const int kMaxTelephoneEventDuration = 60000;   // Actual limit is 2^16
 
+class ProxySink : public webrtc::AudioSinkInterface {
+ public:
+  ProxySink(AudioSinkInterface* sink) : sink_(sink) {}
+
+  void OnData(const Data& audio) override { sink_->OnData(audio); }
+
+ private:
+  webrtc::AudioSinkInterface* sink_;
+};
+
 bool ValidateStreamParams(const StreamParams& sp) {
   if (sp.ssrcs.empty()) {
     LOG(LS_ERROR) << "No SSRCs in stream parameters: " << sp.ToString();
@@ -2186,6 +2196,9 @@ void WebRtcVoiceMediaChannel::OnPacketReceived(
     }
     default_recv_ssrc_ = ssrc;
     SetOutputVolume(default_recv_ssrc_, default_recv_volume_);
+    rtc::scoped_ptr<webrtc::AudioSinkInterface> proxy_sink(
+        default_sink_ ? new ProxySink(default_sink_.get()) : nullptr);
+    SetRawAudioSink(default_recv_ssrc_, std::move(proxy_sink));

[tommi, 2016/01/13 23:14:24]

Do we actually want to make this call if default_sink_ is null?  I was thinking
we'd just do something like

if (default_sink_)
  SetRawAudioSink(default_recv_ssrc_, new ProxySink(default_sink_.get()));

In either case, there's a concern that we might be overriding/disconnecting a
different sink if one was already assigned.  I guess we can check that later if
it's indeed a possibility.

[Taylor Brandstetter, 2016/01/13 23:55:12]

I had "if (default_sink_)" originally, but I was suggested to change it. I'll
put it back since it makes more sense now.

Also, at this point it will be impossible to override an existing sink, since
the stream was just created (inside AddRecvStream).

[the sun, 2016/01/14 09:24:39]

Yes, now that we can avoid allocating an object it makes sense.

   }
 
   // Forward packet to Call. If the SSRC is unknown we'll return after this.
@@ -2414,7 +2427,17 @@ void WebRtcVoiceMediaChannel::SetRawAudioSink(
     uint32_t ssrc,
     rtc::scoped_ptr<webrtc::AudioSinkInterface> sink) {
   RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
-  LOG(LS_VERBOSE) << "WebRtcVoiceMediaChannel::SetRawAudioSink";
+  LOG(LS_VERBOSE) << "WebRtcVoiceMediaChannel::SetRawAudioSink: ssrc:" << ssrc
+                  << " " << (sink ? "(ptr)" : "NULL");
+  if (ssrc == 0) {
+    if (default_recv_ssrc_ != -1) {
+      rtc::scoped_ptr<webrtc::AudioSinkInterface> proxy_sink(
+          sink ? new ProxySink(sink.get()) : nullptr);
+      SetRawAudioSink(default_recv_ssrc_, std::move(proxy_sink));
+    }
+    default_sink_ = std::move(sink);

[tommi, 2016/01/13 23:14:24]

nit: do you mind moving this above the check for -1 and use default_sink_ like
you do above?  For a while there I read this as |sink| going out of scope and
that the proxy would have a bogus pointer.

[Taylor Brandstetter, 2016/01/13 23:55:12]

Is the sink accessed on any other threads? If so I can't move this line, since
it will implicitly destroy the existing default sink (if there is one). The new
sink needs to be set before the old is destroyed.

[the sun, 2016/01/14 09:24:39]

I'd still like you to avoid the duplicate log lines that will result from
recursively calling SetRawAudioSink.

[tommi (sloooow) - chröme, 2016/01/14 12:34:13]

ah of course

[Taylor Brandstetter, 2016/01/14 15:48:47]

My reasoning is that SetRenderer does the same thing:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/libjin...

That's why I added the SSRC to the log, so you'll see:
"SetRawAudioSink: ssrc 0"
"SetRawAudioSink: ssrc X"

Which I've found is actually more helpful than having a single log line.

[the sun, 2016/01/14 15:51:44]

Acknowledged.

+    return;
+  }
   const auto it = recv_streams_.find(ssrc);
   if (it == recv_streams_.end()) {
     LOG(LS_WARNING) << "SetRawAudioSink: no recv stream" << ssrc;