Decouple FEC controller's decision from the decision making for frame length.
Bug: webrtc:8098
Change-Id: If313c8069a16bfb3c2752dcca7fb1cfca24c1caf
Reviewed-on: https://chromium-review.googlesource.com/643299
Reviewed-by: Michael T <tschumim@webrtc.org>
Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
Commit-Queue: Minyue Li <minyue@webrtc.org>
Cr-Original-Commit-Position: refs/heads/master@{#19627}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: 5fcd1cc4bbba639b1544c3ae9640ac45bfd1ff44diff --git a/modules/audio_coding/audio_network_adaptor/frame_length_controller.cc b/modules/audio_coding/audio_network_adaptor/frame_length_controller.cc
index 49b1d4d..4dd6088 100644
--- a/modules/audio_coding/audio_network_adaptor/frame_length_controller.cc
+++ b/modules/audio_coding/audio_network_adaptor/frame_length_controller.cc
@@ -98,9 +98,7 @@
// current overhead rate OR all the following:
// 2. longer frame length is available AND
// 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
- // AND
- // 5. FEC is not decided or is OFF.
+ // 4. |uplink_packet_loss_fraction| is known to be smaller than a threshold.
auto longer_frame_length_ms = std::next(frame_length_ms_);
if (longer_frame_length_ms == config_.encoder_frame_lengths_ms.end())
@@ -123,8 +121,7 @@
*uplink_bandwidth_bps_ <= increase_threshold->second) &&
(uplink_packet_loss_fraction_ &&
*uplink_packet_loss_fraction_ <=
- config_.fl_increasing_packet_loss_fraction) &&
- !config.enable_fec.value_or(false);
+ config_.fl_increasing_packet_loss_fraction);
}
bool FrameLengthController::FrameLengthDecreasingDecision(
@@ -137,7 +134,6 @@
// 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,
- // 5. FEC is decided ON.
if (frame_length_ms_ == config_.encoder_frame_lengths_ms.begin())
return false;
@@ -160,8 +156,7 @@
*uplink_bandwidth_bps_ >= decrease_threshold->second) ||
(uplink_packet_loss_fraction_ &&
*uplink_packet_loss_fraction_ >=
- config_.fl_decreasing_packet_loss_fraction) ||
- config.enable_fec.value_or(false);
+ config_.fl_decreasing_packet_loss_fraction);
}
} // namespace webrtc
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 beff4ed..d2b535c 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
@@ -99,10 +99,8 @@
}
void CheckDecision(FrameLengthController* controller,
- const rtc::Optional<bool>& enable_fec,
int expected_frame_length_ms) {
AudioEncoderRuntimeConfig config;
- config.enable_fec = enable_fec;
controller->MakeDecision(&config);
EXPECT_EQ(rtc::Optional<int>(expected_frame_length_ms),
config.frame_length_ms);
@@ -116,7 +114,7 @@
UpdateNetworkMetrics(
controller.get(), rtc::Optional<int>(kFl60msTo20msBandwidthBps),
rtc::Optional<float>(), rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 20);
+ CheckDecision(controller.get(), 20);
}
TEST(FrameLengthControllerTest, DecreaseTo20MsOnHighUplinkPacketLossFraction) {
@@ -125,13 +123,7 @@
UpdateNetworkMetrics(controller.get(), rtc::Optional<int>(),
rtc::Optional<float>(kFlDecreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 20);
-}
-
-TEST(FrameLengthControllerTest, DecreaseTo20MsWhenFecIsOn) {
- auto controller =
- CreateController(CreateChangeCriteriaFor20msAnd60ms(), {20, 60}, 60);
- CheckDecision(controller.get(), rtc::Optional<bool>(true), 20);
+ CheckDecision(controller.get(), 20);
}
TEST(FrameLengthControllerTest,
@@ -140,7 +132,7 @@
CreateController(CreateChangeCriteriaFor20msAnd60ms(), {60}, 60);
// Set FEC on that would cause frame length to decrease if receiver frame
// length range included 20ms.
- CheckDecision(controller.get(), rtc::Optional<bool>(true), 60);
+ CheckDecision(controller.get(), 60);
}
TEST(FrameLengthControllerTest, Maintain60MsOnMultipleConditions) {
@@ -154,7 +146,7 @@
rtc::Optional<int>(kMediumBandwidthBps),
rtc::Optional<float>(kMediumPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 60);
+ CheckDecision(controller.get(), 60);
}
TEST(FrameLengthControllerTest, IncreaseTo60MsOnMultipleConditions) {
@@ -169,31 +161,31 @@
rtc::Optional<int>(kFl20msTo60msBandwidthBps),
rtc::Optional<float>(kFlIncreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 60);
+ CheckDecision(controller.get(), 60);
}
TEST(FrameLengthControllerTest, IncreaseTo60MsOnVeryLowUplinkBandwidth) {
auto controller =
CreateController(CreateChangeCriteriaFor20msAnd60ms(), {20, 60}, 20);
- // We set packet loss fraction to kFlDecreasingPacketLossFraction, and FEC on,
- // both of which should have prevented frame length to increase, if the uplink
- // bandwidth was not this low.
+ // We set packet loss fraction to kFlDecreasingPacketLossFraction, which
+ // should have prevented frame length to increase, if the uplink bandwidth
+ // was not this low.
UpdateNetworkMetrics(controller.get(), rtc::Optional<int>(VeryLowBitrate(20)),
rtc::Optional<float>(kFlIncreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(true), 60);
+ CheckDecision(controller.get(), 60);
}
TEST(FrameLengthControllerTest, Maintain60MsOnVeryLowUplinkBandwidth) {
auto controller =
CreateController(CreateChangeCriteriaFor20msAnd60ms(), {20, 60}, 60);
- // We set packet loss fraction to FlDecreasingPacketLossFraction, and FEC on,
- // both of which should have caused the frame length to decrease, if the
- // uplink bandwidth was not this low.
+ // We set packet loss fraction to FlDecreasingPacketLossFraction, which should
+ // have caused the frame length to decrease, if the uplink bandwidth was not
+ // this low.
UpdateNetworkMetrics(controller.get(), rtc::Optional<int>(VeryLowBitrate(20)),
rtc::Optional<float>(kFlIncreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(true), 60);
+ CheckDecision(controller.get(), 60);
}
TEST(FrameLengthControllerTest, UpdateMultipleNetworkMetricsAtOnce) {
@@ -212,7 +204,7 @@
network_metrics.uplink_packet_loss_fraction =
rtc::Optional<float>(kFlIncreasingPacketLossFraction);
controller->UpdateNetworkMetrics(network_metrics);
- CheckDecision(controller.get(), rtc::Optional<bool>(), 60);
+ CheckDecision(controller.get(), 60);
}
TEST(FrameLengthControllerTest,
@@ -225,7 +217,7 @@
rtc::Optional<int>(kFl20msTo60msBandwidthBps),
rtc::Optional<float>(kFlIncreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 20);
+ CheckDecision(controller.get(), 20);
}
TEST(FrameLengthControllerTest, Maintain20MsOnMediumUplinkBandwidth) {
@@ -235,7 +227,7 @@
rtc::Optional<int>(kMediumBandwidthBps),
rtc::Optional<float>(kFlIncreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 20);
+ CheckDecision(controller.get(), 20);
}
TEST(FrameLengthControllerTest, Maintain20MsOnMediumUplinkPacketLossFraction) {
@@ -247,19 +239,7 @@
rtc::Optional<int>(kFl20msTo60msBandwidthBps),
rtc::Optional<float>(kMediumPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 20);
-}
-
-TEST(FrameLengthControllerTest, Maintain20MsWhenFecIsOn) {
- auto controller =
- CreateController(CreateChangeCriteriaFor20msAnd60ms(), {20, 60}, 20);
- // Use a low uplink bandwidth and a low uplink packet loss fraction that would
- // cause frame length to increase if FEC was not ON.
- UpdateNetworkMetrics(controller.get(),
- rtc::Optional<int>(kFl20msTo60msBandwidthBps),
- rtc::Optional<float>(kMediumPacketLossFraction),
- rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(true), 20);
+ CheckDecision(controller.get(), 20);
}
TEST(FrameLengthControllerTest, Maintain60MsWhenNo120msCriteriaIsSet) {
@@ -269,7 +249,7 @@
rtc::Optional<int>(kFl60msTo120msBandwidthBps),
rtc::Optional<float>(kFlIncreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 60);
+ CheckDecision(controller.get(), 60);
}
TEST(FrameLengthControllerTest, From120MsTo20MsOnHighUplinkBandwidth) {
@@ -279,12 +259,12 @@
UpdateNetworkMetrics(
controller.get(), rtc::Optional<int>(kFl60msTo20msBandwidthBps),
rtc::Optional<float>(), rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 60);
+ CheckDecision(controller.get(), 60);
UpdateNetworkMetrics(
controller.get(), rtc::Optional<int>(kFl60msTo20msBandwidthBps),
rtc::Optional<float>(), rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 20);
+ CheckDecision(controller.get(), 20);
}
TEST(FrameLengthControllerTest, From120MsTo20MsOnHighUplinkPacketLossFraction) {
@@ -294,52 +274,42 @@
UpdateNetworkMetrics(controller.get(), rtc::Optional<int>(),
rtc::Optional<float>(kFlDecreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 60);
+ CheckDecision(controller.get(), 60);
UpdateNetworkMetrics(controller.get(), rtc::Optional<int>(),
rtc::Optional<float>(kFlDecreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 20);
-}
-
-TEST(FrameLengthControllerTest, From120MsTo20MsWhenFecIsOn) {
- auto controller = CreateController(CreateChangeCriteriaFor20ms60msAnd120ms(),
- {20, 60, 120}, 120);
- // It takes two steps for frame length to go from 120ms to 20ms.
- CheckDecision(controller.get(), rtc::Optional<bool>(true), 60);
- CheckDecision(controller.get(), rtc::Optional<bool>(true), 20);
+ CheckDecision(controller.get(), 20);
}
TEST(FrameLengthControllerTest, Maintain120MsOnVeryLowUplinkBandwidth) {
auto controller = CreateController(CreateChangeCriteriaFor20ms60msAnd120ms(),
{20, 60, 120}, 120);
- // We set packet loss fraction to FlDecreasingPacketLossFraction, and FEC on,
- // both of which should have caused the frame length to decrease, if the
- // uplink bandwidth was not this low.
+ // We set packet loss fraction to FlDecreasingPacketLossFraction, which should
+ // have caused the frame length to decrease, if the uplink bandwidth was not
+ // this low.
UpdateNetworkMetrics(controller.get(), rtc::Optional<int>(VeryLowBitrate(60)),
rtc::Optional<float>(kFlDecreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(true), 120);
+ CheckDecision(controller.get(), 120);
}
TEST(FrameLengthControllerTest, From60MsTo120MsOnVeryLowUplinkBandwidth) {
auto controller = CreateController(CreateChangeCriteriaFor20ms60msAnd120ms(),
{20, 60, 120}, 60);
- // We set packet loss fraction to FlDecreasingPacketLossFraction, and FEC on,
- // both of which should have prevented frame length to increase, if the uplink
- // bandwidth was not this low.
+ // We set packet loss fraction to FlDecreasingPacketLossFraction, which should
+ // have prevented frame length to increase, if the uplink bandwidth was not
+ // this low.
UpdateNetworkMetrics(controller.get(), rtc::Optional<int>(VeryLowBitrate(60)),
rtc::Optional<float>(kFlDecreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(true), 120);
+ CheckDecision(controller.get(), 120);
}
TEST(FrameLengthControllerTest, From20MsTo120MsOnMultipleConditions) {
// Increase to 120ms 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.
+ // 2. |uplink_packet_loss_fraction| is known to be smaller than a threshold.
auto controller = CreateController(CreateChangeCriteriaFor20ms60msAnd120ms(),
{20, 60, 120}, 20);
// It takes two steps for frame length to go from 20ms to 120ms.
@@ -347,12 +317,12 @@
rtc::Optional<int>(kFl60msTo120msBandwidthBps),
rtc::Optional<float>(kFlIncreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 60);
+ CheckDecision(controller.get(), 60);
UpdateNetworkMetrics(controller.get(),
rtc::Optional<int>(kFl60msTo120msBandwidthBps),
rtc::Optional<float>(kFlIncreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 120);
+ CheckDecision(controller.get(), 120);
}
TEST(FrameLengthControllerTest, Stall60MsIf120MsNotInReceiverFrameLengthRange) {
@@ -362,12 +332,12 @@
rtc::Optional<int>(kFl60msTo120msBandwidthBps),
rtc::Optional<float>(kFlIncreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 60);
+ CheckDecision(controller.get(), 60);
UpdateNetworkMetrics(controller.get(),
rtc::Optional<int>(kFl60msTo120msBandwidthBps),
rtc::Optional<float>(kFlIncreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 60);
+ CheckDecision(controller.get(), 60);
}
TEST(FrameLengthControllerTest, CheckBehaviorOnChangingNetworkMetrics) {
@@ -377,37 +347,37 @@
rtc::Optional<int>(kMediumBandwidthBps),
rtc::Optional<float>(kFlIncreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 20);
+ CheckDecision(controller.get(), 20);
UpdateNetworkMetrics(controller.get(),
rtc::Optional<int>(kFl20msTo60msBandwidthBps),
rtc::Optional<float>(kFlIncreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 60);
+ CheckDecision(controller.get(), 60);
UpdateNetworkMetrics(controller.get(),
rtc::Optional<int>(kFl60msTo120msBandwidthBps),
rtc::Optional<float>(kMediumPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 60);
+ CheckDecision(controller.get(), 60);
UpdateNetworkMetrics(controller.get(),
rtc::Optional<int>(kFl60msTo120msBandwidthBps),
rtc::Optional<float>(kFlIncreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 120);
+ CheckDecision(controller.get(), 120);
UpdateNetworkMetrics(controller.get(),
rtc::Optional<int>(kFl120msTo60msBandwidthBps),
rtc::Optional<float>(kFlIncreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 60);
+ CheckDecision(controller.get(), 60);
UpdateNetworkMetrics(controller.get(),
rtc::Optional<int>(kMediumBandwidthBps),
rtc::Optional<float>(kFlDecreasingPacketLossFraction),
rtc::Optional<size_t>(kOverheadBytesPerPacket));
- CheckDecision(controller.get(), rtc::Optional<bool>(), 20);
+ CheckDecision(controller.get(), 20);
}
} // namespace webrtc
