Reset RobustThroughputEstimator if recv timestamp jump backwards

Start using RobustThoughputEstimator in DelayBasedBwe test in preparation for making it default.
Experiments has not showed significant metric changes. However, simulations has showed that RobustThroughputEstimator better follow the actually receive rate better. Especially during bursts of sent packets. Code is also simpler.

Bug: webrtc:13402 chromium:1411666
Change-Id: I83cfa1fc15486982b18cc22fbd0752ff59c1c1b4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/317600
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40644}
This commit is contained in:
Per K 2023-08-25 16:17:25 +02:00 committed by WebRTC LUCI CQ
parent 14e5d4ce5e
commit a041a97f63
7 changed files with 53 additions and 104 deletions

View File

@ -123,7 +123,7 @@ TEST_F(DelayBasedBweTest, ProbeDetectionSlowerArrivalHighBitrate) {
TEST_F(DelayBasedBweTest, GetExpectedBwePeriodMs) {
auto default_interval = bitrate_estimator_->GetExpectedBwePeriod();
EXPECT_GT(default_interval.ms(), 0);
CapacityDropTestHelper(1, true, 333, 0);
CapacityDropTestHelper(1, true, 533, 0);
auto interval = bitrate_estimator_->GetExpectedBwePeriod();
EXPECT_GT(interval.ms(), 0);
EXPECT_NE(interval.ms(), default_interval.ms());
@ -142,11 +142,11 @@ TEST_F(DelayBasedBweTest, RateIncreaseReordering) {
RateIncreaseReorderingTestHelper(730000);
}
TEST_F(DelayBasedBweTest, RateIncreaseRtpTimestamps) {
RateIncreaseRtpTimestampsTestHelper(622);
RateIncreaseRtpTimestampsTestHelper(617);
}
TEST_F(DelayBasedBweTest, CapacityDropOneStream) {
CapacityDropTestHelper(1, false, 300, 0);
CapacityDropTestHelper(1, false, 500, 0);
}
TEST_F(DelayBasedBweTest, CapacityDropPosOffsetChange) {
@ -158,7 +158,7 @@ TEST_F(DelayBasedBweTest, CapacityDropNegOffsetChange) {
}
TEST_F(DelayBasedBweTest, CapacityDropOneStreamWrap) {
CapacityDropTestHelper(1, true, 333, 0);
CapacityDropTestHelper(1, true, 533, 0);
}
TEST_F(DelayBasedBweTest, TestTimestampGrouping) {
@ -193,19 +193,16 @@ TEST_F(DelayBasedBweTest, TestInitialOveruse) {
// Needed to initialize the AimdRateControl.
bitrate_estimator_->SetStartBitrate(kStartBitrate);
// Produce 30 frames (in 1/3 second) and give them to the estimator.
// Produce 40 frames (in 1/3 second) and give them to the estimator.
int64_t bitrate_bps = kStartBitrate.bps();
bool seen_overuse = false;
for (int i = 0; i < 30; ++i) {
for (int i = 0; i < 40; ++i) {
bool overuse = GenerateAndProcessFrame(kDummySsrc, bitrate_bps);
// The purpose of this test is to ensure that we back down even if we don't
// have any acknowledged bitrate estimate yet. Hence, if the test works
// as expected, we should not have a measured bitrate yet.
EXPECT_FALSE(acknowledged_bitrate_estimator_->bitrate().has_value());
if (overuse) {
EXPECT_TRUE(bitrate_observer_.updated());
EXPECT_NEAR(bitrate_observer_.latest_bitrate(), kStartBitrate.bps() / 2,
15000);
EXPECT_LE(bitrate_observer_.latest_bitrate(), kInitialCapacity.bps());
EXPECT_GT(bitrate_observer_.latest_bitrate(),
0.8 * kInitialCapacity.bps());
bitrate_bps = bitrate_observer_.latest_bitrate();
seen_overuse = true;
break;
@ -215,8 +212,8 @@ TEST_F(DelayBasedBweTest, TestInitialOveruse) {
}
}
EXPECT_TRUE(seen_overuse);
EXPECT_NEAR(bitrate_observer_.latest_bitrate(), kStartBitrate.bps() / 2,
15000);
EXPECT_LE(bitrate_observer_.latest_bitrate(), kInitialCapacity.bps());
EXPECT_GT(bitrate_observer_.latest_bitrate(), 0.8 * kInitialCapacity.bps());
}
TEST_F(DelayBasedBweTest, TestTimestampPrecisionHandling) {
@ -254,61 +251,4 @@ TEST_F(DelayBasedBweTest, TestTimestampPrecisionHandling) {
}
}
class DelayBasedBweTestWithBackoffTimeoutExperiment : public DelayBasedBweTest {
public:
DelayBasedBweTestWithBackoffTimeoutExperiment()
: DelayBasedBweTest(
"WebRTC-BweAimdRateControlConfig/initial_backoff_interval:200ms/") {
}
};
// This test subsumes and improves DelayBasedBweTest.TestInitialOveruse above.
TEST_F(DelayBasedBweTestWithBackoffTimeoutExperiment, TestInitialOveruse) {
const DataRate kStartBitrate = DataRate::KilobitsPerSec(300);
const DataRate kInitialCapacity = DataRate::KilobitsPerSec(200);
const uint32_t kDummySsrc = 0;
// High FPS to ensure that we send a lot of packets in a short time.
const int kFps = 90;
stream_generator_->AddStream(new test::RtpStream(kFps, kStartBitrate.bps()));
stream_generator_->set_capacity_bps(kInitialCapacity.bps());
// Needed to initialize the AimdRateControl.
bitrate_estimator_->SetStartBitrate(kStartBitrate);
// Produce 30 frames (in 1/3 second) and give them to the estimator.
int64_t bitrate_bps = kStartBitrate.bps();
bool seen_overuse = false;
for (int frames = 0; frames < 30 && !seen_overuse; ++frames) {
bool overuse = GenerateAndProcessFrame(kDummySsrc, bitrate_bps);
// The purpose of this test is to ensure that we back down even if we don't
// have any acknowledged bitrate estimate yet. Hence, if the test works
// as expected, we should not have a measured bitrate yet.
EXPECT_FALSE(acknowledged_bitrate_estimator_->bitrate().has_value());
if (overuse) {
EXPECT_TRUE(bitrate_observer_.updated());
EXPECT_NEAR(bitrate_observer_.latest_bitrate(), kStartBitrate.bps() / 2,
15000);
bitrate_bps = bitrate_observer_.latest_bitrate();
seen_overuse = true;
} else if (bitrate_observer_.updated()) {
bitrate_bps = bitrate_observer_.latest_bitrate();
bitrate_observer_.Reset();
}
}
EXPECT_TRUE(seen_overuse);
// Continue generating an additional 15 frames (equivalent to 167 ms) and
// verify that we don't back down further.
for (int frames = 0; frames < 15 && seen_overuse; ++frames) {
bool overuse = GenerateAndProcessFrame(kDummySsrc, bitrate_bps);
EXPECT_FALSE(overuse);
if (bitrate_observer_.updated()) {
bitrate_bps = bitrate_observer_.latest_bitrate();
EXPECT_GE(bitrate_bps, kStartBitrate.bps() / 2 - 15000);
EXPECT_LE(bitrate_bps, kInitialCapacity.bps() + 15000);
bitrate_observer_.Reset();
}
}
}
} // namespace webrtc

View File

@ -144,11 +144,9 @@ int64_t StreamGenerator::GenerateFrame(std::vector<PacketResult>* packets,
}
} // namespace test
DelayBasedBweTest::DelayBasedBweTest() : DelayBasedBweTest("") {}
DelayBasedBweTest::DelayBasedBweTest(absl::string_view field_trial_string)
: field_trial(
std::make_unique<test::ScopedFieldTrials>(field_trial_string)),
DelayBasedBweTest::DelayBasedBweTest()
: field_trial(std::make_unique<test::ScopedFieldTrials>(
"WebRTC-Bwe-RobustThroughputEstimatorSettings/enabled:true/")),
clock_(100000000),
acknowledged_bitrate_estimator_(
AcknowledgedBitrateEstimatorInterface::Create(&field_trial_config_)),

View File

@ -118,7 +118,6 @@ class StreamGenerator {
class DelayBasedBweTest : public ::testing::Test {
public:
DelayBasedBweTest();
explicit DelayBasedBweTest(absl::string_view field_trial_string);
~DelayBasedBweTest() override;
protected:

View File

@ -20,6 +20,7 @@
#include "api/units/time_delta.h"
#include "api/units/timestamp.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
namespace webrtc {
@ -76,6 +77,16 @@ void RobustThroughputEstimator::IncomingPacketFeedbackVector(
i > 0 && window_[i].receive_time < window_[i - 1].receive_time; i--) {
std::swap(window_[i], window_[i - 1]);
}
constexpr TimeDelta kMaxReorderingTime = TimeDelta::Seconds(1);
const TimeDelta receive_delta =
(window_.back().receive_time - packet.receive_time);
if (receive_delta > kMaxReorderingTime) {
RTC_LOG(LS_WARNING)
<< "Severe packet re-ordering or timestamps offset changed: "
<< receive_delta;
window_.clear();
latest_discarded_send_time_ = Timestamp::MinusInfinity();
}
}
// Remove old packets.

View File

@ -381,6 +381,29 @@ TEST(RobustThroughputEstimatorTest, DeepReordering) {
0.05 * send_rate.bytes_per_sec<double>()); // Allow 5% error
}
}
TEST(RobustThroughputEstimatorTest, ResetsIfReceiveClockChangeBackwards) {
FeedbackGenerator feedback_generator;
RobustThroughputEstimator throughput_estimator(
CreateRobustThroughputEstimatorSettings(
"WebRTC-Bwe-RobustThroughputEstimatorSettings/"
"enabled:true/"));
DataRate send_rate(DataRate::BytesPerSec(100000));
DataRate recv_rate(DataRate::BytesPerSec(100000));
std::vector<PacketResult> packet_feedback =
feedback_generator.CreateFeedbackVector(20, DataSize::Bytes(1000),
send_rate, recv_rate);
throughput_estimator.IncomingPacketFeedbackVector(packet_feedback);
EXPECT_EQ(throughput_estimator.bitrate(), send_rate);
feedback_generator.AdvanceReceiveClock(TimeDelta::Seconds(-2));
send_rate = DataRate::BytesPerSec(200000);
recv_rate = DataRate::BytesPerSec(200000);
packet_feedback = feedback_generator.CreateFeedbackVector(
20, DataSize::Bytes(1000), send_rate, recv_rate);
throughput_estimator.IncomingPacketFeedbackVector(packet_feedback);
EXPECT_EQ(throughput_estimator.bitrate(), send_rate);
}
TEST(RobustThroughputEstimatorTest, StreamPausedAndResumed) {
FeedbackGenerator feedback_generator;

View File

@ -79,22 +79,11 @@ AimdRateControl::AimdRateControl(const FieldTrialsView& key_value_config,
rtt_(kDefaultRtt),
send_side_(send_side),
no_bitrate_increase_in_alr_(
key_value_config.IsEnabled("WebRTC-DontIncreaseDelayBasedBweInAlr")),
initial_backoff_interval_("initial_backoff_interval"),
link_capacity_fix_("link_capacity_fix") {
key_value_config.IsEnabled("WebRTC-DontIncreaseDelayBasedBweInAlr")) {
ParseFieldTrial(
{&disable_estimate_bounded_increase_,
&use_current_estimate_as_min_upper_bound_},
key_value_config.Lookup("WebRTC-Bwe-EstimateBoundedIncrease"));
// E.g
// WebRTC-BweAimdRateControlConfig/initial_backoff_interval:100ms/
ParseFieldTrial({&initial_backoff_interval_, &link_capacity_fix_},
key_value_config.Lookup("WebRTC-BweAimdRateControlConfig"));
if (initial_backoff_interval_) {
RTC_LOG(LS_INFO) << "Using aimd rate control with initial back-off interval"
" "
<< ToString(*initial_backoff_interval_) << ".";
}
RTC_LOG(LS_INFO) << "Using aimd rate control with back off factor " << beta_;
}
@ -143,19 +132,10 @@ bool AimdRateControl::TimeToReduceFurther(Timestamp at_time,
}
bool AimdRateControl::InitialTimeToReduceFurther(Timestamp at_time) const {
if (!initial_backoff_interval_) {
return ValidEstimate() &&
TimeToReduceFurther(at_time,
LatestEstimate() / 2 - DataRate::BitsPerSec(1));
}
// TODO(terelius): We could use the RTT (clamped to suitable limits) instead
// of a fixed bitrate_reduction_interval.
if (time_last_bitrate_decrease_.IsInfinite() ||
at_time - time_last_bitrate_decrease_ >= *initial_backoff_interval_) {
return true;
}
return false;
}
DataRate AimdRateControl::LatestEstimate() const {
return current_bitrate_;
@ -307,7 +287,7 @@ void AimdRateControl::ChangeBitrate(const RateControlInput& input,
// Set bit rate to something slightly lower than the measured throughput
// to get rid of any self-induced delay.
decreased_bitrate = estimated_throughput * beta_;
if (decreased_bitrate > current_bitrate_ && !link_capacity_fix_) {
if (decreased_bitrate > current_bitrate_) {
// TODO(terelius): The link_capacity estimate may be based on old
// throughput measurements. Relying on them may lead to unnecessary
// BWE drops.

View File

@ -108,8 +108,6 @@ class AimdRateControl {
FieldTrialParameter<bool> use_current_estimate_as_min_upper_bound_{"c_upper",
false};
absl::optional<DataRate> last_decrease_;
FieldTrialOptional<TimeDelta> initial_backoff_interval_;
FieldTrialFlag link_capacity_fix_;
};
} // namespace webrtc