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}

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 @@ class ReceiveSideCongestionController : public CallStatsObserver,
     RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(WrappingBitrateEstimator);
   };
 
+  const FieldTrialBasedConfig field_trial_config_;
   WrappingBitrateEstimator remote_bitrate_estimator_;
   RemoteEstimatorProxy remote_estimator_proxy_;
 };

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,

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);
       }
     }

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 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<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 @@ class RemoteEstimatorProxy : public RemoteBitrateEstimator {
 
   Clock* const clock_;
   TransportFeedbackSenderInterface* const feedback_sender_;
+  const TransportWideFeedbackConfig send_config_;
   int64_t last_process_time_ms_;
 
   rtc::CriticalSection lock_;

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<uint16_t> SequenceNumbers(
     const rtcp::TransportFeedback& feedback_packet) {
   std::vector<uint16_t> 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<MockTransportFeedbackSender> 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) {