Moving WebRtcVoiceMediaChannel::SendSetCodec to AudioSendStream.

webrtc/audio/audio_send_stream.cc

Index: webrtc/audio/audio_send_stream.cc
diff --git a/webrtc/audio/audio_send_stream.cc b/webrtc/audio/audio_send_stream.cc
index ee84b96aaa3700a8e7d937e0cf3d4ec1dfd0990d..18d035a7d5ee78f0a3ca6c685edfd467f505fe9b 100644
--- a/webrtc/audio/audio_send_stream.cc
+++ b/webrtc/audio/audio_send_stream.cc
@@ -10,6 +10,7 @@
 
 #include "webrtc/audio/audio_send_stream.h"
 
+#include <algorithm>
 #include <string>
 
 #include "webrtc/audio/audio_state.h"
@@ -30,6 +31,17 @@
 #include "webrtc/voice_engine/voice_engine_impl.h"
 
 namespace webrtc {
+
+namespace {
+
+constexpr char kOpusCodecName[] = "opus";
+
+bool IsCodec(const webrtc::CodecInst& codec, const char* ref_name) {
+  return (_stricmp(codec.plname, ref_name) == 0);
+}
+
+}  // namespace
+
 std::string AudioSendStream::Config::Rtp::ToString() const {
   std::stringstream ss;
   ss << "{ssrc: " << ssrc;
@@ -102,6 +114,7 @@ AudioSendStream::AudioSendStream(
       RTC_NOTREACHED() << "Registering unsupported RTP extension.";
     }
   }
+  SetSendCodecs();

[the sun, 2016/10/13 13:15:05]

if (!SetupSendCodec()) {
  LOG(LS_ERROR) << "Failed to set up send codec state.";
}

 }
 
 AudioSendStream::~AudioSendStream() {
@@ -285,5 +298,140 @@ VoiceEngine* AudioSendStream::voice_engine() const {
   RTC_DCHECK(voice_engine);
   return voice_engine;
 }
+
+// Apply current codec settings to a single voe::Channel used for sending.
+bool AudioSendStream::SetSendCodecs() {

[the sun, 2016/10/13 13:15:05]

Rename to SetupSendCodec() instead.

+  ScopedVoEInterface<VoECodec> codec(voice_engine());
+  const int channel = config_.voe_channel_id;
+
+  // Disable VAD and FEC unless we know the other side wants them.
+  codec->SetVADStatus(channel, false);
+  codec->SetFECStatus(channel, false);
+
+  // Set the codec immediately, since SetVADStatus() depends on whether

[the sun, 2016/10/13 13:15:05]

Do we need to check whether the codec_inst is initialized and not go further if
not? If codec_inst.pt == -1 codecs->SetSendCodec() will probably fail. Is this a
condition to worry about in practise?

[minyue-webrtc, 2016/10/17 07:41:59]

SetSendCodec checks pltype and return false if it is <0 or >127, that is what we
can rely on.

+  // the current codec is mono or stereo.
+  if (!SetSendCodec(config_.send_codec_spec.codec_inst)) {
+    return false;
+  }
+
+  // FEC should be enabled after SetSendCodec.
+  if (config_.send_codec_spec.enable_codec_fec) {
+    LOG(LS_INFO) << "Attempt to enable codec internal FEC on channel "
+                 << channel;
+    if (codec->SetFECStatus(channel, true) == -1) {
+      // Enable codec internal FEC. Treat any failure as fatal internal error.
+      // TODO(minyue): use normal logging.
+      // LOG_RTCERR2(SetFECStatus, channel, true);
+      return false;
+    }
+  }
+
+  if (IsCodec(config_.send_codec_spec.codec_inst, kOpusCodecName)) {
+    // DTX and maxplaybackrate should be set after SetSendCodec. Because current
+    // send codec has to be Opus.
+
+    // Set Opus internal DTX.
+    LOG(LS_INFO) << "Attempt to "
+                 << (config_.send_codec_spec.enable_opus_dtx ? "enable"
+                                                             : "disable")
+                 << " Opus DTX on channel " << channel;
+    if (codec->SetOpusDtx(channel, config_.send_codec_spec.enable_opus_dtx)) {
+      // TODO(minyue): use normal logging.
+      // LOG_RTCERR2(SetOpusDtx, channel,
+      // config_.send_codec_spec.enable_opus_dtx);
+      return false;
+    }
+
+    // If opus_max_playback_rate <= 0, the default maximum playback rate
+    // (48 kHz) will be used.
+    if (config_.send_codec_spec.opus_max_playback_rate > 0) {
+      LOG(LS_INFO) << "Attempt to set maximum playback rate to "
+                   << config_.send_codec_spec.opus_max_playback_rate
+                   << " Hz on channel " << channel;
+      if (codec->SetOpusMaxPlaybackRate(
+              channel, config_.send_codec_spec.opus_max_playback_rate) == -1) {
+        // TODO(minyue): use normal logging.
+        // LOG_RTCERR2(SetOpusMaxPlaybackRate, channel,
+        //             config_.send_codec_spec.opus_max_playback_rate);
+        return false;
+      }
+    }
+  }
+
+  // Set the CN payloadtype and the VAD status.
+  if (config_.send_codec_spec.cng_payload_type != -1) {
+    // The CN payload type for 8000 Hz clockrate is fixed at 13.
+    if (config_.send_codec_spec.cng_plfreq != 8000) {
+      webrtc::PayloadFrequencies cn_freq;
+      switch (config_.send_codec_spec.cng_plfreq) {
+        case 16000:
+          cn_freq = webrtc::kFreq16000Hz;
+          break;
+        case 32000:
+          cn_freq = webrtc::kFreq32000Hz;
+          break;
+        default:
+          RTC_NOTREACHED();
+          return false;
+      }
+      if (codec->SetSendCNPayloadType(channel,
+                                      config_.send_codec_spec.cng_payload_type,
+                                      cn_freq) == -1) {
+        // TODO(minyue): use normal logging.
+        // LOG_RTCERR3(SetSendCNPayloadType, channel,
+        //            config_.send_codec_spec.cng_payload_type, cn_freq);
+
+        // TODO(ajm): This failure condition will be removed from VoE.
+        // Restore the return here when we update to a new enough webrtc.
+        //
+        // Not returning false because the SetSendCNPayloadType will fail if
+        // the channel is already sending.
+        // This can happen if the remote description is applied twice, for
+        // example in the case of ROAP on top of JSEP, where both side will
+        // send the offer.
+      }
+    }
+
+    // Only turn on VAD if we have a CN payload type that matches the
+    // clockrate for the codec we are going to use.
+    if (config_.send_codec_spec.cng_plfreq ==
+            config_.send_codec_spec.codec_inst.plfreq &&
+        config_.send_codec_spec.codec_inst.channels == 1) {
+      // TODO(minyue): If CN frequency == 48000 Hz is allowed, consider the
+      // interaction between VAD and Opus FEC.
+      LOG(LS_INFO) << "Enabling VAD";
+      if (codec->SetVADStatus(channel, true) == -1) {
+        // TODO(minyue): use normal logging.
+        // LOG_RTCERR2(SetVADStatus, channel, true);
+        return false;
+      }
+    }
+  }
+  return true;
+}
+
+bool AudioSendStream::SetSendCodec(const webrtc::CodecInst& send_codec) {
+  // TODO(minyue): avoid ToString
+  // LOG(LS_INFO) << "Send channel " << channel <<  " selected voice codec "
+  //              << ToString(send_codec) << ", bitrate=" << send_codec.rate;
+
+  ScopedVoEInterface<VoECodec> codec(voice_engine());
+  int channel = config_.voe_channel_id;
+
+  webrtc::CodecInst current_codec = {0};
+  if (codec->GetSendCodec(channel, current_codec) == 0 &&

[the sun, 2016/10/13 13:15:05]

Add a TODO to look into whether this optimization is really needed.

Also, remove this function and add the code inline in SetSendCodecs()

[minyue-webrtc, 2016/10/17 07:41:59]

yes, added TODO and moved the function inline.

+      (send_codec == current_codec)) {
+    // Codec is already configured, we can return without setting it again.
+    return true;
+  }
+
+  if (codec->SetSendCodec(channel, send_codec) == -1) {
+    // TODO(minyue): use normal logging.
+    // LOG_RTCERR2(SetSendCodec, channel, ToString(send_codec));
+    return false;
+  }
+  return true;
+}
+
 }  // namespace internal
 }  // namespace webrtc