Injectable audio encoders: BuiltinAudioEncoderFactory

webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc

Index: webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc
diff --git a/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc b/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc
index 515f6c0f4215cf21cb8f1491c1014493df243003..2eedb5285e4f6139495d99d02e7caf2ba8808f8f 100644
--- a/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc
+++ b/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc
@@ -10,6 +10,8 @@
 
 #include "webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.h"
 
+#include <stdlib.h>
+
 #include <algorithm>
 #include <iterator>
 
@@ -17,6 +19,7 @@
 #include "webrtc/base/logging.h"
 #include "webrtc/base/numerics/exp_filter.h"
 #include "webrtc/base/safe_conversions.h"
+#include "webrtc/base/string_to_number.h"
 #include "webrtc/base/timeutils.h"
 #include "webrtc/common_types.h"
 #include "webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.h"
@@ -28,6 +31,28 @@ namespace webrtc {
 
 namespace {
 
+// 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;
+const int kOpusBitrateFbBps = 32000;
+
+// Opus bitrate should be in the range between 6000 and 510000.
+const int kOpusMinBitrateBps = 6000;
+const int kOpusMaxBitrateBps = 510000;
+
+// TODO(ossu): These are from kCodecPrefs in webrtcvoiceengine.cc
+const int kOpusSupportedFrameLengths[] = {10, 20, 40, 60};
+
 constexpr int kSampleRateHz = 48000;
 
 // Opus API allows a min bitrate of 500bps, but Opus documentation suggests
@@ -37,9 +62,9 @@ constexpr int kMinBitrateBps = 6000;
 constexpr int kMaxBitrateBps = 512000;
 
 #if WEBRTC_OPUS_SUPPORT_120MS_PTIME
-constexpr int kSupportedFrameLengths[] = {20, 60, 120};
+constexpr int kANASupportedFrameLengths[] = {20, 60, 120};
 #else
-constexpr int kSupportedFrameLengths[] = {20, 60};
+constexpr int kANASupportedFrameLengths[] = {20, 60};
 #endif
 
 // PacketLossFractionSmoother uses an exponential filter with a time constant
@@ -103,6 +128,151 @@ float OptimizePacketLossRate(float new_loss_rate, float old_loss_rate) {
 
 }  // namespace
 
+rtc::Optional<AudioFormatInfo> AudioEncoderOpus::QueryAudioFormat(
+    const SdpAudioFormat& format) {
+  if (STR_CASE_CMP(format.name.c_str(), GetPayloadName()) == 0 &&
+      format.clockrate_hz == 48000 && format.num_channels == 2) {

[kwiberg-webrtc, 2017/02/19 21:41:10]

Invert this test and return early, to save lots of indentation.

[ossu, 2017/03/14 20:25:11]

I didn't do this, but I simplified the function enough that I've now only one
failure return, and it's hit if any of the two nested ifs fail.

+    int num_channels = 1;
+    const auto stereo_param = format.parameters.find("stereo");
+    if (stereo_param != format.parameters.end()) {
+      if (stereo_param->second == "1") {
+        num_channels = 2;
+      } else if (stereo_param->second != "0") {
+        return rtc::Optional<AudioFormatInfo>();
+      }
+    }
+
+    int max_playback_rate = 48000;
+    const auto maxplaybackrate_param =
+        format.parameters.find("maxplaybackrate");
+    if (maxplaybackrate_param != format.parameters.end()) {
+      const auto opt_max_playback_rate =
+          rtc::StringToNumber<int>(maxplaybackrate_param->second);
+      if (opt_max_playback_rate) {
+        max_playback_rate = *opt_max_playback_rate;
+        if (max_playback_rate <= 0) {
+          return rtc::Optional<AudioFormatInfo>();
+        }
+      }
+    }
+
+    int bitrate;
+    if (max_playback_rate && max_playback_rate <= 8000) {

[kwiberg-webrtc, 2017/02/19 21:41:10]

max_playback_rate is an int, isn't it? Is 0 special?

[ossu, 2017/02/20 12:20:26]

I'll look through this whole function. It duplicates some of what CreatConfig
does - it would be nice to DRY that up.

+      bitrate = kOpusBitrateNbBps * num_channels;
+    } else if (max_playback_rate && max_playback_rate <= 16000) {
+      bitrate = kOpusBitrateWbBps * num_channels;
+    } else {
+      bitrate = kOpusBitrateFbBps * num_channels;
+    }
+
+    AudioFormatInfo info(48000, num_channels, bitrate, kOpusMinBitrateBps,
+                         kOpusMaxBitrateBps);
+    info.allow_comfort_noise = false;
+    info.supports_network_adaption = true;
+
+    return rtc::Optional<AudioFormatInfo>(info);
+  }
+  return rtc::Optional<AudioFormatInfo>();
+}
+
+AudioEncoderOpus::Config AudioEncoderOpus::CreateConfig(
+    int payload_type,
+    const SdpAudioFormat& format) {
+  auto get_param_int =
+      [&format] (const std::string& param) {
+        auto it = format.parameters.find(param);
+        if (it != format.parameters.end()) {
+          return rtc::StringToNumber<int>(it->second.c_str());
+        }
+        return rtc::Optional<int>();
+      };
+
+  auto get_param =
+      [&format] (const std::string& param) {
+        auto it = format.parameters.find(param);
+        return (it == format.parameters.end())
+            ? std::string()
+            : it->second;
+      };
+
+  AudioEncoderOpus::Config config;
+  config.num_channels = (get_param("stereo") == "1") ? 2 : 1;
+  config.fec_enabled = (get_param("useinbandfec") == "1");
+  config.dtx_enabled = (get_param("usedtx") == "1");
+  const auto max_playback_rate = get_param_int("maxplaybackrate");
+  if (max_playback_rate && *max_playback_rate > 0) {
+    config.max_playback_rate_hz = *max_playback_rate;
+  }
+  const auto max_average_bitrate = get_param_int("maxaveragebitrate");
+  bool use_param = true;
+  int bitrate = 0;
+  if (max_average_bitrate && *max_average_bitrate > 0) {
+    bitrate = *max_average_bitrate;
+  } else {
+    if (max_playback_rate && *max_playback_rate <= 8000) {
+      bitrate = kOpusBitrateNbBps * config.num_channels;
+    } else if (max_playback_rate && *max_playback_rate <= 16000) {
+      bitrate = kOpusBitrateWbBps * config.num_channels;
+    } else {
+      bitrate = kOpusBitrateFbBps * config.num_channels;
+    }
+    use_param = false;
+  }
+
+  if (bitrate < kOpusMinBitrateBps || bitrate > kOpusMaxBitrateBps) {
+    bitrate = (bitrate < kOpusMinBitrateBps) ? kOpusMinBitrateBps
+        : kOpusMaxBitrateBps;
+    std::string rate_source =
+        use_param ? "Codec parameter \"maxaveragebitrate\""
+        : "Default Opus bitrate";
+    LOG(LS_WARNING) << rate_source
+                    << " is invalid and is replaced by: " << bitrate;
+  }
+  config.bitrate_bps = rtc::Optional<int>(bitrate);
+  config.payload_type = payload_type;
+  config.application = config.num_channels == 1 ? AudioEncoderOpus::kVoip
+                                                : AudioEncoderOpus::kAudio;
+#if WEBRTC_OPUS_VARIABLE_COMPLEXITY
+  config.low_rate_complexity = 9;
+#endif
+
+  // TODO(ossu): What to do if ptime is not between minptime and maxptime?
+  const auto ptime = get_param_int("ptime");
+  if (ptime) {
+    // Expects kOpusSupportedFrameLengths to be sorted.
+    for (const int supported_frame_length : kOpusSupportedFrameLengths) {
+      if (supported_frame_length >= *ptime) {
+        config.frame_size_ms = *ptime;
+      }
+    }
+  }
+
+  const int min_frame_length_ms =
+      std::min(std::max(get_param_int("minptime").value_or(10), 10), 60);
+  const int max_frame_length_ms =
+      std::min(std::max(get_param_int("maxptime").value_or(60), 10), 60);
+  if (min_frame_length_ms <= max_frame_length_ms) {
+    for (const int frame_length_ms : kANASupportedFrameLengths) {
+      if (frame_length_ms >= min_frame_length_ms &&
+          frame_length_ms <= max_frame_length_ms) {
+        config.supported_frame_lengths_ms.push_back(frame_length_ms);
+      }
+    }
+  }
+
+  // As a fallback, just pick the whole set of supported frame lengths.
+  if (config.supported_frame_lengths_ms.empty()) {
+    for (const int frame_length_ms : kANASupportedFrameLengths) {
+      config.supported_frame_lengths_ms.push_back(frame_length_ms);
+    }
+  }
+
+  RTC_DCHECK(std::is_sorted(config.supported_frame_lengths_ms.begin(),
+                            config.supported_frame_lengths_ms.end()));
+
+  return config;
+}

[kwiberg-webrtc, 2017/02/19 21:41:10]

This function is very long. It has several parts that could be separate
functions, I believe. (As a bonus, writing unit tests will become much easier!)

[ossu, 2017/02/20 12:20:26]

Agreed.

[ossu, 2017/03/14 20:25:11]

Moved common functionality from QueryAudioFormat and CreateConfig into separate
functions, making everything easier to follow. The only big lump still in
CreateConfig is the stuff pertaining to frame lengths. I don't know the
specifics well enough, but I believe this could be cleaned up a bit, especially
if we get info about ANA during creation. Maybe this can wait until after this
set of CLs, though.

+
 class AudioEncoderOpus::PacketLossFractionSmoother {
  public:
   explicit PacketLossFractionSmoother(const Clock* clock)
@@ -206,7 +376,11 @@ AudioEncoderOpus::AudioEncoderOpus(
 }
 
 AudioEncoderOpus::AudioEncoderOpus(const CodecInst& codec_inst)
-    : AudioEncoderOpus(CreateConfig(codec_inst), nullptr) {}
+    : AudioEncoderOpus(webrtc::CreateConfig(codec_inst), nullptr) {}

[kwiberg-webrtc, 2017/02/19 21:41:10]

Wait... webrtc::CreateConfig?

[ossu, 2017/02/20 12:20:26]

When I added a CreateConfig for my case, it needed to live in AudioEncoderOpus,
but the old one didn't. So just CreateConfig was confusing the compiler. I'll
tidy this up, so they're both under AudioEncoderOpus. At least one of them, I'd
like to keep public as well, since that would help other users set up their own
factories including Opus, without having to duplicate all the logic in
CreateConfig.

+
+AudioEncoderOpus::AudioEncoderOpus(int payload_type,
+                                   const SdpAudioFormat& format)
+    : AudioEncoderOpus(CreateConfig(payload_type, format), nullptr) {}
 
 AudioEncoderOpus::~AudioEncoderOpus() {
   RTC_CHECK_EQ(0, WebRtcOpus_EncoderFree(inst_));
@@ -367,8 +541,8 @@ void AudioEncoderOpus::SetReceiverFrameLengthRange(int min_frame_length_ms,
   RTC_DCHECK(!audio_network_adaptor_);
 
   config_.supported_frame_lengths_ms.clear();
-  std::copy_if(std::begin(kSupportedFrameLengths),
-               std::end(kSupportedFrameLengths),
+  std::copy_if(std::begin(kANASupportedFrameLengths),
+               std::end(kANASupportedFrameLengths),
                std::back_inserter(config_.supported_frame_lengths_ms),
                [&](int frame_length_ms) {
                  return frame_length_ms >= min_frame_length_ms &&