diff --git a/modules/congestion_controller/goog_cc/BUILD.gn b/modules/congestion_controller/goog_cc/BUILD.gn index 554a83c966..bab967b399 100644 --- a/modules/congestion_controller/goog_cc/BUILD.gn +++ b/modules/congestion_controller/goog_cc/BUILD.gn @@ -19,8 +19,6 @@ config("bwe_test_logging") { rtc_static_library("goog_cc") { configs += [ ":bwe_test_logging" ] sources = [ - "alr_detector.cc", - "alr_detector.h", "goog_cc_factory.cc", "goog_cc_network_control.cc", "goog_cc_network_control.h", @@ -35,6 +33,7 @@ rtc_static_library("goog_cc") { } deps = [ + ":alr_detector", ":delay_based_bwe", ":estimators", "../..:module_api", @@ -50,13 +49,34 @@ rtc_static_library("goog_cc") { "../../../system_wrappers:field_trial_api", "../../../system_wrappers:metrics_api", "../../bitrate_controller", - "../../pacing", "../../remote_bitrate_estimator", "../../rtp_rtcp:rtp_rtcp_format", "//third_party/abseil-cpp/absl/types:optional", ] } +rtc_source_set("alr_detector") { + sources = [ + "alr_detector.cc", + "alr_detector.h", + ] + if (!build_with_chromium && is_clang) { + # Suppress warnings from the Chromium Clang plugin (bugs.webrtc.org/163). + suppressed_configs += [ "//build/config/clang:find_bad_constructs" ] + } + deps = [ + "../../..:webrtc_common", + "../../../:typedefs", + "../../../logging:rtc_event_log_api", + "../../../logging:rtc_event_pacing", + "../../../rtc_base:checks", + "../../../rtc_base:rtc_base_approved", + "../../../rtc_base/experiments:alr_experiment", + "../../../system_wrappers:field_trial_api", + "../../pacing:interval_budget", + "//third_party/abseil-cpp/absl/types:optional", + ] +} rtc_source_set("estimators") { configs += [ ":bwe_test_logging" ] sources = [ @@ -138,6 +158,7 @@ if (rtc_include_tests) { "trendline_estimator_unittest.cc", ] deps = [ + ":alr_detector", ":delay_based_bwe", ":estimators", ":goog_cc", diff --git a/modules/congestion_controller/goog_cc/alr_detector.cc b/modules/congestion_controller/goog_cc/alr_detector.cc index 941809d2a4..0a2cc74569 100644 --- a/modules/congestion_controller/goog_cc/alr_detector.cc +++ b/modules/congestion_controller/goog_cc/alr_detector.cc @@ -20,12 +20,12 @@ #include "rtc_base/experiments/alr_experiment.h" #include "rtc_base/format_macros.h" #include "rtc_base/logging.h" +#include "rtc_base/numerics/safe_conversions.h" #include "rtc_base/ptr_util.h" #include "rtc_base/timeutils.h" #include "system_wrappers/include/field_trial.h" namespace webrtc { -namespace webrtc_cc { AlrDetector::AlrDetector() : AlrDetector(nullptr) {} AlrDetector::AlrDetector(RtcEventLog* event_log) @@ -93,5 +93,4 @@ absl::optional AlrDetector::GetApplicationLimitedRegionStartTime() const { return alr_started_time_ms_; } -} // namespace webrtc_cc } // namespace webrtc diff --git a/modules/congestion_controller/goog_cc/alr_detector.h b/modules/congestion_controller/goog_cc/alr_detector.h index 36fb952356..2a99ae8039 100644 --- a/modules/congestion_controller/goog_cc/alr_detector.h +++ b/modules/congestion_controller/goog_cc/alr_detector.h @@ -14,7 +14,6 @@ #include "absl/types/optional.h" #include "common_types.h" // NOLINT(build/include) #include "modules/pacing/interval_budget.h" -#include "modules/pacing/paced_sender.h" #include "rtc_base/rate_statistics.h" #include "typedefs.h" // NOLINT(build/include) @@ -22,8 +21,6 @@ namespace webrtc { class RtcEventLog; -namespace webrtc_cc { - // Application limited region detector is a class that utilizes signals of // elapsed time and bytes sent to estimate whether network traffic is // currently limited by the application's ability to generate traffic. @@ -70,8 +67,6 @@ class AlrDetector { RtcEventLog* event_log_; }; - -} // namespace webrtc_cc } // namespace webrtc #endif // MODULES_CONGESTION_CONTROLLER_GOOG_CC_ALR_DETECTOR_H_ diff --git a/modules/congestion_controller/send_side_congestion_controller.cc b/modules/congestion_controller/send_side_congestion_controller.cc index 3065c8b25b..7b61d0eaf6 100644 --- a/modules/congestion_controller/send_side_congestion_controller.cc +++ b/modules/congestion_controller/send_side_congestion_controller.cc @@ -19,7 +19,6 @@ #include "modules/bitrate_controller/include/bitrate_controller.h" #include "modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.h" #include "modules/congestion_controller/probe_controller.h" -#include "modules/pacing/alr_detector.h" #include "modules/remote_bitrate_estimator/include/bwe_defines.h" #include "rtc_base/checks.h" #include "rtc_base/format_macros.h" diff --git a/modules/pacing/BUILD.gn b/modules/pacing/BUILD.gn index 2b41eeeade..3de1d4a32d 100644 --- a/modules/pacing/BUILD.gn +++ b/modules/pacing/BUILD.gn @@ -10,12 +10,8 @@ import("../../webrtc.gni") rtc_static_library("pacing") { sources = [ - "alr_detector.cc", - "alr_detector.h", "bitrate_prober.cc", "bitrate_prober.h", - "interval_budget.cc", - "interval_budget.h", "paced_sender.cc", "paced_sender.h", "pacer.h", @@ -35,6 +31,7 @@ rtc_static_library("pacing") { } deps = [ + ":interval_budget", "..:module_api", "../../:typedefs", "../../:webrtc_common", @@ -47,6 +44,7 @@ rtc_static_library("pacing") { "../../system_wrappers", "../../system_wrappers:field_trial_api", "../../system_wrappers:runtime_enabled_features_api", + "../congestion_controller/goog_cc:alr_detector", "../remote_bitrate_estimator", "../rtp_rtcp", "../rtp_rtcp:rtp_rtcp_format", @@ -55,18 +53,37 @@ rtc_static_library("pacing") { ] } +rtc_source_set("interval_budget") { + sources = [ + "interval_budget.cc", + "interval_budget.h", + ] + + if (!build_with_chromium && is_clang) { + # Suppress warnings from the Chromium Clang plugin (bugs.webrtc.org/163). + suppressed_configs += [ "//build/config/clang:find_bad_constructs" ] + } + + deps = [ + # "../../:typedefs", + "../../:webrtc_common", + "../../rtc_base:checks", + "../../rtc_base:rtc_base_approved", + ] +} + if (rtc_include_tests) { rtc_source_set("pacing_unittests") { testonly = true sources = [ - "alr_detector_unittest.cc", "bitrate_prober_unittest.cc", "interval_budget_unittest.cc", "paced_sender_unittest.cc", "packet_router_unittest.cc", ] deps = [ + ":interval_budget", ":pacing", "../../rtc_base:checks", "../../rtc_base:rtc_base_approved", diff --git a/modules/pacing/alr_detector.cc b/modules/pacing/alr_detector.cc deleted file mode 100644 index 1d16a71e67..0000000000 --- a/modules/pacing/alr_detector.cc +++ /dev/null @@ -1,86 +0,0 @@ -/* - * Copyright (c) 2016 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "modules/pacing/alr_detector.h" - -#include -#include -#include - -#include "logging/rtc_event_log/events/rtc_event_alr_state.h" -#include "logging/rtc_event_log/rtc_event_log.h" -#include "rtc_base/checks.h" -#include "rtc_base/experiments/alr_experiment.h" -#include "rtc_base/format_macros.h" -#include "rtc_base/logging.h" -#include "rtc_base/ptr_util.h" -#include "rtc_base/timeutils.h" -#include "system_wrappers/include/field_trial.h" - -namespace webrtc { -AlrDetector::AlrDetector() : AlrDetector(nullptr) {} - -AlrDetector::AlrDetector(RtcEventLog* event_log) - : bandwidth_usage_percent_(kDefaultAlrBandwidthUsagePercent), - alr_start_budget_level_percent_(kDefaultAlrStartBudgetLevelPercent), - alr_stop_budget_level_percent_(kDefaultAlrStopBudgetLevelPercent), - alr_budget_(0, true), - event_log_(event_log) { - RTC_CHECK(AlrExperimentSettings::MaxOneFieldTrialEnabled()); - absl::optional experiment_settings = - AlrExperimentSettings::CreateFromFieldTrial( - AlrExperimentSettings::kScreenshareProbingBweExperimentName); - if (!experiment_settings) { - experiment_settings = AlrExperimentSettings::CreateFromFieldTrial( - AlrExperimentSettings::kStrictPacingAndProbingExperimentName); - } - if (experiment_settings) { - alr_stop_budget_level_percent_ = - experiment_settings->alr_stop_budget_level_percent; - alr_start_budget_level_percent_ = - experiment_settings->alr_start_budget_level_percent; - bandwidth_usage_percent_ = experiment_settings->alr_bandwidth_usage_percent; - } -} - -AlrDetector::~AlrDetector() {} - -void AlrDetector::OnBytesSent(size_t bytes_sent, int64_t delta_time_ms) { - alr_budget_.UseBudget(bytes_sent); - alr_budget_.IncreaseBudget(delta_time_ms); - bool state_changed = false; - if (alr_budget_.budget_level_percent() > alr_start_budget_level_percent_ && - !alr_started_time_ms_) { - alr_started_time_ms_.emplace(rtc::TimeMillis()); - state_changed = true; - } else if (alr_budget_.budget_level_percent() < - alr_stop_budget_level_percent_ && - alr_started_time_ms_) { - state_changed = true; - alr_started_time_ms_.reset(); - } - if (event_log_ && state_changed) { - event_log_->Log( - rtc::MakeUnique(alr_started_time_ms_.has_value())); - } -} - -void AlrDetector::SetEstimatedBitrate(int bitrate_bps) { - RTC_DCHECK(bitrate_bps); - const auto target_rate_kbps = static_cast(bitrate_bps) * - bandwidth_usage_percent_ / (1000 * 100); - alr_budget_.set_target_rate_kbps(rtc::dchecked_cast(target_rate_kbps)); -} - -absl::optional AlrDetector::GetApplicationLimitedRegionStartTime() - const { - return alr_started_time_ms_; -} -} // namespace webrtc diff --git a/modules/pacing/alr_detector.h b/modules/pacing/alr_detector.h deleted file mode 100644 index cffd6c0c1a..0000000000 --- a/modules/pacing/alr_detector.h +++ /dev/null @@ -1,72 +0,0 @@ -/* - * Copyright (c) 2016 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef MODULES_PACING_ALR_DETECTOR_H_ -#define MODULES_PACING_ALR_DETECTOR_H_ - -#include "absl/types/optional.h" -#include "common_types.h" // NOLINT(build/include) -#include "modules/pacing/interval_budget.h" -#include "modules/pacing/paced_sender.h" -#include "rtc_base/rate_statistics.h" -#include "typedefs.h" // NOLINT(build/include) - -namespace webrtc { - -class RtcEventLog; - -// Application limited region detector is a class that utilizes signals of -// elapsed time and bytes sent to estimate whether network traffic is -// currently limited by the application's ability to generate traffic. -// -// AlrDetector provides a signal that can be utilized to adjust -// estimate bandwidth. -// Note: This class is not thread-safe. -class AlrDetector { - public: - AlrDetector(); - explicit AlrDetector(RtcEventLog* event_log); - ~AlrDetector(); - - void OnBytesSent(size_t bytes_sent, int64_t delta_time_ms); - - // Set current estimated bandwidth. - void SetEstimatedBitrate(int bitrate_bps); - - // Returns time in milliseconds when the current application-limited region - // started or empty result if the sender is currently not application-limited. - absl::optional GetApplicationLimitedRegionStartTime() const; - - // Sent traffic percentage as a function of network capacity used to determine - // application-limited region. ALR region start when bandwidth usage drops - // below kAlrStartUsagePercent and ends when it raises above - // kAlrEndUsagePercent. NOTE: This is intentionally conservative at the moment - // until BW adjustments of application limited region is fine tuned. - static constexpr int kDefaultAlrBandwidthUsagePercent = 65; - static constexpr int kDefaultAlrStartBudgetLevelPercent = 80; - static constexpr int kDefaultAlrStopBudgetLevelPercent = 50; - - void UpdateBudgetWithElapsedTime(int64_t delta_time_ms); - void UpdateBudgetWithBytesSent(size_t bytes_sent); - - private: - int bandwidth_usage_percent_; - int alr_start_budget_level_percent_; - int alr_stop_budget_level_percent_; - - IntervalBudget alr_budget_; - absl::optional alr_started_time_ms_; - - RtcEventLog* event_log_; -}; - -} // namespace webrtc - -#endif // MODULES_PACING_ALR_DETECTOR_H_ diff --git a/modules/pacing/alr_detector_unittest.cc b/modules/pacing/alr_detector_unittest.cc deleted file mode 100644 index 13eb65f561..0000000000 --- a/modules/pacing/alr_detector_unittest.cc +++ /dev/null @@ -1,170 +0,0 @@ -/* - * Copyright (c) 2016 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "modules/pacing/alr_detector.h" - -#include "rtc_base/experiments/alr_experiment.h" -#include "test/field_trial.h" -#include "test/gtest.h" - -namespace { - -constexpr int kEstimatedBitrateBps = 300000; - -} // namespace - -namespace webrtc { - -namespace { -class SimulateOutgoingTrafficIn { - public: - explicit SimulateOutgoingTrafficIn(AlrDetector* alr_detector) - : alr_detector_(alr_detector) { - RTC_CHECK(alr_detector_); - } - - SimulateOutgoingTrafficIn& ForTimeMs(int time_ms) { - interval_ms_ = time_ms; - ProduceTraffic(); - return *this; - } - - SimulateOutgoingTrafficIn& AtPercentOfEstimatedBitrate(int usage_percentage) { - usage_percentage_.emplace(usage_percentage); - ProduceTraffic(); - return *this; - } - - private: - void ProduceTraffic() { - if (!interval_ms_ || !usage_percentage_) - return; - const int kTimeStepMs = 10; - for (int t = 0; t < *interval_ms_; t += kTimeStepMs) { - alr_detector_->OnBytesSent(kEstimatedBitrateBps * *usage_percentage_ * - kTimeStepMs / (8 * 100 * 1000), - kTimeStepMs); - } - int remainder_ms = *interval_ms_ % kTimeStepMs; - if (remainder_ms > 0) { - alr_detector_->OnBytesSent(kEstimatedBitrateBps * *usage_percentage_ * - remainder_ms / (8 * 100 * 1000), - kTimeStepMs); - } - } - AlrDetector* const alr_detector_; - absl::optional interval_ms_; - absl::optional usage_percentage_; -}; -} // namespace - -class LegacyAlrDetectorTest : public testing::Test { - public: - void SetUp() override { - alr_detector_.SetEstimatedBitrate(kEstimatedBitrateBps); - } - - protected: - AlrDetector alr_detector_; -}; - -TEST_F(LegacyAlrDetectorTest, AlrDetection) { - // Start in non-ALR state. - EXPECT_FALSE(alr_detector_.GetApplicationLimitedRegionStartTime()); - - // Stay in non-ALR state when usage is close to 100%. - SimulateOutgoingTrafficIn(&alr_detector_) - .ForTimeMs(1000) - .AtPercentOfEstimatedBitrate(90); - EXPECT_FALSE(alr_detector_.GetApplicationLimitedRegionStartTime()); - - // Verify that we ALR starts when bitrate drops below 20%. - SimulateOutgoingTrafficIn(&alr_detector_) - .ForTimeMs(1500) - .AtPercentOfEstimatedBitrate(20); - EXPECT_TRUE(alr_detector_.GetApplicationLimitedRegionStartTime()); - - // Verify that ALR ends when usage is above 65%. - SimulateOutgoingTrafficIn(&alr_detector_) - .ForTimeMs(4000) - .AtPercentOfEstimatedBitrate(100); - EXPECT_FALSE(alr_detector_.GetApplicationLimitedRegionStartTime()); -} - -TEST_F(LegacyAlrDetectorTest, ShortSpike) { - // Start in non-ALR state. - EXPECT_FALSE(alr_detector_.GetApplicationLimitedRegionStartTime()); - - // Verify that we ALR starts when bitrate drops below 20%. - SimulateOutgoingTrafficIn(&alr_detector_) - .ForTimeMs(1000) - .AtPercentOfEstimatedBitrate(20); - EXPECT_TRUE(alr_detector_.GetApplicationLimitedRegionStartTime()); - - // Verify that we stay in ALR region even after a short bitrate spike. - SimulateOutgoingTrafficIn(&alr_detector_) - .ForTimeMs(100) - .AtPercentOfEstimatedBitrate(150); - EXPECT_TRUE(alr_detector_.GetApplicationLimitedRegionStartTime()); - - // ALR ends when usage is above 65%. - SimulateOutgoingTrafficIn(&alr_detector_) - .ForTimeMs(3000) - .AtPercentOfEstimatedBitrate(100); - EXPECT_FALSE(alr_detector_.GetApplicationLimitedRegionStartTime()); -} - -TEST_F(LegacyAlrDetectorTest, BandwidthEstimateChanges) { - // Start in non-ALR state. - EXPECT_FALSE(alr_detector_.GetApplicationLimitedRegionStartTime()); - - // ALR starts when bitrate drops below 20%. - SimulateOutgoingTrafficIn(&alr_detector_) - .ForTimeMs(1000) - .AtPercentOfEstimatedBitrate(20); - EXPECT_TRUE(alr_detector_.GetApplicationLimitedRegionStartTime()); - - // When bandwidth estimate drops the detector should stay in ALR mode and quit - // it shortly afterwards as the sender continues sending the same amount of - // traffic. This is necessary to ensure that ProbeController can still react - // to the BWE drop by initiating a new probe. - alr_detector_.SetEstimatedBitrate(kEstimatedBitrateBps / 5); - EXPECT_TRUE(alr_detector_.GetApplicationLimitedRegionStartTime()); - SimulateOutgoingTrafficIn(&alr_detector_) - .ForTimeMs(1000) - .AtPercentOfEstimatedBitrate(50); - EXPECT_FALSE(alr_detector_.GetApplicationLimitedRegionStartTime()); -} - -TEST_F(LegacyAlrDetectorTest, ParseControlFieldTrial) { - webrtc::test::ScopedFieldTrials field_trial( - "WebRTC-ProbingScreenshareBwe/Control/"); - absl::optional parsed_params = - AlrExperimentSettings::CreateFromFieldTrial( - "WebRTC-ProbingScreenshareBwe"); - EXPECT_FALSE(static_cast(parsed_params)); -} - -TEST_F(LegacyAlrDetectorTest, ParseActiveFieldTrial) { - webrtc::test::ScopedFieldTrials field_trial( - "WebRTC-ProbingScreenshareBwe/1.1,2875,85,20,-20,1/"); - absl::optional parsed_params = - AlrExperimentSettings::CreateFromFieldTrial( - "WebRTC-ProbingScreenshareBwe"); - ASSERT_TRUE(static_cast(parsed_params)); - EXPECT_EQ(1.1f, parsed_params->pacing_factor); - EXPECT_EQ(2875, parsed_params->max_paced_queue_time); - EXPECT_EQ(85, parsed_params->alr_bandwidth_usage_percent); - EXPECT_EQ(20, parsed_params->alr_start_budget_level_percent); - EXPECT_EQ(-20, parsed_params->alr_stop_budget_level_percent); - EXPECT_EQ(1, parsed_params->group_id); -} - -} // namespace webrtc diff --git a/modules/pacing/paced_sender.cc b/modules/pacing/paced_sender.cc index 53dec05dc6..eda640e426 100644 --- a/modules/pacing/paced_sender.cc +++ b/modules/pacing/paced_sender.cc @@ -17,8 +17,8 @@ #include #include +#include "modules/congestion_controller/goog_cc/alr_detector.h" #include "modules/include/module_common_types.h" -#include "modules/pacing/alr_detector.h" #include "modules/pacing/bitrate_prober.h" #include "modules/pacing/interval_budget.h" #include "modules/pacing/round_robin_packet_queue.h" @@ -280,7 +280,7 @@ void PacedSender::Process() { if (packet_counter_ > 0) { PacedPacketInfo pacing_info; size_t bytes_sent = SendPadding(1, pacing_info); - alr_detector_->OnBytesSent(bytes_sent, elapsed_time_ms); + alr_detector_->OnBytesSent(bytes_sent, now_us / 1000); } } } @@ -355,7 +355,7 @@ void PacedSender::Process() { if (!probing_send_failure_) prober_->ProbeSent(clock_->TimeInMilliseconds(), bytes_sent); } - alr_detector_->OnBytesSent(bytes_sent, elapsed_time_ms); + alr_detector_->OnBytesSent(bytes_sent, now_us / 1000); } void PacedSender::ProcessThreadAttached(ProcessThread* process_thread) {