Remove default receive channel from WVoE; baby step 1.

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 130 matching lines @@
 // NOTE(ajm): Don't use hardcoded paths on platforms not explicitly specified
 // below.
 #if defined(CHROMEOS)
 const char kAecDumpByAudioOptionFilename[] = "/tmp/audio.aecdump";
 #elif defined(ANDROID)
 const char kAecDumpByAudioOptionFilename[] = "/sdcard/audio.aecdump";
 #else
 const char kAecDumpByAudioOptionFilename[] = "audio.aecdump";
 #endif
 
+bool ValidateStreamParams(const StreamParams& sp) {

[pthatcher1, 2015/10/06 18:19:52]

To go along with VerifyUniquePayloadTypes, you might call this VerifyOneSsrc.

[the sun, 2015/10/07 10:50:25]

That may not be the only thing it checks, going forward. I'm using the same name
as we do in WebRtcVideoChannel2.

+  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;
+}
+
 // Dumps an AudioCodec in RFC 2327-ish format.
 std::string ToString(const AudioCodec& codec) {
   std::stringstream ss;
   ss << codec.name << "/" << codec.clockrate << "/" << codec.channels
      << " (" << codec.id << ")";
   return ss.str();
 }
 
 std::string ToString(const webrtc::CodecInst& codec) {
   std::stringstream ss;
@@ skipping 53 matching lines @@
     if (c.Matches(codec)) {
       if (found_codec != NULL) {
         *found_codec = c;
       }
       return true;
     }
   }
   return false;
 }
 
