commit e1bbef1e6bf9b88d6bfcbdbf7139c31ba1cc1af5
author Jakob Ivarsson <jakobi@webrtc.org> Thu Nov 25 14:34:57 2021
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Nov 25 16:30:17 2021
tree 41e1f9e8392a1421bbc495ede7b5ac88ef997ea0
parent 144e5bf87dadd75224f716e8c9e7d035235414f1

Add options to only NACK if there is a valid RTT and if loss rate is below a configured value.

Bug: webrtc:10178
Change-Id: I16a74ed6fb380cecaf82a303bb14bf215c944a73
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/238988
Reviewed-by: Ivo Creusen <ivoc@webrtc.org>
Commit-Queue: Jakob Ivarsson <jakobi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35419}

modules/audio_coding/neteq/nack_tracker.cc

diff --git a/modules/audio_coding/neteq/nack_tracker.cc b/modules/audio_coding/neteq/nack_tracker.cc
index 0edefa5..35afb73 100644
--- a/modules/audio_coding/neteq/nack_tracker.cc
+++ b/modules/audio_coding/neteq/nack_tracker.cc
@@ -32,15 +32,17 @@
   auto parser = StructParametersParser::Create(
       "packet_loss_forget_factor", &packet_loss_forget_factor,
       "ms_per_loss_percent", &ms_per_loss_percent, "never_nack_multiple_times",
-      &never_nack_multiple_times);
+      &never_nack_multiple_times, "require_valid_rtt", &require_valid_rtt,
+      "max_loss_rate", &max_loss_rate);
   parser->Parse(
       webrtc::field_trial::FindFullName(kNackTrackerConfigFieldTrial));
   RTC_LOG(LS_INFO) << "Nack tracker config:"
                       " packet_loss_forget_factor="
                    << packet_loss_forget_factor
                    << " ms_per_loss_percent=" << ms_per_loss_percent
-                   << " never_nack_multiple_times="
-                   << never_nack_multiple_times;
+                   << " never_nack_multiple_times=" << never_nack_multiple_times
+                   << " require_valid_rtt=" << require_valid_rtt
+                   << " max_loss_rate=" << max_loss_rate;
 }
 
 NackTracker::NackTracker()
@@ -223,6 +225,13 @@
 std::vector<uint16_t> NackTracker::GetNackList(int64_t round_trip_time_ms) {
   RTC_DCHECK_GE(round_trip_time_ms, 0);
   std::vector<uint16_t> sequence_numbers;
+  if (config_.require_valid_rtt && round_trip_time_ms == 0) {
+    return sequence_numbers;
+  }
+  if (packet_loss_rate_ >
+      static_cast<uint32_t>(config_.max_loss_rate * (1 << 30))) {
+    return sequence_numbers;
+  }
   // The estimated packet loss is between 0 and 1, so we need to multiply by 100
   // here.
   int max_wait_ms =

modules/audio_coding/neteq/nack_tracker.h

diff --git a/modules/audio_coding/neteq/nack_tracker.h b/modules/audio_coding/neteq/nack_tracker.h
index f95aa0b..0cc95b0 100644
--- a/modules/audio_coding/neteq/nack_tracker.h
+++ b/modules/audio_coding/neteq/nack_tracker.h
@@ -109,6 +109,10 @@
     int ms_per_loss_percent = 20;
     // If true, never nack packets more than once.
     bool never_nack_multiple_times = false;
+    // Only nack if the RTT is valid.
+    bool require_valid_rtt = false;
+    // Do not nack if the loss rate is above this value.
+    double max_loss_rate = 1.0;
   };
 
   struct NackElement {

modules/audio_coding/neteq/nack_tracker_unittest.cc

diff --git a/modules/audio_coding/neteq/nack_tracker_unittest.cc b/modules/audio_coding/neteq/nack_tracker_unittest.cc
index 4639727..bcc5120 100644
--- a/modules/audio_coding/neteq/nack_tracker_unittest.cc
+++ b/modules/audio_coding/neteq/nack_tracker_unittest.cc
@@ -480,19 +480,19 @@
   nack.SetMaxNackListSize(kNackListSize);
   uint16_t seq_num = 0;
   uint32_t timestamp = 0x87654321;
-  auto add_packet = [&nack, &seq_num, &timestamp] {
-    nack.UpdateLastReceivedPacket(seq_num, timestamp);
+  auto add_packet = [&nack, &seq_num, &timestamp](bool received) {
+    if (received) {
+      nack.UpdateLastReceivedPacket(seq_num, timestamp);
+    }
     seq_num++;
     timestamp += kTimestampIncrement;
   };
   // Add some packets, but every fourth packet is lost.
   for (int i = 0; i < 300; i++) {
-    add_packet();
-    add_packet();
-    add_packet();
-    // The next packet is lost.
-    seq_num++;
-    timestamp += kTimestampIncrement;
+    add_packet(true);
+    add_packet(true);
+    add_packet(true);
+    add_packet(false);
   }
   // 1 << 28 is 0.25 in Q30. We expect the packet loss estimate to be within
   // 0.01 of that.
@@ -514,4 +514,52 @@
   EXPECT_TRUE(nack.GetNackList(0).empty());
 }
 
+TEST(NackTrackerTest, DoNotNackIfLossRateIsTooHigh) {
+  test::ScopedFieldTrials field_trials(
+      "WebRTC-Audio-NetEqNackTrackerConfig/max_loss_rate:0.4/");
+  const int kNackListSize = 200;
+  NackTracker nack;
+  nack.UpdateSampleRate(kSampleRateHz);
+  nack.SetMaxNackListSize(kNackListSize);
+  uint16_t seq_num = 0;
+  uint32_t timestamp = 0x87654321;
+  auto add_packet = [&nack, &seq_num, &timestamp](bool received) {
+    if (received) {
+      nack.UpdateLastReceivedPacket(seq_num, timestamp);
+    }
+    seq_num++;
+    timestamp += kTimestampIncrement;
+  };
+  for (int i = 0; i < 500; i++) {
+    add_packet(true);
+    add_packet(false);
+  }
+  // Expect 50% loss rate which is higher that the configured maximum 40%.
+  EXPECT_NEAR(nack.GetPacketLossRateForTest(), 1 << 29, (1 << 30) / 100);
+  EXPECT_TRUE(nack.GetNackList(0).empty());
+}
+
+TEST(NackTrackerTest, OnlyNackIfRttIsValid) {
+  test::ScopedFieldTrials field_trials(
+      "WebRTC-Audio-NetEqNackTrackerConfig/require_valid_rtt:true/");
+  const int kNackListSize = 200;
+  NackTracker nack;
+  nack.UpdateSampleRate(kSampleRateHz);
+  nack.SetMaxNackListSize(kNackListSize);
+  uint16_t seq_num = 0;
+  uint32_t timestamp = 0x87654321;
+  auto add_packet = [&nack, &seq_num, &timestamp](bool received) {
+    if (received) {
+      nack.UpdateLastReceivedPacket(seq_num, timestamp);
+    }
+    seq_num++;
+    timestamp += kTimestampIncrement;
+  };
+  add_packet(true);
+  add_packet(false);
+  add_packet(true);
+  EXPECT_TRUE(nack.GetNackList(0).empty());
+  EXPECT_FALSE(nack.GetNackList(10).empty());
+}
+
 }  // namespace webrtc