Filter out small packets from delay-based overuse detection.

The change is behind a field trial. The intention is to use this
to (heuristically) base the bandwidth estimate only on video packets
even if both audio and video packets have transport sequence numbers.

Bug: webrtc:10932
Change-Id: I6cc5bb9ab6f1a3f25b84ee6ac78e4abb4112032e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/150787
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29031}

modules/congestion_controller/goog_cc/delay_based_bwe.cc

@@ -158,9 +158,10 @@ void DelayBasedBwe::IncomingPacketFeedback(const PacketResult& packet_feedback,
       packet_feedback.sent_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,
+  delay_detector_->Update(
-                          packet_feedback.sent_packet.send_time.ms(),
+      t_delta, ts_delta_ms, packet_feedback.sent_packet.send_time.ms(),
-                          packet_feedback.receive_time.ms(), calculated_deltas);
+      packet_feedback.receive_time.ms(),
+      packet_feedback.sent_packet.size.bytes(), calculated_deltas);
 }
 
 DataRate DelayBasedBwe::TriggerOveruse(Timestamp at_time,

modules/congestion_controller/goog_cc/delay_increase_detector_interface.h

@@ -28,6 +28,7 @@ class DelayIncreaseDetectorInterface {
                       double send_delta_ms,
                       int64_t send_time_ms,
                       int64_t arrival_time_ms,
+                      size_t packet_size,
                       bool calculated_deltas) = 0;
 
   virtual BandwidthUsage State() const = 0;

modules/congestion_controller/goog_cc/trendline_estimator.cc

@@ -19,11 +19,29 @@
 #include "modules/remote_bitrate_estimator/include/bwe_defines.h"
 #include "modules/remote_bitrate_estimator/test/bwe_test_logging.h"
 #include "rtc_base/checks.h"
+#include "rtc_base/experiments/struct_parameters_parser.h"
 #include "rtc_base/logging.h"
 #include "rtc_base/numerics/safe_minmax.h"
 
 namespace webrtc {
 
+constexpr char BweIgnoreSmallPacketsSettings::kKey[];
+
+BweIgnoreSmallPacketsSettings::BweIgnoreSmallPacketsSettings(
+    const WebRtcKeyValueConfig* key_value_config) {
+  Parser()->Parse(
+      key_value_config->Lookup(BweIgnoreSmallPacketsSettings::kKey));
+}
+
+std::unique_ptr<StructParametersParser>
+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.
@@ -84,23 +102,14 @@ constexpr int kDeltaCounterMax = 1000;
 TrendlineEstimator::TrendlineEstimator(
     const WebRtcKeyValueConfig* key_value_config,
     NetworkStatePredictor* network_state_predictor)
-    : TrendlineEstimator(
+    : ignore_small_packets_(key_value_config),
-          key_value_config->Lookup(kBweWindowSizeInPacketsExperiment)
+      fraction_large_packets_(0.5),
-                      .find("Enabled") == 0
+      window_size_(key_value_config->Lookup(kBweWindowSizeInPacketsExperiment)
-              ? ReadTrendlineFilterWindowSize(key_value_config)
+                               .find("Enabled") == 0
-              : kDefaultTrendlineWindowSize,
+                       ? ReadTrendlineFilterWindowSize(key_value_config)
-          kDefaultTrendlineSmoothingCoeff,
+                       : kDefaultTrendlineWindowSize),
-          kDefaultTrendlineThresholdGain,
+      smoothing_coef_(kDefaultTrendlineSmoothingCoeff),
-          network_state_predictor) {}
+      threshold_gain_(kDefaultTrendlineThresholdGain),
-
-TrendlineEstimator::TrendlineEstimator(
-    size_t window_size,
-    double smoothing_coef,
-    double threshold_gain,
-    NetworkStatePredictor* network_state_predictor)
-    : window_size_(window_size),
-      smoothing_coef_(smoothing_coef),
-      threshold_gain_(threshold_gain),
       num_of_deltas_(0),
       first_arrival_time_ms_(-1),
       accumulated_delay_(0),
@@ -120,51 +129,78 @@ TrendlineEstimator::TrendlineEstimator(
       network_state_predictor_(network_state_predictor) {
   RTC_LOG(LS_INFO)
       << "Using Trendline filter for delay change estimation with window size "
-      << window_size_;
+      << window_size_ << " and field trial "
+      << ignore_small_packets_.Parser()->Encode();
 }
 
 TrendlineEstimator::~TrendlineEstimator() {}
 
+void TrendlineEstimator::UpdateTrendline(double recv_delta_ms,
+                                         double send_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);
+  if (first_arrival_time_ms_ == -1)
+    first_arrival_time_ms_ = arrival_time_ms;
+
+  // Exponential backoff filter.
+  accumulated_delay_ += delta_ms;
+  BWE_TEST_LOGGING_PLOT(1, "accumulated_delay_ms", arrival_time_ms,
+                        accumulated_delay_);
+  smoothed_delay_ = smoothing_coef_ * smoothed_delay_ +
+                    (1 - smoothing_coef_) * accumulated_delay_;
+  BWE_TEST_LOGGING_PLOT(1, "smoothed_delay_ms", arrival_time_ms,
+                        smoothed_delay_);
+
+  // Simple linear regression.
+  delay_hist_.push_back(std::make_pair(
+      static_cast<double>(arrival_time_ms - first_arrival_time_ms_),
+      smoothed_delay_));
+  if (delay_hist_.size() > window_size_)
+    delay_hist_.pop_front();
+  double trend = prev_trend_;
+  if (delay_hist_.size() == window_size_) {
+    // Update trend_ if it is possible to fit a line to the data. The delay
+    // trend can be seen as an estimate of (send_rate - capacity)/capacity.
+    // 0 < trend < 1   ->  the delay increases, queues are filling up
+    //   trend == 0    ->  the delay does not change
+    //   trend < 0     ->  the delay decreases, queues are being emptied
+    trend = LinearFitSlope(delay_hist_).value_or(trend);
+  }
+  BWE_TEST_LOGGING_PLOT(1, "trendline_slope", arrival_time_ms, trend);
+
+  Detect(trend, send_delta_ms, arrival_time_ms);
+}
+
 void TrendlineEstimator::Update(double recv_delta_ms,
                                 double send_delta_ms,
                                 int64_t send_time_ms,
                                 int64_t arrival_time_ms,
+                                size_t packet_size,
                                 bool calculated_deltas) {
   if (calculated_deltas) {
-    const double delta_ms = recv_delta_ms - send_delta_ms;
+    UpdateTrendline(recv_delta_ms, send_delta_ms, send_time_ms, arrival_time_ms,
-    ++num_of_deltas_;
+                    packet_size);
-    num_of_deltas_ = std::min(num_of_deltas_, kDeltaCounterMax);
-    if (first_arrival_time_ms_ == -1)
-      first_arrival_time_ms_ = arrival_time_ms;
-
-    // Exponential backoff filter.
-    accumulated_delay_ += delta_ms;
-    BWE_TEST_LOGGING_PLOT(1, "accumulated_delay_ms", arrival_time_ms,
-                          accumulated_delay_);
-    smoothed_delay_ = smoothing_coef_ * smoothed_delay_ +
-                      (1 - smoothing_coef_) * accumulated_delay_;
-    BWE_TEST_LOGGING_PLOT(1, "smoothed_delay_ms", arrival_time_ms,
-                          smoothed_delay_);
-
-    // Simple linear regression.
-    delay_hist_.push_back(std::make_pair(
-        static_cast<double>(arrival_time_ms - first_arrival_time_ms_),
-        smoothed_delay_));
-    if (delay_hist_.size() > window_size_)
-      delay_hist_.pop_front();
-    double trend = prev_trend_;
-    if (delay_hist_.size() == window_size_) {
-      // Update trend_ if it is possible to fit a line to the data. The delay
-      // trend can be seen as an estimate of (send_rate - capacity)/capacity.
-      // 0 < trend < 1   ->  the delay increases, queues are filling up
-      //   trend == 0    ->  the delay does not change
-      //   trend < 0     ->  the delay decreases, queues are being emptied
-      trend = LinearFitSlope(delay_hist_).value_or(trend);
-    }
-
-    BWE_TEST_LOGGING_PLOT(1, "trendline_slope", arrival_time_ms, trend);
-
-    Detect(trend, send_delta_ms, arrival_time_ms);
   }
   if (network_state_predictor_) {
     hypothesis_predicted_ = network_state_predictor_->Update(

modules/congestion_controller/goog_cc/trendline_estimator.h

@@ -14,6 +14,7 @@
 #include <stdint.h>
 
 #include <deque>
+#include <memory>
 #include <utility>
 
 #include "api/network_state_predictor.h"
@@ -21,24 +22,29 @@
 #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<StructParametersParser> Parser();
+};
+
 class TrendlineEstimator : public DelayIncreaseDetectorInterface {
  public:
   TrendlineEstimator(const WebRtcKeyValueConfig* key_value_config,
                      NetworkStatePredictor* network_state_predictor);
-  // |window_size| is the number of points required to compute a trend line.
-  // |smoothing_coef| controls how much we smooth out the delay before fitting
-  // the trend line. |threshold_gain| is used to scale the trendline slope for
-  // comparison to the old threshold. Once the old estimator has been removed
-  // (or the thresholds been merged into the estimators), we can just set the
-  // threshold instead of setting a gain.|network_state_predictor| is used to
-  // bettter predict network state.
-  TrendlineEstimator(size_t window_size,
-                     double smoothing_coef,
-                     double threshold_gain,
-                     NetworkStatePredictor* network_state_predictor);
 
   ~TrendlineEstimator() override;
 
@@ -48,13 +54,16 @@ class TrendlineEstimator : public DelayIncreaseDetectorInterface {
               double send_delta_ms,
               int64_t send_time_ms,
               int64_t arrival_time_ms,
+              size_t packet_size,
               bool calculated_deltas) override;
 
-  BandwidthUsage State() const override;
+  void UpdateTrendline(double recv_delta_ms,
+                       double send_delta_ms,
+                       int64_t send_time_ms,
+                       int64_t arrival_time_ms,
+                       size_t packet_size);
 
- protected:
+  BandwidthUsage State() const override;
-  // Used in unit tests.
-  double modified_trend() const { return prev_trend_ * threshold_gain_; }
 
  private:
   friend class GoogCcStatePrinter;
@@ -63,6 +72,11 @@ 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_;

modules/congestion_controller/goog_cc/trendline_estimator_unittest.cc

@@ -10,69 +10,142 @@
 
 #include "modules/congestion_controller/goog_cc/trendline_estimator.h"
 
+#include <algorithm>
+#include <numeric>
+#include <vector>
+
+#include "api/transport/field_trial_based_config.h"
 #include "rtc_base/random.h"
 #include "test/gtest.h"
 
 namespace webrtc {
-
 namespace {
-constexpr size_t kWindowSize = 20;
+
-constexpr double kSmoothing = 0.0;
+class PacketTimeGenerator {
-constexpr double kGain = 1;
-constexpr int64_t kAvgTimeBetweenPackets = 10;
-constexpr size_t kPacketCount = 2 * kWindowSize + 1;
-class TrendlineEstimatorForTest : public TrendlineEstimator {
  public:
-  using TrendlineEstimator::modified_trend;
+  PacketTimeGenerator(int64_t initial_clock, double time_between_packets)
-  using TrendlineEstimator::TrendlineEstimator;
+      : initial_clock_(initial_clock),
+        time_between_packets_(time_between_packets),
+        packets_(0) {}
+  int64_t operator()() {
+    return initial_clock_ + time_between_packets_ * packets_++;
+  }
+
+ private:
+  const int64_t initial_clock_;
+  const double time_between_packets_;
+  size_t packets_;
 };
-void TestEstimator(double slope, double jitter_stddev, double tolerance) {
+
-  TrendlineEstimatorForTest estimator(kWindowSize, kSmoothing, kGain, nullptr);
+class TrendlineEstimatorTest : public testing::Test {
-  Random random(0x1234567);
+ public:
-  int64_t send_times[kPacketCount];
+  TrendlineEstimatorTest()
-  int64_t recv_times[kPacketCount];
+      : send_times(kPacketCount),
-  int64_t send_start_time = random.Rand(1000000);
+        recv_times(kPacketCount),
-  int64_t recv_start_time = random.Rand(1000000);
+        packet_sizes(kPacketCount),
-  for (size_t i = 0; i < kPacketCount; ++i) {
+        config(),
-    send_times[i] = send_start_time + i * kAvgTimeBetweenPackets;
+        estimator(&config, nullptr),
-    double latency = i * kAvgTimeBetweenPackets / (1 - slope);
+        count(1) {
-    double jitter = random.Gaussian(0, jitter_stddev);
+    std::fill(packet_sizes.begin(), packet_sizes.end(), kPacketSizeBytes);
-    recv_times[i] = recv_start_time + latency + jitter;
   }
-  for (size_t i = 1; i < kPacketCount; ++i) {
+
-    double recv_delta = recv_times[i] - recv_times[i - 1];
+  void RunTestUntilStateChange() {
-    double send_delta = send_times[i] - send_times[i - 1];
+    RTC_DCHECK_EQ(send_times.size(), kPacketCount);
-    estimator.Update(recv_delta, send_delta, 0, recv_times[i], true);
+    RTC_DCHECK_EQ(recv_times.size(), kPacketCount);
-    if (i < kWindowSize)
+    RTC_DCHECK_EQ(packet_sizes.size(), kPacketCount);
-      EXPECT_NEAR(estimator.modified_trend(), 0, 0.001);
+    RTC_DCHECK_GE(count, 1);
-    else
+    RTC_DCHECK_LT(count, kPacketCount);
-      EXPECT_NEAR(estimator.modified_trend(), slope, tolerance);
+
+    auto initial_state = estimator.State();
+    for (; count < kPacketCount; count++) {
+      double recv_delta = recv_times[count] - recv_times[count - 1];
+      double send_delta = send_times[count] - send_times[count - 1];
+      estimator.Update(recv_delta, send_delta, send_times[count],
+                       recv_times[count], packet_sizes[count], true);
+      if (estimator.State() != initial_state) {
+        return;
+      }
+    }
   }
-}
+
+ protected:
+  const size_t kPacketCount = 25;
+  const size_t kPacketSizeBytes = 1200;
+  std::vector<int64_t> send_times;
+  std::vector<int64_t> recv_times;
+  std::vector<size_t> packet_sizes;
+  const FieldTrialBasedConfig config;
+  TrendlineEstimator estimator;
+  size_t count;
+};
 }  // namespace
 
-TEST(TrendlineEstimator, PerfectLineSlopeOneHalf) {
+TEST_F(TrendlineEstimatorTest, Normal) {
-  TestEstimator(0.5, 0, 0.001);
+  PacketTimeGenerator send_time_generator(123456789 /*initial clock*/,
+                                          20 /*20 ms between sent packets*/);
+  std::generate(send_times.begin(), send_times.end(), send_time_generator);
+
+  PacketTimeGenerator recv_time_generator(987654321 /*initial clock*/,
+                                          20 /*delivered at the same pace*/);
+  std::generate(recv_times.begin(), recv_times.end(), recv_time_generator);
+
+  EXPECT_EQ(estimator.State(), BandwidthUsage::kBwNormal);
+  RunTestUntilStateChange();
+  EXPECT_EQ(estimator.State(), BandwidthUsage::kBwNormal);
+  EXPECT_EQ(count, kPacketCount);  // All packets processed
 }
 
-TEST(TrendlineEstimator, PerfectLineSlopeMinusOne) {
+TEST_F(TrendlineEstimatorTest, Overusing) {
-  TestEstimator(-1, 0, 0.001);
+  PacketTimeGenerator send_time_generator(123456789 /*initial clock*/,
+                                          20 /*20 ms between sent packets*/);
+  std::generate(send_times.begin(), send_times.end(), send_time_generator);
+
+  PacketTimeGenerator recv_time_generator(987654321 /*initial clock*/,
+                                          1.1 * 20 /*10% slower delivery*/);
+  std::generate(recv_times.begin(), recv_times.end(), recv_time_generator);
+
+  EXPECT_EQ(estimator.State(), BandwidthUsage::kBwNormal);
+  RunTestUntilStateChange();
+  EXPECT_EQ(estimator.State(), BandwidthUsage::kBwOverusing);
+  RunTestUntilStateChange();
+  EXPECT_EQ(estimator.State(), BandwidthUsage::kBwOverusing);
+  EXPECT_EQ(count, kPacketCount);  // All packets processed
 }
 
-TEST(TrendlineEstimator, PerfectLineSlopeZero) {
+TEST_F(TrendlineEstimatorTest, Underusing) {
-  TestEstimator(0, 0, 0.001);
+  PacketTimeGenerator send_time_generator(123456789 /*initial clock*/,
+                                          20 /*20 ms between sent packets*/);
+  std::generate(send_times.begin(), send_times.end(), send_time_generator);
+
+  PacketTimeGenerator recv_time_generator(987654321 /*initial clock*/,
+                                          0.85 * 20 /*15% faster delivery*/);
+  std::generate(recv_times.begin(), recv_times.end(), recv_time_generator);
+
+  EXPECT_EQ(estimator.State(), BandwidthUsage::kBwNormal);
+  RunTestUntilStateChange();
+  EXPECT_EQ(estimator.State(), BandwidthUsage::kBwUnderusing);
+  RunTestUntilStateChange();
+  EXPECT_EQ(estimator.State(), BandwidthUsage::kBwUnderusing);
+  EXPECT_EQ(count, kPacketCount);  // All packets processed
 }
 
-TEST(TrendlineEstimator, JitteryLineSlopeOneHalf) {
+TEST_F(TrendlineEstimatorTest, IncludesSmallPacketsByDefault) {
-  TestEstimator(0.5, kAvgTimeBetweenPackets / 3.0, 0.01);
+  PacketTimeGenerator send_time_generator(123456789 /*initial clock*/,
-}
+                                          20 /*20 ms between sent packets*/);
+  std::generate(send_times.begin(), send_times.end(), send_time_generator);
 
-TEST(TrendlineEstimator, JitteryLineSlopeMinusOne) {
+  PacketTimeGenerator recv_time_generator(987654321 /*initial clock*/,
-  TestEstimator(-1, kAvgTimeBetweenPackets / 3.0, 0.075);
+                                          1.1 * 20 /*10% slower delivery*/);
-}
+  std::generate(recv_times.begin(), recv_times.end(), recv_time_generator);
 
-TEST(TrendlineEstimator, JitteryLineSlopeZero) {
+  std::fill(packet_sizes.begin(), packet_sizes.end(), 100);
-  TestEstimator(0, kAvgTimeBetweenPackets / 3.0, 0.02);
+
+  EXPECT_EQ(estimator.State(), BandwidthUsage::kBwNormal);
+  RunTestUntilStateChange();
+  EXPECT_EQ(estimator.State(), BandwidthUsage::kBwOverusing);
+  RunTestUntilStateChange();
+  EXPECT_EQ(estimator.State(), BandwidthUsage::kBwOverusing);
+  EXPECT_EQ(count, kPacketCount);  // All packets processed
 }
 
 }  // namespace webrtc