From 0e1a558fb31cbddda51f2f0b9a6a3058d6d10281 Mon Sep 17 00:00:00 2001 From: Ying Wang Date: Fri, 16 Aug 2019 16:45:30 +0200 Subject: [PATCH] Allowing 40ms audio frame length. Currently 20ms, 60ms and 120ms frame length are supported. The motivation is to better adapt audio bit rate to network conditions with more frame length choices. This is continuation of https://webrtc-review.googlesource.com/c/src/+/146206, since crodbro is out of office, I created this commit for continuing the code review. Bug: webrtc:10820 Change-Id: I0e35e91b524f63686bfdf767b7a95c51aeb24716 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146780 Reviewed-by: Minyue Li Reviewed-by: Alex Narest Reviewed-by: Bjorn Mellem Commit-Queue: Ying Wang Cr-Commit-Position: refs/heads/master@{#28882} --- .../audio_network_adaptor/config.proto | 16 ++++ .../controller_manager.cc | 50 +++++++++-- .../controller_manager_unittest.cc | 6 +- .../frame_length_controller_unittest.cc | 87 ++++++++++++++++++- 4 files changed, 145 insertions(+), 14 deletions(-) diff --git a/modules/audio_coding/audio_network_adaptor/config.proto b/modules/audio_coding/audio_network_adaptor/config.proto index 6d1cd42d46..90c58e5c7d 100644 --- a/modules/audio_coding/audio_network_adaptor/config.proto +++ b/modules/audio_coding/audio_network_adaptor/config.proto @@ -98,6 +98,22 @@ message FrameLengthController { // Offset to apply to the per-packet overhead when decreasing frame length. optional int32 fl_decrease_overhead_offset = 8; + + // Uplink bandwidth below which frame length can switch from 20ms to 40ms. In + // current implementation, defining this will invalidate + // fl_20ms_to_60ms_bandwidth_bps. + optional int32 fl_20ms_to_40ms_bandwidth_bps = 9; + + // Uplink bandwidth above which frame length should switch from 40ms to 20ms. + optional int32 fl_40ms_to_20ms_bandwidth_bps = 10; + + // Uplink bandwidth below which frame length can switch from 40ms to 60ms. + optional int32 fl_40ms_to_60ms_bandwidth_bps = 11; + + // Uplink bandwidth above which frame length should switch from 60ms to 40ms. + // In current implementation, defining this will invalidate + // fl_60ms_to_20ms_bandwidth_bps. + optional int32 fl_60ms_to_40ms_bandwidth_bps = 12; } message ChannelController { diff --git a/modules/audio_coding/audio_network_adaptor/controller_manager.cc b/modules/audio_coding/audio_network_adaptor/controller_manager.cc index 4c0e61c6ad..f22df54165 100644 --- a/modules/audio_coding/audio_network_adaptor/controller_manager.cc +++ b/modules/audio_coding/audio_network_adaptor/controller_manager.cc @@ -118,21 +118,53 @@ std::unique_ptr CreateFrameLengthController( int min_encoder_bitrate_bps) { RTC_CHECK(config.has_fl_increasing_packet_loss_fraction()); RTC_CHECK(config.has_fl_decreasing_packet_loss_fraction()); - RTC_CHECK(config.has_fl_20ms_to_60ms_bandwidth_bps()); - RTC_CHECK(config.has_fl_60ms_to_20ms_bandwidth_bps()); std::map - fl_changing_bandwidths_bps = { - {FrameLengthController::Config::FrameLengthChange(20, 60), - config.fl_20ms_to_60ms_bandwidth_bps()}, - {FrameLengthController::Config::FrameLengthChange(60, 20), - config.fl_60ms_to_20ms_bandwidth_bps()}}; + fl_changing_bandwidths_bps; - if (config.has_fl_60ms_to_120ms_bandwidth_bps() && - config.has_fl_120ms_to_60ms_bandwidth_bps()) { + if (config.has_fl_20ms_to_60ms_bandwidth_bps()) { + fl_changing_bandwidths_bps.insert( + std::make_pair(FrameLengthController::Config::FrameLengthChange(20, 60), + config.fl_20ms_to_60ms_bandwidth_bps())); + } + + if (config.has_fl_60ms_to_20ms_bandwidth_bps()) { + fl_changing_bandwidths_bps.insert( + std::make_pair(FrameLengthController::Config::FrameLengthChange(60, 20), + config.fl_60ms_to_20ms_bandwidth_bps())); + } + + if (config.has_fl_20ms_to_40ms_bandwidth_bps()) { + fl_changing_bandwidths_bps.insert( + std::make_pair(FrameLengthController::Config::FrameLengthChange(20, 40), + config.fl_20ms_to_40ms_bandwidth_bps())); + } + + if (config.has_fl_40ms_to_20ms_bandwidth_bps()) { + fl_changing_bandwidths_bps.insert( + std::make_pair(FrameLengthController::Config::FrameLengthChange(40, 20), + config.fl_40ms_to_20ms_bandwidth_bps())); + } + + if (config.has_fl_40ms_to_60ms_bandwidth_bps()) { + fl_changing_bandwidths_bps.insert( + std::make_pair(FrameLengthController::Config::FrameLengthChange(40, 60), + config.fl_40ms_to_60ms_bandwidth_bps())); + } + + if (config.has_fl_60ms_to_40ms_bandwidth_bps()) { + fl_changing_bandwidths_bps.insert( + std::make_pair(FrameLengthController::Config::FrameLengthChange(60, 40), + config.fl_60ms_to_40ms_bandwidth_bps())); + } + + if (config.has_fl_60ms_to_120ms_bandwidth_bps()) { fl_changing_bandwidths_bps.insert(std::make_pair( FrameLengthController::Config::FrameLengthChange(60, 120), config.fl_60ms_to_120ms_bandwidth_bps())); + } + + if (config.has_fl_120ms_to_60ms_bandwidth_bps()) { fl_changing_bandwidths_bps.insert(std::make_pair( FrameLengthController::Config::FrameLengthChange(120, 60), config.fl_120ms_to_60ms_bandwidth_bps())); diff --git a/modules/audio_coding/audio_network_adaptor/controller_manager_unittest.cc b/modules/audio_coding/audio_network_adaptor/controller_manager_unittest.cc index 7fa4096702..c267b791c3 100644 --- a/modules/audio_coding/audio_network_adaptor/controller_manager_unittest.cc +++ b/modules/audio_coding/audio_network_adaptor/controller_manager_unittest.cc @@ -249,8 +249,10 @@ void AddFrameLengthControllerConfig( controller_config_ext->mutable_frame_length_controller(); controller_config->set_fl_decreasing_packet_loss_fraction(0.05f); controller_config->set_fl_increasing_packet_loss_fraction(0.04f); - controller_config->set_fl_20ms_to_60ms_bandwidth_bps(72000); - controller_config->set_fl_60ms_to_20ms_bandwidth_bps(88000); + controller_config->set_fl_20ms_to_40ms_bandwidth_bps(80000); + controller_config->set_fl_40ms_to_20ms_bandwidth_bps(88000); + controller_config->set_fl_40ms_to_60ms_bandwidth_bps(72000); + controller_config->set_fl_60ms_to_40ms_bandwidth_bps(80000); auto scoring_point = controller_config_ext->mutable_scoring_point(); scoring_point->set_uplink_bandwidth_bps(kChracteristicBandwithBps[1]); 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 9db98536b6..0ffa54a1ed 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 @@ -30,8 +30,13 @@ constexpr int kFl20msTo60msBandwidthBps = 40000; constexpr int kFl60msTo20msBandwidthBps = 50000; constexpr int kFl60msTo120msBandwidthBps = 30000; constexpr int kFl120msTo60msBandwidthBps = 40000; +constexpr int kFl20msTo40msBandwidthBps = 45000; +constexpr int kFl40msTo20msBandwidthBps = 50000; +constexpr int kFl40msTo60msBandwidthBps = 40000; +constexpr int kFl60msTo40msBandwidthBps = 45000; + constexpr int kMediumBandwidthBps = - (kFl60msTo20msBandwidthBps + kFl20msTo60msBandwidthBps) / 2; + (kFl40msTo20msBandwidthBps + kFl20msTo40msBandwidthBps) / 2; constexpr float kMediumPacketLossFraction = (kFlDecreasingPacketLossFraction + kFlIncreasingPacketLossFraction) / 2; const std::set kDefaultEncoderFrameLengthsMs = {20, 40, 60, 120}; @@ -65,6 +70,15 @@ CreateChangeCriteriaFor20msAnd60ms() { kFl60msTo20msBandwidthBps}}; } +std::map +CreateChangeCriteriaFor20msAnd40ms() { + return std::map{ + {FrameLengthController::Config::FrameLengthChange(20, 40), + kFl20msTo40msBandwidthBps}, + {FrameLengthController::Config::FrameLengthChange(40, 20), + kFl40msTo20msBandwidthBps}}; +} + std::map CreateChangeCriteriaFor20ms60msAnd120ms() { return std::map{ @@ -78,6 +92,36 @@ CreateChangeCriteriaFor20ms60msAnd120ms() { kFl120msTo60msBandwidthBps}}; } +std::map +CreateChangeCriteriaFor20ms40ms60msAnd120ms() { + return std::map{ + {FrameLengthController::Config::FrameLengthChange(20, 60), + kFl20msTo60msBandwidthBps}, + {FrameLengthController::Config::FrameLengthChange(60, 20), + kFl60msTo20msBandwidthBps}, + {FrameLengthController::Config::FrameLengthChange(20, 40), + kFl20msTo40msBandwidthBps}, + {FrameLengthController::Config::FrameLengthChange(40, 20), + kFl40msTo20msBandwidthBps}, + {FrameLengthController::Config::FrameLengthChange(40, 60), + kFl40msTo60msBandwidthBps}, + {FrameLengthController::Config::FrameLengthChange(60, 40), + kFl60msTo40msBandwidthBps}, + {FrameLengthController::Config::FrameLengthChange(60, 120), + kFl60msTo120msBandwidthBps}, + {FrameLengthController::Config::FrameLengthChange(120, 60), + kFl120msTo60msBandwidthBps}}; +} + +std::map +CreateChangeCriteriaFor40msAnd60ms() { + return std::map{ + {FrameLengthController::Config::FrameLengthChange(40, 60), + kFl40msTo60msBandwidthBps}, + {FrameLengthController::Config::FrameLengthChange(60, 40), + kFl60msTo40msBandwidthBps}}; +} + void UpdateNetworkMetrics( FrameLengthController* controller, const absl::optional& uplink_bandwidth_bps, @@ -138,6 +182,28 @@ TEST(FrameLengthControllerTest, CheckDecision(controller.get(), 60); } +TEST(FrameLengthControllerTest, IncreaseTo40MsOnMultipleConditions) { + // Increase to 40ms frame length if + // 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 + // AND + // 3. FEC is not decided or OFF. + auto controller = CreateController(CreateChangeCriteriaFor20msAnd40ms(), + kDefaultEncoderFrameLengthsMs, 20); + UpdateNetworkMetrics(controller.get(), kFl20msTo40msBandwidthBps, + kFlIncreasingPacketLossFraction, + kOverheadBytesPerPacket); + CheckDecision(controller.get(), 40); +} + +TEST(FrameLengthControllerTest, DecreaseTo40MsOnHighUplinkBandwidth) { + auto controller = CreateController(CreateChangeCriteriaFor40msAnd60ms(), + kDefaultEncoderFrameLengthsMs, 40); + UpdateNetworkMetrics(controller.get(), kFl60msTo40msBandwidthBps, + absl::nullopt, kOverheadBytesPerPacket); + CheckDecision(controller.get(), 40); +} + TEST(FrameLengthControllerTest, Maintain60MsOnMultipleConditions) { // Maintain 60ms frame length if // 1. |uplink_bandwidth_bps| is at medium level, @@ -328,13 +394,23 @@ TEST(FrameLengthControllerTest, Stall60MsIf120MsNotInReceiverFrameLengthRange) { } TEST(FrameLengthControllerTest, CheckBehaviorOnChangingNetworkMetrics) { - auto controller = CreateController(CreateChangeCriteriaFor20ms60msAnd120ms(), - kDefaultEncoderFrameLengthsMs, 20); + auto controller = + CreateController(CreateChangeCriteriaFor20ms40ms60msAnd120ms(), + kDefaultEncoderFrameLengthsMs, 20); UpdateNetworkMetrics(controller.get(), kMediumBandwidthBps, kFlIncreasingPacketLossFraction, kOverheadBytesPerPacket); CheckDecision(controller.get(), 20); + UpdateNetworkMetrics(controller.get(), kFl20msTo40msBandwidthBps, + kFlIncreasingPacketLossFraction, + kOverheadBytesPerPacket); + CheckDecision(controller.get(), 40); + + UpdateNetworkMetrics(controller.get(), kFl60msTo40msBandwidthBps, + kMediumPacketLossFraction, kOverheadBytesPerPacket); + CheckDecision(controller.get(), 40); + UpdateNetworkMetrics(controller.get(), kFl20msTo60msBandwidthBps, kFlIncreasingPacketLossFraction, kOverheadBytesPerPacket); @@ -354,6 +430,11 @@ TEST(FrameLengthControllerTest, CheckBehaviorOnChangingNetworkMetrics) { kOverheadBytesPerPacket); CheckDecision(controller.get(), 60); + UpdateNetworkMetrics(controller.get(), kFl60msTo40msBandwidthBps, + kFlDecreasingPacketLossFraction, + kOverheadBytesPerPacket); + CheckDecision(controller.get(), 40); + UpdateNetworkMetrics(controller.get(), kMediumBandwidthBps, kFlDecreasingPacketLossFraction, kOverheadBytesPerPacket);