Base A/V synchronization on sync_labels.

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 1603 matching lines @@
 }
 
 int WebRtcVoiceEngine::CreateMediaVoiceChannel() {
   return CreateVoiceChannel(voe_wrapper_.get());
 }
 
 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
   // callback.
   // This method is called on the libjingle worker thread.
   // TODO(xians): Make sure Start() is called only once.
   void Start(AudioRenderer* renderer) {
     rtc::CritScope lock(&lock_);
     DCHECK(renderer != NULL);
     if (renderer_ != NULL) {
@@ skipping 40 matching lines @@
   // never been called, this callback won't be triggered.
   void OnClose() override {
     rtc::CritScope lock(&lock_);
     // Set |renderer_| to NULL to make sure no more callback will get into
     // the renderer.
     renderer_ = NULL;
   }
 
   // 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.
   // PeerConnection will make sure invalidating the pointer before the object
   // goes away.
   AudioRenderer* renderer_;
 
   // Protects |renderer_| in Start(), Stop() and OnClose().
   rtc::CriticalSection lock_;
 };
@@ skipping 772 matching lines @@
 
     ConfigureSendChannel(channel);
   }
 
   // Save the channel to send_channels_, so that RemoveSendStream() can still
   // delete the channel in case failure happens below.
   webrtc::AudioTransport* audio_transport =
       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
   // the rest of the SSRC(s) need to be set after SetSendCodec has been called
   // (with a codec requires multiple SSRC(s)).
   if (engine()->voe()->rtp()->SetLocalSSRC(channel, sp.first_ssrc()) == -1) {
     LOG_RTCERR2(SetSendSSRC, channel, sp.first_ssrc());
     return false;
   }
 
@@ skipping 72 matching lines @@
   if (ssrc == 0) {
     LOG(LS_WARNING) << "AddRecvStream with 0 ssrc is not supported.";
     return false;
   }
 
   if (receive_channels_.find(ssrc) != receive_channels_.end()) {
     LOG(LS_ERROR) << "Stream already exists with ssrc " << ssrc;
     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 =
       engine()->voe()->base()->audio_transport();
   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_);
   }
 
   // Create a new channel for receiving audio data.
   int channel = engine()->CreateMediaVoiceChannel();
   if (channel == -1) {
     LOG_RTCERR0(CreateChannel);
     return false;
   }
 
   if (!ConfigureRecvChannel(channel)) {
     DeleteChannel(channel);
     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 #"
                << channel << ".";
   return true;
 }
 
 bool WebRtcVoiceMediaChannel::ConfigureRecvChannel(int channel) {
   // Configure to use external transport, like our default channel.
   if (engine()->voe()->network()->RegisterExternalTransport(
@@ skipping 826 matching lines @@
   unsigned int ulevel;
   int ret =
       engine()->voe()->volume()->GetSpeechOutputLevel(channel, ulevel);
   return (ret == 0) ? static_cast<int>(ulevel) : -1;
 }
 
 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) {
   ChannelMap::iterator it = send_channels_.find(ssrc);
   if (it != send_channels_.end())
     return it->second->channel();
 
   return -1;
 }
 
 void WebRtcVoiceMediaChannel::SetCall(webrtc::Call* call) {
   DCHECK(thread_checker_.CalledOnValidThread());
   for (const auto& it : receive_channels_) {
     TryRemoveAudioRecvStream(it.first);
   }
   call_ = call;
   for (const auto& it : receive_channels_) {
-    TryAddAudioRecvStream(it.first);
+    TryAddAudioRecvStream(it.first, it.second);
   }
 }
 
 bool WebRtcVoiceMediaChannel::GetRedSendCodec(const AudioCodec& red_codec,
     const std::vector<AudioCodec>& all_codecs, webrtc::CodecInst* send_codec) {
   // Get the RED encodings from the parameter with no name. This may
   // change based on what is discussed on the Jingle list.
   // The encoding parameter is of the form "a/b"; we only support where
   // a == b. Verify this and parse out the value into red_pt.
   // If the parameter value is absent (as it will be until we wire up the
@@ skipping 132 matching lines @@
     id = extension->id;
     uri = extension->uri;
   }
   if ((engine()->voe()->rtp()->*setter)(channel_id, enable, id) != 0) {
     LOG_RTCERR4(*setter, uri, channel_id, enable, id);
     return false;
   }
   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) {
   DCHECK(thread_checker_.CalledOnValidThread());
   // If we are hooked up to a webrtc::Call, assume there is an
   // AudioReceiveStream to destroy too.
   if (call_) {
     auto stream_it = receive_streams_.find(ssrc);
     if (stream_it != receive_streams_.end()) {
       call_->DestroyAudioReceiveStream(stream_it->second);
@@ skipping 10 matching lines @@
 
 int WebRtcSoundclipStream::Rewind() {
   mem_.Rewind();
   // Return -1 to keep VoiceEngine from looping.
   return (loop_) ? 0 : -1;
 }
 
 }  // namespace cricket
 
 #endif  // HAVE_WEBRTC_VOICE