Expose send parameters for RemoteEstimatorProxy for field trial.
Bug: None
Change-Id: I14c74f55ed534fff19042423dad63641bd0fa047
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/133187
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27662}diff --git a/modules/congestion_controller/include/receive_side_congestion_controller.h b/modules/congestion_controller/include/receive_side_congestion_controller.h
index fdf210f..1f798b6 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 <memory>
#include <vector>
+#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 @@
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 f886b8e..9f674d1 100644
--- a/modules/congestion_controller/receive_side_congestion_controller.cc
+++ b/modules/congestion_controller/receive_side_congestion_controller.cc
@@ -122,7 +122,7 @@
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 5220d15..bdfe648 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 @@
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 @@
// 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 @@
// 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 fa9f7c8..52107cf 100644
--- a/modules/remote_bitrate_estimator/remote_estimator_proxy.h
+++ b/modules/remote_bitrate_estimator/remote_estimator_proxy.h
@@ -14,8 +14,10 @@
#include <map>
#include <vector>
+#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 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 @@
void SetSendPeriodicFeedback(bool send_periodic_feedback);
private:
+ struct TransportWideFeedbackConfig {
+ FieldTrialParameter<TimeDelta> back_window{"wind", TimeDelta::ms(500)};
+ FieldTrialParameter<TimeDelta> min_interval{"min", TimeDelta::ms(50)};
+ FieldTrialParameter<TimeDelta> max_interval{"max", TimeDelta::ms(250)};
+ FieldTrialParameter<TimeDelta> 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 @@
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 2dafdbf..4be1289 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 kMaxSmallDeltaMs =
(rtcp::TransportFeedback::kDeltaScaleFactor * 0xFF) / 1000;
+constexpr int kBackWindowMs = 500;
+constexpr int kMinSendIntervalMs = 50;
+constexpr int kMaxSendIntervalMs = 250;
+constexpr int kDefaultSendIntervalMs = 100;
+
std::vector<uint16_t> SequenceNumbers(
const rtcp::TransportFeedback& feedback_packet) {
std::vector<uint16_t> sequence_numbers;
@@ -58,7 +64,8 @@
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 @@
}
void Process() {
- clock_.AdvanceTimeMilliseconds(
- RemoteEstimatorProxy::kDefaultSendIntervalMs);
+ clock_.AdvanceTimeMilliseconds(kDefaultSendIntervalMs);
proxy_.Process();
}
+ FieldTrialBasedConfig field_trial_config_;
SimulatedClock clock_;
::testing::StrictMock<MockTransportFeedbackSender> router_;
RemoteEstimatorProxy proxy_;
@@ -305,8 +312,7 @@
}
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, 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 @@
// 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) {
