Move some send stream configuration into webrtc::AudioSendStream.

webrtc/audio/audio_send_stream.cc

 /*
  *  Copyright (c) 2015 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 16 matching lines @@
   std::stringstream ss;
   ss << "{ssrc: " << ssrc;
   ss << ", extensions: [";
   for (size_t i = 0; i < extensions.size(); ++i) {
     ss << extensions[i].ToString();
     if (i != extensions.size() - 1) {
       ss << ", ";
     }
   }
   ss << ']';
+  ss << ", c_name: " << c_name;
   ss << '}';
   return ss.str();
 }
 
 std::string AudioSendStream::Config::ToString() const {
   std::stringstream ss;
   ss << "{rtp: " << rtp.ToString();
   ss << ", voe_channel_id: " << voe_channel_id;
   // TODO(solenberg): Encoder config.
   ss << ", cng_payload_type: " << cng_payload_type;
   ss << ", red_payload_type: " << red_payload_type;
   ss << '}';
   return ss.str();
 }
 
 namespace internal {
 
 AudioSendStream::AudioSendStream(
     const webrtc::AudioSendStream::Config& config,
     const rtc::scoped_refptr<webrtc::AudioState>& audio_state)
     : config_(config), audio_state_(audio_state) {
   LOG(LS_INFO) << "AudioSendStream: " << config_.ToString();
   RTC_DCHECK_NE(config_.voe_channel_id, -1);
   RTC_DCHECK(audio_state_.get());
+
+  const int channel_id = config.voe_channel_id;
+  ScopedVoEInterface<VoERTP_RTCP> rtp(voice_engine());
+  RTC_CHECK_EQ(0, rtp->SetRTCPStatus(channel_id, true));
+  RTC_CHECK_EQ(0, rtp->SetLocalSSRC(channel_id, config.rtp.ssrc));
+  RTC_CHECK_EQ(0, rtp->SetRTCP_CNAME(channel_id, config.rtp.c_name.c_str()));

[tommi, 2015/11/12 13:44:57]

Are all these CHECKs reasonable?  I think checks are good but I think CHECKs
should be there for a reason (usually there would be a security issue if  CHECK
hits), but if the implementation is simple enough that it shouldn't fail, DCHECK
might be the way to go.  CHECK isn't free (as opposed to DCHECK), so my concern
is code bloat.

[the sun, 2015/11/12 15:16:44]

DCHECK is stripped out in non-debug builds, right?

Here's the condition in checks.h:
#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))

The code here is required in all builds, hence the CHECK. Going forward, the
methods on the underlying Channel object will be called directly and changed as
much as possible to not return any error codes unless necessary.

[tommi, 2015/11/12 16:36:18]

Yes, DCHECK is stripped out.  You don't have to put the expressions themselves
into the DCHECK though.

[the sun, 2015/11/16 13:49:51]

You're right. There will be some code bloat because of the CHECK. Also, since
I've actually verified that these calls will never fail in our case, DCHECKs
should be used (it's a logic assumption, not a potential runtime issue).

+  for (const auto& extension : config.rtp.extensions) {
+    // One-byte-extension local identifiers are in the range 1-14 inclusive.
+    RTC_DCHECK_GE(extension.id, 1);
+    RTC_DCHECK_LE(extension.id, 14);
+    if (extension.name == RtpExtension::kAbsSendTime) {
+      RTC_CHECK_EQ(0, rtp->SetSendAbsoluteSenderTimeStatus(channel_id, true,
+                                                           extension.id));
+    } else if (extension.name == RtpExtension::kAudioLevel) {
+      RTC_CHECK_EQ(0, rtp->SetSendAudioLevelIndicationStatus(channel_id, true,
+                                                             extension.id));
+    } else {
+      RTC_NOTREACHED() << "Registering unsupported RTP extension.";
+    }
+  }
 }
 
 AudioSendStream::~AudioSendStream() {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   LOG(LS_INFO) << "~AudioSendStream: " << config_.ToString();
 }
 
+void AudioSendStream::Start() {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+}
+
+void AudioSendStream::Stop() {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+}
+
+void AudioSendStream::SignalNetworkState(NetworkState state) {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+}
+
+bool AudioSendStream::DeliverRtcp(const uint8_t* packet, size_t length) {
+  // TODO(solenberg): Tests call this function on a network thread, libjingle
+  // calls on the worker thread. We should move towards always using a network
+  // thread. Then this check can be enabled.
+  // RTC_DCHECK(!thread_checker_.CalledOnValidThread());
+  return false;
+}
+
 webrtc::AudioSendStream::Stats AudioSendStream::GetStats() const {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   webrtc::AudioSendStream::Stats stats;
   stats.local_ssrc = config_.rtp.ssrc;
-  internal::AudioState* audio_state =
-      static_cast<internal::AudioState*>(audio_state_.get());
-  VoiceEngine* voice_engine = audio_state->voice_engine();
-  ScopedVoEInterface<VoEAudioProcessing> processing(voice_engine);
-  ScopedVoEInterface<VoECodec> codec(voice_engine);
-  ScopedVoEInterface<VoERTP_RTCP> rtp(voice_engine);
-  ScopedVoEInterface<VoEVolumeControl> volume(voice_engine);
+  ScopedVoEInterface<VoEAudioProcessing> processing(voice_engine());
+  ScopedVoEInterface<VoECodec> codec(voice_engine());
+  ScopedVoEInterface<VoERTP_RTCP> rtp(voice_engine());
+  ScopedVoEInterface<VoEVolumeControl> volume(voice_engine());
   unsigned int ssrc = 0;
   webrtc::CallStatistics call_stats = {0};
+  // TODO(solenberg): Change error code checking to RTC_CHECK_EQ(..., -1), if
+  //                  possible...
   if (rtp->GetLocalSSRC(config_.voe_channel_id, ssrc) == -1 ||
       rtp->GetRTCPStatistics(config_.voe_channel_id, call_stats) == -1) {
     return stats;
   }
 
   stats.bytes_sent = call_stats.bytesSent;
   stats.packets_sent = call_stats.packetsSent;
 
   webrtc::CodecInst codec_inst = {0};
   if (codec->GetSendCodec(config_.voe_channel_id, codec_inst) != -1) {
@@ skipping 55 matching lines @@
     int erl = -100;
     int erle = -100;
     int dummy1 = 0;
     int dummy2 = 0;
     if (processing->GetEchoMetrics(erl, erle, dummy1, dummy2) != -1) {
       stats.echo_return_loss = erl;
       stats.echo_return_loss_enhancement = erle;
     }
   }
 
+  internal::AudioState* audio_state =
+      static_cast<internal::AudioState*>(audio_state_.get());
   stats.typing_noise_detected = audio_state->typing_noise_detected();
 
   return stats;
 }
 
 const webrtc::AudioSendStream::Config& AudioSendStream::config() const {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   return config_;
 }
 
-void AudioSendStream::Start() {
-  RTC_DCHECK(thread_checker_.CalledOnValidThread());
-}
-
-void AudioSendStream::Stop() {
-  RTC_DCHECK(thread_checker_.CalledOnValidThread());
-}
-
-void AudioSendStream::SignalNetworkState(NetworkState state) {
-  RTC_DCHECK(thread_checker_.CalledOnValidThread());
-}
-
-bool AudioSendStream::DeliverRtcp(const uint8_t* packet, size_t length) {
-  // TODO(solenberg): Tests call this function on a network thread, libjingle
-  // calls on the worker thread. We should move towards always using a network
-  // thread. Then this check can be enabled.
-  // RTC_DCHECK(!thread_checker_.CalledOnValidThread());
-  return false;
+VoiceEngine* AudioSendStream::voice_engine() const {
+  internal::AudioState* audio_state =
+      static_cast<internal::AudioState*>(audio_state_.get());
+  VoiceEngine* voice_engine = audio_state->voice_engine();
+  RTC_DCHECK(voice_engine);
+  return voice_engine;
 }
 }  // namespace internal
 }  // namespace webrtc