Injectable audio encoders: WebRtcVoiceEngine and company

webrtc/call/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.
  */
 
 #include "webrtc/call/audio_send_stream.h"
 
 #include <string>
 
-namespace {
-
-std::string ToString(const webrtc::CodecInst& codec_inst) {
-  std::stringstream ss;
-  ss << "{pltype: " << codec_inst.pltype;
-  ss << ", plname: \"" << codec_inst.plname << "\"";
-  ss << ", plfreq: " << codec_inst.plfreq;
-  ss << ", pacsize: " << codec_inst.pacsize;
-  ss << ", channels: " << codec_inst.channels;
-  ss << ", rate: " << codec_inst.rate;
-  ss << '}';
-  return ss.str();
-}
-}  // namespace
-
 namespace webrtc {
 
 AudioSendStream::Stats::Stats() = default;
 AudioSendStream::Stats::~Stats() = default;
 
 AudioSendStream::Config::Config(Transport* send_transport)
     : send_transport(send_transport) {}
 
 AudioSendStream::Config::~Config() = default;
 
@@ skipping 23 matching lines @@
       ss << ", ";
     }
   }
   ss << ']';
   ss << ", nack: " << nack.ToString();
   ss << ", c_name: " << c_name;
   ss << '}';
   return ss.str();
 }
 
-AudioSendStream::Config::SendCodecSpec::SendCodecSpec() {
-  webrtc::CodecInst empty_inst = {0};
-  codec_inst = empty_inst;
-  codec_inst.pltype = -1;
-}
+AudioSendStream::Config::SendCodecSpec::SendCodecSpec()
+    : format("", 0, 0) {}

[kwiberg-webrtc, 2017/03/01 12:26:47]

Why have a default constructor for this type? rtc::Optional<SendCodecSpec> seems
like a more natural choice.

But we usually default-construct these and then set one field at a time, don't
we... ick.

[ossu, 2017/03/02 01:30:28]

That's the big problem, currently. Eventually, the SendCodecSpec could be
created from, say, an SdpAudioFormat and optionally a CNG payload id, and then
the rest of the fields could be filled in. In the current implementation, the
WebRtcAudioSendStream wrapper uses a SendCodecSpec and gets called repeatedly
with different parts of the information, and not necessarily in a reasonable
order. I'll fix that at some point, but not right now.

[ossu, 2017/03/20 18:19:48]

Seems I didn't really understand the flow previously. Now it seems to me
creating a stream before a SendCodecSpec can be constructed is completely
reasonable. I've made it optional and made its constructor better.

+
+AudioSendStream::Config::SendCodecSpec::~SendCodecSpec() = default;
 
 std::string AudioSendStream::Config::SendCodecSpec::ToString() const {
   std::stringstream ss;
   ss << "{nack_enabled: " << (nack_enabled ? "true" : "false");
   ss << ", transport_cc_enabled: " << (transport_cc_enabled ? "true" : "false");
-  ss << ", enable_codec_fec: " << (enable_codec_fec ? "true" : "false");
-  ss << ", enable_opus_dtx: " << (enable_opus_dtx ? "true" : "false");
-  ss << ", opus_max_playback_rate: " << opus_max_playback_rate;
   ss << ", cng_payload_type: " << cng_payload_type;
-  ss << ", cng_plfreq: " << cng_plfreq;
-  ss << ", min_ptime: " << min_ptime_ms;
-  ss << ", max_ptime: " << max_ptime_ms;
-  ss << ", codec_inst: " << ::ToString(codec_inst);
+  ss << ", payload_type: " << payload_type;
+  ss << ", format: " << format;
   ss << '}';
   return ss.str();
 }
 
 bool AudioSendStream::Config::SendCodecSpec::operator==(
     const AudioSendStream::Config::SendCodecSpec& rhs) const {
   if (nack_enabled == rhs.nack_enabled &&
       transport_cc_enabled == rhs.transport_cc_enabled &&
-      enable_codec_fec == rhs.enable_codec_fec &&
-      enable_opus_dtx == rhs.enable_opus_dtx &&
-      opus_max_playback_rate == rhs.opus_max_playback_rate &&
       cng_payload_type == rhs.cng_payload_type &&
-      cng_plfreq == rhs.cng_plfreq && max_ptime_ms == rhs.max_ptime_ms &&
-      min_ptime_ms == rhs.min_ptime_ms && codec_inst == rhs.codec_inst) {
+      payload_type == rhs.payload_type && format == rhs.format &&
+      target_bitrate_bps == rhs.target_bitrate_bps) {
     return true;
   }
   return false;
 }
 }  // namespace webrtc