From 9b29d69650144203d3e688c5bfe5a0fa2983aa0f Mon Sep 17 00:00:00 2001 From: Minyue Li Date: Fri, 16 Aug 2019 12:03:39 +0200 Subject: [PATCH] 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 Reviewed-by: Ivo Creusen Reviewed-by: Henrik Lundin Cr-Commit-Position: refs/heads/master@{#28873} --- .../frame_length_controller.cc | 66 ++++++++++++------- .../frame_length_controller.h | 6 +- .../frame_length_controller_unittest.cc | 53 +++++++-------- .../codecs/opus/audio_encoder_opus.cc | 4 +- .../opus/audio_encoder_opus_unittest.cc | 10 +-- 5 files changed, 79 insertions(+), 60 deletions(-) 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 3cb91fdb30..36e9eb9d9e 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 @@ void FrameLengthController::MakeDecision(AudioEncoderRuntimeConfig* config) { 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::Config::FrameLengthChange::operator<( } 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 @@ bool FrameLengthController::FrameLengthIncreasingDecision( // 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 @@ bool FrameLengthController::FrameLengthIncreasingDecision( 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 @@ bool FrameLengthController::FrameLengthDecreasingDecision( // 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 @@ bool FrameLengthController::FrameLengthDecreasingDecision( 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 0268ddc611..74a787e1c1 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 @@ class FrameLengthController final : public Controller { 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 6709336f7c..9db98536b6 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 @@ constexpr int kMediumBandwidthBps = (kFl60msTo20msBandwidthBps + kFl20msTo60msBandwidthBps) / 2; constexpr float kMediumPacketLossFraction = (kFlDecreasingPacketLossFraction + kFlIncreasingPacketLossFraction) / 2; +const std::set kDefaultEncoderFrameLengthsMs = {20, 40, 60, 120}; int VeryLowBitrate(int frame_length_ms) { return kMinEncoderBitrateBps + kPreventOveruseMarginBps + @@ -112,16 +113,16 @@ void CheckDecision(FrameLengthController* controller, } // 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 @@ TEST(FrameLengthControllerTest, Maintain60MsOnMultipleConditions) { // 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 @@ TEST(FrameLengthControllerTest, IncreaseTo60MsOnMultipleConditions) { // 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, IncreaseTo60MsOnMultipleConditions) { } 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, IncreaseTo60MsOnVeryLowUplinkBandwidth) { } 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 @@ TEST(FrameLengthControllerTest, UpdateMultipleNetworkMetricsAtOnce) { // 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, } 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, Maintain20MsOnMediumUplinkBandwidth) { } 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, Maintain20MsOnMediumUplinkPacketLossFraction) { } 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, Maintain60MsWhenNo120msCriteriaIsSet) { 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, From120MsTo20MsOnHighUplinkBandwidth) { 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, From120MsTo20MsOnHighUplinkPacketLossFraction) { 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, Maintain120MsOnVeryLowUplinkBandwidth) { 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 @@ TEST(FrameLengthControllerTest, From20MsTo120MsOnMultipleConditions) { // 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, Stall60MsIf120MsNotInReceiverFrameLengthRange) { 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 d15a2422b1..f901d3ca11 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 @@ constexpr int kDefaultMaxPlaybackRate = 48000; // 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 8ae9ee7520..3870ecd071 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 @@ TEST_P(AudioEncoderOpusTest, SetReceiverFrameLengthRange) { 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 @@ TEST(AudioEncoderOpusTest, TestConfigFromInvalidParams) { const webrtc::SdpAudioFormat format("opus", 48000, 2); const auto default_config = *AudioEncoderOpus::SdpToConfig(format); #if WEBRTC_OPUS_SUPPORT_120MS_PTIME - const std::vector default_supported_frame_lengths_ms({20, 60, 120}); + const std::vector default_supported_frame_lengths_ms({20, 40, 60, 120}); #else - const std::vector default_supported_frame_lengths_ms({20, 60}); + const std::vector default_supported_frame_lengths_ms({20, 40, 60}); #endif AudioEncoderOpusConfig config;