Storing raw audio sink for default audio track.

talk/media/webrtc/webrtcvoiceengine.cc

 /*
  * libjingle
  * Copyright 2004 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  *  1. Redistributions of source code must retain the above copyright notice,
  *     this list of conditions and the following disclaimer.
  *  2. Redistributions in binary form must reproduce the above copyright notice,
@@ skipping 114 matching lines @@
 #else
 const char kAecDumpByAudioOptionFilename[] = "audio.aecdump";
 #endif
 
 // Constants from voice_engine_defines.h.
 const int kMinTelephoneEventCode = 0;           // RFC4733 (Section 2.3.1)
 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();
     return false;
   }
   if (sp.ssrcs.size() > 1) {
     LOG(LS_ERROR) << "Multiple SSRCs in stream parameters: " << sp.ToString();
     return false;
   }
   return true;
@@ skipping 2034 matching lines @@
   if (default_recv_ssrc_ == -1 && GetReceiveChannelId(ssrc) == -1) {
     StreamParams sp;
     sp.ssrcs.push_back(ssrc);
     LOG(LS_INFO) << "Creating default receive stream for SSRC=" << ssrc << ".";
     if (!AddRecvStream(sp)) {
       LOG(LS_WARNING) << "Could not create default receive stream.";
       return;
     }
     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.
   const webrtc::PacketTime webrtc_packet_time(packet_time.timestamp,
                                               packet_time.not_before);
   webrtc::PacketReceiver::DeliveryStatus delivery_result =
       call_->Receiver()->DeliverPacket(webrtc::MediaType::AUDIO,
           reinterpret_cast<const uint8_t*>(packet->data()), packet->size(),
           webrtc_packet_time);
   if (webrtc::PacketReceiver::DELIVERY_OK != delivery_result) {
@@ skipping 208 matching lines @@
     info->receivers.push_back(rinfo);
   }
 
   return true;
 }
 
 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;
     return;
   }
   it->second->SetRawAudioSink(std::move(sink));
 }
 
 int WebRtcVoiceMediaChannel::GetOutputLevel(int channel) {
   unsigned int ulevel = 0;
@@ skipping 72 matching lines @@
     }
   } else {
     LOG(LS_INFO) << "Stopping playout for channel #" << channel;
     engine()->voe()->base()->StopPlayout(channel);
   }
   return true;
 }
 }  // namespace cricket
 
 #endif  // HAVE_WEBRTC_VOICE