Injectable audio encoders: WebRtcVoiceEngine and company

webrtc/call/audio_send_stream.cc

Index: webrtc/call/audio_send_stream.cc
diff --git a/webrtc/call/audio_send_stream.cc b/webrtc/call/audio_send_stream.cc
index 6091462470dba0786850e12b889577653706b397..adc15403dbca15ba83e8297c37b0f6ea6028b839 100644
--- a/webrtc/call/audio_send_stream.cc
+++ b/webrtc/call/audio_send_stream.cc
@@ -12,21 +12,6 @@
 
 #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;
@@ -44,7 +29,8 @@ std::string AudioSendStream::Config::ToString() const {
   ss << ", voe_channel_id: " << voe_channel_id;
   ss << ", min_bitrate_bps: " << min_bitrate_bps;
   ss << ", max_bitrate_bps: " << max_bitrate_bps;
-  ss << ", send_codec_spec: " << send_codec_spec.ToString();
+  ss << ", send_codec_spec: "
+     << (send_codec_spec ? send_codec_spec->ToString() : "<unset>");
   ss << '}';
   return ss.str();
 }
@@ -70,24 +56,19 @@ std::string AudioSendStream::Config::Rtp::ToString() const {
   return ss.str();
 }
 
-AudioSendStream::Config::SendCodecSpec::SendCodecSpec() {
-  webrtc::CodecInst empty_inst = {0};
-  codec_inst = empty_inst;
-  codec_inst.pltype = -1;
-}
+AudioSendStream::Config::SendCodecSpec::SendCodecSpec(
+    int payload_type,
+    const SdpAudioFormat& format)
+    : payload_type(payload_type), format(format) {}
+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 << ", cng_payload_type: " << cng_payload_type.value_or(-1);

[the sun, 2017/04/24 14:48:49]

<unset>? Would be good to use one way to denote an unset Optional in logs.

[ossu, 2017/04/24 16:20:30]

Unfortunately, operator<< can only take one type and *cng_payload_type is (at
best) int. I wrote some code to properly print Optionals, but was asked to only
include it in test builds since "it would be nice if we could avoid using
iostream."

I guess I could refactor it into:
  ss << ", cng_payload_type: " << (cng_payload_type ?
std::to_string(*cng_payload_type) : "<unset>")

+  ss << ", payload_type: " << payload_type;
+  ss << ", format: " << format;
   ss << '}';
   return ss.str();
 }
@@ -96,12 +77,9 @@ 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;