diff --git a/modules/congestion_controller/include/receive_side_congestion_controller.h b/modules/congestion_controller/include/receive_side_congestion_controller.h index fdf210f3f6..1f798b60a7 100644 --- a/modules/congestion_controller/include/receive_side_congestion_controller.h +++ b/modules/congestion_controller/include/receive_side_congestion_controller.h @@ -14,6 +14,7 @@ #include #include +#include "api/transport/field_trial_based_config.h" #include "modules/remote_bitrate_estimator/remote_estimator_proxy.h" #include "rtc_base/constructor_magic.h" #include "rtc_base/critical_section.h" @@ -93,6 +94,7 @@ class ReceiveSideCongestionController : public CallStatsObserver, RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(WrappingBitrateEstimator); }; + const FieldTrialBasedConfig field_trial_config_; WrappingBitrateEstimator remote_bitrate_estimator_; RemoteEstimatorProxy remote_estimator_proxy_; }; diff --git a/modules/congestion_controller/receive_side_congestion_controller.cc b/modules/congestion_controller/receive_side_congestion_controller.cc index f886b8e019..9f674d160d 100644 --- a/modules/congestion_controller/receive_side_congestion_controller.cc +++ b/modules/congestion_controller/receive_side_congestion_controller.cc @@ -122,7 +122,7 @@ ReceiveSideCongestionController::ReceiveSideCongestionController( Clock* clock, PacketRouter* packet_router) : remote_bitrate_estimator_(packet_router, clock), - remote_estimator_proxy_(clock, packet_router) {} + remote_estimator_proxy_(clock, packet_router, &field_trial_config_) {} void ReceiveSideCongestionController::OnReceivedPacket( int64_t arrival_time_ms, diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc index 5220d15f88..bdfe648b06 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy.cc +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.cc @@ -21,11 +21,6 @@ namespace webrtc { -// TODO(sprang): Tune these! -const int RemoteEstimatorProxy::kBackWindowMs = 500; -const int RemoteEstimatorProxy::kMinSendIntervalMs = 50; -const int RemoteEstimatorProxy::kMaxSendIntervalMs = 250; -const int RemoteEstimatorProxy::kDefaultSendIntervalMs = 100; // Impossible to request feedback older than what can be represented by 15 bits. const int RemoteEstimatorProxy::kMaxNumberOfPackets = (1 << 15); @@ -36,13 +31,15 @@ static constexpr int64_t kMaxTimeMs = RemoteEstimatorProxy::RemoteEstimatorProxy( Clock* clock, - TransportFeedbackSenderInterface* feedback_sender) + TransportFeedbackSenderInterface* feedback_sender, + const WebRtcKeyValueConfig* key_value_config) : clock_(clock), feedback_sender_(feedback_sender), + send_config_(key_value_config), last_process_time_ms_(-1), media_ssrc_(0), feedback_packet_count_(0), - send_interval_ms_(kDefaultSendIntervalMs), + send_interval_ms_(send_config_.default_interval->ms()), send_periodic_feedback_(true) {} RemoteEstimatorProxy::~RemoteEstimatorProxy() {} @@ -97,10 +94,10 @@ void RemoteEstimatorProxy::OnBitrateChanged(int bitrate_bps) { // TwccReport size at 250ms interval is 36 byte. // AverageTwccReport = (TwccReport(50ms) + TwccReport(250ms)) / 2 constexpr int kTwccReportSize = 20 + 8 + 10 + 30; - constexpr double kMinTwccRate = - kTwccReportSize * 8.0 * 1000.0 / kMaxSendIntervalMs; - constexpr double kMaxTwccRate = - kTwccReportSize * 8.0 * 1000.0 / kMinSendIntervalMs; + const double kMinTwccRate = + kTwccReportSize * 8.0 * 1000.0 / send_config_.max_interval->ms(); + const double kMaxTwccRate = + kTwccReportSize * 8.0 * 1000.0 / send_config_.min_interval->ms(); // Let TWCC reports occupy 5% of total bandwidth. rtc::CritScope cs(&lock_); @@ -133,7 +130,7 @@ void RemoteEstimatorProxy::OnPacketArrival( // Start new feedback packet, cull old packets. for (auto it = packet_arrival_times_.begin(); it != packet_arrival_times_.end() && it->first < seq && - arrival_time - it->second >= kBackWindowMs;) { + arrival_time - it->second >= send_config_.back_window->ms();) { it = packet_arrival_times_.erase(it); } } diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy.h b/modules/remote_bitrate_estimator/remote_estimator_proxy.h index fa9f7c8e91..52107cfc39 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy.h +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.h @@ -14,8 +14,10 @@ #include #include +#include "api/transport/webrtc_key_value_config.h" #include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "rtc_base/critical_section.h" +#include "rtc_base/experiments/field_trial_parser.h" #include "rtc_base/numerics/sequence_number_util.h" namespace webrtc { @@ -32,12 +34,9 @@ class TransportFeedback; class RemoteEstimatorProxy : public RemoteBitrateEstimator { public: - static const int kMinSendIntervalMs; - static const int kMaxSendIntervalMs; - static const int kDefaultSendIntervalMs; - static const int kBackWindowMs; RemoteEstimatorProxy(Clock* clock, - TransportFeedbackSenderInterface* feedback_sender); + TransportFeedbackSenderInterface* feedback_sender, + const WebRtcKeyValueConfig* key_value_config); ~RemoteEstimatorProxy() override; void IncomingPacket(int64_t arrival_time_ms, @@ -54,6 +53,20 @@ class RemoteEstimatorProxy : public RemoteBitrateEstimator { void SetSendPeriodicFeedback(bool send_periodic_feedback); private: + struct TransportWideFeedbackConfig { + FieldTrialParameter back_window{"wind", TimeDelta::ms(500)}; + FieldTrialParameter min_interval{"min", TimeDelta::ms(50)}; + FieldTrialParameter max_interval{"max", TimeDelta::ms(250)}; + FieldTrialParameter default_interval{"def", TimeDelta::ms(100)}; + explicit TransportWideFeedbackConfig( + const WebRtcKeyValueConfig* key_value_config) { + ParseFieldTrial( + {&back_window, &min_interval, &max_interval, &default_interval}, + key_value_config->Lookup( + "WebRTC-Bwe-TransportWideFeedbackIntervals")); + } + }; + static const int kMaxNumberOfPackets; void OnPacketArrival(uint16_t sequence_number, int64_t arrival_time, @@ -75,6 +88,7 @@ class RemoteEstimatorProxy : public RemoteBitrateEstimator { Clock* const clock_; TransportFeedbackSenderInterface* const feedback_sender_; + const TransportWideFeedbackConfig send_config_; int64_t last_process_time_ms_; rtc::CriticalSection lock_; diff --git a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc b/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc index 2dafdbfc73..4be1289c70 100644 --- a/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc +++ b/modules/remote_bitrate_estimator/remote_estimator_proxy_unittest.cc @@ -9,6 +9,7 @@ */ #include "modules/remote_bitrate_estimator/remote_estimator_proxy.h" +#include "api/transport/field_trial_based_config.h" #include "modules/pacing/packet_router.h" #include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "system_wrappers/include/clock.h" @@ -30,6 +31,11 @@ constexpr int64_t kBaseTimeMs = 123; constexpr int64_t kMaxSmallDeltaMs = (rtcp::TransportFeedback::kDeltaScaleFactor * 0xFF) / 1000; +constexpr int kBackWindowMs = 500; +constexpr int kMinSendIntervalMs = 50; +constexpr int kMaxSendIntervalMs = 250; +constexpr int kDefaultSendIntervalMs = 100; + std::vector SequenceNumbers( const rtcp::TransportFeedback& feedback_packet) { std::vector sequence_numbers; @@ -58,7 +64,8 @@ class MockTransportFeedbackSender : public TransportFeedbackSenderInterface { class RemoteEstimatorProxyTest : public ::testing::Test { public: - RemoteEstimatorProxyTest() : clock_(0), proxy_(&clock_, &router_) {} + RemoteEstimatorProxyTest() + : clock_(0), proxy_(&clock_, &router_, &field_trial_config_) {} protected: void IncomingPacket(uint16_t seq, @@ -77,11 +84,11 @@ class RemoteEstimatorProxyTest : public ::testing::Test { } void Process() { - clock_.AdvanceTimeMilliseconds( - RemoteEstimatorProxy::kDefaultSendIntervalMs); + clock_.AdvanceTimeMilliseconds(kDefaultSendIntervalMs); proxy_.Process(); } + FieldTrialBasedConfig field_trial_config_; SimulatedClock clock_; ::testing::StrictMock router_; RemoteEstimatorProxy proxy_; @@ -305,8 +312,7 @@ TEST_F(RemoteEstimatorProxyTest, ResendsTimestampsOnReordering) { } TEST_F(RemoteEstimatorProxyTest, RemovesTimestampsOutOfScope) { - const int64_t kTimeoutTimeMs = - kBaseTimeMs + RemoteEstimatorProxy::kBackWindowMs; + const int64_t kTimeoutTimeMs = kBaseTimeMs + kBackWindowMs; IncomingPacket(kBaseSeq + 2, kBaseTimeMs); @@ -361,15 +367,13 @@ TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsZeroBeforeFirstProcess) { TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsDefaultOnUnkownBitrate) { Process(); - EXPECT_EQ(RemoteEstimatorProxy::kDefaultSendIntervalMs, - proxy_.TimeUntilNextProcess()); + EXPECT_EQ(kDefaultSendIntervalMs, proxy_.TimeUntilNextProcess()); } TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsMinIntervalOn300kbps) { Process(); proxy_.OnBitrateChanged(300000); - EXPECT_EQ(RemoteEstimatorProxy::kMinSendIntervalMs, - proxy_.TimeUntilNextProcess()); + EXPECT_EQ(kMinSendIntervalMs, proxy_.TimeUntilNextProcess()); } TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsMaxIntervalOn0kbps) { @@ -378,15 +382,13 @@ TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsMaxIntervalOn0kbps) { // bitrate is small. We choose 0 bps as a special case, which also tests // erroneous behaviors like division-by-zero. proxy_.OnBitrateChanged(0); - EXPECT_EQ(RemoteEstimatorProxy::kMaxSendIntervalMs, - proxy_.TimeUntilNextProcess()); + EXPECT_EQ(kMaxSendIntervalMs, proxy_.TimeUntilNextProcess()); } TEST_F(RemoteEstimatorProxyTest, TimeUntilNextProcessIsMaxIntervalOn20kbps) { Process(); proxy_.OnBitrateChanged(20000); - EXPECT_EQ(RemoteEstimatorProxy::kMaxSendIntervalMs, - proxy_.TimeUntilNextProcess()); + EXPECT_EQ(kMaxSendIntervalMs, proxy_.TimeUntilNextProcess()); } TEST_F(RemoteEstimatorProxyTest, TwccReportsUse5PercentOfAvailableBandwidth) {