From 0e5501f0fff8d5cfffa35b2a0198be61ef84b076 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Fri, 24 Mar 2023 12:42:51 +0100 Subject: [PATCH] Cleanup OveruseDetector from tracers of old experimentation experiment was cleaned up in https://webrtc-review.googlesource.com/c/src/+/284922 Bug: webrtc:4711 Change-Id: I19e1ba9716a5b0375fa4c5cf8c69e6bb2b03de6b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299041 Commit-Queue: Danil Chapovalov Reviewed-by: Per Kjellander Cr-Commit-Position: refs/heads/main@{#39671} --- .../overuse_detector.cc | 28 ++++------- .../overuse_detector.h | 22 ++++----- .../overuse_detector_unittest.cc | 49 ++++++++----------- .../remote_bitrate_estimator_abs_send_time.cc | 5 +- .../remote_bitrate_estimator_single_stream.cc | 9 ++-- 5 files changed, 44 insertions(+), 69 deletions(-) diff --git a/modules/remote_bitrate_estimator/overuse_detector.cc b/modules/remote_bitrate_estimator/overuse_detector.cc index bd2d756876..888f18cd9e 100644 --- a/modules/remote_bitrate_estimator/overuse_detector.cc +++ b/modules/remote_bitrate_estimator/overuse_detector.cc @@ -21,25 +21,17 @@ #include "rtc_base/numerics/safe_minmax.h" namespace webrtc { +namespace { -const double kMaxAdaptOffsetMs = 15.0; -const double kOverUsingTimeThreshold = 10; -const int kMaxNumDeltas = 60; +constexpr double kMaxAdaptOffsetMs = 15.0; +constexpr double kOverUsingTimeThreshold = 10; +constexpr int kMaxNumDeltas = 60; +constexpr double kUp = 0.0087; +constexpr double kDown = 0.039; -OveruseDetector::OveruseDetector(const FieldTrialsView* key_value_config) - // Experiment is on by default, but can be disabled with finch by setting - // the field trial string to "WebRTC-AdaptiveBweThreshold/Disabled/". - : k_up_(0.0087), - k_down_(0.039), - overusing_time_threshold_(kOverUsingTimeThreshold), - threshold_(12.5), - last_update_ms_(-1), - prev_offset_(0.0), - time_over_using_(-1), - overuse_counter_(0), - hypothesis_(BandwidthUsage::kBwNormal) {} +} // namespace -OveruseDetector::~OveruseDetector() {} +OveruseDetector::OveruseDetector() = default; BandwidthUsage OveruseDetector::State() const { return hypothesis_; @@ -66,7 +58,7 @@ BandwidthUsage OveruseDetector::Detect(double offset, time_over_using_ += ts_delta; } overuse_counter_++; - if (time_over_using_ > overusing_time_threshold_ && overuse_counter_ > 1) { + if (time_over_using_ > kOverUsingTimeThreshold && overuse_counter_ > 1) { if (offset >= prev_offset_) { time_over_using_ = 0; overuse_counter_ = 0; @@ -100,7 +92,7 @@ void OveruseDetector::UpdateThreshold(double modified_offset, int64_t now_ms) { return; } - const double k = fabs(modified_offset) < threshold_ ? k_down_ : k_up_; + const double k = fabs(modified_offset) < threshold_ ? kDown : kUp; const int64_t kMaxTimeDeltaMs = 100; int64_t time_delta_ms = std::min(now_ms - last_update_ms_, kMaxTimeDeltaMs); threshold_ += k * (fabs(modified_offset) - threshold_) * time_delta_ms; diff --git a/modules/remote_bitrate_estimator/overuse_detector.h b/modules/remote_bitrate_estimator/overuse_detector.h index 07ae8734c4..444e8eece5 100644 --- a/modules/remote_bitrate_estimator/overuse_detector.h +++ b/modules/remote_bitrate_estimator/overuse_detector.h @@ -12,19 +12,19 @@ #include -#include "api/field_trials_view.h" #include "api/network_state_predictor.h" namespace webrtc { class OveruseDetector { public: - explicit OveruseDetector(const FieldTrialsView* key_value_config); - virtual ~OveruseDetector(); + OveruseDetector(); OveruseDetector(const OveruseDetector&) = delete; OveruseDetector& operator=(const OveruseDetector&) = delete; + ~OveruseDetector() = default; + // Update the detection state based on the estimated inter-arrival time delta // offset. `timestamp_delta` is the delta between the last timestamp which the // estimated offset is based on and the last timestamp on which the last @@ -41,17 +41,13 @@ class OveruseDetector { private: void UpdateThreshold(double modified_offset, int64_t now_ms); - void InitializeExperiment(const FieldTrialsView& key_value_config); - const double k_up_; - const double k_down_; - const double overusing_time_threshold_; - double threshold_; - int64_t last_update_ms_; - double prev_offset_; - double time_over_using_; - int overuse_counter_; - BandwidthUsage hypothesis_; + double threshold_ = 12.5; + int64_t last_update_ms_ = -1; + double prev_offset_ = 0.0; + double time_over_using_ = -1; + int overuse_counter_ = 0; + BandwidthUsage hypothesis_ = BandwidthUsage::kBwNormal; }; } // namespace webrtc diff --git a/modules/remote_bitrate_estimator/overuse_detector_unittest.cc b/modules/remote_bitrate_estimator/overuse_detector_unittest.cc index 82786521d7..c4c72b1de5 100644 --- a/modules/remote_bitrate_estimator/overuse_detector_unittest.cc +++ b/modules/remote_bitrate_estimator/overuse_detector_unittest.cc @@ -17,7 +17,6 @@ #include #include -#include "api/transport/field_trial_based_config.h" #include "modules/remote_bitrate_estimator/inter_arrival.h" #include "modules/remote_bitrate_estimator/overuse_estimator.h" #include "rtc_base/random.h" @@ -34,15 +33,10 @@ class OveruseDetectorTest : public ::testing::Test { : now_ms_(0), receive_time_ms_(0), rtp_timestamp_(10 * 90), - overuse_detector_(), inter_arrival_(5 * 90, kRtpTimestampToMs), random_(123456789) {} protected: - void SetUp() override { - overuse_detector_.reset(new OveruseDetector(&field_trials_)); - } - int Run100000Samples(int packets_per_frame, size_t packet_size, int mean_ms, @@ -59,7 +53,7 @@ class OveruseDetectorTest : public ::testing::Test { receive_time_ms_, now_ms_ + static_cast( random_.Gaussian(0, standard_deviation_ms) + 0.5)); - if (BandwidthUsage::kBwOverusing == overuse_detector_->State()) { + if (BandwidthUsage::kBwOverusing == overuse_detector_.State()) { if (last_overuse + 1 != i) { unique_overuse++; } @@ -85,7 +79,7 @@ class OveruseDetectorTest : public ::testing::Test { receive_time_ms_, now_ms_ + static_cast( random_.Gaussian(0, standard_deviation_ms) + 0.5)); - if (BandwidthUsage::kBwOverusing == overuse_detector_->State()) { + if (BandwidthUsage::kBwOverusing == overuse_detector_.State()) { return i + 1; } } @@ -103,18 +97,17 @@ class OveruseDetectorTest : public ::testing::Test { ×tamp_delta, &time_delta, &size_delta)) { double timestamp_delta_ms = timestamp_delta / 90.0; overuse_estimator_.Update(time_delta, timestamp_delta_ms, size_delta, - overuse_detector_->State(), receive_time_ms); - overuse_detector_->Detect(overuse_estimator_.offset(), timestamp_delta_ms, - overuse_estimator_.num_of_deltas(), - receive_time_ms); + overuse_detector_.State(), receive_time_ms); + overuse_detector_.Detect(overuse_estimator_.offset(), timestamp_delta_ms, + overuse_estimator_.num_of_deltas(), + receive_time_ms); } } - const FieldTrialBasedConfig field_trials_; int64_t now_ms_; int64_t receive_time_ms_; uint32_t rtp_timestamp_; - std::unique_ptr overuse_detector_; + OveruseDetector overuse_detector_; OveruseEstimator overuse_estimator_; InterArrival inter_arrival_; Random random_; @@ -143,7 +136,7 @@ TEST_F(OveruseDetectorTest, SimpleNonOveruse30fps) { UpdateDetector(rtp_timestamp, now_ms_, packet_size); now_ms_ += frame_duration_ms; rtp_timestamp += frame_duration_ms * 90; - EXPECT_EQ(BandwidthUsage::kBwNormal, overuse_detector_->State()); + EXPECT_EQ(BandwidthUsage::kBwNormal, overuse_detector_.State()); } } @@ -161,7 +154,7 @@ TEST_F(OveruseDetectorTest, SimpleNonOveruseWithReceiveVariance) { } else { now_ms_ += frame_duration_ms + 5; } - EXPECT_EQ(BandwidthUsage::kBwNormal, overuse_detector_->State()); + EXPECT_EQ(BandwidthUsage::kBwNormal, overuse_detector_.State()); } } @@ -179,7 +172,7 @@ TEST_F(OveruseDetectorTest, SimpleNonOveruseWithRtpTimestampVariance) { } else { rtp_timestamp += (frame_duration_ms + 5) * 90; } - EXPECT_EQ(BandwidthUsage::kBwNormal, overuse_detector_->State()); + EXPECT_EQ(BandwidthUsage::kBwNormal, overuse_detector_.State()); } } @@ -237,7 +230,7 @@ TEST_F(OveruseDetectorTest, OveruseWithLowVariance2000Kbit30fps) { } else { now_ms_ += frame_duration_ms + offset; } - EXPECT_EQ(BandwidthUsage::kBwNormal, overuse_detector_->State()); + EXPECT_EQ(BandwidthUsage::kBwNormal, overuse_detector_.State()); } // Simulate a higher send pace, that is too high. // Total build up of 30 ms. @@ -250,10 +243,10 @@ TEST_F(OveruseDetectorTest, OveruseWithLowVariance2000Kbit30fps) { UpdateDetector(rtp_timestamp, now_ms_, packet_size); now_ms_ += frame_duration_ms + drift_per_frame_ms * 6; rtp_timestamp += frame_duration_ms * 90; - EXPECT_EQ(BandwidthUsage::kBwNormal, overuse_detector_->State()); + EXPECT_EQ(BandwidthUsage::kBwNormal, overuse_detector_.State()); } UpdateDetector(rtp_timestamp, now_ms_, packet_size); - EXPECT_EQ(BandwidthUsage::kBwOverusing, overuse_detector_->State()); + EXPECT_EQ(BandwidthUsage::kBwOverusing, overuse_detector_.State()); } TEST_F(OveruseDetectorTest, LowGaussianVariance30Kbit3fps) { @@ -567,7 +560,7 @@ TEST_F(OveruseDetectorTest, ThresholdAdapts) { bool overuse_detected = false; for (int i = 0; i < kBatchLength; ++i) { BandwidthUsage overuse_state = - overuse_detector_->Detect(kOffset, kTsDelta, num_deltas, now_ms); + overuse_detector_.Detect(kOffset, kTsDelta, num_deltas, now_ms); if (overuse_state == BandwidthUsage::kBwOverusing) { overuse_detected = true; } @@ -580,7 +573,7 @@ TEST_F(OveruseDetectorTest, ThresholdAdapts) { overuse_detected = false; for (int i = 0; i < kBatchLength; ++i) { BandwidthUsage overuse_state = - overuse_detector_->Detect(1.1 * kOffset, kTsDelta, num_deltas, now_ms); + overuse_detector_.Detect(1.1 * kOffset, kTsDelta, num_deltas, now_ms); if (overuse_state == BandwidthUsage::kBwOverusing) { overuse_detected = true; } @@ -593,7 +586,7 @@ TEST_F(OveruseDetectorTest, ThresholdAdapts) { overuse_detected = false; for (int i = 0; i < kBatchLength; ++i) { BandwidthUsage overuse_state = - overuse_detector_->Detect(kOffset, kTsDelta, num_deltas, now_ms); + overuse_detector_.Detect(kOffset, kTsDelta, num_deltas, now_ms); if (overuse_state == BandwidthUsage::kBwOverusing) { overuse_detected = true; } @@ -605,7 +598,7 @@ TEST_F(OveruseDetectorTest, ThresholdAdapts) { // Pass in a low offset to make the threshold adapt down. for (int i = 0; i < 15 * kBatchLength; ++i) { BandwidthUsage overuse_state = - overuse_detector_->Detect(0.7 * kOffset, kTsDelta, num_deltas, now_ms); + overuse_detector_.Detect(0.7 * kOffset, kTsDelta, num_deltas, now_ms); if (overuse_state == BandwidthUsage::kBwOverusing) { overuse_detected = true; } @@ -617,7 +610,7 @@ TEST_F(OveruseDetectorTest, ThresholdAdapts) { // Make sure the original offset now again triggers overuse. for (int i = 0; i < kBatchLength; ++i) { BandwidthUsage overuse_state = - overuse_detector_->Detect(kOffset, kTsDelta, num_deltas, now_ms); + overuse_detector_.Detect(kOffset, kTsDelta, num_deltas, now_ms); if (overuse_state == BandwidthUsage::kBwOverusing) { overuse_detected = true; } @@ -640,7 +633,7 @@ TEST_F(OveruseDetectorTest, DoesntAdaptToSpikes) { bool overuse_detected = false; for (int i = 0; i < kBatchLength; ++i) { BandwidthUsage overuse_state = - overuse_detector_->Detect(kOffset, kTsDelta, num_deltas, now_ms); + overuse_detector_.Detect(kOffset, kTsDelta, num_deltas, now_ms); if (overuse_state == BandwidthUsage::kBwOverusing) { overuse_detected = true; } @@ -654,7 +647,7 @@ TEST_F(OveruseDetectorTest, DoesntAdaptToSpikes) { overuse_detected = false; for (int i = 0; i < kShortBatchLength; ++i) { BandwidthUsage overuse_state = - overuse_detector_->Detect(kLargeOffset, kTsDelta, num_deltas, now_ms); + overuse_detector_.Detect(kLargeOffset, kTsDelta, num_deltas, now_ms); if (overuse_state == BandwidthUsage::kBwOverusing) { overuse_detected = true; } @@ -667,7 +660,7 @@ TEST_F(OveruseDetectorTest, DoesntAdaptToSpikes) { overuse_detected = false; for (int i = 0; i < kBatchLength; ++i) { BandwidthUsage overuse_state = - overuse_detector_->Detect(kOffset, kTsDelta, num_deltas, now_ms); + overuse_detector_.Detect(kOffset, kTsDelta, num_deltas, now_ms); if (overuse_state == BandwidthUsage::kBwOverusing) { overuse_detected = true; } diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc index a07348c766..021fd82972 100644 --- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc +++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc @@ -97,10 +97,7 @@ void RemoteBitrateEstimatorAbsSendTime::MaybeAddCluster( RemoteBitrateEstimatorAbsSendTime::RemoteBitrateEstimatorAbsSendTime( RemoteBitrateObserver* observer, Clock* clock) - : clock_(clock), - observer_(observer), - detector_(&field_trials_), - remote_rate_(&field_trials_) { + : clock_(clock), observer_(observer), remote_rate_(&field_trials_) { RTC_DCHECK(clock_); RTC_DCHECK(observer_); RTC_LOG(LS_INFO) << "RemoteBitrateEstimatorAbsSendTime: Instantiating."; diff --git a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc index bc06d4e4b5..ae3e7d8465 100644 --- a/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc +++ b/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc @@ -41,11 +41,9 @@ enum { kTimestampGroupLengthMs = 5 }; static const double kTimestampToMs = 1.0 / 90.0; struct RemoteBitrateEstimatorSingleStream::Detector { - explicit Detector(int64_t last_packet_time_ms, - const FieldTrialsView* key_value_config) + explicit Detector(int64_t last_packet_time_ms) : last_packet_time_ms(last_packet_time_ms), - inter_arrival(90 * kTimestampGroupLengthMs, kTimestampToMs), - detector(key_value_config) {} + inter_arrival(90 * kTimestampGroupLengthMs, kTimestampToMs) {} int64_t last_packet_time_ms; InterArrival inter_arrival; @@ -99,8 +97,7 @@ void RemoteBitrateEstimatorSingleStream::IncomingPacket( // automatically cleaned up when we have one RemoteBitrateEstimator per REMB // group. std::pair insert_result = - overuse_detectors_.insert( - std::make_pair(ssrc, new Detector(now_ms, &field_trials_))); + overuse_detectors_.insert(std::make_pair(ssrc, new Detector(now_ms))); it = insert_result.first; } Detector* estimator = it->second;