Base A/V synchronization on sync_labels.

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 84a26c51b8558e02bfb6d25bda85494bc15eee0d..10de6ae0b8174fc2b85ef15dff51dced5fdf2910 100644
--- a/talk/media/webrtc/webrtcvoiceengine.cc
+++ b/talk/media/webrtc/webrtcvoiceengine.cc
@@ -1621,11 +1621,12 @@ class WebRtcVoiceMediaChannel::WebRtcVoiceChannelRenderer
     : public AudioRenderer::Sink {
  public:
   WebRtcVoiceChannelRenderer(int ch,
+                             const std::string& sync_label,
                              webrtc::AudioTransport* voe_audio_transport)
       : channel_(ch),
+        sync_label_(sync_label),
         voe_audio_transport_(voe_audio_transport),
-        renderer_(NULL) {
-  }
+        renderer_(NULL) {}
   ~WebRtcVoiceChannelRenderer() override { Stop(); }
 
   // Starts the rendering by setting a sink to the renderer to get data
@@ -1686,9 +1687,11 @@ class WebRtcVoiceMediaChannel::WebRtcVoiceChannelRenderer
 
   // Accessor to the VoE channel ID.
   int channel() const { return channel_; }
+  const std::string& sync_label() const { return sync_label_; }

[the sun, 2015/06/11 11:52:03]

Adding the sync label here feels backwards to me. I think going forward we will
be better served with a map ssrc->StreamParams, e.g. receive_stream_params_. and
to look there for the sync label.

[pbos-webrtc, 2015/06/11 14:48:52]

Done.

 
  private:
   const int channel_;
+  const std::string sync_label_;
   webrtc::AudioTransport* const voe_audio_transport_;
 
   // Raw pointer to AudioRenderer owned by LocalAudioTrackHandler.
@@ -2481,7 +2484,7 @@ bool WebRtcVoiceMediaChannel::AddSendStream(const StreamParams& sp) {
       engine()->voe()->base()->audio_transport();
   send_channels_.insert(std::make_pair(
       sp.first_ssrc(),
-      new WebRtcVoiceChannelRenderer(channel, audio_transport)));
+      new WebRtcVoiceChannelRenderer(channel, sp.sync_label, audio_transport)));

[the sun, 2015/06/11 11:52:03]

This will never be used in practise?

[pbos-webrtc, 2015/06/11 14:48:52]

Correct.

 
   // Set the send (local) SSRC.
   // If there are multiple send SSRCs, we can only set the first one here, and
@@ -2574,8 +2577,6 @@ bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) {
     return false;
   }
 
-  TryAddAudioRecvStream(ssrc);
-
   // Reuse default channel for recv stream in non-conference mode call
   // when the default channel is not being used.
   webrtc::AudioTransport* audio_transport =
@@ -2583,9 +2584,11 @@ bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) {
   if (!InConferenceMode() && default_receive_ssrc_ == 0) {
     LOG(LS_INFO) << "Recv stream " << ssrc << " reuse default channel";
     default_receive_ssrc_ = ssrc;
-    receive_channels_.insert(std::make_pair(
-        default_receive_ssrc_,
-        new WebRtcVoiceChannelRenderer(voe_channel(), audio_transport)));
+    WebRtcVoiceChannelRenderer* channel_renderer =
+        new WebRtcVoiceChannelRenderer(voe_channel(), sp.sync_label,
+                                       audio_transport);
+    receive_channels_.insert(std::make_pair(ssrc, channel_renderer));
+    TryAddAudioRecvStream(ssrc, channel_renderer);
     return SetPlayout(voe_channel(), playout_);
   }
 
@@ -2601,9 +2604,10 @@ bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) {
     return false;
   }
 
-  receive_channels_.insert(
-      std::make_pair(
-          ssrc, new WebRtcVoiceChannelRenderer(channel, audio_transport)));
+  WebRtcVoiceChannelRenderer* channel_renderer =
+      new WebRtcVoiceChannelRenderer(channel, sp.sync_label, audio_transport);
+  receive_channels_.insert(std::make_pair(ssrc, channel_renderer));
+  TryAddAudioRecvStream(ssrc, channel_renderer);
 
   LOG(LS_INFO) << "New audio stream " << ssrc
                << " registered to VoiceEngine channel #"
@@ -3450,7 +3454,7 @@ int WebRtcVoiceMediaChannel::GetReceiveChannelNum(uint32 ssrc) {
   ChannelMap::iterator it = receive_channels_.find(ssrc);
   if (it != receive_channels_.end())
     return it->second->channel();
-  return (ssrc == default_receive_ssrc_) ?  voe_channel() : -1;
+  return (ssrc == default_receive_ssrc_) ? voe_channel() : -1;
 }
 
 int WebRtcVoiceMediaChannel::GetSendChannelNum(uint32 ssrc) {
@@ -3468,7 +3472,7 @@ void WebRtcVoiceMediaChannel::SetCall(webrtc::Call* call) {
   }
   call_ = call;
   for (const auto& it : receive_channels_) {
-    TryAddAudioRecvStream(it.first);
+    TryAddAudioRecvStream(it.first, it.second);
   }
 }
 
@@ -3621,17 +3625,23 @@ bool WebRtcVoiceMediaChannel::SetHeaderExtension(ExtensionSetterFunction setter,
   return true;
 }
 
-void WebRtcVoiceMediaChannel::TryAddAudioRecvStream(uint32 ssrc) {
+void WebRtcVoiceMediaChannel::TryAddAudioRecvStream(
+    uint32 ssrc,
+    WebRtcVoiceChannelRenderer* channel) {

[the sun, 2015/06/11 11:52:03]

No need to take the ..Renderer* as argument. We can look it up from the
receive_channels_ map. Yes, I know you will say it requires an unnecessary map
lookup, to which I will answer it doesn't matter here (1. few objects in the
map, 2. code due for refactoring) - keep the interface simple instead!

[pbos-webrtc, 2015/06/11 14:48:52]

Done.

   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(receive_streams_.find(ssrc) == receive_streams_.end());
   // If we are hooked up to a webrtc::Call, create an AudioReceiveStream too.
-  if (call_ && options_.combined_audio_video_bwe.GetWithDefaultIfUnset(false)) {
-    DCHECK(receive_streams_.find(ssrc) == receive_streams_.end());
-    webrtc::AudioReceiveStream::Config config;
-    config.rtp.remote_ssrc = ssrc;
+  if (!call_)

[the sun, 2015/06/11 11:52:03]

{} please.

[pbos-webrtc, 2015/06/11 14:48:52]

Gah, done.

+    return;
+  webrtc::AudioReceiveStream::Config config;
+  config.rtp.remote_ssrc = ssrc;
+  // Only add RTP extensions if we support combined AV BWE.
+  if (options_.combined_audio_video_bwe.GetWithDefaultIfUnset(false))
     config.rtp.extensions = recv_rtp_extensions_;
-    webrtc::AudioReceiveStream* s = call_->CreateAudioReceiveStream(config);
-    receive_streams_.insert(std::make_pair(ssrc, s));
-  }
+  config.channel_id = channel->channel();
+  config.sync_group = channel->sync_label();
+  webrtc::AudioReceiveStream* s = call_->CreateAudioReceiveStream(config);
+  receive_streams_.insert(std::make_pair(ssrc, s));
 }
 
 void WebRtcVoiceMediaChannel::TryRemoveAudioRecvStream(uint32 ssrc) {