+bool VerifyUniquePayloadTypes(const std::vector<AudioCodec>& codecs) {
+  if (codecs.empty()) {
+    return true;
+  }
+  std::vector<int> payload_types;
+  for (const AudioCodec& codec : codecs) {
+    payload_types.push_back(codec.id);
+  }
+  std::sort(payload_types.begin(), payload_types.end());
+  auto it = std::unique(payload_types.begin(), payload_types.end());
+  return it == payload_types.end();
+}
+
 bool IsNackEnabled(const AudioCodec& codec) {
   return codec.HasFeedbackParam(FeedbackParam(kRtcpFbParamNack,
                                               kParamValueEmpty));
 }
 
 int SelectPacketSize(const CodecPref& codec_pref, int ptime_ms) {
   int selected_packet_size_ms = codec_pref.packet_sizes_ms[0];
   for (int packet_size_ms : codec_pref.packet_sizes_ms) {
     if (packet_size_ms && packet_size_ms <= ptime_ms) {
       selected_packet_size_ms = packet_size_ms;
@@ skipping 1220 matching lines @@
   options_.SetAll(options);
 
   if (send_ != SEND_NOTHING) {
     if (!engine()->ApplyOptions(options_)) {
       LOG(LS_WARNING) <<
           "Failed to apply engine options during channel SetOptions.";
       return false;
     }
   }
 
+  if (!SetRecvOptions(voe_channel(), options)) {
+    return false;
+  }
+  for (const auto& ch : receive_channels_) {
+    if (!SetRecvOptions(ch.second->channel(), options)) {
+      return false;
+    }
+  }
+  if (dscp_option_changed) {
+    rtc::DiffServCodePoint dscp = rtc::DSCP_DEFAULT;
+    if (options_.dscp.GetWithDefaultIfUnset(false))
+      dscp = kAudioDscpValue;
+    if (MediaChannel::SetDscp(dscp) != 0) {
+      LOG(LS_WARNING) << "Failed to set DSCP settings for audio channel";
+    }
+  }
+  RecreateAudioReceiveStreams();
+  LOG(LS_INFO) << "Set voice channel options.  Current options: "
+               << options_.ToString();
+  return true;
+}
+
+bool WebRtcVoiceMediaChannel::SetRecvOptions(int channel_id,
+                                             const AudioOptions& options) {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+
   // Receiver-side auto gain control happens per channel, so set it here from
-  // options. Note that, like conference mode, setting it on the engine won't
-  // have the desired effect, since voice channels don't inherit options from
-  // the media engine when those options are applied per-channel.
+  // options. Note that voice channels don't inherit options from the media
+  // engine when those options are applied per-channel.
   bool rx_auto_gain_control;
   if (options.rx_auto_gain_control.Get(&rx_auto_gain_control)) {
     if (engine()->voe()->processing()->SetRxAgcStatus(
-            voe_channel(), rx_auto_gain_control,
+            channel_id, rx_auto_gain_control,
             webrtc::kAgcFixedDigital) == -1) {
       LOG_RTCERR1(SetRxAgcStatus, rx_auto_gain_control);
       return false;
     } else {
       LOG(LS_VERBOSE) << "Rx auto gain set to " << rx_auto_gain_control
                       << " with mode " << webrtc::kAgcFixedDigital;
     }
   }
   if (options.rx_agc_target_dbov.IsSet() ||
       options.rx_agc_digital_compression_gain.IsSet() ||
       options.rx_agc_limiter.IsSet()) {
     webrtc::AgcConfig config;
     // If only some of the options are being overridden, get the current
     // settings for the channel and bail if they aren't available.
     if (!options.rx_agc_target_dbov.IsSet() ||
         !options.rx_agc_digital_compression_gain.IsSet() ||
         !options.rx_agc_limiter.IsSet()) {
       if (engine()->voe()->processing()->GetRxAgcConfig(
-              voe_channel(), config) != 0) {
+              channel_id, config) != 0) {
         LOG(LS_ERROR) << "Failed to get default rx agc configuration for "
-                      << "channel " << voe_channel() << ". Since not all rx "
+                      << "channel " << channel_id << ". Since not all rx "
                       << "agc options are specified, unable to safely set rx "
                       << "agc options.";
         return false;
       }
     }
     config.targetLeveldBOv =
         options.rx_agc_target_dbov.GetWithDefaultIfUnset(
             config.targetLeveldBOv);
     config.digitalCompressionGaindB =
         options.rx_agc_digital_compression_gain.GetWithDefaultIfUnset(
             config.digitalCompressionGaindB);
     config.limiterEnable = options.rx_agc_limiter.GetWithDefaultIfUnset(
         config.limiterEnable);
     if (engine()->voe()->processing()->SetRxAgcConfig(
-            voe_channel(), config) == -1) {
-      LOG_RTCERR4(SetRxAgcConfig, voe_channel(), config.targetLeveldBOv,
+            channel_id, config) == -1) {
+      LOG_RTCERR4(SetRxAgcConfig, channel_id, config.targetLeveldBOv,
                   config.digitalCompressionGaindB, config.limiterEnable);
       return false;
     }
   }
-  if (dscp_option_changed) {
-    rtc::DiffServCodePoint dscp = rtc::DSCP_DEFAULT;
-    if (options_.dscp.GetWithDefaultIfUnset(false))
-      dscp = kAudioDscpValue;
-    if (MediaChannel::SetDscp(dscp) != 0) {
-      LOG(LS_WARNING) << "Failed to set DSCP settings for audio channel";
-    }
-  }
-
-  RecreateAudioReceiveStreams();
-
-  LOG(LS_INFO) << "Set voice channel options.  Current options: "
-               << options_.ToString();
   return true;
 }
 
 bool WebRtcVoiceMediaChannel::SetRecvCodecs(
     const std::vector<AudioCodec>& codecs) {
+  // Set the payload types to be used for incoming media.
+  LOG(LS_INFO) << "Setting receive voice codecs.";
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
-  // Set the payload types to be used for incoming media.
-  LOG(LS_INFO) << "Setting receive voice codecs:";
+
+  if (!VerifyUniquePayloadTypes(codecs)) {
+    LOG(LS_ERROR) << "Codec payload types overlap.";
+    return false;
+  }

[pthatcher1, 2015/10/06 18:19:52]

Can we add a unit test for this?

[the sun, 2015/10/07 10:50:25]

There is one already:
WebRtcVoiceEngineTestFake.SetRecvCodecsDuplicatePayloadType

 
   std::vector<AudioCodec> new_codecs;
   // Find all new codecs. We allow adding new codecs but don't allow changing
   // the payload type of codecs that is already configured since we might
   // already be receiving packets with that payload type.
   for (const AudioCodec& codec : codecs) {
     AudioCodec old_codec;
     if (FindCodec(recv_codecs_, codec, &old_codec)) {
       if (old_codec.id != codec.id) {
         LOG(LS_ERROR) << codec.name << " payload type changed.";
@@ skipping 677 matching lines @@
   if (send_channels_.empty())
     ChangeSend(SEND_NOTHING);
 
   return true;
 }
 
 bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   LOG(LS_INFO) << "AddRecvStream: " << sp.ToString();
 
-  rtc::CritScope lock(&receive_channels_cs_);
-
-  if (!VERIFY(sp.ssrcs.size() == 1))
-    return false;
-  uint32 ssrc = sp.first_ssrc();
-
-  if (ssrc == 0) {
-    LOG(LS_WARNING) << "AddRecvStream with 0 ssrc is not supported.";
+  if (!ValidateStreamParams(sp)) {
     return false;
   }
 
+  uint32 ssrc = sp.first_ssrc();
+  if (ssrc == 0) {
+    LOG(LS_WARNING) << "AddRecvStream with ssrc==0 is not supported.";
+    return false;
+  }
+
+  rtc::CritScope lock(&receive_channels_cs_);
+
   if (receive_channels_.find(ssrc) != receive_channels_.end()) {
     LOG(LS_ERROR) << "Stream already exists with ssrc " << ssrc;
     return false;
   }
 
   RTC_DCHECK(receive_stream_params_.find(ssrc) == receive_stream_params_.end());
 
   // Reuse default channel for recv stream in non-conference mode call
   // when the default channel is not being used.
   webrtc::AudioTransport* audio_transport =
@@ skipping 35 matching lines @@
 
 bool WebRtcVoiceMediaChannel::ConfigureRecvChannel(int channel) {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   // Configure to use external transport, like our default channel.
   if (engine()->voe()->network()->RegisterExternalTransport(
           channel, *this) == -1) {
     LOG_RTCERR2(SetExternalTransport, channel, this);
     return false;
   }
 
+  if (!SetRecvOptions(channel, options_)) {
+    return false;
+  }

[pthatcher1, 2015/10/06 18:19:52]

Can we add a unit test to cover that we are setting the options for all receive
streams and not just the default one?

+
   // Use the same SSRC as our default channel (so the RTCP reports are correct).
   unsigned int send_ssrc = 0;
   webrtc::VoERTP_RTCP* rtp = engine()->voe()->rtp();
   if (rtp->GetLocalSSRC(voe_channel(), send_ssrc) == -1) {
     LOG_RTCERR1(GetSendSSRC, channel);
     return false;
   }
   if (rtp->SetLocalSSRC(channel, send_ssrc) == -1) {
     LOG_RTCERR1(SetSendSSRC, channel);
     return false;
@@ skipping 349 matching lines @@
   // Sending channels need all RTCP packets with feedback information.
   // Even sender reports can contain attached report blocks.
   // Receiving channels need sender reports in order to create
   // correct receiver reports.
   int type = 0;
   if (!GetRtcpType(packet->data(), packet->size(), &type)) {
     LOG(LS_WARNING) << "Failed to parse type from received RTCP packet";
     return;
   }
 
-  // If it is a sender report, find the channel that is listening.
+  // If it is a sender report, find the receive channel that is listening.
   bool has_sent_to_default_channel = false;
   if (type == kRtcpTypeSR) {
-    int which_channel =
-        GetReceiveChannelId(ParseSsrc(packet->data(), packet->size(), true));
-    if (which_channel != -1) {
+    uint32 ssrc = 0;
+    if (!GetRtcpSsrc(packet->data(), packet->size(), &ssrc)) {
+      return;
+    }
+    int recv_channel_id = GetReceiveChannelId(ssrc);
+    if (recv_channel_id != -1) {
       engine()->voe()->network()->ReceivedRTCPPacket(
-          which_channel, packet->data(), packet->size());
+          recv_channel_id, packet->data(), packet->size());
 
-      if (IsDefaultChannel(which_channel))
+      if (IsDefaultChannel(recv_channel_id))
         has_sent_to_default_channel = true;
     }
   }
 
   // SR may continue RR and any RR entry may correspond to any one of the send
   // channels. So all RTCP packets must be forwarded all send channels. VoE
   // will filter out RR internally.
   for (const auto& ch : send_channels_) {
     // Make sure not sending the same packet to default channel more than once.
     if (IsDefaultChannel(ch.second->channel()) &&
@@ skipping 529 matching lines @@
       LOG(LS_WARNING) << "Unknown codec " << ToString(codec);
       return false;
     }
   }
   return true;
 }
 
 }  // namespace cricket
 
 #endif  // HAVE_WEBRTC_VOICE