Make ANA frame length controller more robust to encoder frame lengths.

Bug: webrtc:10820
Change-Id: Ic3a30976d0181de9cdd35e44d4c5439cadad4812
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/149163
Commit-Queue: Minyue Li <minyue@webrtc.org>
Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
Reviewed-by: Henrik Lundin <henrik.lundin@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28873}
diff --git a/modules/audio_coding/audio_network_adaptor/frame_length_controller.cc b/modules/audio_coding/audio_network_adaptor/frame_length_controller.cc
index 3cb91fd..36e9eb9 100644
--- a/modules/audio_coding/audio_network_adaptor/frame_length_controller.cc
+++ b/modules/audio_coding/audio_network_adaptor/frame_length_controller.cc
@@ -75,10 +75,8 @@
   RTC_DCHECK(!config->frame_length_ms);
 
   if (FrameLengthIncreasingDecision(*config)) {
-    ++frame_length_ms_;
     prev_decision_increase_ = true;
   } else if (FrameLengthDecreasingDecision(*config)) {
-    --frame_length_ms_;
     prev_decision_increase_ = false;
   }
   config->last_fl_change_increase = prev_decision_increase_;
@@ -99,7 +97,7 @@
 }
 
 bool FrameLengthController::FrameLengthIncreasingDecision(
-    const AudioEncoderRuntimeConfig& config) const {
+    const AudioEncoderRuntimeConfig& config) {
   // Increase frame length if
   // 1. |uplink_bandwidth_bps| is known to be smaller or equal than
   //    |min_encoder_bitrate_bps| plus |prevent_overuse_margin_bps| plus the
@@ -108,12 +106,17 @@
   // 3. |uplink_bandwidth_bps| is known to be smaller than a threshold AND
   // 4. |uplink_packet_loss_fraction| is known to be smaller than a threshold.
 
+  // Find next frame length to which a criterion is defined to shift from
+  // current frame length.
   auto longer_frame_length_ms = std::next(frame_length_ms_);
-  if (longer_frame_length_ms == config_.encoder_frame_lengths_ms.end())
-    return false;
-
-  auto increase_threshold = config_.fl_changing_bandwidths_bps.find(
-      Config::FrameLengthChange(*frame_length_ms_, *longer_frame_length_ms));
+  auto increase_threshold = config_.fl_changing_bandwidths_bps.end();
+  while (longer_frame_length_ms != config_.encoder_frame_lengths_ms.end()) {
+    increase_threshold = config_.fl_changing_bandwidths_bps.find(
+        Config::FrameLengthChange(*frame_length_ms_, *longer_frame_length_ms));
+    if (increase_threshold != config_.fl_changing_bandwidths_bps.end())
+      break;
+    longer_frame_length_ms = std::next(longer_frame_length_ms);
+  }
 
   if (increase_threshold == config_.fl_changing_bandwidths_bps.end())
     return false;
@@ -134,18 +137,23 @@
               OverheadRateBps(*overhead_bytes_per_packet_ +
                                   config_.fl_increase_overhead_offset,
                               *frame_length_ms_)) {
+    frame_length_ms_ = longer_frame_length_ms;
     return true;
   }
 
