Ensure loss spikes are ignored

If all packets are dropped for a period of time, an observation window will have the same length as the period when packets are dropped.
If later, no packets are lost, there is no point in loss based bwe backing down.
Therefore, ignore the observation with most loss and least loss when calculating an instant upper bound.


Bug: webrtc:42222865
Change-Id: I1d0125d6c76e68018b2aec1ecaa9b65729963136
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/356380
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Diep Bui <diepbp@google.com>
Cr-Commit-Position: refs/heads/main@{#42772}

modules/congestion_controller/goog_cc/loss_based_bwe_v2.cc

@@ -809,6 +809,13 @@ double LossBasedBweV2::GetAverageReportedByteLossRatio() const {
 
   DataSize total_bytes = DataSize::Zero();
   DataSize lost_bytes = DataSize::Zero();
+  double min_loss_rate = 1.0;
+  double max_loss_rate = 0.0;
+  DataSize min_lost_bytes = DataSize::Zero();
+  DataSize max_lost_bytes = DataSize::Zero();
+  DataSize min_bytes_received = DataSize::Zero();
+  DataSize max_bytes_received = DataSize::Zero();
+
   for (const Observation& observation : observations_) {
     if (!observation.IsInitialized()) {
       continue;
@@ -819,8 +826,25 @@ double LossBasedBweV2::GetAverageReportedByteLossRatio() const {
                                               observation.id];
     total_bytes += instant_temporal_weight * observation.size;
     lost_bytes += instant_temporal_weight * observation.lost_size;
+
+    double loss_rate = !observation.size.IsZero()
+                           ? observation.lost_size / observation.size
+                           : 0.0;
+    if (num_observations_ > 3) {
+      if (loss_rate > max_loss_rate) {
+        max_loss_rate = loss_rate;
+        max_lost_bytes = instant_temporal_weight * observation.lost_size;
+        max_bytes_received = instant_temporal_weight * observation.size;
+      }
+      if (loss_rate < min_loss_rate) {
+        min_loss_rate = loss_rate;
+        min_lost_bytes = instant_temporal_weight * observation.lost_size;
+        min_bytes_received = instant_temporal_weight * observation.size;
+      }
+    }
   }
-  return lost_bytes / total_bytes;
+  return (lost_bytes - min_lost_bytes - max_lost_bytes) /
+         (total_bytes - max_bytes_received - min_bytes_received);
 }
 
 DataRate LossBasedBweV2::GetCandidateBandwidthUpperBound() const {

modules/congestion_controller/goog_cc/loss_based_bwe_v2.h

@@ -171,6 +171,9 @@ class LossBasedBweV2 {
   // Returns `0.0` if not enough loss statistics have been received.
   double GetAverageReportedLossRatio() const;
   double GetAverageReportedPacketLossRatio() const;
+  // Calculates the average loss ratio over the last `observation_window_size`
+  // observations but skips the observation with min and max loss ratio in order
+  // to filter out loss spikes.
   double GetAverageReportedByteLossRatio() const;
   std::vector<ChannelParameters> GetCandidates(bool in_alr) const;
   DataRate GetCandidateBandwidthUpperBound() const;

modules/congestion_controller/goog_cc/loss_based_bwe_v2_test.cc

@@ -1793,6 +1793,55 @@ TEST_F(LossBasedBweV2Test, UseByteLossRate) {
       DataRate::KilobitsPerSec(150));
 }
 
+TEST_F(LossBasedBweV2Test, UseByteLossRateIgnoreLossSpike) {
+  ExplicitKeyValueConfig key_value_config(
+      "WebRTC-Bwe-LossBasedBweV2/"
+      "UseByteLossRate:true,ObservationWindowSize:5/");
+  LossBasedBweV2 loss_based_bandwidth_estimator(&key_value_config);
+  const DataRate kDelayBasedEstimate = DataRate::KilobitsPerSec(500);
+  loss_based_bandwidth_estimator.SetBandwidthEstimate(kDelayBasedEstimate);
+
+  // Fill the observation window.
+  for (int i = 0; i < 5; ++i) {
+    loss_based_bandwidth_estimator.UpdateBandwidthEstimate(
+        CreatePacketResultsWithReceivedPackets(
+            /*first_packet_timestamp=*/Timestamp::Zero() +
+            i * kObservationDurationLowerBound),
+        kDelayBasedEstimate,
+        /*in_alr=*/false);
+  }
+  loss_based_bandwidth_estimator.UpdateBandwidthEstimate(
+      CreatePacketResultsWith100pLossRate(
+          /*first_packet_timestamp=*/Timestamp::Zero() +
+          5 * kObservationDurationLowerBound),
+      kDelayBasedEstimate,
+      /*in_alr=*/false);
+  loss_based_bandwidth_estimator.UpdateBandwidthEstimate(
+      CreatePacketResultsWithReceivedPackets(
+          /*first_packet_timestamp=*/Timestamp::Zero() +
+          6 * kObservationDurationLowerBound),
+      kDelayBasedEstimate,
+      /*in_alr=*/false);
+  EXPECT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state,
+            LossBasedState::kDelayBasedEstimate);
+  EXPECT_EQ(
+      loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate,
+      kDelayBasedEstimate);
+
+  // But if more loss happen in a new observation, BWE back down.
+  loss_based_bandwidth_estimator.UpdateBandwidthEstimate(
+      CreatePacketResultsWith100pLossRate(
+          /*first_packet_timestamp=*/Timestamp::Zero() +
+          7 * kObservationDurationLowerBound),
+      kDelayBasedEstimate,
+      /*in_alr=*/false);
+  EXPECT_EQ(loss_based_bandwidth_estimator.GetLossBasedResult().state,
+            LossBasedState::kDecreasing);
+  EXPECT_LT(
+      loss_based_bandwidth_estimator.GetLossBasedResult().bandwidth_estimate,
+      kDelayBasedEstimate);
+}
+
 TEST_F(LossBasedBweV2Test, PaceAtLossBasedEstimate) {
   ExplicitKeyValueConfig key_value_config(ShortObservationConfig(
       "PaceAtLossBasedEstimate:true,PaddingDuration:1000ms"));