iSAC encoder: Make it possible to change target bitrate at any time

Not just at construction time.

Bug: webrtc:11704
Change-Id: I952c7dbe20774cc976065c7d2f992a80074ebf63
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177663
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31550}

modules/audio_coding/BUILD.gn

@@ -386,6 +386,8 @@ rtc_source_set("isac_common") {
     "../../api/units:time_delta",
     "../../rtc_base:checks",
     "../../rtc_base:rtc_base_approved",
+    "../../rtc_base:safe_minmax",
+    "../../system_wrappers:field_trial",
   ]
   absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ]
 }

modules/audio_coding/codecs/isac/audio_encoder_isac_t.h

@@ -19,6 +19,7 @@
 #include "api/scoped_refptr.h"
 #include "api/units/time_delta.h"
 #include "rtc_base/constructor_magic.h"
+#include "system_wrappers/include/field_trial.h"
 
 namespace webrtc {
 
@@ -48,6 +49,13 @@ class AudioEncoderIsacT final : public AudioEncoder {
   size_t Num10MsFramesInNextPacket() const override;
   size_t Max10MsFramesInAPacket() const override;
   int GetTargetBitrate() const override;
+  void SetTargetBitrate(int target_bps) override;
+  void OnReceivedTargetAudioBitrate(int target_bps) override;
+  void OnReceivedUplinkBandwidth(
+      int target_audio_bitrate_bps,
+      absl::optional<int64_t> bwe_period_ms) override;
+  void OnReceivedUplinkAllocation(BitrateAllocationUpdate update) override;
+  void OnReceivedOverhead(size_t overhead_bytes_per_packet) override;
   EncodedInfo EncodeImpl(uint32_t rtp_timestamp,
                          rtc::ArrayView<const int16_t> audio,
                          rtc::Buffer* encoded) override;
@@ -60,7 +68,13 @@ class AudioEncoderIsacT final : public AudioEncoder {
   // STREAM_MAXW16_60MS for iSAC fix (60 ms).
   static const size_t kSufficientEncodeBufferSizeBytes = 400;
 
-  static const int kDefaultBitRate = 32000;
+  static constexpr int kDefaultBitRate = 32000;
+  static constexpr int kMinBitrateBps = 10000;
+  static constexpr int MaxBitrateBps(int sample_rate_hz) {
+    return sample_rate_hz == 32000 ? 56000 : 32000;
+  }
+
+  void SetTargetBitrate(int target_bps, bool subtract_per_packet_overhead);
 
   // Recreate the iSAC encoder instance with the given settings, and save them.
   void RecreateEncoderInstance(const Config& config);
@@ -77,6 +91,15 @@ class AudioEncoderIsacT final : public AudioEncoder {
   // Timestamp of the previously encoded packet.
   uint32_t last_encoded_timestamp_;
 
+  // Cache the value of the "WebRTC-SendSideBwe-WithOverhead" field trial.
+  const bool send_side_bwe_with_overhead_ =
+      field_trial::IsEnabled("WebRTC-SendSideBwe-WithOverhead");
+
+  // When we send a packet, expect this many bytes of headers to be added to it.
+  // Start out with a reasonable default that we can use until we receive a real
+  // value.
+  DataSize overhead_per_packet_ = DataSize::Bytes(28);
+
   RTC_DISALLOW_COPY_AND_ASSIGN(AudioEncoderIsacT);
 };
 

modules/audio_coding/codecs/isac/audio_encoder_isac_t_impl.h

@@ -12,6 +12,7 @@
 #define MODULES_AUDIO_CODING_CODECS_ISAC_AUDIO_ENCODER_ISAC_T_IMPL_H_
 
 #include "rtc_base/checks.h"
+#include "rtc_base/numerics/safe_minmax.h"
 
 namespace webrtc {
 
@@ -80,6 +81,51 @@ int AudioEncoderIsacT<T>::GetTargetBitrate() const {
   return config_.bit_rate == 0 ? kDefaultBitRate : config_.bit_rate;
 }
 
+template <typename T>
+void AudioEncoderIsacT<T>::SetTargetBitrate(int target_bps) {
+  // Set target bitrate directly without subtracting per-packet overhead,
+  // because that's what AudioEncoderOpus does.
+  SetTargetBitrate(target_bps,
+                   /*subtract_per_packet_overhead=*/false);
+}
+
+template <typename T>
+void AudioEncoderIsacT<T>::OnReceivedTargetAudioBitrate(int target_bps) {
+  // Set target bitrate directly without subtracting per-packet overhead,
+  // because that's what AudioEncoderOpus does.
+  SetTargetBitrate(target_bps,
+                   /*subtract_per_packet_overhead=*/false);
+}
+
+template <typename T>
+void AudioEncoderIsacT<T>::OnReceivedUplinkBandwidth(
+    int target_audio_bitrate_bps,
+    absl::optional<int64_t> /*bwe_period_ms*/) {
+  // Set target bitrate, subtracting the per-packet overhead if
+  // WebRTC-SendSideBwe-WithOverhead is enabled, because that's what
+  // AudioEncoderOpus does.
+  SetTargetBitrate(
+      target_audio_bitrate_bps,
+      /*subtract_per_packet_overhead=*/send_side_bwe_with_overhead_);
+}
+
+template <typename T>
+void AudioEncoderIsacT<T>::OnReceivedUplinkAllocation(
+    BitrateAllocationUpdate update) {
+  // Set target bitrate, subtracting the per-packet overhead if
+  // WebRTC-SendSideBwe-WithOverhead is enabled, because that's what
+  // AudioEncoderOpus does.
+  SetTargetBitrate(
+      update.target_bitrate.bps<int>(),
+      /*subtract_per_packet_overhead=*/send_side_bwe_with_overhead_);
+}
+
+template <typename T>
+void AudioEncoderIsacT<T>::OnReceivedOverhead(
+    size_t overhead_bytes_per_packet) {
+  overhead_per_packet_ = DataSize::Bytes(overhead_bytes_per_packet);
+}
+
 template <typename T>
 AudioEncoder::EncodedInfo AudioEncoderIsacT<T>::EncodeImpl(
     uint32_t rtp_timestamp,
@@ -126,6 +172,21 @@ AudioEncoderIsacT<T>::GetFrameLengthRange() const {
            TimeDelta::Millis(config_.frame_size_ms)}};
 }
 
+template <typename T>
+void AudioEncoderIsacT<T>::SetTargetBitrate(int target_bps,
+                                            bool subtract_per_packet_overhead) {
+  if (subtract_per_packet_overhead) {
+    const DataRate overhead_rate =
+        overhead_per_packet_ / TimeDelta::Millis(config_.frame_size_ms);
+    target_bps -= overhead_rate.bps();
+  }
+  target_bps = rtc::SafeClamp(target_bps, kMinBitrateBps,
+                              MaxBitrateBps(config_.sample_rate_hz));
+  int result = T::Control(isac_state_, target_bps, config_.frame_size_ms);
+  RTC_DCHECK_EQ(result, 0);
+  config_.bit_rate = target_bps;
+}
+
 template <typename T>
 void AudioEncoderIsacT<T>::RecreateEncoderInstance(const Config& config) {
   RTC_CHECK(config.IsOk());

modules/audio_coding/codecs/isac/isac_webrtc_api_test.cc

@@ -9,6 +9,7 @@
  */
 
 #include <array>
+#include <map>
 #include <memory>
 #include <vector>
 
@@ -159,6 +160,33 @@ TEST_P(EncoderTest, TestDifferentBitrates) {
   EXPECT_LT(num_bytes_low, num_bytes_high);
 }
 
+// Encodes an input audio sequence first with a low, then with a high target
+// bitrate *using the same encoder* and checks that the number of emitted bytes
+// in the first case is less than in the second case.
+TEST_P(EncoderTest, TestDynamicBitrateChange) {
+  constexpr int kLowBps = 20000;
+  constexpr int kHighBps = 25000;
+  constexpr int kStartBps = 30000;
+  auto encoder = CreateEncoder(GetIsacImpl(), GetSampleRateHz(),
+                               GetFrameSizeMs(), kStartBps);
+  std::map<int, int> num_bytes;
+  constexpr int kNumFrames = 200;  // 2 seconds.
+  for (int bitrate_bps : {kLowBps, kHighBps}) {
+    auto pcm_file = GetPcmTestFileReader(GetSampleRateHz());
+    encoder->OnReceivedTargetAudioBitrate(bitrate_bps);
+    for (int i = 0; i < kNumFrames; ++i) {
+      AudioFrame in;
+      pcm_file->Read10MsData(in);
+      rtc::Buffer buf;
+      encoder->Encode(/*rtp_timestamp=*/0, AudioFrameToView(in), &buf);
+      num_bytes[bitrate_bps] += buf.size();
+    }
+  }
+  // kHighBps / kLowBps == 1.25, so require the high-bitrate run to produce at
+  // least 1.2 times the number of bytes.
+  EXPECT_LT(1.2 * num_bytes[kLowBps], num_bytes[kHighBps]);
+}
+
 // Checks that, given a target bitrate, the encoder does not overshoot too much.
 TEST_P(EncoderTest, DoNotOvershootTargetBitrate) {
   for (int bitrate_bps : {10000, 15000, 20000, 26000, 32000}) {

modules/audio_coding/neteq/audio_decoder_unittest.cc

@@ -536,7 +536,11 @@ TEST_F(AudioDecoderIsacFloatTest, EncodeDecode) {
 }
 
 TEST_F(AudioDecoderIsacFloatTest, SetTargetBitrate) {
-  TestSetAndGetTargetBitratesWithFixedCodec(audio_encoder_.get(), 32000);
+  EXPECT_EQ(10000, SetAndGetTargetBitrate(audio_encoder_.get(), 9999));
+  EXPECT_EQ(10000, SetAndGetTargetBitrate(audio_encoder_.get(), 10000));
+  EXPECT_EQ(23456, SetAndGetTargetBitrate(audio_encoder_.get(), 23456));
+  EXPECT_EQ(32000, SetAndGetTargetBitrate(audio_encoder_.get(), 32000));
+  EXPECT_EQ(32000, SetAndGetTargetBitrate(audio_encoder_.get(), 32001));
 }
 
 TEST_F(AudioDecoderIsacSwbTest, EncodeDecode) {
@@ -549,7 +553,11 @@ TEST_F(AudioDecoderIsacSwbTest, EncodeDecode) {
 }
 
 TEST_F(AudioDecoderIsacSwbTest, SetTargetBitrate) {
-  TestSetAndGetTargetBitratesWithFixedCodec(audio_encoder_.get(), 32000);
+  EXPECT_EQ(10000, SetAndGetTargetBitrate(audio_encoder_.get(), 9999));
+  EXPECT_EQ(10000, SetAndGetTargetBitrate(audio_encoder_.get(), 10000));
+  EXPECT_EQ(23456, SetAndGetTargetBitrate(audio_encoder_.get(), 23456));
+  EXPECT_EQ(56000, SetAndGetTargetBitrate(audio_encoder_.get(), 56000));
+  EXPECT_EQ(56000, SetAndGetTargetBitrate(audio_encoder_.get(), 56001));
 }
 
 TEST_F(AudioDecoderIsacFixTest, EncodeDecode) {
@@ -569,7 +577,11 @@ TEST_F(AudioDecoderIsacFixTest, EncodeDecode) {
 }
 
 TEST_F(AudioDecoderIsacFixTest, SetTargetBitrate) {
-  TestSetAndGetTargetBitratesWithFixedCodec(audio_encoder_.get(), 32000);
+  EXPECT_EQ(10000, SetAndGetTargetBitrate(audio_encoder_.get(), 9999));
+  EXPECT_EQ(10000, SetAndGetTargetBitrate(audio_encoder_.get(), 10000));
+  EXPECT_EQ(23456, SetAndGetTargetBitrate(audio_encoder_.get(), 23456));
+  EXPECT_EQ(32000, SetAndGetTargetBitrate(audio_encoder_.get(), 32000));
+  EXPECT_EQ(32000, SetAndGetTargetBitrate(audio_encoder_.get(), 32001));
 }
 
 TEST_F(AudioDecoderG722Test, EncodeDecode) {