-  return (uplink_bandwidth_bps_ &&
-          *uplink_bandwidth_bps_ <= increase_threshold->second) &&
-         (uplink_packet_loss_fraction_ &&
-          *uplink_packet_loss_fraction_ <=
-              config_.fl_increasing_packet_loss_fraction);
+  if ((uplink_bandwidth_bps_ &&
+       *uplink_bandwidth_bps_ <= increase_threshold->second) &&
+      (uplink_packet_loss_fraction_ &&
+       *uplink_packet_loss_fraction_ <=
+           config_.fl_increasing_packet_loss_fraction)) {
+    frame_length_ms_ = longer_frame_length_ms;
+    return true;
+  }
+  return false;
 }
 
 bool FrameLengthController::FrameLengthDecreasingDecision(
-    const AudioEncoderRuntimeConfig& config) const {
+    const AudioEncoderRuntimeConfig& config) {
   // Decrease frame length if
   // 1. shorter frame length is available AND
   // 2. |uplink_bandwidth_bps| is known to be bigger than
@@ -154,12 +162,18 @@
   // one or more of the followings:
   // 3. |uplink_bandwidth_bps| is known to be larger than a threshold,
   // 4. |uplink_packet_loss_fraction| is known to be larger than a threshold,
-  if (frame_length_ms_ == config_.encoder_frame_lengths_ms.begin())
-    return false;
 
-  auto shorter_frame_length_ms = std::prev(frame_length_ms_);
-  auto decrease_threshold = config_.fl_changing_bandwidths_bps.find(
-      Config::FrameLengthChange(*frame_length_ms_, *shorter_frame_length_ms));
+  // Find next frame length to which a criterion is defined to shift from
+  // current frame length.
+  auto shorter_frame_length_ms = frame_length_ms_;
+  auto decrease_threshold = config_.fl_changing_bandwidths_bps.end();
+  while (shorter_frame_length_ms != config_.encoder_frame_lengths_ms.begin()) {
+    shorter_frame_length_ms = std::prev(shorter_frame_length_ms);
+    decrease_threshold = config_.fl_changing_bandwidths_bps.find(
+        Config::FrameLengthChange(*frame_length_ms_, *shorter_frame_length_ms));
+    if (decrease_threshold != config_.fl_changing_bandwidths_bps.end())
+      break;
+  }
 
   if (decrease_threshold == config_.fl_changing_bandwidths_bps.end())
     return false;
@@ -173,11 +187,15 @@
     return false;
   }
 
-  return (uplink_bandwidth_bps_ &&
-          *uplink_bandwidth_bps_ >= decrease_threshold->second) ||
-         (uplink_packet_loss_fraction_ &&
-          *uplink_packet_loss_fraction_ >=
-              config_.fl_decreasing_packet_loss_fraction);
+  if ((uplink_bandwidth_bps_ &&
+       *uplink_bandwidth_bps_ >= decrease_threshold->second) ||
+      (uplink_packet_loss_fraction_ &&
+       *uplink_packet_loss_fraction_ >=
+           config_.fl_decreasing_packet_loss_fraction)) {
+    frame_length_ms_ = shorter_frame_length_ms;
+    return true;
+  }
+  return false;
 }
 
 }  // namespace webrtc
diff --git a/modules/audio_coding/audio_network_adaptor/frame_length_controller.h b/modules/audio_coding/audio_network_adaptor/frame_length_controller.h
index 0268ddc..74a787e 100644
--- a/modules/audio_coding/audio_network_adaptor/frame_length_controller.h
+++ b/modules/audio_coding/audio_network_adaptor/frame_length_controller.h
@@ -67,11 +67,9 @@
   void MakeDecision(AudioEncoderRuntimeConfig* config) override;
 
  private:
-  bool FrameLengthIncreasingDecision(
-      const AudioEncoderRuntimeConfig& config) const;
+  bool FrameLengthIncreasingDecision(const AudioEncoderRuntimeConfig& config);
 
-  bool FrameLengthDecreasingDecision(
-      const AudioEncoderRuntimeConfig& config) const;
+  bool FrameLengthDecreasingDecision(const AudioEncoderRuntimeConfig& config);
 
   const Config config_;
 
diff --git a/modules/audio_coding/audio_network_adaptor/frame_length_controller_unittest.cc b/modules/audio_coding/audio_network_adaptor/frame_length_controller_unittest.cc
index 6709336..9db9853 100644
--- a/modules/audio_coding/audio_network_adaptor/frame_length_controller_unittest.cc
+++ b/modules/audio_coding/audio_network_adaptor/frame_length_controller_unittest.cc
@@ -34,6 +34,7 @@
     (kFl60msTo20msBandwidthBps + kFl20msTo60msBandwidthBps) / 2;
 constexpr float kMediumPacketLossFraction =
     (kFlDecreasingPacketLossFraction + kFlIncreasingPacketLossFraction) / 2;
