From 7bad75b3906ae78b67b2a8cec095d877deb58215 Mon Sep 17 00:00:00 2001 From: Mirko Bonadei Date: Wed, 10 Feb 2021 09:34:31 +0000 Subject: [PATCH] Revert "Replace field trials with WebRtcKeyValueConfig in LossBasedBandwidthEstimation" This reverts commit 51f8e09540b1816236ceb1eaa540a7adb019b393. Reason for revert: Breaks downstream project. Original change's description: > Replace field trials with WebRtcKeyValueConfig in LossBasedBandwidthEstimation > > > Bug: webrtc:10335 > Change-Id: I85d62b9b63e0b6ec5dd4b957738a67a9a11e3a1f > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/205627 > Reviewed-by: Christoffer Rodbro > Commit-Queue: Per Kjellander > Cr-Commit-Position: refs/heads/master@{#33210} TBR=perkj@webrtc.org,crodbro@webrtc.org Change-Id: I220a0e5316c54c435d04bc2bbd714b9d9b92be26 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:10335 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/206645 Reviewed-by: Mirko Bonadei Commit-Queue: Mirko Bonadei Cr-Commit-Position: refs/heads/master@{#33214} --- .../loss_based_bandwidth_estimation.cc | 20 +++++++------------ .../goog_cc/loss_based_bandwidth_estimation.h | 6 ++---- .../goog_cc/send_side_bandwidth_estimation.cc | 1 - 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/modules/congestion_controller/goog_cc/loss_based_bandwidth_estimation.cc b/modules/congestion_controller/goog_cc/loss_based_bandwidth_estimation.cc index 0d36fbe23d..1d2aab8521 100644 --- a/modules/congestion_controller/goog_cc/loss_based_bandwidth_estimation.cc +++ b/modules/congestion_controller/goog_cc/loss_based_bandwidth_estimation.cc @@ -14,9 +14,9 @@ #include #include -#include "absl/strings/match.h" #include "api/units/data_rate.h" #include "api/units/time_delta.h" +#include "system_wrappers/include/field_trial.h" namespace webrtc { namespace { @@ -71,16 +71,10 @@ double ExponentialUpdate(TimeDelta window, TimeDelta interval) { return 1.0f - exp(interval / window * -1.0); } -bool IsEnabled(const webrtc::WebRtcKeyValueConfig& key_value_config, - absl::string_view name) { - return absl::StartsWith(key_value_config.Lookup(name), "Enabled"); -} - } // namespace -LossBasedControlConfig::LossBasedControlConfig( - const WebRtcKeyValueConfig* key_value_config) - : enabled(IsEnabled(*key_value_config, kBweLossBasedControl)), +LossBasedControlConfig::LossBasedControlConfig() + : enabled(field_trial::IsEnabled(kBweLossBasedControl)), min_increase_factor("min_incr", 1.02), max_increase_factor("max_incr", 1.08), increase_low_rtt("incr_low_rtt", TimeDelta::Millis(200)), @@ -98,6 +92,7 @@ LossBasedControlConfig::LossBasedControlConfig( allow_resets("resets", false), decrease_interval("decr_intvl", TimeDelta::Millis(300)), loss_report_timeout("timeout", TimeDelta::Millis(6000)) { + std::string trial_string = field_trial::FindFullName(kBweLossBasedControl); ParseFieldTrial( {&min_increase_factor, &max_increase_factor, &increase_low_rtt, &increase_high_rtt, &decrease_factor, &loss_window, &loss_max_window, @@ -105,15 +100,14 @@ LossBasedControlConfig::LossBasedControlConfig( &loss_bandwidth_balance_increase, &loss_bandwidth_balance_decrease, &loss_bandwidth_balance_exponent, &allow_resets, &decrease_interval, &loss_report_timeout}, - key_value_config->Lookup(kBweLossBasedControl)); + trial_string); } LossBasedControlConfig::LossBasedControlConfig(const LossBasedControlConfig&) = default; LossBasedControlConfig::~LossBasedControlConfig() = default; -LossBasedBandwidthEstimation::LossBasedBandwidthEstimation( - const WebRtcKeyValueConfig* key_value_config) - : config_(key_value_config), +LossBasedBandwidthEstimation::LossBasedBandwidthEstimation() + : config_(LossBasedControlConfig()), average_loss_(0), average_loss_max_(0), loss_based_bitrate_(DataRate::Zero()), diff --git a/modules/congestion_controller/goog_cc/loss_based_bandwidth_estimation.h b/modules/congestion_controller/goog_cc/loss_based_bandwidth_estimation.h index 2032c3e516..b63363cadd 100644 --- a/modules/congestion_controller/goog_cc/loss_based_bandwidth_estimation.h +++ b/modules/congestion_controller/goog_cc/loss_based_bandwidth_estimation.h @@ -14,7 +14,6 @@ #include #include "api/transport/network_types.h" -#include "api/transport/webrtc_key_value_config.h" #include "api/units/data_rate.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" @@ -23,7 +22,7 @@ namespace webrtc { struct LossBasedControlConfig { - explicit LossBasedControlConfig(const WebRtcKeyValueConfig* key_value_config); + LossBasedControlConfig(); LossBasedControlConfig(const LossBasedControlConfig&); LossBasedControlConfig& operator=(const LossBasedControlConfig&) = default; ~LossBasedControlConfig(); @@ -47,8 +46,7 @@ struct LossBasedControlConfig { class LossBasedBandwidthEstimation { public: - explicit LossBasedBandwidthEstimation( - const WebRtcKeyValueConfig* key_value_config); + LossBasedBandwidthEstimation(); void Update(Timestamp at_time, DataRate min_bitrate, TimeDelta last_round_trip_time); diff --git a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc index f45946462c..8de2a91114 100644 --- a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc +++ b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc @@ -226,7 +226,6 @@ SendSideBandwidthEstimation::SendSideBandwidthEstimation( low_loss_threshold_(kDefaultLowLossThreshold), high_loss_threshold_(kDefaultHighLossThreshold), bitrate_threshold_(kDefaultBitrateThreshold), - loss_based_bandwidth_estimation_(key_value_config), receiver_limit_caps_only_("Enabled") { RTC_DCHECK(event_log); if (BweLossExperimentIsEnabled()) {