From ad10222289272cd8728c0ebb7cd80301a5a70937 Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Wed, 25 Sep 2019 14:38:26 +0200 Subject: [PATCH] Cleanup of unused field trials and options in SendSideBandwidthEstimation Bug: webrtc:9883 Change-Id: Icbf4d6cb84da51f800343675f181e41b7cc45a6a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/154422 Reviewed-by: Niels Moller Commit-Queue: Sebastian Jansson Cr-Commit-Position: refs/heads/master@{#29306} --- .../goog_cc/send_side_bandwidth_estimation.cc | 96 +++++-------------- .../goog_cc/send_side_bandwidth_estimation.h | 14 +-- 2 files changed, 30 insertions(+), 80 deletions(-) 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 15480f14cc..e03b4a2a0d 100644 --- a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc +++ b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc @@ -37,8 +37,6 @@ constexpr TimeDelta kLowBitrateLogPeriod = TimeDelta::Millis<10000>(); constexpr TimeDelta kRtcEventLogPeriod = TimeDelta::Millis<5000>(); // Expecting that RTCP feedback is sent uniformly within [0.5, 1.5]s intervals. constexpr TimeDelta kMaxRtcpFeedbackInterval = TimeDelta::Millis<5000>(); -constexpr int kFeedbackTimeoutIntervals = 3; -constexpr TimeDelta kTimeoutInterval = TimeDelta::Millis<1000>(); constexpr float kDefaultLowLossThreshold = 0.02f; constexpr float kDefaultHighLossThreshold = 0.1f; @@ -112,11 +110,15 @@ LinkCapacityTracker::LinkCapacityTracker() LinkCapacityTracker::~LinkCapacityTracker() {} -void LinkCapacityTracker::OnOveruse(DataRate delay_based_bitrate, - Timestamp at_time) { - capacity_estimate_bps_ = - std::min(capacity_estimate_bps_, delay_based_bitrate.bps()); - last_link_capacity_update_ = at_time; +void LinkCapacityTracker::UpdateDelayBasedEstimate( + Timestamp at_time, + DataRate delay_based_bitrate) { + if (delay_based_bitrate < last_delay_based_estimate_) { + capacity_estimate_bps_ = + std::min(capacity_estimate_bps_, delay_based_bitrate.bps()); + last_link_capacity_update_ = at_time; + } + last_delay_based_estimate_ = delay_based_bitrate; } void LinkCapacityTracker::OnStartingRate(DataRate start_rate) { @@ -124,13 +126,17 @@ void LinkCapacityTracker::OnStartingRate(DataRate start_rate) { capacity_estimate_bps_ = start_rate.bps(); } -void LinkCapacityTracker::OnRateUpdate(DataRate acknowledged, +void LinkCapacityTracker::OnRateUpdate(absl::optional acknowledged, + DataRate target, Timestamp at_time) { - if (acknowledged.bps() > capacity_estimate_bps_) { + if (!acknowledged) + return; + DataRate acknowledged_target = std::min(*acknowledged, target); + if (acknowledged_target.bps() > capacity_estimate_bps_) { TimeDelta delta = at_time - last_link_capacity_update_; double alpha = delta.IsFinite() ? exp(-(delta / tracking_rate.Get())) : 0; capacity_estimate_bps_ = alpha * capacity_estimate_bps_ + - (1 - alpha) * acknowledged.bps(); + (1 - alpha) * acknowledged_target.bps(); } last_link_capacity_update_ = at_time; } @@ -150,8 +156,6 @@ RttBasedBackoff::RttBasedBackoff() : rtt_limit_("limit", TimeDelta::PlusInfinity()), drop_fraction_("fraction", 0.5), drop_interval_("interval", TimeDelta::ms(300)), - persist_on_route_change_("persist"), - safe_timeout_("safe_timeout", true), bandwidth_floor_("floor", DataRate::kbps(5)), // By initializing this to plus infinity, we make sure that we never // trigger rtt backoff unless packet feedback is enabled. @@ -159,18 +163,10 @@ RttBasedBackoff::RttBasedBackoff() last_propagation_rtt_(TimeDelta::Zero()), last_packet_sent_(Timestamp::MinusInfinity()) { ParseFieldTrial( - {&rtt_limit_, &drop_fraction_, &drop_interval_, &persist_on_route_change_, - &safe_timeout_, &bandwidth_floor_}, + {&rtt_limit_, &drop_fraction_, &drop_interval_, &bandwidth_floor_}, field_trial::FindFullName("WebRTC-Bwe-MaxRttLimit")); } -void RttBasedBackoff::OnRouteChange() { - if (!persist_on_route_change_) { - last_propagation_rtt_update_ = Timestamp::PlusInfinity(); - last_propagation_rtt_ = TimeDelta::Zero(); - } -} - void RttBasedBackoff::UpdatePropagationRtt(Timestamp at_time, TimeDelta propagation_rtt) { last_propagation_rtt_update_ = at_time; @@ -180,12 +176,10 @@ void RttBasedBackoff::UpdatePropagationRtt(Timestamp at_time, TimeDelta RttBasedBackoff::CorrectedRtt(Timestamp at_time) const { TimeDelta time_since_rtt = at_time - last_propagation_rtt_update_; TimeDelta timeout_correction = time_since_rtt; - if (safe_timeout_) { - // Avoid timeout when no packets are being sent. - TimeDelta time_since_packet_sent = at_time - last_packet_sent_; - timeout_correction = - std::max(time_since_rtt - time_since_packet_sent, TimeDelta::Zero()); - } + // Avoid timeout when no packets are being sent. + TimeDelta time_since_packet_sent = at_time - last_packet_sent_; + timeout_correction = + std::max(time_since_rtt - time_since_packet_sent, TimeDelta::Zero()); return timeout_correction + last_propagation_rtt_; } @@ -202,7 +196,6 @@ SendSideBandwidthEstimation::SendSideBandwidthEstimation(RtcEventLog* event_log) has_decreased_since_last_fraction_loss_(false), last_loss_feedback_(Timestamp::MinusInfinity()), last_loss_packet_report_(Timestamp::MinusInfinity()), - last_timeout_(Timestamp::MinusInfinity()), last_fraction_loss_(0), last_logged_fraction_loss_(0), last_round_trip_time_(TimeDelta::Zero()), @@ -217,8 +210,6 @@ SendSideBandwidthEstimation::SendSideBandwidthEstimation(RtcEventLog* event_log) rampup_uma_stats_updated_(kNumUmaRampupMetrics, false), event_log_(event_log), last_rtc_event_log_(Timestamp::MinusInfinity()), - in_timeout_experiment_( - webrtc::field_trial::IsEnabled("WebRTC-FeedbackTimeout")), low_loss_threshold_(kDefaultLowLossThreshold), high_loss_threshold_(kDefaultHighLossThreshold), bitrate_threshold_(kDefaultBitrateThreshold) { @@ -249,7 +240,6 @@ void SendSideBandwidthEstimation::OnRouteChange() { has_decreased_since_last_fraction_loss_ = false; last_loss_feedback_ = Timestamp::MinusInfinity(); last_loss_packet_report_ = Timestamp::MinusInfinity(); - last_timeout_ = Timestamp::MinusInfinity(); last_fraction_loss_ = 0; last_logged_fraction_loss_ = 0; last_round_trip_time_ = TimeDelta::Zero(); @@ -262,8 +252,6 @@ void SendSideBandwidthEstimation::OnRouteChange() { uma_update_state_ = kNoUpdate; uma_rtt_state_ = kNoUpdate; last_rtc_event_log_ = Timestamp::MinusInfinity(); - - rtt_backoff_.OnRouteChange(); } void SendSideBandwidthEstimation::SetBitrates( @@ -323,10 +311,7 @@ void SendSideBandwidthEstimation::UpdateReceiverEstimate(Timestamp at_time, void SendSideBandwidthEstimation::UpdateDelayBasedEstimate(Timestamp at_time, DataRate bitrate) { - if (bitrate < delay_based_bitrate_) { - link_capacity_.OnOveruse(bitrate, at_time); - } - + link_capacity_.UpdateDelayBasedEstimate(at_time, bitrate); delay_based_bitrate_ = bitrate; CapBitrateToThresholds(at_time, current_bitrate_); } @@ -474,7 +459,6 @@ void SendSideBandwidthEstimation::UpdateEstimate(Timestamp at_time) { } TimeDelta time_since_loss_packet_report = at_time - last_loss_packet_report_; - TimeDelta time_since_loss_feedback = at_time - last_loss_feedback_; if (time_since_loss_packet_report < 1.2 * kMaxRtcpFeedbackInterval) { // We only care about loss above a given bitrate threshold. float loss = last_fraction_loss_ / 256.0f; @@ -521,21 +505,6 @@ void SendSideBandwidthEstimation::UpdateEstimate(Timestamp at_time) { } } } - } else if (time_since_loss_feedback > - kFeedbackTimeoutIntervals * kMaxRtcpFeedbackInterval && - (last_timeout_.IsInfinite() || - at_time - last_timeout_ > kTimeoutInterval)) { - if (in_timeout_experiment_) { - RTC_LOG(LS_WARNING) << "Feedback timed out (" - << ToString(time_since_loss_feedback) - << "), reducing bitrate."; - new_bitrate = new_bitrate * 0.8; - // Reset accumulators since we've already acted on missing feedback and - // shouldn't to act again on these old lost packets. - lost_packets_since_last_loss_update_ = 0; - expected_packets_since_last_loss_update_ = 0; - last_timeout_ = at_time; - } } CapBitrateToThresholds(at_time, new_bitrate); @@ -583,25 +552,9 @@ DataRate SendSideBandwidthEstimation::MaybeRampupOrBackoff(DataRate new_bitrate, // inlining of very similar functionality. const TimeDelta time_since_loss_packet_report = at_time - last_loss_packet_report_; - const TimeDelta time_since_loss_feedback = at_time - last_loss_feedback_; if (time_since_loss_packet_report < 1.2 * kMaxRtcpFeedbackInterval) { new_bitrate = min_bitrate_history_.front().second * 1.08; new_bitrate += DataRate::bps(1000); - } else if (time_since_loss_feedback > - kFeedbackTimeoutIntervals * kMaxRtcpFeedbackInterval && - (last_timeout_.IsInfinite() || - at_time - last_timeout_ > kTimeoutInterval)) { - if (in_timeout_experiment_) { - RTC_LOG(LS_WARNING) << "Feedback timed out (" - << ToString(time_since_loss_feedback) - << "), reducing bitrate."; - new_bitrate = new_bitrate * 0.8; - // Reset accumulators since we've already acted on missing feedback and - // shouldn't to act again on these old lost packets. - lost_packets_since_last_loss_update_ = 0; - expected_packets_since_last_loss_update_ = 0; - last_timeout_ = at_time; - } } return new_bitrate; } @@ -645,9 +598,6 @@ void SendSideBandwidthEstimation::CapBitrateToThresholds(Timestamp at_time, } current_bitrate_ = bitrate; - if (acknowledged_rate_) { - link_capacity_.OnRateUpdate(std::min(current_bitrate_, *acknowledged_rate_), - at_time); - } + link_capacity_.OnRateUpdate(acknowledged_rate_, current_bitrate_, at_time); } } // namespace webrtc diff --git a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h index d2ab240fc8..8c2538fbee 100644 --- a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h +++ b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h @@ -35,9 +35,13 @@ class LinkCapacityTracker { public: LinkCapacityTracker(); ~LinkCapacityTracker(); - void OnOveruse(DataRate delay_based_bitrate, Timestamp at_time); + // Call when a new delay-based estimate is available. + void UpdateDelayBasedEstimate(Timestamp at_time, + DataRate delay_based_bitrate); void OnStartingRate(DataRate start_rate); - void OnRateUpdate(DataRate acknowledged, Timestamp at_time); + void OnRateUpdate(absl::optional acknowledged, + DataRate target, + Timestamp at_time); void OnRttBackoff(DataRate backoff_rate, Timestamp at_time); DataRate estimate() const; @@ -45,21 +49,19 @@ class LinkCapacityTracker { FieldTrialParameter tracking_rate; double capacity_estimate_bps_ = 0; Timestamp last_link_capacity_update_ = Timestamp::MinusInfinity(); + DataRate last_delay_based_estimate_ = DataRate::PlusInfinity(); }; class RttBasedBackoff { public: RttBasedBackoff(); ~RttBasedBackoff(); - void OnRouteChange(); void UpdatePropagationRtt(Timestamp at_time, TimeDelta propagation_rtt); TimeDelta CorrectedRtt(Timestamp at_time) const; FieldTrialParameter rtt_limit_; FieldTrialParameter drop_fraction_; FieldTrialParameter drop_interval_; - FieldTrialFlag persist_on_route_change_; - FieldTrialParameter safe_timeout_; FieldTrialParameter bandwidth_floor_; public: @@ -149,7 +151,6 @@ class SendSideBandwidthEstimation { bool has_decreased_since_last_fraction_loss_; Timestamp last_loss_feedback_; Timestamp last_loss_packet_report_; - Timestamp last_timeout_; uint8_t last_fraction_loss_; uint8_t last_logged_fraction_loss_; TimeDelta last_round_trip_time_; @@ -165,7 +166,6 @@ class SendSideBandwidthEstimation { std::vector rampup_uma_stats_updated_; RtcEventLog* event_log_; Timestamp last_rtc_event_log_; - bool in_timeout_experiment_; float low_loss_threshold_; float high_loss_threshold_; DataRate bitrate_threshold_;