diff --git a/modules/congestion_controller/goog_cc/delay_based_bwe.cc b/modules/congestion_controller/goog_cc/delay_based_bwe.cc index 2b62891daa..0a84284572 100644 --- a/modules/congestion_controller/goog_cc/delay_based_bwe.cc +++ b/modules/congestion_controller/goog_cc/delay_based_bwe.cc @@ -42,6 +42,22 @@ constexpr uint32_t kFixedSsrc = 0; } // namespace +constexpr char BweIgnoreSmallPacketsSettings::kKey[]; + +BweIgnoreSmallPacketsSettings::BweIgnoreSmallPacketsSettings( + const WebRtcKeyValueConfig* key_value_config) { + Parser()->Parse( + key_value_config->Lookup(BweIgnoreSmallPacketsSettings::kKey)); +} + +std::unique_ptr +BweIgnoreSmallPacketsSettings::Parser() { + return StructParametersParser::Create("smoothing", &smoothing_factor, // + "fraction_large", &fraction_large, // + "large", &large_threshold, // + "small", &small_threshold); +} + DelayBasedBwe::Result::Result() : updated(false), probe(false), @@ -63,6 +79,8 @@ DelayBasedBwe::DelayBasedBwe(const WebRtcKeyValueConfig* key_value_config, NetworkStatePredictor* network_state_predictor) : event_log_(event_log), key_value_config_(key_value_config), + ignore_small_(key_value_config), + fraction_large_packets_(0.5), network_state_predictor_(network_state_predictor), inter_arrival_(), delay_detector_( @@ -75,7 +93,12 @@ DelayBasedBwe::DelayBasedBwe(const WebRtcKeyValueConfig* key_value_config, prev_state_(BandwidthUsage::kBwNormal), alr_limited_backoff_enabled_( key_value_config->Lookup("WebRTC-Bwe-AlrLimitedBackoff") - .find("Enabled") == 0) {} + .find("Enabled") == 0) { + RTC_LOG(LS_INFO) << "Initialized DelayBasedBwe with field trial " + << ignore_small_.Parser()->Encode() + << " and alr limited backoff " + << (alr_limited_backoff_enabled_ ? "enabled" : "disabled"); +} DelayBasedBwe::~DelayBasedBwe() {} @@ -151,18 +174,33 @@ void DelayBasedBwe::IncomingPacketFeedback(const PacketResult& packet_feedback, // so wrapping works properly. uint32_t timestamp = send_time_24bits << kAbsSendTimeInterArrivalUpshift; + // Ignore "small" packets if many/most packets in the call are "large". The + // packet size may have a significant effect on the propagation delay, + // especially at low bandwidths. Variations in packet size will then show up + // as noise in the delay measurement. By default, we include all packets. + DataSize packet_size = packet_feedback.sent_packet.size; + if (!ignore_small_.small_threshold.IsZero()) { + double is_large = + static_cast(packet_size >= ignore_small_.large_threshold); + fraction_large_packets_ += + ignore_small_.smoothing_factor * (is_large - fraction_large_packets_); + if (packet_size <= ignore_small_.small_threshold && + fraction_large_packets_ >= ignore_small_.fraction_large) { + return; + } + } + uint32_t ts_delta = 0; int64_t t_delta = 0; int size_delta = 0; bool calculated_deltas = inter_arrival_->ComputeDeltas( timestamp, packet_feedback.receive_time.ms(), at_time.ms(), - packet_feedback.sent_packet.size.bytes(), &ts_delta, &t_delta, - &size_delta); + packet_size.bytes(), &ts_delta, &t_delta, &size_delta); double ts_delta_ms = (1000.0 * ts_delta) / (1 << kInterArrivalShift); - delay_detector_->Update( - t_delta, ts_delta_ms, packet_feedback.sent_packet.send_time.ms(), - packet_feedback.receive_time.ms(), - packet_feedback.sent_packet.size.bytes(), calculated_deltas); + delay_detector_->Update(t_delta, ts_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 a2331b6223..03845949a4 100644 --- a/modules/congestion_controller/goog_cc/delay_based_bwe.h +++ b/modules/congestion_controller/goog_cc/delay_based_bwe.h @@ -27,11 +27,27 @@ #include "modules/remote_bitrate_estimator/include/bwe_defines.h" #include "modules/remote_bitrate_estimator/inter_arrival.h" #include "rtc_base/constructor_magic.h" +#include "rtc_base/experiments/struct_parameters_parser.h" #include "rtc_base/race_checker.h" namespace webrtc { class RtcEventLog; +struct BweIgnoreSmallPacketsSettings { + static constexpr char kKey[] = "WebRTC-BweIgnoreSmallPacketsFix"; + + BweIgnoreSmallPacketsSettings() = default; + explicit BweIgnoreSmallPacketsSettings( + const WebRtcKeyValueConfig* key_value_config); + + double smoothing_factor = 0.1; + double fraction_large = 1.0; + DataSize large_threshold = DataSize::Zero(); + DataSize small_threshold = DataSize::Zero(); + + std::unique_ptr Parser(); +}; + class DelayBasedBwe { public: struct Result { @@ -86,6 +102,12 @@ class DelayBasedBwe { rtc::RaceChecker network_race_; RtcEventLog* const event_log_; const WebRtcKeyValueConfig* const key_value_config_; + + // Filtering out small packets. Intention is to base the detection only + // on video packets even if we have TWCC sequence numbers for audio. + BweIgnoreSmallPacketsSettings ignore_small_; + double fraction_large_packets_; + NetworkStatePredictor* network_state_predictor_; std::unique_ptr inter_arrival_; std::unique_ptr delay_detector_; @@ -96,7 +118,6 @@ class DelayBasedBwe { bool has_once_detected_overuse_; BandwidthUsage prev_state_; bool alr_limited_backoff_enabled_; - RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(DelayBasedBwe); }; diff --git a/modules/congestion_controller/goog_cc/trendline_estimator.cc b/modules/congestion_controller/goog_cc/trendline_estimator.cc index 130acbfe85..d8d984ead9 100644 --- a/modules/congestion_controller/goog_cc/trendline_estimator.cc +++ b/modules/congestion_controller/goog_cc/trendline_estimator.cc @@ -25,23 +25,6 @@ namespace webrtc { -constexpr char BweIgnoreSmallPacketsSettings::kKey[]; - -BweIgnoreSmallPacketsSettings::BweIgnoreSmallPacketsSettings( - const WebRtcKeyValueConfig* key_value_config) { - Parser()->Parse( - key_value_config->Lookup(BweIgnoreSmallPacketsSettings::kKey)); -} - -std::unique_ptr -BweIgnoreSmallPacketsSettings::Parser() { - return StructParametersParser::Create( - "smoothing_factor", &smoothing_factor, // - "min_fraction_large_packets", &min_fraction_large_packets, // - "large_packet_size", &large_packet_size, // - "ignored_size", &ignored_size); -} - namespace { // Parameters for linear least squares fit of regression line to noisy data. @@ -102,9 +85,7 @@ constexpr int kDeltaCounterMax = 1000; TrendlineEstimator::TrendlineEstimator( const WebRtcKeyValueConfig* key_value_config, NetworkStatePredictor* network_state_predictor) - : ignore_small_packets_(key_value_config), - fraction_large_packets_(0.5), - window_size_(key_value_config->Lookup(kBweWindowSizeInPacketsExperiment) + : window_size_(key_value_config->Lookup(kBweWindowSizeInPacketsExperiment) .find("Enabled") == 0 ? ReadTrendlineFilterWindowSize(key_value_config) : kDefaultTrendlineWindowSize), @@ -129,8 +110,7 @@ TrendlineEstimator::TrendlineEstimator( network_state_predictor_(network_state_predictor) { RTC_LOG(LS_INFO) << "Using Trendline filter for delay change estimation with window size " - << window_size_ << ", field trial " - << ignore_small_packets_.Parser()->Encode() << " and " + << window_size_ << " and " << (network_state_predictor_ ? "injected" : "no") << " network state predictor"; } @@ -142,23 +122,6 @@ void TrendlineEstimator::UpdateTrendline(double recv_delta_ms, int64_t send_time_ms, int64_t arrival_time_ms, size_t packet_size) { - if (ignore_small_packets_.ignored_size > 0) { - // Process the packet if it is "large" or if all packets in the call are - // "small". The packet size may have a significant effect on the propagation - // delay, especially at low bandwidths. Variations in packet size will then - // show up as noise in the delay measurement. - // By default, we include all packets. - fraction_large_packets_ = - (1 - ignore_small_packets_.smoothing_factor) * fraction_large_packets_ + - ignore_small_packets_.smoothing_factor * - (packet_size >= ignore_small_packets_.large_packet_size); - if (packet_size <= ignore_small_packets_.ignored_size && - fraction_large_packets_ >= - ignore_small_packets_.min_fraction_large_packets) { - return; - } - } - const double delta_ms = recv_delta_ms - send_delta_ms; ++num_of_deltas_; num_of_deltas_ = std::min(num_of_deltas_, kDeltaCounterMax); diff --git a/modules/congestion_controller/goog_cc/trendline_estimator.h b/modules/congestion_controller/goog_cc/trendline_estimator.h index c48fcf0cfa..0f70943fe5 100644 --- a/modules/congestion_controller/goog_cc/trendline_estimator.h +++ b/modules/congestion_controller/goog_cc/trendline_estimator.h @@ -22,25 +22,9 @@ #include "modules/congestion_controller/goog_cc/delay_increase_detector_interface.h" #include "modules/remote_bitrate_estimator/include/bwe_defines.h" #include "rtc_base/constructor_magic.h" -#include "rtc_base/experiments/struct_parameters_parser.h" namespace webrtc { -struct BweIgnoreSmallPacketsSettings { - static constexpr char kKey[] = "WebRTC-BweIgnoreSmallPackets"; - - BweIgnoreSmallPacketsSettings() = default; - explicit BweIgnoreSmallPacketsSettings( - const WebRtcKeyValueConfig* key_value_config); - - double smoothing_factor = 0.1; - double min_fraction_large_packets = 1.0; - unsigned large_packet_size = 0; - unsigned ignored_size = 0; - - std::unique_ptr Parser(); -}; - class TrendlineEstimator : public DelayIncreaseDetectorInterface { public: TrendlineEstimator(const WebRtcKeyValueConfig* key_value_config, @@ -72,11 +56,6 @@ class TrendlineEstimator : public DelayIncreaseDetectorInterface { void UpdateThreshold(double modified_offset, int64_t now_ms); - // Filtering out small packets. (Intention is to base the detection only - // on video packets even if we have TWCC sequence number for audio.) - BweIgnoreSmallPacketsSettings ignore_small_packets_; - double fraction_large_packets_; - // Parameters. const size_t window_size_; const double smoothing_coef_;