Audio encoder tests: Create audio encoders the new way

Specifically, don't expect the ACM to be able to create encoders; we
have to give it an encoder that we make ourselves.

The new way of creating encoders used a 32 kbit/s bitrate
unconditionally for iSAC; I had to change it to 32 kbit/s for 16 kHz
and 56 kbit/s for 32 kHz, which is what the old way of creating
encoders has used since forever.

I also had to change some test expectations on Opus, because the new
way defaults to 32 kbit/s for mono and 64 kbit/s for stereo (which I
believe to be correct), while the old way defaults to 64 kbit/s in
both cases.

Bug: webrtc:8396
Change-Id: I3aab944175a8e27f4c63380e822b27e839bba7f2
Reviewed-on: https://webrtc-review.googlesource.com/94540
Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24375}

api/audio_codecs/isac/audio_encoder_isac_float.cc

@@ -24,6 +24,7 @@ AudioEncoderIsacFloat::SdpToConfig(const SdpAudioFormat& format) {
       format.num_channels == 1) {
     Config config;
     config.sample_rate_hz = format.clockrate_hz;
+    config.bit_rate = format.clockrate_hz == 16000 ? 32000 : 56000;
     if (config.sample_rate_hz == 16000) {
       // For sample rate 16 kHz, optionally use 60 ms frames, instead of the
       // default 30 ms.
@@ -65,9 +66,10 @@ std::unique_ptr<AudioEncoder> AudioEncoderIsacFloat::MakeAudioEncoder(
     absl::optional<AudioCodecPairId> /*codec_pair_id*/) {
   RTC_DCHECK(config.IsOk());
   AudioEncoderIsacFloatImpl::Config c;
+  c.payload_type = payload_type;
   c.sample_rate_hz = config.sample_rate_hz;
   c.frame_size_ms = config.frame_size_ms;
-  c.payload_type = payload_type;
+  c.bit_rate = config.bit_rate;
   return absl::make_unique<AudioEncoderIsacFloatImpl>(c);
 }
 

api/audio_codecs/isac/audio_encoder_isac_float.h

@@ -28,12 +28,30 @@ namespace webrtc {
 struct AudioEncoderIsacFloat {
   struct Config {
     bool IsOk() const {
-      return (sample_rate_hz == 16000 &&
+      switch (sample_rate_hz) {
-              (frame_size_ms == 30 || frame_size_ms == 60)) ||
+        case 16000:
-             (sample_rate_hz == 32000 && frame_size_ms == 30);
+          if (frame_size_ms != 30 && frame_size_ms != 60) {
+            return false;
+          }
+          if (bit_rate < 10000 || bit_rate > 32000) {
+            return false;
+          }
+          return true;
+        case 32000:
+          if (frame_size_ms != 30) {
+            return false;
+          }
+          if (bit_rate < 10000 || bit_rate > 56000) {
+            return false;
+          }
+          return true;
+        default:
+          return false;
+      }
     }
     int sample_rate_hz = 16000;
     int frame_size_ms = 30;
+    int bit_rate = 32000;  // Limit on short-term average bit rate, in bits/s.
   };
   static absl::optional<Config> SdpToConfig(const SdpAudioFormat& audio_format);
   static void AppendSupportedEncoders(std::vector<AudioCodecSpec>* specs);

modules/audio_coding/BUILD.gn

@@ -1406,8 +1406,10 @@ if (rtc_include_tests) {
              "../../api/audio:audio_frame_api",
              "../../rtc_base:checks",
              ":audio_coding",
+             ":audio_format_conversion",
              ":neteq_tools",
              "../../api/audio_codecs:builtin_audio_decoder_factory",
+             "../../api/audio_codecs:builtin_audio_encoder_factory",
              "../../api/audio_codecs:audio_codecs_api",
              "../../rtc_base:rtc_base_approved",
              "../../test:test_support",

modules/audio_coding/acm2/acm_send_test.cc

@@ -16,10 +16,13 @@
 
 #include "api/audio_codecs/audio_encoder.h"
 #include "api/audio_codecs/builtin_audio_decoder_factory.h"
+#include "api/audio_codecs/builtin_audio_encoder_factory.h"
+#include "modules/audio_coding/codecs/audio_format_conversion.h"
 #include "modules/audio_coding/include/audio_coding_module.h"
 #include "modules/audio_coding/neteq/tools/input_audio_file.h"
 #include "modules/audio_coding/neteq/tools/packet.h"
 #include "rtc_base/checks.h"
+#include "rtc_base/stringencode.h"
 #include "test/gtest.h"
 
 namespace webrtc {
@@ -65,20 +68,26 @@ bool AcmSendTestOldApi::RegisterCodec(const char* payload_name,
                                            sampling_freq_hz, channels));
   codec.pltype = payload_type;
   codec.pacsize = frame_size_samples;
-  codec_registered_ = (acm_->RegisterSendCodec(codec) == 0);
+  auto factory = CreateBuiltinAudioEncoderFactory();
+  SdpAudioFormat format = CodecInstToSdp(codec);
+  format.parameters["ptime"] = rtc::ToString(rtc::CheckedDivExact(
+      frame_size_samples, rtc::CheckedDivExact(sampling_freq_hz, 1000)));
+  acm_->SetEncoder(
+      factory->MakeAudioEncoder(payload_type, format, absl::nullopt));
+  codec_registered_ = true;
   input_frame_.num_channels_ = channels;
   assert(input_block_size_samples_ * input_frame_.num_channels_ <=
          AudioFrame::kMaxDataSizeSamples);
   return codec_registered_;
 }
 
-bool AcmSendTestOldApi::RegisterExternalCodec(
+void AcmSendTestOldApi::RegisterExternalCodec(
-    AudioEncoder* external_speech_encoder) {
+    std::unique_ptr<AudioEncoder> external_speech_encoder) {
-  acm_->RegisterExternalSendCodec(external_speech_encoder);
   input_frame_.num_channels_ = external_speech_encoder->NumChannels();
+  acm_->SetEncoder(std::move(external_speech_encoder));
   assert(input_block_size_samples_ * input_frame_.num_channels_ <=
          AudioFrame::kMaxDataSizeSamples);
-  return codec_registered_ = true;
+  codec_registered_ = true;
 }
 
 std::unique_ptr<Packet> AcmSendTestOldApi::NextPacket() {

modules/audio_coding/acm2/acm_send_test.h

@@ -42,8 +42,9 @@ class AcmSendTestOldApi : public AudioPacketizationCallback,
                      int payload_type,
                      int frame_size_samples);
 
-  // Registers an external send codec. Returns true on success, false otherwise.
+  // Registers an external send codec.
-  bool RegisterExternalCodec(AudioEncoder* external_speech_encoder);
+  void RegisterExternalCodec(
+      std::unique_ptr<AudioEncoder> external_speech_encoder);
 
   // Inherited from PacketSource.
   std::unique_ptr<Packet> NextPacket() override;

modules/audio_coding/acm2/audio_coding_module_unittest.cc

@@ -1148,13 +1148,14 @@ class AcmSenderBitExactnessOldApi : public ::testing::Test,
                                      payload_type, frame_size_samples);
   }
 
-  bool RegisterExternalSendCodec(AudioEncoder* external_speech_encoder,
+  void RegisterExternalSendCodec(
-                                 int payload_type) {
+      std::unique_ptr<AudioEncoder> external_speech_encoder,
+      int payload_type) {
     payload_type_ = payload_type;
     frame_size_rtp_timestamps_ = rtc::checked_cast<uint32_t>(
         external_speech_encoder->Num10MsFramesInNextPacket() *
         external_speech_encoder->RtpTimestampRateHz() / 100);
-    return send_test_->RegisterExternalCodec(external_speech_encoder);
+    send_test_->RegisterExternalCodec(std::move(external_speech_encoder));
   }
 
   // Runs the test. SetUpSender() and RegisterSendCodec() must have been called
@@ -1249,11 +1250,11 @@ class AcmSenderBitExactnessOldApi : public ::testing::Test,
                                   codec_frame_size_rtp_timestamps));
   }
 
-  void SetUpTestExternalEncoder(AudioEncoder* external_speech_encoder,
+  void SetUpTestExternalEncoder(
-                                int payload_type) {
+      std::unique_ptr<AudioEncoder> external_speech_encoder,
+      int payload_type) {
     ASSERT_TRUE(SetUpSender());
-    ASSERT_TRUE(
+    RegisterExternalSendCodec(std::move(external_speech_encoder), payload_type);
-        RegisterExternalSendCodec(external_speech_encoder, payload_type));
   }
 
   std::unique_ptr<test::AcmSendTestOldApi> send_test_;
@@ -1460,8 +1461,8 @@ TEST_F(AcmSenderBitExactnessOldApi, Opus_stereo_20ms) {
 TEST_F(AcmSenderBitExactnessNewApi, MAYBE_OpusFromFormat_stereo_20ms) {
   const auto config = AudioEncoderOpus::SdpToConfig(
       SdpAudioFormat("opus", 48000, 2, {{"stereo", "1"}}));
-  const auto encoder = AudioEncoderOpus::MakeAudioEncoder(*config, 120);
+  ASSERT_NO_FATAL_FAILURE(SetUpTestExternalEncoder(
-  ASSERT_NO_FATAL_FAILURE(SetUpTestExternalEncoder(encoder.get(), 120));
+      AudioEncoderOpus::MakeAudioEncoder(*config, 120), 120));
   Run(AcmReceiverBitExactnessOldApi::PlatformChecksum(
           "3e285b74510e62062fbd8142dacd16e9",
           "3e285b74510e62062fbd8142dacd16e9",
@@ -1499,8 +1500,8 @@ TEST_F(AcmSenderBitExactnessOldApi, Opus_stereo_20ms_voip) {
 TEST_F(AcmSenderBitExactnessNewApi, OpusFromFormat_stereo_20ms_voip) {
   const auto config = AudioEncoderOpus::SdpToConfig(
       SdpAudioFormat("opus", 48000, 2, {{"stereo", "1"}}));
-  const auto encoder = AudioEncoderOpus::MakeAudioEncoder(*config, 120);
+  ASSERT_NO_FATAL_FAILURE(SetUpTestExternalEncoder(
-  ASSERT_NO_FATAL_FAILURE(SetUpTestExternalEncoder(encoder.get(), 120));
+      AudioEncoderOpus::MakeAudioEncoder(*config, 120), 120));
   // If not set, default will be kAudio in case of stereo.
   EXPECT_EQ(0, send_test_->acm()->SetOpusApplication(kVoip));
   Run(AcmReceiverBitExactnessOldApi::PlatformChecksum(
@@ -1550,9 +1551,10 @@ class AcmSetBitRateTest : public ::testing::Test {
                                      payload_type, frame_size_samples);
   }
 
-  bool RegisterExternalSendCodec(AudioEncoder* external_speech_encoder,
+  void RegisterExternalSendCodec(
-                                 int payload_type) {
+      std::unique_ptr<AudioEncoder> external_speech_encoder,
-    return send_test_->RegisterExternalCodec(external_speech_encoder);
+      int payload_type) {
+    send_test_->RegisterExternalCodec(std::move(external_speech_encoder));
   }
 
   void RunInner(int min_expected_total_bits, int max_expected_total_bits) {
@@ -1611,9 +1613,9 @@ TEST_F(AcmSetBitRateOldApi, Opus_48khz_20ms_10kbps) {
 TEST_F(AcmSetBitRateNewApi, OpusFromFormat_48khz_20ms_10kbps) {
   const auto config = AudioEncoderOpus::SdpToConfig(
       SdpAudioFormat("opus", 48000, 2, {{"maxaveragebitrate", "10000"}}));
-  const auto encoder = AudioEncoderOpus::MakeAudioEncoder(*config, 107);
   ASSERT_TRUE(SetUpSender());
-  ASSERT_TRUE(RegisterExternalSendCodec(encoder.get(), 107));
+  RegisterExternalSendCodec(AudioEncoderOpus::MakeAudioEncoder(*config, 107),
+                            107);
   RunInner(8000, 12000);
 }
 
@@ -1625,9 +1627,9 @@ TEST_F(AcmSetBitRateOldApi, Opus_48khz_20ms_50kbps) {
 TEST_F(AcmSetBitRateNewApi, OpusFromFormat_48khz_20ms_50kbps) {
   const auto config = AudioEncoderOpus::SdpToConfig(
       SdpAudioFormat("opus", 48000, 2, {{"maxaveragebitrate", "50000"}}));
-  const auto encoder = AudioEncoderOpus::MakeAudioEncoder(*config, 107);
   ASSERT_TRUE(SetUpSender());
-  ASSERT_TRUE(RegisterExternalSendCodec(encoder.get(), 107));
+  RegisterExternalSendCodec(AudioEncoderOpus::MakeAudioEncoder(*config, 107),
+                            107);
   RunInner(40000, 60000);
 }
 
@@ -1650,9 +1652,9 @@ TEST_F(AcmSetBitRateOldApi, MAYBE_Opus_48khz_20ms_100kbps) {
 TEST_F(AcmSetBitRateNewApi, MAYBE_OpusFromFormat_48khz_20ms_100kbps) {
   const auto config = AudioEncoderOpus::SdpToConfig(
       SdpAudioFormat("opus", 48000, 2, {{"maxaveragebitrate", "100000"}}));
-  const auto encoder = AudioEncoderOpus::MakeAudioEncoder(*config, 107);
   ASSERT_TRUE(SetUpSender());
-  ASSERT_TRUE(RegisterExternalSendCodec(encoder.get(), 107));
+  RegisterExternalSendCodec(AudioEncoderOpus::MakeAudioEncoder(*config, 107),
+                            107);
   RunInner(80000, 120000);
 }
 
@@ -1724,17 +1726,17 @@ class AcmChangeBitRateOldApi : public AcmSetBitRateOldApi {
 
 TEST_F(AcmChangeBitRateOldApi, Opus_48khz_20ms_10kbps_2) {
   ASSERT_NO_FATAL_FAILURE(SetUpTest("opus", 48000, 1, 107, 960, 960));
-  Run(10000, 32200, 5208);
+  Run(10000, 14096, 4232);
 }
 
 TEST_F(AcmChangeBitRateOldApi, Opus_48khz_20ms_50kbps_2) {
   ASSERT_NO_FATAL_FAILURE(SetUpTest("opus", 48000, 1, 107, 960, 960));
-  Run(50000, 32200, 23928);
+  Run(50000, 14096, 22552);
 }
 
 TEST_F(AcmChangeBitRateOldApi, Opus_48khz_20ms_100kbps_2) {
   ASSERT_NO_FATAL_FAILURE(SetUpTest("opus", 48000, 1, 107, 960, 960));
-  Run(100000, 32200, 50448);
+  Run(100000, 14096, 49472);
 }
 
 // These next 2 tests ensure that the SetBitRate function has no effect on PCM
@@ -1754,36 +1756,33 @@ TEST_F(AcmSenderBitExactnessOldApi, External_Pcmu_20ms) {
   codec_inst.pacsize = 160;
   codec_inst.pltype = 0;
   AudioEncoderPcmU encoder(codec_inst);
-  MockAudioEncoder mock_encoder;
+  auto mock_encoder = absl::make_unique<MockAudioEncoder>();
   // Set expectations on the mock encoder and also delegate the calls to the
   // real encoder.
-  EXPECT_CALL(mock_encoder, SampleRateHz())
+  EXPECT_CALL(*mock_encoder, SampleRateHz())
       .Times(AtLeast(1))
       .WillRepeatedly(Invoke(&encoder, &AudioEncoderPcmU::SampleRateHz));
-  EXPECT_CALL(mock_encoder, NumChannels())
+  EXPECT_CALL(*mock_encoder, NumChannels())
       .Times(AtLeast(1))
       .WillRepeatedly(Invoke(&encoder, &AudioEncoderPcmU::NumChannels));
-  EXPECT_CALL(mock_encoder, RtpTimestampRateHz())
+  EXPECT_CALL(*mock_encoder, RtpTimestampRateHz())
       .Times(AtLeast(1))
       .WillRepeatedly(Invoke(&encoder, &AudioEncoderPcmU::RtpTimestampRateHz));
-  EXPECT_CALL(mock_encoder, Num10MsFramesInNextPacket())
+  EXPECT_CALL(*mock_encoder, Num10MsFramesInNextPacket())
       .Times(AtLeast(1))
       .WillRepeatedly(
           Invoke(&encoder, &AudioEncoderPcmU::Num10MsFramesInNextPacket));
-  EXPECT_CALL(mock_encoder, GetTargetBitrate())
+  EXPECT_CALL(*mock_encoder, GetTargetBitrate())
       .Times(AtLeast(1))
       .WillRepeatedly(Invoke(&encoder, &AudioEncoderPcmU::GetTargetBitrate));
-  EXPECT_CALL(mock_encoder, EncodeImpl(_, _, _))
+  EXPECT_CALL(*mock_encoder, EncodeImpl(_, _, _))
       .Times(AtLeast(1))
       .WillRepeatedly(Invoke(
           &encoder, static_cast<AudioEncoder::EncodedInfo (AudioEncoder::*)(
                         uint32_t, rtc::ArrayView<const int16_t>, rtc::Buffer*)>(
                         &AudioEncoderPcmU::Encode)));
-  EXPECT_CALL(mock_encoder, SetFec(_))
-      .Times(AtLeast(1))
-      .WillRepeatedly(Invoke(&encoder, &AudioEncoderPcmU::SetFec));
   ASSERT_NO_FATAL_FAILURE(
-      SetUpTestExternalEncoder(&mock_encoder, codec_inst.pltype));
+      SetUpTestExternalEncoder(std::move(mock_encoder), codec_inst.pltype));
   Run("81a9d4c0bb72e9becc43aef124c981e9", "8f9b8750bd80fe26b6cbf6659b89f0f9",
       50, test::AcmReceiveTestOldApi::kMonoOutput);
 }