+const std::set<int> kDefaultEncoderFrameLengthsMs = {20, 40, 60, 120};
 
 int VeryLowBitrate(int frame_length_ms) {
   return kMinEncoderBitrateBps + kPreventOveruseMarginBps +
@@ -112,16 +113,16 @@
 }  // namespace
 
 TEST(FrameLengthControllerTest, DecreaseTo20MsOnHighUplinkBandwidth) {
-  auto controller =
-      CreateController(CreateChangeCriteriaFor20msAnd60ms(), {20, 60}, 60);
+  auto controller = CreateController(CreateChangeCriteriaFor20msAnd60ms(),
+                                     kDefaultEncoderFrameLengthsMs, 60);
   UpdateNetworkMetrics(controller.get(), kFl60msTo20msBandwidthBps,
                        absl::nullopt, kOverheadBytesPerPacket);
   CheckDecision(controller.get(), 20);
 }
 
 TEST(FrameLengthControllerTest, DecreaseTo20MsOnHighUplinkPacketLossFraction) {
-  auto controller =
-      CreateController(CreateChangeCriteriaFor20msAnd60ms(), {20, 60}, 60);
+  auto controller = CreateController(CreateChangeCriteriaFor20msAnd60ms(),
+                                     kDefaultEncoderFrameLengthsMs, 60);
   UpdateNetworkMetrics(controller.get(), absl::nullopt,
                        kFlDecreasingPacketLossFraction,
                        kOverheadBytesPerPacket);
@@ -142,8 +143,8 @@
   // 1. |uplink_bandwidth_bps| is at medium level,
   // 2. |uplink_packet_loss_fraction| is at medium,
   // 3. FEC is not decided ON.
-  auto controller =
-      CreateController(CreateChangeCriteriaFor20msAnd60ms(), {20, 60}, 60);
+  auto controller = CreateController(CreateChangeCriteriaFor20msAnd60ms(),
+                                     kDefaultEncoderFrameLengthsMs, 60);
   UpdateNetworkMetrics(controller.get(), kMediumBandwidthBps,
                        kMediumPacketLossFraction, kOverheadBytesPerPacket);
   CheckDecision(controller.get(), 60);
@@ -155,8 +156,8 @@
   // 2. |uplink_packet_loss_fraction| is known to be smaller than a threshold
   //    AND
   // 3. FEC is not decided or OFF.
-  auto controller =
-      CreateController(CreateChangeCriteriaFor20msAnd60ms(), {20, 60}, 20);
+  auto controller = CreateController(CreateChangeCriteriaFor20msAnd60ms(),
+                                     kDefaultEncoderFrameLengthsMs, 20);
   UpdateNetworkMetrics(controller.get(), kFl20msTo60msBandwidthBps,
                        kFlIncreasingPacketLossFraction,
                        kOverheadBytesPerPacket);
@@ -164,8 +165,8 @@
 }
 
 TEST(FrameLengthControllerTest, IncreaseTo60MsOnVeryLowUplinkBandwidth) {
-  auto controller =
-      CreateController(CreateChangeCriteriaFor20msAnd60ms(), {20, 60}, 20);
+  auto controller = CreateController(CreateChangeCriteriaFor20msAnd60ms(),
+                                     kDefaultEncoderFrameLengthsMs, 20);
   // We set packet loss fraction to kFlDecreasingPacketLossFraction, which
   // should have prevented frame length to increase, if the uplink bandwidth
   // was not this low.
@@ -176,8 +177,8 @@
 }
 
 TEST(FrameLengthControllerTest, Maintain60MsOnVeryLowUplinkBandwidth) {
-  auto controller =
-      CreateController(CreateChangeCriteriaFor20msAnd60ms(), {20, 60}, 60);
+  auto controller = CreateController(CreateChangeCriteriaFor20msAnd60ms(),
+                                     kDefaultEncoderFrameLengthsMs, 60);
   // We set packet loss fraction to FlDecreasingPacketLossFraction, which should
   // have caused the frame length to decrease, if the uplink bandwidth was not
   // this low.
@@ -195,8 +196,8 @@
   // FrameLengthController::UpdateNetworkMetrics(...) can handle multiple
   // network updates at once. This is, however, not a common use case in current
   // audio_network_adaptor_impl.cc.
-  auto controller =
-      CreateController(CreateChangeCriteriaFor20msAnd60ms(), {20, 60}, 20);
+  auto controller = CreateController(CreateChangeCriteriaFor20msAnd60ms(),
+                                     kDefaultEncoderFrameLengthsMs, 20);
   Controller::NetworkMetrics network_metrics;
   network_metrics.uplink_bandwidth_bps = kFl20msTo60msBandwidthBps;
   network_metrics.uplink_packet_loss_fraction = kFlIncreasingPacketLossFraction;
@@ -217,8 +218,8 @@
 }
 
 TEST(FrameLengthControllerTest, Maintain20MsOnMediumUplinkBandwidth) {
-  auto controller =
-      CreateController(CreateChangeCriteriaFor20msAnd60ms(), {20, 60}, 20);
+  auto controller = CreateController(CreateChangeCriteriaFor20msAnd60ms(),
+                                     kDefaultEncoderFrameLengthsMs, 20);
   UpdateNetworkMetrics(controller.get(), kMediumBandwidthBps,
                        kFlIncreasingPacketLossFraction,
                        kOverheadBytesPerPacket);
@@ -226,8 +227,8 @@
 }
 
 TEST(FrameLengthControllerTest, Maintain20MsOnMediumUplinkPacketLossFraction) {
-  auto controller =
-      CreateController(CreateChangeCriteriaFor20msAnd60ms(), {20, 60}, 20);
+  auto controller = CreateController(CreateChangeCriteriaFor20msAnd60ms(),
+                                     kDefaultEncoderFrameLengthsMs, 20);
   // Use a low uplink bandwidth that would cause frame length to increase if
   // uplink packet loss fraction was low.
   UpdateNetworkMetrics(controller.get(), kFl20msTo60msBandwidthBps,
@@ -236,8 +237,8 @@
 }
 
 TEST(FrameLengthControllerTest, Maintain60MsWhenNo120msCriteriaIsSet) {
-  auto controller =
-      CreateController(CreateChangeCriteriaFor20msAnd60ms(), {20, 60, 120}, 60);
+  auto controller = CreateController(CreateChangeCriteriaFor20msAnd60ms(),
+                                     kDefaultEncoderFrameLengthsMs, 60);
   UpdateNetworkMetrics(controller.get(), kFl60msTo120msBandwidthBps,
                        kFlIncreasingPacketLossFraction,
                        kOverheadBytesPerPacket);
@@ -246,7 +247,7 @@
 
 TEST(FrameLengthControllerTest, From120MsTo20MsOnHighUplinkBandwidth) {
   auto controller = CreateController(CreateChangeCriteriaFor20ms60msAnd120ms(),
-                                     {20, 60, 120}, 120);
+                                     kDefaultEncoderFrameLengthsMs, 120);
   // It takes two steps for frame length to go from 120ms to 20ms.
   UpdateNetworkMetrics(controller.get(), kFl60msTo20msBandwidthBps,
                        absl::nullopt, kOverheadBytesPerPacket);
@@ -259,7 +260,7 @@
 
 TEST(FrameLengthControllerTest, From120MsTo20MsOnHighUplinkPacketLossFraction) {
   auto controller = CreateController(CreateChangeCriteriaFor20ms60msAnd120ms(),
-                                     {20, 60, 120}, 120);
+                                     kDefaultEncoderFrameLengthsMs, 120);
   // It takes two steps for frame length to go from 120ms to 20ms.
   UpdateNetworkMetrics(controller.get(), absl::nullopt,
                        kFlDecreasingPacketLossFraction,
@@ -274,7 +275,7 @@
 
 TEST(FrameLengthControllerTest, Maintain120MsOnVeryLowUplinkBandwidth) {
   auto controller = CreateController(CreateChangeCriteriaFor20ms60msAnd120ms(),
-                                     {20, 60, 120}, 120);
+                                     kDefaultEncoderFrameLengthsMs, 120);
   // We set packet loss fraction to FlDecreasingPacketLossFraction, which should
   // have caused the frame length to decrease, if the uplink bandwidth was not
   // this low.
@@ -286,7 +287,7 @@
 
 TEST(FrameLengthControllerTest, From60MsTo120MsOnVeryLowUplinkBandwidth) {
   auto controller = CreateController(CreateChangeCriteriaFor20ms60msAnd120ms(),
-                                     {20, 60, 120}, 60);
+                                     kDefaultEncoderFrameLengthsMs, 60);
   // We set packet loss fraction to FlDecreasingPacketLossFraction, which should
   // have prevented frame length to increase, if the uplink bandwidth was not
   // this low.
@@ -301,7 +302,7 @@
   // 1. |uplink_bandwidth_bps| is known to be smaller than a threshold AND
   // 2. |uplink_packet_loss_fraction| is known to be smaller than a threshold.
   auto controller = CreateController(CreateChangeCriteriaFor20ms60msAnd120ms(),
-                                     {20, 60, 120}, 20);
+                                     kDefaultEncoderFrameLengthsMs, 20);
   // It takes two steps for frame length to go from 20ms to 120ms.
   UpdateNetworkMetrics(controller.get(), kFl60msTo120msBandwidthBps,
                        kFlIncreasingPacketLossFraction,
@@ -328,7 +329,7 @@
 
 TEST(FrameLengthControllerTest, CheckBehaviorOnChangingNetworkMetrics) {
   auto controller = CreateController(CreateChangeCriteriaFor20ms60msAnd120ms(),
-                                     {20, 60, 120}, 20);
+                                     kDefaultEncoderFrameLengthsMs, 20);
   UpdateNetworkMetrics(controller.get(), kMediumBandwidthBps,
                        kFlIncreasingPacketLossFraction,
                        kOverheadBytesPerPacket);
diff --git a/modules/audio_coding/codecs/opus/audio_encoder_opus.cc b/modules/audio_coding/codecs/opus/audio_encoder_opus.cc
index d15a242..f901d3c 100644
--- a/modules/audio_coding/codecs/opus/audio_encoder_opus.cc
+++ b/modules/audio_coding/codecs/opus/audio_encoder_opus.cc
@@ -55,10 +55,10 @@
 
 // These two lists must be sorted from low to high
 #if WEBRTC_OPUS_SUPPORT_120MS_PTIME
-constexpr int kANASupportedFrameLengths[] = {20, 60, 120};
+constexpr int kANASupportedFrameLengths[] = {20, 40, 60, 120};
 constexpr int kOpusSupportedFrameLengths[] = {10, 20, 40, 60, 120};
 #else
-constexpr int kANASupportedFrameLengths[] = {20, 60};
+constexpr int kANASupportedFrameLengths[] = {20, 40, 60};
 constexpr int kOpusSupportedFrameLengths[] = {10, 20, 40, 60};
 #endif
 
diff --git a/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc b/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc
index 8ae9ee7..3870ecd 100644
--- a/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc
+++ b/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc
@@ -334,9 +334,11 @@
               ElementsAre(states->encoder->next_frame_length_ms()));
   states->encoder->SetReceiverFrameLengthRange(0, 12345);
   states->encoder->SetReceiverFrameLengthRange(21, 60);
-  EXPECT_THAT(states->encoder->supported_frame_lengths_ms(), ElementsAre(60));
+  EXPECT_THAT(states->encoder->supported_frame_lengths_ms(),
+              ElementsAre(40, 60));
   states->encoder->SetReceiverFrameLengthRange(20, 59);
-  EXPECT_THAT(states->encoder->supported_frame_lengths_ms(), ElementsAre(20));
+  EXPECT_THAT(states->encoder->supported_frame_lengths_ms(),
+              ElementsAre(20, 40));
 }
 
 TEST_P(AudioEncoderOpusTest,
@@ -780,9 +782,9 @@
   const webrtc::SdpAudioFormat format("opus", 48000, 2);
   const auto default_config = *AudioEncoderOpus::SdpToConfig(format);
 #if WEBRTC_OPUS_SUPPORT_120MS_PTIME
-  const std::vector<int> default_supported_frame_lengths_ms({20, 60, 120});
+  const std::vector<int> default_supported_frame_lengths_ms({20, 40, 60, 120});
 #else
-  const std::vector<int> default_supported_frame_lengths_ms({20, 60});
+  const std::vector<int> default_supported_frame_lengths_ms({20, 40, 60});
 #endif
 
   AudioEncoderOpusConfig config;