Injectable audio encoders: WebRtcVoiceEngine and company

webrtc/media/engine/webrtcvoiceengine.cc

 /*
  *  Copyright (c) 2004 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 19 matching lines @@
 #include "webrtc/base/stringutils.h"
 #include "webrtc/base/trace_event.h"
 #include "webrtc/media/base/audiosource.h"
 #include "webrtc/media/base/mediaconstants.h"
 #include "webrtc/media/base/streamparams.h"
 #include "webrtc/media/engine/payload_type_mapper.h"
 #include "webrtc/media/engine/webrtcmediaengine.h"
 #include "webrtc/media/engine/webrtcvoe.h"
 #include "webrtc/modules/audio_coding/acm2/rent_a_codec.h"
 #include "webrtc/modules/audio_mixer/audio_mixer_impl.h"
+#include "webrtc/modules/audio_coding/codecs/builtin_audio_encoder_factory.h"
 #include "webrtc/modules/audio_processing/include/audio_processing.h"
 #include "webrtc/system_wrappers/include/field_trial.h"
 #include "webrtc/system_wrappers/include/trace.h"
 
 namespace cricket {
 namespace {
 
 const int kDefaultTraceFilter = webrtc::kTraceNone | webrtc::kTraceTerseInfo |
                                 webrtc::kTraceWarning | webrtc::kTraceError |
                                 webrtc::kTraceCritical;
@@ skipping 15 matching lines @@
 constexpr int kNackRtpHistoryMs = 5000;
 
 // Check to verify that the define for the intelligibility enhancer is properly
 // set.
 #if !defined(WEBRTC_INTELLIGIBILITY_ENHANCER) || \
     (WEBRTC_INTELLIGIBILITY_ENHANCER != 0 &&     \
      WEBRTC_INTELLIGIBILITY_ENHANCER != 1)
 #error "Set WEBRTC_INTELLIGIBILITY_ENHANCER to either 0 or 1"
 #endif
 
-// Codec parameters for Opus.
-// draft-spittka-payload-rtp-opus-03
-
-// Recommended bitrates:
-// 8-12 kb/s for NB speech,
-// 16-20 kb/s for WB speech,
-// 28-40 kb/s for FB speech,
-// 48-64 kb/s for FB mono music, and
-// 64-128 kb/s for FB stereo music.
-// The current implementation applies the following values to mono signals,
-// and multiplies them by 2 for stereo.
-const int kOpusBitrateNbBps = 12000;
-const int kOpusBitrateWbBps = 20000;
+// For SendSideBwe, Opus bitrate should be in the range between 6000 and 32000.

[the sun, 2017/03/16 08:48:19]

32000? 

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

Yup, seems so. I've just rearranged the code a bit and added an explanatory
comment. Perhaps I should remove the comment and move the constants close to
their use instead?

+const int kOpusMinBitrateBps = 6000;
 const int kOpusBitrateFbBps = 32000;
 
-// Opus bitrate should be in the range between 6000 and 510000.
-const int kOpusMinBitrateBps = 6000;
-const int kOpusMaxBitrateBps = 510000;
-
-// iSAC bitrate should be <= 56000.
-const int kIsacMaxBitrateBps = 56000;
-
 // Default audio dscp value.
 // See http://tools.ietf.org/html/rfc2474 for details.
 // See also http://tools.ietf.org/html/draft-jennings-rtcweb-qos-00
 const rtc::DiffServCodePoint kAudioDscpValue = rtc::DSCP_EF;
 
 // Constants from voice_engine_defines.h.
 const int kMinTelephoneEventCode = 0;           // RFC4733 (Section 2.3.1)
 const int kMaxTelephoneEventCode = 255;
 const int kMinTelephoneEventDuration = 100;
 const int kMaxTelephoneEventDuration = 60000;   // Actual limit is 2^16
@@ skipping 19 matching lines @@
   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 << ")";
+  ss << codec.name << "/" << codec.clockrate << "/" << codec.channels;
+  if (!codec.params.empty()) {
+    ss << " {";
+        for (const auto& param : codec.params) {
+          ss << " " << param.first << "=" << param.second;
+        }

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

Broken indentation. Also, isn't there a second parameter map?

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

Wow, what happened here? :)
I guess you're talking about the feedback params? Yes, there is. It would
probably make sense to print them as well.

+    ss << " }";
+  }
+  ss << " (" << codec.id << ")";
   return ss.str();
 }
 
 std::string ToString(const webrtc::CodecInst& codec) {
   std::stringstream ss;
   ss << codec.plname << "/" << codec.plfreq << "/" << codec.channels
      << " (" << codec.pltype << ")";
   return ss.str();
 }
 
@@ skipping 25 matching lines @@
   }
   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();
 }
 
-// Return true if codec.params[feature] == "1", false otherwise.
-bool IsCodecFeatureEnabled(const AudioCodec& codec, const char* feature) {
-  int value;
-  return codec.GetParam(feature, &value) && value == 1;
-}
-
 rtc::Optional<std::string> GetAudioNetworkAdaptorConfig(
     const AudioOptions& options) {
   if (options.audio_network_adaptor && *options.audio_network_adaptor &&
       options.audio_network_adaptor_config) {
     // Turn on audio network adaptor only when |options_.audio_network_adaptor|
     // equals true and |options_.audio_network_adaptor_config| has a value.
     return options.audio_network_adaptor_config;
   }
   return rtc::Optional<std::string>();
 }
 
-// Returns integer parameter params[feature] if it is defined. Returns
-// |default_value| otherwise.
-int GetCodecFeatureInt(const AudioCodec& codec,
-                       const char* feature,
-                       int default_value) {
-  int value = 0;
-  if (codec.GetParam(feature, &value)) {
-    return value;
-  }
-  return default_value;
-}
-
-// Use params[kCodecParamMaxAverageBitrate] if it is defined, use codec.bitrate
-// otherwise. If the value (either from params or codec.bitrate) <=0, use the
-// default configuration. If the value is beyond feasible bit rate of Opus,
-// clamp it. Returns the Opus bit rate for operation.
-int GetOpusBitrate(const AudioCodec& codec, int max_playback_rate) {
-  int bitrate = 0;
-  bool use_param = true;
-  if (!codec.GetParam(kCodecParamMaxAverageBitrate, &bitrate)) {
-    bitrate = codec.bitrate;
-    use_param = false;
-  }
-  if (bitrate <= 0) {
-    if (max_playback_rate <= 8000) {
-      bitrate = kOpusBitrateNbBps;
-    } else if (max_playback_rate <= 16000) {
-      bitrate = kOpusBitrateWbBps;
-    } else {
-      bitrate = kOpusBitrateFbBps;
-    }
-
-    if (IsCodecFeatureEnabled(codec, kCodecParamStereo)) {
-      bitrate *= 2;
-    }
-  } else if (bitrate < kOpusMinBitrateBps || bitrate > kOpusMaxBitrateBps) {
-    bitrate = (bitrate < kOpusMinBitrateBps) ? kOpusMinBitrateBps
-                                             : kOpusMaxBitrateBps;
-    std::string rate_source =
-        use_param ? "Codec parameter \"maxaveragebitrate\"" :
-            "Supplied Opus bitrate";
-    LOG(LS_WARNING) << rate_source
-                    << " is invalid and is replaced by: "
-                    << bitrate;
-  }
-  return bitrate;
-}
-
-void GetOpusConfig(const AudioCodec& codec,
-                   webrtc::CodecInst* voe_codec,
-                   bool* enable_codec_fec,
-                   int* max_playback_rate,
-                   bool* enable_codec_dtx,
-                   int* min_ptime_ms,
-                   int* max_ptime_ms) {
-  *enable_codec_fec = IsCodecFeatureEnabled(codec, kCodecParamUseInbandFec);
-  *enable_codec_dtx = IsCodecFeatureEnabled(codec, kCodecParamUseDtx);
-  *max_playback_rate = GetCodecFeatureInt(codec, kCodecParamMaxPlaybackRate,
-                                          kOpusDefaultMaxPlaybackRate);
-  *max_ptime_ms =
-      GetCodecFeatureInt(codec, kCodecParamMaxPTime, kOpusDefaultMaxPTime);
-  *min_ptime_ms =
-      GetCodecFeatureInt(codec, kCodecParamMinPTime, kOpusDefaultMinPTime);
-  if (*max_ptime_ms < *min_ptime_ms) {
-    // If min ptime or max ptime defined by codec parameter is wrong, we use
-    // the default values.
-    *max_ptime_ms = kOpusDefaultMaxPTime;
-    *min_ptime_ms = kOpusDefaultMinPTime;
-  }
-
-  // If OPUS, change what we send according to the "stereo" codec
-  // parameter, and not the "channels" parameter.  We set
-  // voe_codec.channels to 2 if "stereo=1" and 1 otherwise.  If
-  // the bitrate is not specified, i.e. is <= zero, we set it to the
-  // appropriate default value for mono or stereo Opus.
-  voe_codec->channels = IsCodecFeatureEnabled(codec, kCodecParamStereo) ? 2 : 1;
-  voe_codec->rate = GetOpusBitrate(codec, *max_playback_rate);
-}
-
 webrtc::AudioState::Config MakeAudioStateConfig(
     VoEWrapper* voe_wrapper,
     rtc::scoped_refptr<webrtc::AudioMixer> audio_mixer) {
   webrtc::AudioState::Config config;
   config.voice_engine = voe_wrapper->engine();
   if (audio_mixer) {
     config.audio_mixer = audio_mixer;
   } else {
     config.audio_mixer = webrtc::AudioMixerImpl::Create();
   }
   return config;
 }
 
 class WebRtcVoiceCodecs final {
  public:
-  // TODO(solenberg): Do this filtering once off-line, add a simple AudioCodec
-  // list and add a test which verifies VoE supports the listed codecs.
-  static std::vector<AudioCodec> SupportedSendCodecs() {
-    std::vector<AudioCodec> result;
-    // Iterate first over our preferred codecs list, so that the results are
-    // added in order of preference.
-    for (size_t i = 0; i < arraysize(kCodecPrefs); ++i) {
-      const CodecPref* pref = &kCodecPrefs[i];
-      for (webrtc::CodecInst voe_codec : webrtc::acm2::RentACodec::Database()) {
-        // Change the sample rate of G722 to 8000 to match SDP.
-        MaybeFixupG722(&voe_codec, 8000);
-        // Skip uncompressed formats.
-        if (IsCodec(voe_codec, kL16CodecName)) {
-          continue;
-        }
-
-        if (!IsCodec(voe_codec, pref->name) ||
-            pref->clockrate != voe_codec.plfreq ||
-            pref->channels != voe_codec.channels) {
-          // Not a match.
-          continue;
-        }
-
-        AudioCodec codec(pref->payload_type, voe_codec.plname, voe_codec.plfreq,
-                         voe_codec.rate, voe_codec.channels);
-        LOG(LS_INFO) << "Adding supported codec: " << ToString(codec);
-        if (IsCodec(codec, kIsacCodecName)) {
-          // Indicate auto-bitrate in signaling.
-          codec.bitrate = 0;
-        }
-        if (IsCodec(codec, kOpusCodecName)) {
-          // Only add fmtp parameters that differ from the spec.
-          if (kPreferredMinPTime != kOpusDefaultMinPTime) {
-            codec.params[kCodecParamMinPTime] =
-                rtc::ToString(kPreferredMinPTime);
-          }
-          if (kPreferredMaxPTime != kOpusDefaultMaxPTime) {
-            codec.params[kCodecParamMaxPTime] =
-                rtc::ToString(kPreferredMaxPTime);
-          }
-          codec.SetParam(kCodecParamUseInbandFec, 1);
-          codec.AddFeedbackParam(
-              FeedbackParam(kRtcpFbParamTransportCc, kParamValueEmpty));
-
-          // TODO(hellner): Add ptime, sprop-stereo, and stereo
-          // when they can be set to values other than the default.
-        }
-        result.push_back(codec);
-      }
-    }
-    return result;
-  }
-
-  static bool ToCodecInst(const AudioCodec& in,
-                          webrtc::CodecInst* out) {
+  static bool ToCodecInst(const AudioCodec& in, webrtc::CodecInst* out) {

[the sun, 2017/03/02 20:37:24]

So this is just needed for tests now? And it is static? If you don't want to
update the tests and get rid of it in this CL, could you at least move it into
webrtcvoiceengine_unittest.cc?

[ossu, 2017/03/02 20:43:27]

I'll try to clean up the tests, so we won't need this one at all. Wanted to get
the functional change up as a patch set so we could have a look at it and decide
if it was better than the other way around. I think so.

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

Cleaned up (or plain removed) all remaining tests using it. Turns out it's still
needed until https://codereview.webrtc.org/2686043006/ lands. I guess whoever
lands last gets to remove this function and all it depends on. :)

     for (webrtc::CodecInst voe_codec : webrtc::acm2::RentACodec::Database()) {
       // Change the sample rate of G722 to 8000 to match SDP.
       MaybeFixupG722(&voe_codec, 8000);
       AudioCodec codec(voe_codec.pltype, voe_codec.plname, voe_codec.plfreq,
                        voe_codec.rate, voe_codec.channels);
-      bool multi_rate = IsCodecMultiRate(voe_codec);
+      const bool multi_rate =
+          IsCodec(codec, kIsacCodecName) || IsCodec(codec, kOpusCodecName);
       // Allow arbitrary rates for ISAC to be specified.
       if (multi_rate) {
         // Set codec.bitrate to 0 so the check for codec.Matches() passes.
         codec.bitrate = 0;
       }
       if (codec.Matches(in)) {
         if (out) {
           // Fixup the payload type.
           voe_codec.pltype = in.id;
 
           // Set bitrate if specified.
           if (multi_rate && in.bitrate != 0) {
             voe_codec.rate = in.bitrate;
           }
 
           // Reset G722 sample rate to 16000 to match WebRTC.
           MaybeFixupG722(&voe_codec, 16000);
 
           *out = voe_codec;
         }
         return true;
       }
     }
     return false;
   }
 
-  static bool IsCodecMultiRate(const webrtc::CodecInst& codec) {
-    for (size_t i = 0; i < arraysize(kCodecPrefs); ++i) {
-      if (IsCodec(codec, kCodecPrefs[i].name) &&
-          kCodecPrefs[i].clockrate == codec.plfreq) {
-        return kCodecPrefs[i].is_multi_rate;
-      }
-    }
-    return false;
-  }
-
-  static int MaxBitrateBps(const webrtc::CodecInst& codec) {
-    for (size_t i = 0; i < arraysize(kCodecPrefs); ++i) {
-      if (IsCodec(codec, kCodecPrefs[i].name) &&
-          kCodecPrefs[i].clockrate == codec.plfreq) {
-        return kCodecPrefs[i].max_bitrate_bps;
-      }
-    }
-    return 0;
-  }
-
-  static rtc::ArrayView<const int> GetPacketSizesMs(
-      const webrtc::CodecInst& codec) {
-    for (size_t i = 0; i < arraysize(kCodecPrefs); ++i) {
-      if (IsCodec(codec, kCodecPrefs[i].name)) {
-        size_t num_packet_sizes = kMaxNumPacketSize;
-        for (int index = 0; index < kMaxNumPacketSize; index++) {
-          if (kCodecPrefs[i].packet_sizes_ms[index] == 0) {
-            num_packet_sizes = index;
-            break;
-          }
-        }
-        return rtc::ArrayView<const int>(kCodecPrefs[i].packet_sizes_ms,
-                                         num_packet_sizes);
-      }
-    }
-    return rtc::ArrayView<const int>();
-  }
-
-  // If the AudioCodec param kCodecParamPTime is set, then we will set it to
-  // codec pacsize if it's valid, or we will pick the next smallest value we
-  // support.
-  // TODO(Brave): Query supported packet sizes from ACM when the API is ready.
-  static bool SetPTimeAsPacketSize(webrtc::CodecInst* codec, int ptime_ms) {
-    for (const CodecPref& codec_pref : kCodecPrefs) {
-      if ((IsCodec(*codec, codec_pref.name) &&
-          codec_pref.clockrate == codec->plfreq) ||
-          IsCodec(*codec, kG722CodecName)) {
-        int packet_size_ms = SelectPacketSize(codec_pref, ptime_ms);
-        if (packet_size_ms) {
-          // Convert unit from milli-seconds to samples.
-          codec->pacsize = (codec->plfreq / 1000) * packet_size_ms;
-          return true;
-        }
-      }
-    }
-    return false;
-  }
-
-  static const AudioCodec* GetPreferredCodec(
-      const std::vector<AudioCodec>& codecs,
-      webrtc::CodecInst* out) {
-    RTC_DCHECK(out);
-    // Select the preferred send codec (the first non-telephone-event/CN codec).
-    for (const AudioCodec& codec : codecs) {
-      if (IsCodec(codec, kDtmfCodecName) || IsCodec(codec, kCnCodecName)) {
-        // Skip telephone-event/CN codecs - they will be handled later.
-        continue;
-      }
-
-      // We'll use the first codec in the list to actually send audio data.
-      // Be sure to use the payload type requested by the remote side.
-      // Ignore codecs we don't know about. The negotiation step should prevent
-      // this, but double-check to be sure.
-      if (!ToCodecInst(codec, out)) {
-        LOG(LS_WARNING) << "Unknown codec " << ToString(codec);
-        continue;
-      }
-      return &codec;
-    }
-    return nullptr;
-  }
-
- private:
-  static const int kMaxNumPacketSize = 6;
-  struct CodecPref {
-    const char* name;
-    int clockrate;
-    size_t channels;
-    int payload_type;
-    bool is_multi_rate;
-    int packet_sizes_ms[kMaxNumPacketSize];
-    int max_bitrate_bps;
-  };
-  // Note: keep the supported packet sizes in ascending order.
-  static const CodecPref kCodecPrefs[14];
-
-  static 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;
-      }
-    }
-    return selected_packet_size_ms;
-  }
-
   // Changes RTP timestamp rate of G722. This is due to the "bug" in the RFC
   // which says that G722 should be advertised as 8 kHz although it is a 16 kHz
   // codec.
   static void MaybeFixupG722(webrtc::CodecInst* voe_codec, int new_plfreq) {
     if (IsCodec(*voe_codec, kG722CodecName)) {
       // If the DCHECK triggers, the codec definition in WebRTC VoiceEngine
       // has changed, and this special case is no longer needed.
       RTC_DCHECK(voe_codec->plfreq != new_plfreq);
       voe_codec->plfreq = new_plfreq;
     }
   }
 };
 
-const WebRtcVoiceCodecs::CodecPref WebRtcVoiceCodecs::kCodecPrefs[14] = {
-#if WEBRTC_OPUS_SUPPORT_120MS_PTIME
-    {kOpusCodecName, 48000, 2, 111, true, {10, 20, 40, 60, 120},
-     kOpusMaxBitrateBps},
-#else
-    {kOpusCodecName, 48000, 2, 111, true, {10, 20, 40, 60}, kOpusMaxBitrateBps},
-#endif
-    {kIsacCodecName, 16000, 1, 103, true, {30, 60}, kIsacMaxBitrateBps},
-    {kIsacCodecName, 32000, 1, 104, true, {30}, kIsacMaxBitrateBps},
-    // G722 should be advertised as 8000 Hz because of the RFC "bug".
-    {kG722CodecName, 8000, 1, 9, false, {10, 20, 30, 40, 50, 60}},
-    {kIlbcCodecName, 8000, 1, 102, false, {20, 30, 40, 60}},
-    {kPcmuCodecName, 8000, 1, 0, false, {10, 20, 30, 40, 50, 60}},
-    {kPcmaCodecName, 8000, 1, 8, false, {10, 20, 30, 40, 50, 60}},
-    {kCnCodecName, 32000, 1, 106, false, {}},
-    {kCnCodecName, 16000, 1, 105, false, {}},
-    {kCnCodecName, 8000, 1, 13, false, {}},
-    {kDtmfCodecName, 48000, 1, 110, false, {}},
-    {kDtmfCodecName, 32000, 1, 112, false, {}},
-    {kDtmfCodecName, 16000, 1, 113, false, {}},
-    {kDtmfCodecName, 8000, 1, 126, false, {}}
-};
-
 // |max_send_bitrate_bps| is the bitrate from "b=" in SDP.
 // |rtp_max_bitrate_bps| is the bitrate from RtpSender::SetParameters.
 rtc::Optional<int> ComputeSendBitrate(int max_send_bitrate_bps,
                                       rtc::Optional<int> rtp_max_bitrate_bps,
-                                      const webrtc::CodecInst& codec_inst) {
+                                      const webrtc::AudioCodecSpec& spec) {
   // If application-configured bitrate is set, take minimum of that and SDP
   // bitrate.
   const int bps = rtp_max_bitrate_bps
                       ? MinPositive(max_send_bitrate_bps, *rtp_max_bitrate_bps)
                       : max_send_bitrate_bps;
-  const int codec_rate = codec_inst.rate;
-
   if (bps <= 0) {
-    return rtc::Optional<int>(codec_rate);
+    return rtc::Optional<int>(spec.info.default_bitrate_bps);
   }
 
-  if (codec_inst.pltype == -1) {
-    return rtc::Optional<int>(codec_rate);
-    ;
-  }
-
-  if (WebRtcVoiceCodecs::IsCodecMultiRate(codec_inst)) {
-    // If codec is multi-rate then just set the bitrate.
-    return rtc::Optional<int>(
-        std::min(bps, WebRtcVoiceCodecs::MaxBitrateBps(codec_inst)));
-  }
-
-  if (bps < codec_inst.rate) {
+  if (bps < spec.info.min_bitrate_bps) {
     // If codec is not multi-rate and |bps| is less than the fixed bitrate then
     // fail. If codec is not multi-rate and |bps| exceeds or equal the fixed
     // bitrate then ignore.
-    LOG(LS_ERROR) << "Failed to set codec " << codec_inst.plname
+    LOG(LS_ERROR) << "Failed to set codec " << spec.format.name
                   << " to bitrate " << bps << " bps"
-                  << ", requires at least " << codec_inst.rate << " bps.";
+                  << ", requires at least " << spec.info.min_bitrate_bps
+                  << " bps.";
     return rtc::Optional<int>();
   }
-  return rtc::Optional<int>(codec_rate);
+
+  if (spec.info.HasFixedBitrate()) {
+    return rtc::Optional<int>(spec.info.default_bitrate_bps);
+  } else {
+    // If codec is multi-rate then just set the bitrate.
+    return rtc::Optional<int>(std::min(bps, spec.info.max_bitrate_bps));
+  }
 }
 
-}  // namespace {
+}  // namespace
 
 bool WebRtcVoiceEngine::ToCodecInst(const AudioCodec& in,
                                     webrtc::CodecInst* out) {
   return WebRtcVoiceCodecs::ToCodecInst(in, out);
 }
 
 WebRtcVoiceEngine::WebRtcVoiceEngine(
     webrtc::AudioDeviceModule* adm,
     const rtc::scoped_refptr<webrtc::AudioDecoderFactory>& decoder_factory,
     rtc::scoped_refptr<webrtc::AudioMixer> audio_mixer)
     : WebRtcVoiceEngine(adm, decoder_factory, audio_mixer, new VoEWrapper()) {
   audio_state_ =
       webrtc::AudioState::Create(MakeAudioStateConfig(voe(), audio_mixer));
 }
 
 WebRtcVoiceEngine::WebRtcVoiceEngine(
     webrtc::AudioDeviceModule* adm,
     const rtc::scoped_refptr<webrtc::AudioDecoderFactory>& decoder_factory,
     rtc::scoped_refptr<webrtc::AudioMixer> audio_mixer,
     VoEWrapper* voe_wrapper)
-    : adm_(adm), decoder_factory_(decoder_factory), voe_wrapper_(voe_wrapper) {
+    : adm_(adm),
+      encoder_factory_(webrtc::CreateBuiltinAudioEncoderFactory()),
+      decoder_factory_(decoder_factory),
+      voe_wrapper_(voe_wrapper) {
   RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
   LOG(LS_INFO) << "WebRtcVoiceEngine::WebRtcVoiceEngine";
   RTC_DCHECK(voe_wrapper);
   RTC_DCHECK(decoder_factory);
 
   signal_thread_checker_.DetachFromThread();
 
   // Load our audio codec list.
   LOG(LS_INFO) << "Supported send codecs in order of preference:";
-  send_codecs_ = WebRtcVoiceCodecs::SupportedSendCodecs();
+  send_codecs_ = CollectCodecs(encoder_factory_->GetSupportedEncoders());
   for (const AudioCodec& codec : send_codecs_) {
     LOG(LS_INFO) << ToString(codec);
   }
 
   LOG(LS_INFO) << "Supported recv codecs in order of preference:";
-  recv_codecs_ = CollectRecvCodecs();
+  recv_codecs_ = CollectCodecs(decoder_factory_->GetSupportedDecoders());
   for (const AudioCodec& codec : recv_codecs_) {
     LOG(LS_INFO) << ToString(codec);
   }
 
   channel_config_.enable_voice_pacing = true;
 
   // Temporarily turn logging level up for the Init() call.
   webrtc::Trace::SetTraceCallback(this);
   webrtc::Trace::set_level_filter(kElevatedTraceFilter);
   LOG(LS_INFO) << webrtc::VoiceEngine::GetVersionString();
@@ skipping 531 matching lines @@
   RTC_DCHECK(adm_);
   return adm_;
 }
 
 webrtc::AudioProcessing* WebRtcVoiceEngine::apm() {
   RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
   RTC_DCHECK(apm_);
   return apm_;
 }
 
-AudioCodecs WebRtcVoiceEngine::CollectRecvCodecs() const {
+AudioCodecs WebRtcVoiceEngine::CollectCodecs(
+    const std::vector<webrtc::AudioCodecSpec>& specs) const {
   PayloadTypeMapper mapper;
   AudioCodecs out;
-  const std::vector<webrtc::AudioCodecSpec>& specs =
-      decoder_factory_->GetSupportedDecoders();
 
   // Only generate CN payload types for these clockrates:
   std::map<int, bool, std::greater<int>> generate_cn = {{ 8000,  false },
                                                         { 16000, false },
                                                         { 32000, false }};
   // Only generate telephone-event payload types for these clockrates:
   std::map<int, bool, std::greater<int>> generate_dtmf = {{ 8000,  false },
                                                           { 16000, false },
                                                           { 32000, false },
                                                           { 48000, false }};
@@ skipping 64 matching lines @@
   WebRtcAudioSendStream(
       int ch,
       webrtc::AudioTransport* voe_audio_transport,
       uint32_t ssrc,
       const std::string& c_name,
       const webrtc::AudioSendStream::Config::SendCodecSpec& send_codec_spec,
       const std::vector<webrtc::RtpExtension>& extensions,
       int max_send_bitrate_bps,
       const rtc::Optional<std::string>& audio_network_adaptor_config,
       webrtc::Call* call,
-      webrtc::Transport* send_transport)
+      webrtc::Transport* send_transport,
+      const rtc::scoped_refptr<webrtc::AudioEncoderFactory>& encoder_factory)
       : voe_audio_transport_(voe_audio_transport),
         call_(call),
         config_(send_transport),
         send_side_bwe_with_overhead_(webrtc::field_trial::FindFullName(
             "WebRTC-SendSideBwe-WithOverhead") == "Enabled"),
         max_send_bitrate_bps_(max_send_bitrate_bps),
         rtp_parameters_(CreateRtpParametersWithOneEncoding()) {
     RTC_DCHECK_GE(ch, 0);
     // TODO(solenberg): Once we're not using FakeWebRtcVoiceEngine anymore:
     // RTC_DCHECK(voe_audio_transport);
     RTC_DCHECK(call);
+    RTC_DCHECK(encoder_factory);
     config_.rtp.ssrc = ssrc;
     config_.rtp.c_name = c_name;
     config_.voe_channel_id = ch;
     config_.rtp.extensions = extensions;
     config_.audio_network_adaptor_config = audio_network_adaptor_config;
+    config_.encoder_factory = encoder_factory;
     rtp_parameters_.encodings[0].ssrc = rtc::Optional<uint32_t>(ssrc);
     RecreateAudioSendStream(send_codec_spec);
   }
 
   ~WebRtcAudioSendStream() override {
     RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
     ClearSource();
     call_->DestroyAudioSendStream(stream_);
   }
 
   void RecreateAudioSendStream(
       const webrtc::AudioSendStream::Config::SendCodecSpec& send_codec_spec) {
     RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
     send_codec_spec_ = send_codec_spec;
     config_.rtp.nack.rtp_history_ms =
         send_codec_spec_.nack_enabled ? kNackRtpHistoryMs : 0;
     config_.send_codec_spec = send_codec_spec_;
-    auto send_rate = ComputeSendBitrate(
-        max_send_bitrate_bps_, rtp_parameters_.encodings[0].max_bitrate_bps,
-        send_codec_spec.codec_inst);
-    if (send_rate) {
-      // Apply a send rate that abides by |max_send_bitrate_bps_| and
-      // |rtp_parameters_| when possible. Otherwise use the codec rate.
-      config_.send_codec_spec.codec_inst.rate = *send_rate;
+    if (send_codec_spec_.format.name != "") {

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

One of these again...

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

Yes. This is the most important one. There are two factors that conspire to
this:
1. AudioSendStream should never allow an invalid or nonexistent format in its
SendCodecSpec, so an rtc::Optional doesn't make sense for the interface.
2. WebRtcAudioSendStream keeps a SendCodecSpec internally and builds it up
during repeated calls to RecreateAudioSendStream. Sometimes, the
WebRtcAudioSendStream is created without a valid SendCodecSpec.

Clearly, this should be fixed, but not in this CL. For the time being, I'd
either have to re-engineer WebRtcAudioSendStream to use a similar, but slightly
different format for building up the SendCodecSpec (maybe ok) or I could just
leave this check in place for now, and deal with it when cleaning up the way
SendStreams are initialized. WDYT? 

[kwiberg-webrtc, 2017/03/02 02:17:55]

Dealing with it when cleaning up sounds OK.

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

SendCodecSpec is now optional. No more string comparison!

+      auto info =
+          config_.encoder_factory->QueryAudioFormat(send_codec_spec_.format);
+      RTC_DCHECK(info);
+      // If a specific target bitrate has been set for the stream, use that as
+      // the new default bitrate when computing send bitrate.
+      if (send_codec_spec_.target_bitrate_bps) {
+        info->default_bitrate_bps =
+            std::max(info->min_bitrate_bps,
+                     std::min(info->max_bitrate_bps,
+                              *send_codec_spec.target_bitrate_bps));
+      }
+
+      audio_codec_spec_.emplace(
+          webrtc::AudioCodecSpec{send_codec_spec_.format, *info});
+
+      config_.send_codec_spec.target_bitrate_bps = ComputeSendBitrate(
+          max_send_bitrate_bps_, rtp_parameters_.encodings[0].max_bitrate_bps,
+          *audio_codec_spec_);
     }
     RecreateAudioSendStream();
   }
 
   void RecreateAudioSendStream(
       const std::vector<webrtc::RtpExtension>& extensions) {
     RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
     config_.rtp.extensions = extensions;
     RecreateAudioSendStream();
   }
 
   void RecreateAudioSendStream(
       const rtc::Optional<std::string>& audio_network_adaptor_config) {
     RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
     if (config_.audio_network_adaptor_config == audio_network_adaptor_config) {
       return;
     }
     config_.audio_network_adaptor_config = audio_network_adaptor_config;
     RecreateAudioSendStream();
   }
 
   bool SetMaxSendBitrate(int bps) {
     RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
+    RTC_DCHECK(audio_codec_spec_);
     auto send_rate =
         ComputeSendBitrate(bps, rtp_parameters_.encodings[0].max_bitrate_bps,
-                           send_codec_spec_.codec_inst);
+                           *audio_codec_spec_);
+
     if (!send_rate) {
       return false;
     }
 
     max_send_bitrate_bps_ = bps;
 
-    if (config_.send_codec_spec.codec_inst.rate != *send_rate) {
-      // Recreate AudioSendStream with new bit rate.
-      config_.send_codec_spec.codec_inst.rate = *send_rate;
+    if (send_rate != config_.send_codec_spec.target_bitrate_bps) {
+      // TODO(ossu): Should not recreate stream!
+      config_.send_codec_spec.target_bitrate_bps = send_rate;
       RecreateAudioSendStream();
     }
     return true;
   }
 
   bool SendTelephoneEvent(int payload_type, int payload_freq, int event,
                           int duration_ms) {
     RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
     RTC_DCHECK(stream_);
     return stream_->SendTelephoneEvent(payload_type, payload_freq, event,
@@ skipping 96 matching lines @@
       LOG(LS_ERROR) << "Attempted to set RtpParameters with modified SSRC";
       return false;
     }
     return true;
   }
 
   bool SetRtpParameters(const webrtc::RtpParameters& parameters) {
     if (!ValidateRtpParameters(parameters)) {
       return false;
     }
-    auto send_rate = ComputeSendBitrate(max_send_bitrate_bps_,
-                                        parameters.encodings[0].max_bitrate_bps,
-                                        send_codec_spec_.codec_inst);
-    if (!send_rate) {
-      return false;
+
+    rtc::Optional<int> send_rate;
+    if (audio_codec_spec_) {
+      send_rate = ComputeSendBitrate(max_send_bitrate_bps_,
+                                     parameters.encodings[0].max_bitrate_bps,
+                                     *audio_codec_spec_);
+      if (!send_rate) {
+        return false;
+      }
     }
 
     rtp_parameters_ = parameters;
 
     // parameters.encodings[0].encodings[0].max_bitrate_bps could have changed.
-    if (config_.send_codec_spec.codec_inst.rate != *send_rate) {
+    if (send_rate && send_rate != config_.send_codec_spec.target_bitrate_bps) {
+      // TODO(ossu): Should not recreate stream!
       // Recreate AudioSendStream with new bit rate.
-      config_.send_codec_spec.codec_inst.rate = *send_rate;
+      config_.send_codec_spec.target_bitrate_bps = send_rate;
       RecreateAudioSendStream();
     } else {
       // parameters.encodings[0].active could have changed.
       UpdateSendState();
     }
     return true;
   }
 
  private:
   void UpdateSendState() {
@@ skipping 14 matching lines @@
       stream_ = nullptr;
     }
     RTC_DCHECK(!stream_);
     if (webrtc::field_trial::FindFullName("WebRTC-Audio-SendSideBwe") ==
         "Enabled") {
       config_.min_bitrate_bps = kOpusMinBitrateBps;
       config_.max_bitrate_bps = kOpusBitrateFbBps;
       // TODO(mflodman): Keep testing this and set proper values.
       // Note: This is an early experiment currently only supported by Opus.
       if (send_side_bwe_with_overhead_) {
-        auto packet_sizes_ms = WebRtcVoiceCodecs::GetPacketSizesMs(
-            config_.send_codec_spec.codec_inst);
-        if (!packet_sizes_ms.empty()) {
-          int max_packet_size_ms =
-              *std::max_element(packet_sizes_ms.begin(), packet_sizes_ms.end());
-          int min_packet_size_ms =
-              *std::min_element(packet_sizes_ms.begin(), packet_sizes_ms.end());
+        const bool is_opus_with_ana =
+            config_.audio_network_adaptor_config &&
+            !STR_CASE_CMP(config_.send_codec_spec.format.name.c_str(),
+                          kOpusCodecName);
+        const int max_packet_size_ms =
+            (is_opus_with_ana && WEBRTC_OPUS_SUPPORT_120MS_PTIME) ? 120 : 60;
+        // Audio network adaptor will just use 20ms and 60ms frame lengths.
+        // The adaptor will only be active for the Opus encoder.
+        const int min_packet_size_ms = is_opus_with_ana ? 20 : 10;
 
-          // Audio network adaptor will just use 20ms and 60ms frame lengths.
-          // The adaptor will only be active for the Opus encoder.
-          if (config_.audio_network_adaptor_config &&
-              IsCodec(config_.send_codec_spec.codec_inst, kOpusCodecName)) {
-#if WEBRTC_OPUS_SUPPORT_120MS_PTIME
-            max_packet_size_ms = 120;
-#else
-            max_packet_size_ms = 60;
-#endif
-            min_packet_size_ms = 20;
-          }
+        // OverheadPerPacket = Ipv4(20B) + UDP(8B) + SRTP(10B) + RTP(12)
+        constexpr int kOverheadPerPacket = 20 + 8 + 10 + 12;
 
-          // OverheadPerPacket = Ipv4(20B) + UDP(8B) + SRTP(10B) + RTP(12)
-          constexpr int kOverheadPerPacket = 20 + 8 + 10 + 12;
+        int min_overhead_bps =
+            kOverheadPerPacket * 8 * 1000 / max_packet_size_ms;
 
-          int min_overhead_bps =
-              kOverheadPerPacket * 8 * 1000 / max_packet_size_ms;
+        int max_overhead_bps =
+            kOverheadPerPacket * 8 * 1000 / min_packet_size_ms;
 
-          int max_overhead_bps =
-              kOverheadPerPacket * 8 * 1000 / min_packet_size_ms;
-
-          config_.min_bitrate_bps = kOpusMinBitrateBps + min_overhead_bps;
-          config_.max_bitrate_bps = kOpusBitrateFbBps + max_overhead_bps;
-        }
+        config_.min_bitrate_bps = kOpusMinBitrateBps + min_overhead_bps;
+        config_.max_bitrate_bps = kOpusBitrateFbBps + max_overhead_bps;
       }
     }
     stream_ = call_->CreateAudioSendStream(config_);
     RTC_CHECK(stream_);
     UpdateSendState();
   }
 
   rtc::ThreadChecker worker_thread_checker_;
   rtc::RaceChecker audio_capture_race_checker_;
   webrtc::AudioTransport* const voe_audio_transport_ = nullptr;
   webrtc::Call* call_ = nullptr;
   webrtc::AudioSendStream::Config config_;
   const bool send_side_bwe_with_overhead_;
   // The stream is owned by WebRtcAudioSendStream and may be reallocated if
   // configuration changes.
   webrtc::AudioSendStream* stream_ = nullptr;
 
   // Raw pointer to AudioSource owned by LocalAudioTrackHandler.
   // PeerConnection will make sure invalidating the pointer before the object
   // goes away.
   AudioSource* source_ = nullptr;
   bool send_ = false;
   bool muted_ = false;
   int max_send_bitrate_bps_;
   webrtc::RtpParameters rtp_parameters_;
   webrtc::AudioSendStream::Config::SendCodecSpec send_codec_spec_;
+  // TODO(ossu): Worst. Names. Ever.
+  rtc::Optional<webrtc::AudioCodecSpec> audio_codec_spec_;
 
   RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(WebRtcAudioSendStream);
 };
 
 class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream {
  public:
   WebRtcAudioReceiveStream(
       int ch,
       uint32_t remote_ssrc,
       uint32_t local_ssrc,
@@ skipping 431 matching lines @@
       if (!dtmf_payload_type_ || codec.clockrate < dtmf_payload_freq_) {
         dtmf_payload_type_ = rtc::Optional<int>(codec.id);
         dtmf_payload_freq_ = codec.clockrate;
       }
     }
   }
 
   // Scan through the list to figure out the codec to use for sending, along
   // with the proper configuration for VAD, CNG, NACK and Opus-specific
   // parameters.
   // TODO(solenberg): Refactor this logic once we create AudioEncoders here.

[the sun, 2017/03/16 08:48:19]

Comment doesn't apply anymore?

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

Acknowledged.

   webrtc::AudioSendStream::Config::SendCodecSpec send_codec_spec;
   {
     send_codec_spec.nack_enabled = send_codec_spec_.nack_enabled;
 
-    // Find send codec (the first non-telephone-event/CN codec).
-    const AudioCodec* codec = WebRtcVoiceCodecs::GetPreferredCodec(
-        codecs, &send_codec_spec.codec_inst);
-    if (!codec) {
-      LOG(LS_WARNING) << "Received empty list of codecs.";
-      return false;
+    const AudioCodec* voice_codec = nullptr;
+    for (const AudioCodec& codec : codecs) {
+      if (!(IsCodec(codec, kCnCodecName) ||
+            IsCodec(codec, kDtmfCodecName) ||
+            IsCodec(codec, kRedCodecName))) {
+        webrtc::SdpAudioFormat format(codec.name, codec.clockrate,
+                                      codec.channels,
+                                      CodecParameterMap(codec.params));
+
+        if (!engine()->encoder_factory_->IsSupportedEncoder(format)) {
+          LOG(LS_WARNING) << "Unknown codec " << ToString(codec);
+          continue;
+        }
+
+        voice_codec = &codec;
+        send_codec_spec.payload_type = voice_codec->id;
+        send_codec_spec.format = format;
+        if (codec.bitrate > 0) {
+          send_codec_spec.target_bitrate_bps =
+              rtc::Optional<int>(codec.bitrate);
+        }
+        send_codec_spec.transport_cc_enabled = HasTransportCc(*voice_codec);
+        send_codec_spec.nack_enabled = HasNack(*voice_codec);
+        bitrate_config_ = GetBitrateConfigForCodec(*voice_codec);
+        break;
+      }
     }
 
-    send_codec_spec.transport_cc_enabled = HasTransportCc(*codec);
-    send_codec_spec.nack_enabled = HasNack(*codec);
-    bitrate_config_ = GetBitrateConfigForCodec(*codec);
-
-    // For Opus as the send codec, we are to determine inband FEC, maximum
-    // playback rate, and opus internal dtx.
-    if (IsCodec(*codec, kOpusCodecName)) {
-      GetOpusConfig(*codec, &send_codec_spec.codec_inst,
-                    &send_codec_spec.enable_codec_fec,
-                    &send_codec_spec.opus_max_playback_rate,
-                    &send_codec_spec.enable_opus_dtx,
-                    &send_codec_spec.min_ptime_ms,
-                    &send_codec_spec.max_ptime_ms);
-    }
-
-    // Set packet size if the AudioCodec param kCodecParamPTime is set.
-    int ptime_ms = 0;
-    if (codec->GetParam(kCodecParamPTime, &ptime_ms)) {
-      if (!WebRtcVoiceCodecs::SetPTimeAsPacketSize(
-          &send_codec_spec.codec_inst, ptime_ms)) {
-        LOG(LS_WARNING) << "Failed to set packet size for codec "
-                        << send_codec_spec.codec_inst.plname;
-        return false;
-      }
-    }
+    if (!voice_codec)
+      return false;
 
     // Loop through the codecs list again to find the CN codec.
     // TODO(solenberg): Break out into a separate function?
     for (const AudioCodec& codec : codecs) {
-      // Ignore codecs we don't know about. The negotiation step should prevent
-      // this, but double-check to be sure.
-      webrtc::CodecInst voe_codec = {0};
-      if (!WebRtcVoiceEngine::ToCodecInst(codec, &voe_codec)) {
-        LOG(LS_WARNING) << "Unknown codec " << ToString(codec);
-        continue;
-      }
-
-      if (IsCodec(codec, kCnCodecName)) {
+      if (IsCodec(codec, kCnCodecName) &&
+          codec.clockrate == voice_codec->clockrate) {
         // Turn voice activity detection/comfort noise on if supported.
         // Set the wideband CN payload type appropriately.
         // (narrowband always uses the static payload type 13).
-        int cng_plfreq = -1;
         switch (codec.clockrate) {
           case 8000:
           case 16000:
           case 32000:
-            cng_plfreq = codec.clockrate;
+            send_codec_spec.cng_payload_type = codec.id;
             break;
           default:
             LOG(LS_WARNING) << "CN frequency " << codec.clockrate
                             << " not supported.";
             continue;
         }
-        send_codec_spec.cng_payload_type = codec.id;
-        send_codec_spec.cng_plfreq = cng_plfreq;
         break;
       }
     }
 
     // Find the telephone-event PT exactly matching the preferred send codec.
     for (const AudioCodec& dtmf_codec : dtmf_codecs) {
-      if (dtmf_codec.clockrate == codec->clockrate) {
+      if (dtmf_codec.clockrate == voice_codec->clockrate) {
         dtmf_payload_type_ = rtc::Optional<int>(dtmf_codec.id);
         dtmf_payload_freq_ = dtmf_codec.clockrate;
         break;
       }
     }
   }
 
   if (send_codec_spec_ != send_codec_spec) {
     send_codec_spec_ = std::move(send_codec_spec);
     // Apply new settings to all streams.
@@ skipping 129 matching lines @@
   // Save the channel to send_streams_, so that RemoveSendStream() can still
   // delete the channel in case failure happens below.
   webrtc::AudioTransport* audio_transport =
       engine()->voe()->base()->audio_transport();
 
   rtc::Optional<std::string> audio_network_adaptor_config =
       GetAudioNetworkAdaptorConfig(options_);
   WebRtcAudioSendStream* stream = new WebRtcAudioSendStream(
       channel, audio_transport, ssrc, sp.cname, send_codec_spec_,
       send_rtp_extensions_, max_send_bitrate_bps_, audio_network_adaptor_config,
-      call_, this);
+      call_, this, engine()->encoder_factory_);
   send_streams_.insert(std::make_pair(ssrc, stream));
 
   // At this point the stream's local SSRC has been updated. If it is the first
   // send stream, make sure that all the receive streams are updated with the
   // same SSRC in order to send receiver reports.
   if (send_streams_.size() == 1) {
     receiver_reports_ssrc_ = ssrc;
     for (const auto& kv : recv_streams_) {
       // TODO(solenberg): Allow applications to set the RTCP SSRC of receive
       // streams instead, so we can avoid recreating the streams here.
@@ skipping 475 matching lines @@
   RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
   const auto it = send_streams_.find(ssrc);
   if (it != send_streams_.end()) {
     return it->second->channel();
   }
   return -1;
 }
 }  // namespace cricket
 
 #endif  // HAVE_WEBRTC_VOICE