Remove the default send channel in WVoE.

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 a3ea0f9a74d2683e24f7a9adb660f51d0d689f84..e3c94ce5eb6de6e561c3d3d249128dbf49ac0afd 100644
--- a/talk/media/webrtc/webrtcvoiceengine.cc
+++ b/talk/media/webrtc/webrtcvoiceengine.cc
@@ -148,6 +148,9 @@ const char kAecDumpByAudioOptionFilename[] = "/sdcard/audio.aecdump";
 const char kAecDumpByAudioOptionFilename[] = "audio.aecdump";
 #endif
 
+// See: https://code.google.com/p/webrtc/issues/detail?id=4740
+const int kDefaultRtcpReceiverReportSsrc = 1;
+
 bool ValidateStreamParams(const StreamParams& sp) {
   if (sp.ssrcs.empty()) {
     LOG(LS_ERROR) << "No SSRCs in stream parameters: " << sp.ToString();
@@ -591,13 +594,7 @@ void WebRtcVoiceEngine::Terminate() {
 
 VoiceMediaChannel* WebRtcVoiceEngine::CreateChannel(webrtc::Call* call,
     const AudioOptions& options) {
-  WebRtcVoiceMediaChannel* ch =
-      new WebRtcVoiceMediaChannel(this, options, call);
-  if (!ch->valid()) {
-    delete ch;
-    return nullptr;
-  }
-  return ch;
+  return new WebRtcVoiceMediaChannel(this, options, call);
 }
 
 bool WebRtcVoiceEngine::SetOptions(const AudioOptions& options) {
@@ -1300,12 +1297,8 @@ void WebRtcVoiceEngine::StopAecDump() {
   }
 }
 
-int WebRtcVoiceEngine::CreateVoiceChannel(VoEWrapper* voice_engine_wrapper) {
-  return voice_engine_wrapper->base()->CreateChannel(voe_config_);
-}
-
-int WebRtcVoiceEngine::CreateMediaVoiceChannel() {
-  return CreateVoiceChannel(voe_wrapper_.get());
+int WebRtcVoiceEngine::CreateVoEChannel() {
+  return voe_wrapper_->base()->CreateChannel(voe_config_);
 }
 
 class WebRtcVoiceMediaChannel::WebRtcVoiceChannelRenderer
@@ -1389,7 +1382,6 @@ WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel(WebRtcVoiceEngine* engine,
                                                  const AudioOptions& options,
                                                  webrtc::Call* call)
     : engine_(engine),
-      default_send_channel_id_(engine->CreateMediaVoiceChannel()),
       send_bitrate_setting_(false),
       send_bitrate_bps_(0),
       options_(),
@@ -1402,36 +1394,29 @@ WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel(WebRtcVoiceEngine* engine,
       send_(SEND_NOTHING),
       call_(call) {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
-  engine->RegisterChannel(this);
-  LOG(LS_VERBOSE) << "WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel "
-                  << default_send_channel_id();
+  LOG(LS_VERBOSE) << "WebRtcVoiceMediaChannel::WebRtcVoiceMediaChannel";
   RTC_DCHECK(nullptr != call);
-  ConfigureSendChannel(default_send_channel_id());
+  engine->RegisterChannel(this);
   SetOptions(options);
 }
 
 WebRtcVoiceMediaChannel::~WebRtcVoiceMediaChannel() {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
-  LOG(LS_VERBOSE) << "WebRtcVoiceMediaChannel::~WebRtcVoiceMediaChannel "
-                  << default_send_channel_id();
+  LOG(LS_VERBOSE) << "WebRtcVoiceMediaChannel::~WebRtcVoiceMediaChannel";
 
-  // Remove any remaining send streams, the default channel will be deleted
-  // later.
+  // Remove any remaining send streams.
   while (!send_channels_.empty()) {
     RemoveSendStream(send_channels_.begin()->first);
   }
 
-  // Unregister ourselves from the engine.
-  engine()->UnregisterChannel(this);
-
-  // Remove any remaining streams.
+  // Remove any remaining receive streams.
   while (!receive_channels_.empty()) {
     RemoveRecvStream(receive_channels_.begin()->first);
   }
   RTC_DCHECK(receive_streams_.empty());
 
-  // Delete the default channel.
-  DeleteChannel(default_send_channel_id());
+  // Unregister ourselves from the engine.
+  engine()->UnregisterChannel(this);
 }
 
 bool WebRtcVoiceMediaChannel::SetSendParameters(
@@ -1770,15 +1755,11 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs(
   }
 
   // Set nack status on receive channels and update |nack_enabled_|.
-  SetNack(receive_channels_, nack_enabled_);
-  return true;
-}
-
-void WebRtcVoiceMediaChannel::SetNack(const ChannelMap& channels,
-                                      bool nack_enabled) {
-  for (const auto& ch : channels) {
-    SetNack(ch.second->channel(), nack_enabled);
+  for (const auto& ch : receive_channels_) {
+    SetNack(ch.second->channel(), nack_enabled_);
   }
+
+  return true;
 }
 
 void WebRtcVoiceMediaChannel::SetNack(int channel, bool nack_enabled) {
@@ -1890,15 +1871,6 @@ bool WebRtcVoiceMediaChannel::SetSendRtpHeaderExtensions(
     return true;
   }
 
-  // The default channel may or may not be in |send_channels_|. Set the rtp
-  // header extensions for default channel regardless.
-
-  if (!SetChannelSendRtpHeaderExtensions(default_send_channel_id(),
-                                         extensions)) {
-    return false;
-  }
-
-  // Loop through all send channels and enable/disable the extensions.
   for (const auto& ch : send_channels_) {
     if (!SetChannelSendRtpHeaderExtensions(ch.second->channel(), extensions)) {
       return false;
@@ -2040,61 +2012,65 @@ bool WebRtcVoiceMediaChannel::SetAudioSend(uint32_t ssrc,
   return true;
 }
 
-// TODO(ronghuawu): Change this method to return bool.
-void WebRtcVoiceMediaChannel::ConfigureSendChannel(int channel) {
-  if (engine()->voe()->network()->RegisterExternalTransport(
-          channel, *this) == -1) {
-    LOG_RTCERR2(RegisterExternalTransport, channel, this);
+int WebRtcVoiceMediaChannel::CreateVoEChannel() {
+  int id = engine()->CreateVoEChannel();
+  if (id == -1) {
+    LOG_RTCERR0(CreateVoEChannel);
+    return -1;
   }
-
-  // Enable RTCP (for quality stats and feedback messages)
-  EnableRtcp(channel);
-
-  // Set RTP header extension for the new channel.
-  SetChannelSendRtpHeaderExtensions(channel, send_extensions_);
+  if (engine()->voe()->network()->RegisterExternalTransport(id, *this) == -1) {
+    LOG_RTCERR2(RegisterExternalTransport, id, this);
+    engine()->voe()->base()->DeleteChannel(id);
+    return -1;
+  }
+  return id;
 }
 
 bool WebRtcVoiceMediaChannel::DeleteChannel(int channel) {
   if (engine()->voe()->network()->DeRegisterExternalTransport(channel) == -1) {
     LOG_RTCERR1(DeRegisterExternalTransport, channel);
   }
-
   if (engine()->voe()->base()->DeleteChannel(channel) == -1) {
     LOG_RTCERR1(DeleteChannel, channel);
     return false;
   }
-
   return true;
 }
 
 bool WebRtcVoiceMediaChannel::AddSendStream(const StreamParams& sp) {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
-  // If the default channel is already used for sending create a new channel
-  // otherwise use the default channel for sending.
-  int channel = GetSendChannelId(sp.first_ssrc());
-  if (channel != -1) {
-    LOG(LS_ERROR) << "Stream already exists with ssrc " << sp.first_ssrc();
+  uint32_t ssrc = sp.first_ssrc();
+  RTC_DCHECK(0 != ssrc);
+
+  if (GetSendChannelId(ssrc) != -1) {
+    LOG(LS_ERROR) << "Stream already exists with ssrc " << ssrc;
     return false;
   }
 
-  bool default_channel_is_available = true;
-  for (const auto& ch : send_channels_) {
-    if (IsDefaultChannel(ch.second->channel())) {
-      default_channel_is_available = false;
-      break;
-    }
+  // Create a new channel for sending audio data.
+  int channel = CreateVoEChannel();
+  if (channel == -1) {
+    return false;

[pthatcher1, 2015/10/15 05:15:04]

This could use a LOG(LS_ERROR).

[the sun, 2015/10/15 12:07:12]

Added LOG(LS_INFO) at start of AddSendStream() instead. CreateVoEChannel()
already logs its errors; now it is evident in what context it fails.

+  }
+
+  // Enable RTCP (for quality stats and feedback messages).
+  if (engine()->voe()->rtp()->SetRTCPStatus(channel, true) == -1) {
+    LOG_RTCERR2(SetRTCPStatus, channel, 1);
+  }
+
+  SetChannelSendRtpHeaderExtensions(channel, send_extensions_);
+
+  // Set the local (send) SSRC.
+  if (engine()->voe()->rtp()->SetLocalSSRC(channel, ssrc) == -1) {
+    LOG_RTCERR2(SetLocalSSRC, channel, ssrc);
+    DeleteChannel(channel);
+    return false;
   }
-  if (default_channel_is_available) {
-    channel = default_send_channel_id();
-  } else {
-    // Create a new channel for sending audio data.
-    channel = engine()->CreateMediaVoiceChannel();
-    if (channel == -1) {
-      LOG_RTCERR0(CreateChannel);
-      return false;
-    }
 
-    ConfigureSendChannel(channel);
+  if (engine()->voe()->rtp()->SetRTCP_CNAME(channel, sp.cname.c_str()) == -1) {
+    LOG_RTCERR2(SetRTCP_CNAME, channel, sp.cname);
+    DeleteChannel(channel);
+    return false;
   }
 
   // Save the channel to send_channels_, so that RemoveSendStream() can still
@@ -2102,45 +2078,37 @@ bool WebRtcVoiceMediaChannel::AddSendStream(const StreamParams& sp) {
   webrtc::AudioTransport* audio_transport =
       engine()->voe()->base()->audio_transport();
   send_channels_.insert(
-      std::make_pair(sp.first_ssrc(),
+      std::make_pair(ssrc,
                      new WebRtcVoiceChannelRenderer(channel, audio_transport)));
 
-  // 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());
+  // Set the current codecs to be used for the new channel.
+  if (!send_codecs_.empty() && !SetSendCodecs(channel, send_codecs_)) {
+    RemoveSendStream(ssrc);
     return false;
   }

[pthatcher1, 2015/10/15 05:15:04]

Why do we do "add and then remove if SetSendCodecs fails" instead of trying to
SetSendCodecs and then add only if it succeeds?  That's how all the other
settings before this work.  What's special about SetSendCodecs?

[the sun, 2015/10/15 12:07:12]

Added comment.

 
   // At this point the channel's local SSRC has been updated. If the channel is
-  // the default channel make sure that all the receive channels are updated as
-  // well. Receive channels have to have the same SSRC as the default channel in
-  // order to send receiver reports with this SSRC.
-  if (IsDefaultChannel(channel)) {
+  // the first send channel make sure that all the receive channels are updated
+  // with the same SSRC in order to send receiver reports.
+  if (send_channels_.size() == 1) {
+    first_send_ssrc_ = ssrc;

[pthatcher1, 2015/10/15 05:15:04]

Would it make sense to call this "receiver_reports_ssrc_" or "rtcp_rr_ssrc_" and
default to 1 instead of -1?

[the sun, 2015/10/15 12:07:12]

Good idea, makes the logic clearer.

     for (const auto& ch : receive_channels_) {
-      if (engine()->voe()->rtp()->SetLocalSSRC(ch.second->channel(),
-                                               sp.first_ssrc()) != 0) {
-        LOG_RTCERR2(SetLocalSSRC, ch.second->channel(), sp.first_ssrc());
+      int recv_channel = ch.second->channel();
+      if (engine()->voe()->rtp()->SetLocalSSRC(recv_channel, ssrc) != 0) {
+        LOG_RTCERR2(SetLocalSSRC, ch.second->channel(), ssrc);
         return false;
       }
+      engine()->voe()->base()->AssociateSendChannel(recv_channel, channel);
+      LOG(LS_INFO) << "VoiceEngine channel #" << recv_channel
+                   << " is associated with channel #" << channel << ".";
     }
   }
 
-  if (engine()->voe()->rtp()->SetRTCP_CNAME(channel, sp.cname.c_str()) == -1) {
-    LOG_RTCERR2(SetRTCP_CNAME, channel, sp.cname);
-    return false;
-  }
-
-  // Set the current codecs to be used for the new channel.
-  if (!send_codecs_.empty() && !SetSendCodecs(channel, send_codecs_))
-    return false;
-
   return ChangeSend(channel, desired_send_);
 }
 
 bool WebRtcVoiceMediaChannel::RemoveSendStream(uint32_t ssrc) {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
   ChannelMap::iterator it = send_channels_.find(ssrc);
   if (it == send_channels_.end()) {
     LOG(LS_WARNING) << "Try to remove stream with ssrc " << ssrc
@@ -2156,21 +2124,15 @@ bool WebRtcVoiceMediaChannel::RemoveSendStream(uint32_t ssrc) {
   delete it->second;
   send_channels_.erase(it);
 
-  if (IsDefaultChannel(channel)) {
-    // Do not delete the default channel since the receive channels depend on
-    // the default channel, recycle it instead.
-    ChangeSend(channel, SEND_NOTHING);
-  } else {
-    // Clean up and delete the send channel.
-    LOG(LS_INFO) << "Removing audio send stream " << ssrc
-                 << " with VoiceEngine channel #" << channel << ".";
-    if (!DeleteChannel(channel))
-      return false;
+  // Clean up and delete the send channel.
+  LOG(LS_INFO) << "Removing audio send stream " << ssrc
+               << " with VoiceEngine channel #" << channel << ".";
+  if (!DeleteChannel(channel)) {
+    return false;
   }
-
-  if (send_channels_.empty())
+  if (send_channels_.empty()) {
     ChangeSend(SEND_NOTHING);
-
+  }
   return true;
 }
 
@@ -2201,9 +2163,8 @@ bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) {
   RTC_DCHECK(receive_stream_params_.find(ssrc) == receive_stream_params_.end());
 
   // Create a new channel for receiving audio data.
-  int channel = engine()->CreateMediaVoiceChannel();
+  int channel = CreateVoEChannel();
   if (channel == -1) {
-    LOG_RTCERR0(CreateChannel);
     return false;
   }
   if (!ConfigureRecvChannel(channel)) {
@@ -2227,34 +2188,23 @@ bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) {
 
 bool WebRtcVoiceMediaChannel::ConfigureRecvChannel(int channel) {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
-  // Configure to use external transport.
-  if (engine()->voe()->network()->RegisterExternalTransport(
-          channel, *this) == -1) {
-    LOG_RTCERR2(SetExternalTransport, channel, this);
-    return false;
-  }
 
-  // Use the same SSRC as our default send channel, so the RTCP reports are
-  // correct.
-  unsigned int send_ssrc = 0;
-  webrtc::VoERTP_RTCP* rtp = engine()->voe()->rtp();
-  if (rtp->GetLocalSSRC(default_send_channel_id(), send_ssrc) == -1) {
-    LOG_RTCERR1(GetSendSSRC, channel);
-    return false;
+  uint32_t rtcp_ssrc = kDefaultRtcpReceiverReportSsrc;
+  if (send_channels_.size() > 0) {
+    rtcp_ssrc = first_send_ssrc_;
+
+    // Associate receive channel with first send channel (so the receive channel
+    // can obtain RTT from the send channel)
+    int send_channel = send_channels_[first_send_ssrc_]->channel();
+    engine()->voe()->base()->AssociateSendChannel(channel, send_channel);
+    LOG(LS_INFO) << "VoiceEngine channel #" << channel
+                 << " is associated with channel #" << send_channel << ".";
   }
-  if (rtp->SetLocalSSRC(channel, send_ssrc) == -1) {
-    LOG_RTCERR1(SetSendSSRC, channel);
+  if (engine()->voe()->rtp()->SetLocalSSRC(channel, rtcp_ssrc) == -1) {
+    LOG_RTCERR1(SetLocalSSRC, channel);
     return false;
   }
 
-  // Associate receive channel to default send channel (so the receive channel
-  // can obtain RTT from the send channel).
-  engine()->voe()->base()->AssociateSendChannel(channel,
-                                                default_send_channel_id());
-  LOG(LS_INFO) << "VoiceEngine channel #"
-               << channel << " is associated with channel #"
-               << default_send_channel_id() << ".";
-
   // Turn off all supported codecs.
   int ncodecs = engine()->voe()->codec()->NumOfCodecs();
   for (int i = 0; i < ncodecs; ++i) {
@@ -2427,6 +2377,7 @@ bool WebRtcVoiceMediaChannel::InsertDtmf(uint32_t ssrc,
                                          int event,
                                          int duration,
                                          int flags) {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
   if (!dtmf_allowed_) {
     return false;
   }
@@ -2435,16 +2386,7 @@ bool WebRtcVoiceMediaChannel::InsertDtmf(uint32_t ssrc,
   if (flags & cricket::DF_SEND) {
     int channel = -1;
     if (ssrc == 0) {
-      bool default_channel_is_inuse = false;
-      for (const auto& ch : send_channels_) {
-        if (IsDefaultChannel(ch.second->channel())) {
-          default_channel_is_inuse = true;
-          break;
-        }
-      }
-      if (default_channel_is_inuse) {
-        channel = default_send_channel_id();
-      } else if (!send_channels_.empty()) {
+      if (send_channels_.size() > 0) {
         channel = send_channels_.begin()->second->channel();
       }
     } else {
@@ -2562,8 +2504,8 @@ void WebRtcVoiceMediaChannel::OnRtcpReceived(
 }
 
 bool WebRtcVoiceMediaChannel::MuteStream(uint32_t ssrc, bool muted) {
-  int channel =
-      (ssrc == 0) ? default_send_channel_id() : GetSendChannelId(ssrc);
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  int channel = GetSendChannelId(ssrc);
   if (channel == -1) {
     LOG(LS_WARNING) << "The specified ssrc " << ssrc << " is not in use.";
     return false;
@@ -2589,8 +2531,9 @@ bool WebRtcVoiceMediaChannel::MuteStream(uint32_t ssrc, bool muted) {
   }
 
   webrtc::AudioProcessing* ap = engine()->voe()->base()->audio_processing();
-  if (ap)
+  if (ap) {
     ap->set_output_will_be_muted(all_muted);
+  }
   return true;
 }
 
@@ -2598,7 +2541,6 @@ bool WebRtcVoiceMediaChannel::MuteStream(uint32_t ssrc, bool muted) {
 // SetMaxSendBitrate() in future.
 bool WebRtcVoiceMediaChannel::SetMaxSendBandwidth(int bps) {
   LOG(LS_INFO) << "WebRtcVoiceMediaChannel::SetMaxSendBandwidth.";
-
   return SetSendBitrateInternal(bps);
 }
 
@@ -2905,18 +2847,6 @@ bool WebRtcVoiceMediaChannel::GetRedSendCodec(const AudioCodec& red_codec,
   return false;
 }
 
-bool WebRtcVoiceMediaChannel::EnableRtcp(int channel) {
-  if (engine()->voe()->rtp()->SetRTCPStatus(channel, true) == -1) {
-    LOG_RTCERR2(SetRTCPStatus, channel, 1);
-    return false;
-  }
-  // TODO(juberti): Enable VQMon and RTCP XR reports, once we know what
-  // what we want to do with them.
-  // engine()->voe().EnableVQMon(default_send_channel_id(), true);
-  // engine()->voe().EnableRTCP_XR(default_send_channel_id(), true);
-  return true;
-}
-
 bool WebRtcVoiceMediaChannel::SetPlayout(int channel, bool playout) {
   if (playout) {
     LOG(LS_INFO) << "Starting playout for channel #" << channel;