Clean up expreiment WebRTC-Bwe-NewInterArrivalDelta
The experiment has been per default enabled since
https://webrtc.googlesource.com/src/+/62b340545f80baaa449c2159a8e44052c74116c9
submitted 20210914.
Bug: webrtc:12269
Change-Id: I730b603f4d0e382758fd4a6df6ccef9d8b76ea82
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/246105
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35683}
diff --git a/modules/congestion_controller/goog_cc/delay_based_bwe.cc b/modules/congestion_controller/goog_cc/delay_based_bwe.cc
index 4c5bdb6..2ae5441 100644
--- a/modules/congestion_controller/goog_cc/delay_based_bwe.cc
+++ b/modules/congestion_controller/goog_cc/delay_based_bwe.cc
@@ -32,21 +32,8 @@
namespace webrtc {
namespace {
constexpr TimeDelta kStreamTimeOut = TimeDelta::Seconds(2);
-
-// Used with field trial "WebRTC-Bwe-NewInterArrivalDelta/Enabled/
constexpr TimeDelta kSendTimeGroupLength = TimeDelta::Millis(5);
-// Used unless field trial "WebRTC-Bwe-NewInterArrivalDelta/Enabled/"
-constexpr int kTimestampGroupLengthMs = 5;
-constexpr int kAbsSendTimeFraction = 18;
-constexpr int kAbsSendTimeInterArrivalUpshift = 8;
-constexpr int kInterArrivalShift =
- kAbsSendTimeFraction + kAbsSendTimeInterArrivalUpshift;
-constexpr int kTimestampGroupTicks =
- (kTimestampGroupLengthMs << kInterArrivalShift) / 1000;
-constexpr double kTimestampToMs =
- 1000.0 / static_cast<double>(1 << kInterArrivalShift);
-
// This ssrc is used to fulfill the current API but will be removed
// after the API has been changed.
constexpr uint32_t kFixedSsrc = 0;
@@ -95,9 +82,6 @@
prev_bitrate_(DataRate::Zero()),
has_once_detected_overuse_(false),
prev_state_(BandwidthUsage::kBwNormal),
- use_new_inter_arrival_delta_(!absl::StartsWith(
- key_value_config->Lookup("WebRTC-Bwe-NewInterArrivalDelta"),
- "Disabled")),
alr_limited_backoff_enabled_(absl::StartsWith(
key_value_config->Lookup("WebRTC-Bwe-AlrLimitedBackoff"),
"Enabled")) {
@@ -162,17 +146,11 @@
// Reset if the stream has timed out.
if (last_seen_packet_.IsInfinite() ||
at_time - last_seen_packet_ > kStreamTimeOut) {
- if (use_new_inter_arrival_delta_) {
- video_inter_arrival_delta_ =
- std::make_unique<InterArrivalDelta>(kSendTimeGroupLength);
- audio_inter_arrival_delta_ =
- std::make_unique<InterArrivalDelta>(kSendTimeGroupLength);
- } else {
- video_inter_arrival_ = std::make_unique<InterArrival>(
- kTimestampGroupTicks, kTimestampToMs, true);
- audio_inter_arrival_ = std::make_unique<InterArrival>(
- kTimestampGroupTicks, kTimestampToMs, true);
- }
+ video_inter_arrival_delta_ =
+ std::make_unique<InterArrivalDelta>(kSendTimeGroupLength);
+ audio_inter_arrival_delta_ =
+ std::make_unique<InterArrivalDelta>(kSendTimeGroupLength);
+
video_delay_detector_.reset(
new TrendlineEstimator(key_value_config_, network_state_predictor_));
audio_delay_detector_.reset(
@@ -203,7 +181,6 @@
}
DataSize packet_size = packet_feedback.sent_packet.size;
- if (use_new_inter_arrival_delta_) {
TimeDelta send_delta = TimeDelta::Zero();
TimeDelta recv_delta = TimeDelta::Zero();
int size_delta = 0;
@@ -221,39 +198,6 @@
packet_feedback.sent_packet.send_time.ms(),
packet_feedback.receive_time.ms(), packet_size.bytes(),
calculated_deltas);
- } else {
- InterArrival* inter_arrival_for_packet =
- (separate_audio_.enabled && packet_feedback.sent_packet.audio)
- ? video_inter_arrival_.get()
- : audio_inter_arrival_.get();
-
- uint32_t send_time_24bits =
- static_cast<uint32_t>(
- ((static_cast<uint64_t>(packet_feedback.sent_packet.send_time.ms())
- << kAbsSendTimeFraction) +
- 500) /
- 1000) &
- 0x00FFFFFF;
- // Shift up send time to use the full 32 bits that inter_arrival works with,
- // so wrapping works properly.
- uint32_t timestamp = send_time_24bits << kAbsSendTimeInterArrivalUpshift;
-
- uint32_t timestamp_delta = 0;
- int64_t recv_delta_ms = 0;
- int size_delta = 0;
-
- bool calculated_deltas = inter_arrival_for_packet->ComputeDeltas(
- timestamp, packet_feedback.receive_time.ms(), at_time.ms(),
- packet_size.bytes(), ×tamp_delta, &recv_delta_ms, &size_delta);
- double send_delta_ms =
- (1000.0 * timestamp_delta) / (1 << kInterArrivalShift);
-
- delay_detector_for_packet->Update(
- recv_delta_ms, send_delta_ms,
- packet_feedback.sent_packet.send_time.ms(),
- packet_feedback.receive_time.ms(), packet_size.bytes(),
- calculated_deltas);
- }
}
DataRate DelayBasedBwe::TriggerOveruse(Timestamp at_time,
diff --git a/modules/congestion_controller/goog_cc/delay_based_bwe.h b/modules/congestion_controller/goog_cc/delay_based_bwe.h
index 85ce6ea..7823f77 100644
--- a/modules/congestion_controller/goog_cc/delay_based_bwe.h
+++ b/modules/congestion_controller/goog_cc/delay_based_bwe.h
@@ -127,7 +127,6 @@
DataRate prev_bitrate_;
bool has_once_detected_overuse_;
BandwidthUsage prev_state_;
- const bool use_new_inter_arrival_delta_;
bool alr_limited_backoff_enabled_;
};
diff --git a/modules/congestion_controller/goog_cc/delay_based_bwe_unittest.cc b/modules/congestion_controller/goog_cc/delay_based_bwe_unittest.cc
index 0cc6068..71b7ee7 100644
--- a/modules/congestion_controller/goog_cc/delay_based_bwe_unittest.cc
+++ b/modules/congestion_controller/goog_cc/delay_based_bwe_unittest.cc
@@ -28,16 +28,7 @@
constexpr float kTargetUtilizationFraction = 0.95f;
} // namespace
-INSTANTIATE_TEST_SUITE_P(
- ,
- DelayBasedBweTest,
- ::testing::Values("", "WebRTC-Bwe-NewInterArrivalDelta/Disabled/"),
- [](::testing::TestParamInfo<std::string> info) {
- return info.param.empty() ? "SafetypedInterArrival"
- : "LegacyInterArrival";
- });
-
-TEST_P(DelayBasedBweTest, ProbeDetection) {
+TEST_F(DelayBasedBweTest, ProbeDetection) {
int64_t now_ms = clock_.TimeInMilliseconds();
// First burst sent at 8 * 1000 / 10 = 800 kbps.
@@ -59,7 +50,7 @@
EXPECT_GT(bitrate_observer_.latest_bitrate(), 1500000u);
}
-TEST_P(DelayBasedBweTest, ProbeDetectionNonPacedPackets) {
+TEST_F(DelayBasedBweTest, ProbeDetectionNonPacedPackets) {
int64_t now_ms = clock_.TimeInMilliseconds();
// First burst sent at 8 * 1000 / 10 = 800 kbps, but with every other packet
// not being paced which could mess things up.
@@ -76,7 +67,7 @@
EXPECT_GT(bitrate_observer_.latest_bitrate(), 800000u);
}
-TEST_P(DelayBasedBweTest, ProbeDetectionFasterArrival) {
+TEST_F(DelayBasedBweTest, ProbeDetectionFasterArrival) {
int64_t now_ms = clock_.TimeInMilliseconds();
// First burst sent at 8 * 1000 / 10 = 800 kbps.
// Arriving at 8 * 1000 / 5 = 1600 kbps.
@@ -91,7 +82,7 @@
EXPECT_FALSE(bitrate_observer_.updated());
}
-TEST_P(DelayBasedBweTest, ProbeDetectionSlowerArrival) {
+TEST_F(DelayBasedBweTest, ProbeDetectionSlowerArrival) {
int64_t now_ms = clock_.TimeInMilliseconds();
// First burst sent at 8 * 1000 / 5 = 1600 kbps.
// Arriving at 8 * 1000 / 7 = 1142 kbps.
@@ -110,7 +101,7 @@
kTargetUtilizationFraction * 1140000u, 10000u);
}
-TEST_P(DelayBasedBweTest, ProbeDetectionSlowerArrivalHighBitrate) {
+TEST_F(DelayBasedBweTest, ProbeDetectionSlowerArrivalHighBitrate) {
int64_t now_ms = clock_.TimeInMilliseconds();
// Burst sent at 8 * 1000 / 1 = 8000 kbps.
// Arriving at 8 * 1000 / 2 = 4000 kbps.
@@ -129,7 +120,7 @@
kTargetUtilizationFraction * 4000000u, 10000u);
}
-TEST_P(DelayBasedBweTest, GetExpectedBwePeriodMs) {
+TEST_F(DelayBasedBweTest, GetExpectedBwePeriodMs) {
auto default_interval = bitrate_estimator_->GetExpectedBwePeriod();
EXPECT_GT(default_interval.ms(), 0);
CapacityDropTestHelper(1, true, 333, 0);
@@ -138,45 +129,45 @@
EXPECT_NE(interval.ms(), default_interval.ms());
}
-TEST_P(DelayBasedBweTest, InitialBehavior) {
+TEST_F(DelayBasedBweTest, InitialBehavior) {
InitialBehaviorTestHelper(730000);
}
-TEST_P(DelayBasedBweTest, RateIncreaseReordering) {
+TEST_F(DelayBasedBweTest, RateIncreaseReordering) {
RateIncreaseReorderingTestHelper(730000);
}
-TEST_P(DelayBasedBweTest, RateIncreaseRtpTimestamps) {
+TEST_F(DelayBasedBweTest, RateIncreaseRtpTimestamps) {
RateIncreaseRtpTimestampsTestHelper(622);
}
-TEST_P(DelayBasedBweTest, CapacityDropOneStream) {
+TEST_F(DelayBasedBweTest, CapacityDropOneStream) {
CapacityDropTestHelper(1, false, 300, 0);
}
-TEST_P(DelayBasedBweTest, CapacityDropPosOffsetChange) {
+TEST_F(DelayBasedBweTest, CapacityDropPosOffsetChange) {
CapacityDropTestHelper(1, false, 867, 30000);
}
-TEST_P(DelayBasedBweTest, CapacityDropNegOffsetChange) {
+TEST_F(DelayBasedBweTest, CapacityDropNegOffsetChange) {
CapacityDropTestHelper(1, false, 933, -30000);
}
-TEST_P(DelayBasedBweTest, CapacityDropOneStreamWrap) {
+TEST_F(DelayBasedBweTest, CapacityDropOneStreamWrap) {
CapacityDropTestHelper(1, true, 333, 0);
}
-TEST_P(DelayBasedBweTest, TestTimestampGrouping) {
+TEST_F(DelayBasedBweTest, TestTimestampGrouping) {
TestTimestampGroupingTestHelper();
}
-TEST_P(DelayBasedBweTest, TestShortTimeoutAndWrap) {
+TEST_F(DelayBasedBweTest, TestShortTimeoutAndWrap) {
// Simulate a client leaving and rejoining the call after 35 seconds. This
// will make abs send time wrap, so if streams aren't timed out properly
// the next 30 seconds of packets will be out of order.
TestWrappingHelper(35);
}
-TEST_P(DelayBasedBweTest, TestLongTimeoutAndWrap) {
+TEST_F(DelayBasedBweTest, TestLongTimeoutAndWrap) {
// Simulate a client leaving and rejoining the call after some multiple of
// 64 seconds later. This will cause a zero difference in abs send times due
// to the wrap, but a big difference in arrival time, if streams aren't
@@ -184,7 +175,7 @@
TestWrappingHelper(10 * 64);
}
-TEST_P(DelayBasedBweTest, TestInitialOveruse) {
+TEST_F(DelayBasedBweTest, TestInitialOveruse) {
const DataRate kStartBitrate = DataRate::KilobitsPerSec(300);
const DataRate kInitialCapacity = DataRate::KilobitsPerSec(200);
const uint32_t kDummySsrc = 0;
@@ -224,16 +215,15 @@
}
class DelayBasedBweTestWithBackoffTimeoutExperiment : public DelayBasedBweTest {
+ public:
+ DelayBasedBweTestWithBackoffTimeoutExperiment()
+ : DelayBasedBweTest(
+ "WebRTC-BweAimdRateControlConfig/initial_backoff_interval:200ms/") {
+ }
};
-INSTANTIATE_TEST_SUITE_P(
- ,
- DelayBasedBweTestWithBackoffTimeoutExperiment,
- ::testing::Values(
- "WebRTC-BweAimdRateControlConfig/initial_backoff_interval:200ms/"));
-
// This test subsumes and improves DelayBasedBweTest.TestInitialOveruse above.
-TEST_P(DelayBasedBweTestWithBackoffTimeoutExperiment, TestInitialOveruse) {
+TEST_F(DelayBasedBweTestWithBackoffTimeoutExperiment, TestInitialOveruse) {
const DataRate kStartBitrate = DataRate::KilobitsPerSec(300);
const DataRate kInitialCapacity = DataRate::KilobitsPerSec(200);
const uint32_t kDummySsrc = 0;
diff --git a/modules/congestion_controller/goog_cc/delay_based_bwe_unittest_helper.cc b/modules/congestion_controller/goog_cc/delay_based_bwe_unittest_helper.cc
index 4c72277..3eb0ae3 100644
--- a/modules/congestion_controller/goog_cc/delay_based_bwe_unittest_helper.cc
+++ b/modules/congestion_controller/goog_cc/delay_based_bwe_unittest_helper.cc
@@ -145,8 +145,11 @@
}
} // namespace test
-DelayBasedBweTest::DelayBasedBweTest()
- : field_trial(std::make_unique<test::ScopedFieldTrials>(GetParam())),
+DelayBasedBweTest::DelayBasedBweTest() : DelayBasedBweTest("") {}
+
+DelayBasedBweTest::DelayBasedBweTest(const std::string& field_trial_string)
+ : field_trial(
+ std::make_unique<test::ScopedFieldTrials>(field_trial_string)),
clock_(100000000),
acknowledged_bitrate_estimator_(
AcknowledgedBitrateEstimatorInterface::Create(&field_trial_config_)),
diff --git a/modules/congestion_controller/goog_cc/delay_based_bwe_unittest_helper.h b/modules/congestion_controller/goog_cc/delay_based_bwe_unittest_helper.h
index 5fb048b..927cf9b 100644
--- a/modules/congestion_controller/goog_cc/delay_based_bwe_unittest_helper.h
+++ b/modules/congestion_controller/goog_cc/delay_based_bwe_unittest_helper.h
@@ -113,9 +113,10 @@
};
} // namespace test
-class DelayBasedBweTest : public ::testing::TestWithParam<std::string> {
+class DelayBasedBweTest : public ::testing::Test {
public:
DelayBasedBweTest();
+ explicit DelayBasedBweTest(const std::string& field_trial_string);
~DelayBasedBweTest() override;
protected:
