R/PLR calculation - time-based window

webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc

Index: webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc
diff --git a/webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc b/webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc
index 5c9ae21c744292f90ff654c73d2a96d7f6467743..3bd580ccefa4dcfcb839e92dfaf11b4e7a646c87 100644
--- a/webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc
+++ b/webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc
@@ -14,123 +14,134 @@
 #include <vector>
 
 #include "webrtc/base/checks.h"
-#include "webrtc/base/mod_ops.h"
 #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
 #include "webrtc/test/gmock.h"
 #include "webrtc/test/gtest.h"
 #include "webrtc/voice_engine/transport_feedback_packet_loss_tracker.h"
 
-using webrtc::rtcp::TransportFeedback;
-
 namespace webrtc {
 
 namespace {
 
 class TransportFeedbackPacketLossTrackerTest
     : public ::testing::TestWithParam<uint16_t> {
+  using TransportFeedback = webrtc::rtcp::TransportFeedback;
+
  public:
   TransportFeedbackPacketLossTrackerTest() = default;
   virtual ~TransportFeedbackPacketLossTrackerTest() = default;
 
  protected:
-  uint16_t base_{GetParam()};
-
- private:
-  RTC_DISALLOW_COPY_AND_ASSIGN(TransportFeedbackPacketLossTrackerTest);
-};
+  void SendPackets(TransportFeedbackPacketLossTracker* tracker,
+                   const std::vector<uint16_t>& sequence_numbers,
+                   int64_t send_time_interval_ms,
+                   bool validate_all = true) {
+    RTC_CHECK_GE(send_time_interval_ms, 0);
+    for (uint16_t sequence_number : sequence_numbers) {
+      tracker->OnPacketAdded(sequence_number, time_ms_);
+      if (validate_all) {
+        tracker->Validate();
+      }
+      time_ms_ += send_time_interval_ms;
+    }
 
-void SendPackets(TransportFeedbackPacketLossTracker* tracker,
-                 const std::vector<uint16_t>& seq_nums,
-                 bool validate_all = true) {
-  for (uint16_t seq_num : seq_nums) {
-    tracker->OnPacketAdded(seq_num);
-    if (validate_all) {
+    // We've either validated after each packet, or, for making sure the UT
+    // doesn't run too long, we might validate only at the end of the range.
+    if (!validate_all) {
       tracker->Validate();
     }
   }
 
-  // We've either validated after each packet, or, for making sure the UT
-  // doesn't run too long, we might validate only at the end of the range.
-  if (!validate_all) {
-    tracker->Validate();
+  void SendPackets(TransportFeedbackPacketLossTracker* tracker,
+                   uint16_t first_seq_num,
+                   size_t num_of_packets,
+                   int64_t send_time_interval_ms,
+                   bool validate_all = true) {
+    RTC_CHECK_GE(send_time_interval_ms, 0);
+    std::vector<uint16_t> sequence_numbers(num_of_packets);
+    std::iota(sequence_numbers.begin(), sequence_numbers.end(), first_seq_num);
+    SendPackets(tracker, sequence_numbers, send_time_interval_ms, validate_all);
   }
-}
 
-void SendPacketRange(TransportFeedbackPacketLossTracker* tracker,
-                     uint16_t first_seq_num,
-                     size_t num_of_seq_nums,
-                     bool validate_all = true) {
-  std::vector<uint16_t> seq_nums(num_of_seq_nums);
-  std::iota(seq_nums.begin(), seq_nums.end(), first_seq_num);
-  SendPackets(tracker, seq_nums, validate_all);
-}
-
-void AddTransportFeedbackAndValidate(
-    TransportFeedbackPacketLossTracker* tracker,
-    uint16_t base_sequence_num,
-    const std::vector<bool>& reception_status_vec) {
-  const int64_t kBaseTimeUs = 1234;  // Irrelevant to this test.
-  TransportFeedback test_feedback;
-  test_feedback.SetBase(base_sequence_num, kBaseTimeUs);
-  uint16_t sequence_num = base_sequence_num;
-  for (bool status : reception_status_vec) {
-    if (status)
-      test_feedback.AddReceivedPacket(sequence_num, kBaseTimeUs);
-    ++sequence_num;
+  void AdvanceClock(int64_t time_delta_ms) {
+    RTC_CHECK_GT(time_delta_ms, 0);
+    time_ms_ += time_delta_ms;
   }
 
-  // TransportFeedback imposes some limitations on what constitutes a legal
-  // status vector. For instance, the vector cannot terminate in a lost
-  // packet. Make sure all limitations are abided by.
-  RTC_CHECK_EQ(base_sequence_num, test_feedback.GetBaseSequence());
-  const auto& vec = test_feedback.GetStatusVector();
-  RTC_CHECK_EQ(reception_status_vec.size(), vec.size());
-  for (size_t i = 0; i < reception_status_vec.size(); i++) {
-    RTC_CHECK_EQ(reception_status_vec[i],
-                 vec[i] != TransportFeedback::StatusSymbol::kNotReceived);
+  void AddTransportFeedbackAndValidate(
+      TransportFeedbackPacketLossTracker* tracker,
+      uint16_t base_sequence_num,
+      const std::vector<bool>& reception_status_vec) {
+    const int64_t kBaseTimeUs = 1234;  // Irrelevant to this test.
+    TransportFeedback test_feedback;
+    test_feedback.SetBase(base_sequence_num, kBaseTimeUs);
+    uint16_t sequence_num = base_sequence_num;
+    for (bool status : reception_status_vec) {
+      if (status)
+        test_feedback.AddReceivedPacket(sequence_num, kBaseTimeUs);
+      ++sequence_num;
+    }
+
+    // TransportFeedback imposes some limitations on what constitutes a legal
+    // status vector. For instance, the vector cannot terminate in a lost
+    // packet. Make sure all limitations are abided by.
+    RTC_CHECK_EQ(base_sequence_num, test_feedback.GetBaseSequence());
+    const auto& vec = test_feedback.GetStatusVector();
+    RTC_CHECK_EQ(reception_status_vec.size(), vec.size());
+    for (size_t i = 0; i < reception_status_vec.size(); i++) {
+      RTC_CHECK_EQ(reception_status_vec[i],
+                   vec[i] != TransportFeedback::StatusSymbol::kNotReceived);
+    }
+
+    tracker->OnReceivedTransportFeedback(test_feedback);
+    tracker->Validate();
   }
 
-  tracker->OnReceivedTransportFeedback(test_feedback);
-  tracker->Validate();
-}
+  // Checks that validty is as expected. If valid, checks also that
+  // value is as expected.
+  void ValidatePacketLossStatistics(
+      const TransportFeedbackPacketLossTracker& tracker,
+      rtc::Optional<float> expected_plr,
+      rtc::Optional<float> expected_rplr) {
+    // TODO(elad.alon): Comparing the rtc::Optional<float> directly would have
+    // given concise code, but less readable error messages. If we modify
+    // the way rtc::Optional is printed, we can get rid of this.
+    rtc::Optional<float> plr = tracker.GetPacketLossRate();
+    EXPECT_EQ(static_cast<bool>(expected_plr), static_cast<bool>(plr));
+    if (expected_plr && plr) {
+      EXPECT_EQ(*expected_plr, *plr);
+    }
 
-// Checks that validty is as expected. If valid, checks also that
-// value is as expected.
-void ValidatePacketLossStatistics(
-    const TransportFeedbackPacketLossTracker& tracker,
-    rtc::Optional<float> expected_plr,
-    rtc::Optional<float> expected_rplr) {
-  // Comparing the rtc::Optional<float> directly would have given concise code,
-  // but less readable error messages.
-  rtc::Optional<float> plr = tracker.GetPacketLossRate();
-  EXPECT_EQ(static_cast<bool>(expected_plr), static_cast<bool>(plr));
-  if (expected_plr && plr) {
-    EXPECT_EQ(*expected_plr, *plr);
+    rtc::Optional<float> rplr = tracker.GetRecoverablePacketLossRate();
+    EXPECT_EQ(static_cast<bool>(expected_rplr), static_cast<bool>(rplr));
+    if (expected_rplr && rplr) {
+      EXPECT_EQ(*expected_rplr, *rplr);
+    }
   }
 
-  rtc::Optional<float> rplr = tracker.GetRecoverablePacketLossRate();
-  EXPECT_EQ(static_cast<bool>(expected_rplr), static_cast<bool>(rplr));
-  if (expected_rplr && rplr) {
-    EXPECT_EQ(*expected_rplr, *rplr);
+  // Convenience function for when both are valid, and explicitly stating
+  // the rtc::Optional<float> constructor is just cumbersome.
+  void ValidatePacketLossStatistics(
+      const TransportFeedbackPacketLossTracker& tracker,
+      float expected_plr,
+      float expected_rplr) {
+    ValidatePacketLossStatistics(tracker, rtc::Optional<float>(expected_plr),
+                                 rtc::Optional<float>(expected_rplr));
   }
-}
 
-// Convenience function for when both are valid, and explicitly stating
-// the rtc::Optional<float> constructor is just cumbersome.
-void ValidatePacketLossStatistics(
-    const TransportFeedbackPacketLossTracker& tracker,
-    float expected_plr,
-    float expected_rplr) {
-  ValidatePacketLossStatistics(tracker,
-                               rtc::Optional<float>(expected_plr),
-                               rtc::Optional<float>(expected_rplr));
-}
+  uint16_t base_{GetParam()};
+
+ private:
+  int64_t time_ms_{0};
+
+  RTC_DISALLOW_COPY_AND_ASSIGN(TransportFeedbackPacketLossTrackerTest);
+};
 
 }  // namespace
 
 // Sanity check on an empty window.
-TEST(TransportFeedbackPacketLossTrackerTest, EmptyWindow) {
-  TransportFeedbackPacketLossTracker tracker(10, 5, 5);
+TEST_P(TransportFeedbackPacketLossTrackerTest, EmptyWindow) {
+  TransportFeedbackPacketLossTracker tracker(5000, 5, 5);
 
   // PLR and RPLR reported as unknown before reception of first feedback.
   ValidatePacketLossStatistics(tracker,
@@ -140,7 +151,7 @@ TEST(TransportFeedbackPacketLossTrackerTest, EmptyWindow) {
 
 // A feedback received for an empty window has no effect.
 TEST_P(TransportFeedbackPacketLossTrackerTest, EmptyWindowFeedback) {
-  TransportFeedbackPacketLossTracker tracker(3, 3, 2);
+  TransportFeedbackPacketLossTracker tracker(5000, 3, 2);
 
   // Feedback doesn't correspond to any packets - ignored.
   AddTransportFeedbackAndValidate(&tracker, base_, {true, false, true});
@@ -149,19 +160,19 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, EmptyWindowFeedback) {
                                rtc::Optional<float>());
 
   // After the packets are transmitted, acking them would have an effect.
-  SendPacketRange(&tracker, base_, 3);
+  SendPackets(&tracker, base_, 3, 10);
   AddTransportFeedbackAndValidate(&tracker, base_, {true, false, true});
   ValidatePacketLossStatistics(tracker, 1.0f / 3.0f, 0.5f);
 }
 
 // Sanity check on partially filled window.
 TEST_P(TransportFeedbackPacketLossTrackerTest, PartiallyFilledWindow) {
-  TransportFeedbackPacketLossTracker tracker(10, 5, 4);
+  TransportFeedbackPacketLossTracker tracker(5000, 5, 4);
 
   // PLR unknown before minimum window size reached.
   // RPLR unknown before minimum pairs reached.
   // Expected window contents: [] -> [1001].
-  SendPacketRange(&tracker, base_, 3);
+  SendPackets(&tracker, base_, 3, 10);
   AddTransportFeedbackAndValidate(&tracker, base_, {true, false, false, true});
   ValidatePacketLossStatistics(tracker,
                                rtc::Optional<float>(),
@@ -170,12 +181,12 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, PartiallyFilledWindow) {
 
 // Sanity check on minimum filled window - PLR known, RPLR unknown.
 TEST_P(TransportFeedbackPacketLossTrackerTest, PlrMinimumFilledWindow) {
-  TransportFeedbackPacketLossTracker tracker(10, 5, 5);
+  TransportFeedbackPacketLossTracker tracker(5000, 5, 5);
 
   // PLR correctly calculated after minimum window size reached.
   // RPLR not necessarily known at that time (not if min-pairs not reached).
   // Expected window contents: [] -> [10011].
-  SendPacketRange(&tracker, base_, 5);
+  SendPackets(&tracker, base_, 5, 10);
   AddTransportFeedbackAndValidate(&tracker, base_,
                                   {true, false, false, true, true});
   ValidatePacketLossStatistics(tracker,
@@ -185,12 +196,12 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, PlrMinimumFilledWindow) {
 
 // Sanity check on minimum filled window - PLR unknown, RPLR known.
 TEST_P(TransportFeedbackPacketLossTrackerTest, RplrMinimumFilledWindow) {
-  TransportFeedbackPacketLossTracker tracker(10, 6, 4);
+  TransportFeedbackPacketLossTracker tracker(5000, 6, 4);
 
   // RPLR correctly calculated after minimum pairs reached.
   // PLR not necessarily known at that time (not if min window not reached).
   // Expected window contents: [] -> [10011].
-  SendPacketRange(&tracker, base_, 5);
+  SendPackets(&tracker, base_, 5, 10);
   AddTransportFeedbackAndValidate(&tracker, base_,
                                   {true, false, false, true, true});
   ValidatePacketLossStatistics(tracker,
@@ -198,11 +209,23 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, RplrMinimumFilledWindow) {
                                rtc::Optional<float>(1.0f / 4.0f));
 }
 
+// If packets are sent close enough together that the clock reading for both
+// is the same, that's handled properly.
+TEST_P(TransportFeedbackPacketLossTrackerTest, SameSentTime) {
+  TransportFeedbackPacketLossTracker tracker(5000, 3, 2);
+
+  // Expected window contents: [] -> [101].
+  SendPackets(&tracker, base_, 3, 0);  // Note: time interval = 0ms.
+  AddTransportFeedbackAndValidate(&tracker, base_, {true, false, true});
+
+  ValidatePacketLossStatistics(tracker, 1.0f / 3.0f, 0.5f);
+}
+
 // Additional reports update PLR and RPLR.
 TEST_P(TransportFeedbackPacketLossTrackerTest, ExtendWindow) {
-  TransportFeedbackPacketLossTracker tracker(20, 5, 5);
+  TransportFeedbackPacketLossTracker tracker(5000, 5, 5);
 
-  SendPacketRange(&tracker, base_, 25);
+  SendPackets(&tracker, base_, 25, 10);
 
   // Expected window contents: [] -> [10011].
   AddTransportFeedbackAndValidate(&tracker, base_,
@@ -222,25 +245,63 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, ExtendWindow) {
   ValidatePacketLossStatistics(tracker, 7.0f / 15.0f, 4.0f / 13.0f);
 }
 
-// Sanity - all packets correctly received.
+// Correct calculation with different packet lengths.
+TEST_P(TransportFeedbackPacketLossTrackerTest, DifferentSentIntervals) {
+  TransportFeedbackPacketLossTracker tracker(5000, 5, 4);
+
+  int64_t now_ms = 0;
+  int64_t frames[] = {20, 60, 120, 20, 60};
+  for (size_t i = 0; i < sizeof(frames) / sizeof(frames[0]); i++) {
+    tracker.OnPacketAdded(static_cast<uint16_t>(base_ + i), now_ms);
+    now_ms += frames[i];
+  }
+
+  // Expected window contents: [] -> [10011].
+  AddTransportFeedbackAndValidate(&tracker, base_,
+                                  {true, false, false, true, true});
+  ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f);
+}
+
+// The window retains information up to sent times that exceed the the max
+// window size. The oldest packets get shifted out of window to make room
+// for the newer ones.
+TEST_P(TransportFeedbackPacketLossTrackerTest, MaxWindowSize) {
+  TransportFeedbackPacketLossTracker tracker(40, 5, 1);
+
+  SendPackets(&tracker, base_, 6, 10, true);
+
+  // Up to the maximum time-span retained (first + 4 * 10ms).
+  // Expected window contents: [] -> [01001].
+  AddTransportFeedbackAndValidate(&tracker, base_,
+                                  {false, true, false, false, true});
+  ValidatePacketLossStatistics(tracker, 3.0f / 5.0f, 2.0f / 4.0f);
+
+  // After the maximum time-span, older entries are discarded to accommodate
+  // newer ones.
+  // Expected window contents: [01001] -> [10011].
+  AddTransportFeedbackAndValidate(&tracker, base_ + 5, {true});
+  ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f);
+}
+
+// All packets received.
 TEST_P(TransportFeedbackPacketLossTrackerTest, AllReceived) {
-  TransportFeedbackPacketLossTracker tracker(10, 5, 4);
+  TransportFeedbackPacketLossTracker tracker(5000, 5, 4);
 
   // Expected window contents: [] -> [11111].
-  SendPacketRange(&tracker, base_, 5);
+  SendPackets(&tracker, base_, 5, 10);
   AddTransportFeedbackAndValidate(&tracker, base_,
                                   {true, true, true, true, true});
   ValidatePacketLossStatistics(tracker, 0.0f, 0.0f);
 }
 
-// Sanity - all packets lost.
+// All packets lost.
 TEST_P(TransportFeedbackPacketLossTrackerTest, AllLost) {
-  TransportFeedbackPacketLossTracker tracker(10, 5, 4);
+  TransportFeedbackPacketLossTracker tracker(5000, 5, 4);
 
+  // Note: The last packet in the feedback does not belong to the stream.
+  // It's only there because we're not allowed to end a feedback with a loss.
   // Expected window contents: [] -> [00000].
-  SendPacketRange(&tracker, base_, 5);
-  // Note: Last acked packet (the received one) does not belong to the stream,
-  // and is only there to make sure the feedback message is legal.
+  SendPackets(&tracker, base_, 5, 10);
   AddTransportFeedbackAndValidate(&tracker, base_,
                                   {false, false, false, false, false, true});
   ValidatePacketLossStatistics(tracker, 1.0f, 0.0f);
@@ -248,9 +309,9 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, AllLost) {
 
 // Repeated reports are ignored.
 TEST_P(TransportFeedbackPacketLossTrackerTest, ReportRepetition) {
-  TransportFeedbackPacketLossTracker tracker(10, 5, 4);
+  TransportFeedbackPacketLossTracker tracker(5000, 5, 4);
 
-  SendPacketRange(&tracker, base_, 5);
+  SendPackets(&tracker, base_, 5, 10);
 
   // Expected window contents: [] -> [10011].
   AddTransportFeedbackAndValidate(&tracker, base_,
@@ -266,9 +327,9 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, ReportRepetition) {
 
 // Report overlap.
 TEST_P(TransportFeedbackPacketLossTrackerTest, ReportOverlap) {
-  TransportFeedbackPacketLossTracker tracker(10, 5, 1);
+  TransportFeedbackPacketLossTracker tracker(5000, 5, 1);
 
-  SendPacketRange(&tracker, base_, 15);
+  SendPackets(&tracker, base_, 15, 10);
 
   // Expected window contents: [] -> [10011].
   AddTransportFeedbackAndValidate(&tracker, base_,
@@ -283,9 +344,9 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, ReportOverlap) {
 
 // Report conflict.
 TEST_P(TransportFeedbackPacketLossTrackerTest, ReportConflict) {
-  TransportFeedbackPacketLossTracker tracker(10, 5, 4);
+  TransportFeedbackPacketLossTracker tracker(5000, 5, 4);
 
-  SendPacketRange(&tracker, base_, 15);
+  SendPackets(&tracker, base_, 15, 10);
 
   // Expected window contents: [] -> [01001].
   AddTransportFeedbackAndValidate(&tracker, base_,
@@ -300,9 +361,9 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, ReportConflict) {
 
 // Skipped packets treated as unknown (not lost).
 TEST_P(TransportFeedbackPacketLossTrackerTest, SkippedPackets) {
-  TransportFeedbackPacketLossTracker tracker(10, 5, 1);
+  TransportFeedbackPacketLossTracker tracker(200 * 10, 5, 1);
 
-  SendPacketRange(&tracker, base_, 200);
+  SendPackets(&tracker, base_, 200, 10);
 
   // Expected window contents: [] -> [10011].
   AddTransportFeedbackAndValidate(&tracker, base_,
@@ -314,56 +375,50 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, SkippedPackets) {
   ValidatePacketLossStatistics(tracker, 3.0f / 8.0f, 2.0f / 6.0f);
 }
 
-// The window retains information up to the configured max-window-size, but
-// starts discarding after that. (Sent packets are not counted.)
-TEST_P(TransportFeedbackPacketLossTrackerTest, MaxWindowSize) {
-  TransportFeedbackPacketLossTracker tracker(10, 10, 1);
-
-  SendPacketRange(&tracker, base_, 200);
-
-  // Up to max-window-size retained.
-  // Expected window contents: [] -> [1010100001].
-  AddTransportFeedbackAndValidate(
-      &tracker, base_,
-      {true, false, true, false, true, false, false, false, false, true});
-  ValidatePacketLossStatistics(tracker, 6.0f / 10.0f, 3.0f / 9.0f);
-
-  // After max-window-size, older entries discarded to accommodate newer ones.
-  // Expected window contents: [1010100001] -> [0000110111].
-  AddTransportFeedbackAndValidate(&tracker, base_ + 10,
-                                  {true, false, true, true, true});
-  ValidatePacketLossStatistics(tracker, 5.0f / 10.0f, 2.0f / 9.0f);
-}
-
-// Inserting a feedback into the middle of a full window works correctly.
-TEST_P(TransportFeedbackPacketLossTrackerTest, InsertIntoMiddle) {
-  TransportFeedbackPacketLossTracker tracker(10, 5, 1);
+// Moving a window, if it excludes some old acked messages, can leave
+// in-window unacked messages intact, and ready to be used later.
+TEST_P(TransportFeedbackPacketLossTrackerTest, MovedWindowRetainsRelevantInfo) {
+  constexpr int64_t max_window_size_ms = 100;
+  TransportFeedbackPacketLossTracker tracker(max_window_size_ms, 5, 1);
 
-  SendPacketRange(&tracker, base_, 300);
+  // Note: All messages in this test are sent 1ms apart from each other.
+  // Therefore, the delta in sequence numbers equals the timestamps delta.
+  SendPackets(&tracker, base_, 4 * max_window_size_ms, 1);
 
   // Expected window contents: [] -> [10101].
   AddTransportFeedbackAndValidate(&tracker, base_,
                                   {true, false, true, false, true});
   ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 2.0f / 4.0f);
 
-  // Expected window contents: [10101] -> [10101-GAP-10001].
-  AddTransportFeedbackAndValidate(&tracker, base_ + 100,
-                                  {true, false, false, false, true});
-  ValidatePacketLossStatistics(tracker, 5.0f / 10.0f, 3.0f / 8.0f);
+  // Expected window contents: [10101] -> [100011].
+  const int64_t moved_oldest_acked = base_ + 2 * max_window_size_ms;
+  const std::vector<bool> feedback = {true, false, false, false, true, true};
+  AddTransportFeedbackAndValidate(&tracker, moved_oldest_acked, feedback);
+  ValidatePacketLossStatistics(tracker, 3.0f / 6.0f, 1.0f / 5.0f);
 
-  // Insert into the middle of this full window - it discards the older data.
-  // Expected window contents: [10101-GAP-10001] -> [11111-GAP-10001].
-  AddTransportFeedbackAndValidate(&tracker, base_ + 50,
-                                  {true, true, true, true, true});
-  ValidatePacketLossStatistics(tracker, 3.0f / 10.0f, 1.0f / 8.0f);
+  // Having acked |feedback.size()| starting with |moved_oldest_acked|, the
+  // newest of the acked ones is now:
+  const int64_t moved_newest_acked = moved_oldest_acked + feedback.size() - 1;
+
+  // Messages that *are* more than the span-limit away from the newest
+  // acked message *are* too old. Acking them would have no effect.
+  AddTransportFeedbackAndValidate(
+      &tracker, moved_newest_acked - max_window_size_ms - 1, {true});
+  ValidatePacketLossStatistics(tracker, 3.0f / 6.0f, 1.0f / 5.0f);
+
+  // Messages that are *not* more than the span-limit away from the newest
+  // acked message are *not* too old. Acking them would have an effect.
+  AddTransportFeedbackAndValidate(
+      &tracker, moved_newest_acked - max_window_size_ms, {true});
+  ValidatePacketLossStatistics(tracker, 3.0f / 7.0f, 1.0f / 5.0f);
 }
 
-// Inserting feedback into the middle of a full window works correctly - can
+// Inserting feedback into the middle of a window works correctly - can
 // complete two pairs.
 TEST_P(TransportFeedbackPacketLossTrackerTest, InsertionCompletesTwoPairs) {
-  TransportFeedbackPacketLossTracker tracker(15, 5, 1);
+  TransportFeedbackPacketLossTracker tracker(1500, 5, 1);
 
-  SendPacketRange(&tracker, base_, 300);
+  SendPackets(&tracker, base_, 15, 10);
 
   // Expected window contents: [] -> [10111].
   AddTransportFeedbackAndValidate(&tracker, base_,
@@ -376,164 +431,29 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, InsertionCompletesTwoPairs) {
   ValidatePacketLossStatistics(tracker, 3.0f / 10.0f, 3.0f / 8.0f);
 
   // Insert in between, closing the gap completely.
-  // Expected window contents: [10111-GAP-10101] -> [101111010101].
+  // Expected window contents: [10111-GAP-10101] -> [101110110101].
   AddTransportFeedbackAndValidate(&tracker, base_ + 5, {false, true});
   ValidatePacketLossStatistics(tracker, 4.0f / 12.0f, 4.0f / 11.0f);
 }
 
-// The window can meaningfully hold up to 0x8000 SENT packets (of which only
-// up to max-window acked messages will be kept and regarded).
-TEST_P(TransportFeedbackPacketLossTrackerTest, SecondQuadrant) {
-  TransportFeedbackPacketLossTracker tracker(20, 5, 1);
-
-  SendPacketRange(&tracker, base_, 0x8000, false);
-
-  // Expected window contents: [] -> [10011].
-  AddTransportFeedbackAndValidate(&tracker, base_,
-                                  {true, false, false, true, true});
-  ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f);
-
-  // Window *does* get updated with inputs from quadrant #2.
-  // Expected window contents: [10011] -> [100111].
-  AddTransportFeedbackAndValidate(&tracker, base_ + 0x4321, {true});
-  ValidatePacketLossStatistics(tracker, 2.0f / 6.0f, 1.0f / 4.0f);
-
-  // Correct recognition of quadrant #2: up to, but not including, base_ +
-  // 0x8000
-  // Expected window contents: [100111] -> [1001111].
-  AddTransportFeedbackAndValidate(&tracker, base_ + 0x7fff, {true});
-  ValidatePacketLossStatistics(tracker, 2.0f / 7.0f, 1.0f / 4.0f);
-}
-
-// After the base has moved due to insertion into the third quadrant, it is
-// still possible to get feedbacks in the middle of the window and obtain the
-// correct PLR and RPLR. Insertion into the middle before the max window size
-// has been achieved does not cause older packets to be dropped.
-TEST_P(TransportFeedbackPacketLossTrackerTest, InsertIntoMiddleAfterBaseMoved) {
-  TransportFeedbackPacketLossTracker tracker(20, 5, 1);
-
-  SendPacketRange(&tracker, base_, 20);
-  SendPacketRange(&tracker, base_ + 0x5000, 20);
-
-  // Expected window contents: [] -> [1001101].
-  AddTransportFeedbackAndValidate(
-      &tracker, base_, {true, false, false, true, true, false, true});
-  ValidatePacketLossStatistics(tracker, 3.0f / 7.0f, 2.0f / 6.0f);
-
-  // Expected window contents: [1001101] -> [101-GAP-1001].
-  SendPacketRange(&tracker, base_ + 0x8000, 4);
-  AddTransportFeedbackAndValidate(&tracker, base_ + 0x8000,
-                                  {true, false, false, true});
-  ValidatePacketLossStatistics(tracker, 3.0f / 7.0f, 2.0f / 5.0f);
-
-  // Inserting into the middle still works after the base has shifted.
-  // Expected window contents:
-  // [101-GAP-1001] -> [101-GAP-100101-GAP-1001]
-  AddTransportFeedbackAndValidate(&tracker, base_ + 0x5000,
-                                  {true, false, false, true, false, true});
-  ValidatePacketLossStatistics(tracker, 6.0f / 13.0f, 4.0f / 10.0f);
-
-  // The base can keep moving after inserting into the middle.
-  // Expected window contents:
-  // [101-GAP-100101-GAP-1001] -> [1-GAP-100101-GAP-100111].
-  SendPacketRange(&tracker, base_ + 0x8000 + 4, 2);
-  AddTransportFeedbackAndValidate(&tracker, base_ + 0x8000 + 4, {true, true});
-  ValidatePacketLossStatistics(tracker, 5.0f / 13.0f, 3.0f / 10.0f);
-}
-
-// After moving the base of the window, the max window size is still observed.
-TEST_P(TransportFeedbackPacketLossTrackerTest,
-       MaxWindowObservedAfterBaseMoved) {
-  TransportFeedbackPacketLossTracker tracker(15, 10, 1);
-
-  // Expected window contents: [] -> [1001110101].
-  SendPacketRange(&tracker, base_, 10);
-  AddTransportFeedbackAndValidate(
-      &tracker, base_,
-      {true, false, false, true, true, true, false, true, false, true});
-  ValidatePacketLossStatistics(tracker, 4.0f / 10.0f, 3.0f / 9.0f);
-
-  // Create gap (on both sides).
-  SendPacketRange(&tracker, base_ + 0x4000, 20);
-
-  // Expected window contents: [1001110101] -> [1110101-GAP-101].
-  SendPacketRange(&tracker, base_ + 0x8000, 3);
-  AddTransportFeedbackAndValidate(&tracker, base_ + 0x8000,
-                                  {true, false, true});
-  ValidatePacketLossStatistics(tracker, 3.0f / 10.0f, 3.0f / 8.0f);
-
-  // Push into middle until max window is reached. The gap is NOT completed.
-  // Expected window contents:
-  // [1110101-GAP-101] -> [1110101-GAP-10001-GAP-101]
-  AddTransportFeedbackAndValidate(&tracker, base_ + 0x4000 + 2,
-                                  {true, false, false, false, true});
-  ValidatePacketLossStatistics(tracker, 6.0f / 15.0f, 4.0f / 12.0f);
-
-  // Pushing new packets into the middle would discard older packets.
-  // Expected window contents:
-  // [1110101-GAP-10001-GAP-101] -> [0101-GAP-10001101-GAP-101]
-  AddTransportFeedbackAndValidate(&tracker, base_ + 0x4000 + 2 + 5,
-                                  {true, false, true});
-  ValidatePacketLossStatistics(tracker, 7.0f / 15.0f, 5.0f / 12.0f);
-}
-
-// A packet with a new enough sequence number might shift enough old feedbacks
-// out  window, that we'd go back to an unknown PLR and RPLR.
-TEST_P(TransportFeedbackPacketLossTrackerTest, NewPacketMovesWindowBase) {
-  TransportFeedbackPacketLossTracker tracker(20, 5, 3);
-
-  SendPacketRange(&tracker, base_, 50);
-  SendPacketRange(&tracker, base_ + 0x4000 - 1, 6);  // Gap
-
-  // Expected window contents: [] -> [1001110101].
-  AddTransportFeedbackAndValidate(
-      &tracker, base_,
-      {true, false, false, true, true, true, false, true, false, true});
-  ValidatePacketLossStatistics(tracker, 4.0f / 10.0f, 3.0f / 9.0f);
-
-  // A new sent packet with a new enough sequence number could shift enough
-  // acked packets out of window, that we'd go back to an unknown PLR
-  // and RPLR. This *doesn't* // necessarily mean all of the old ones
-  // were discarded, though.
-  // Expected window contents: [1001110101] -> [01].
-  SendPacketRange(&tracker, base_ + 0x8006, 2);
-  ValidatePacketLossStatistics(tracker,
-                               rtc::Optional<float>(),  // Still invalid.
-                               rtc::Optional<float>());
-
-  // Even if those messages are acked, we'd still might be in unknown PLR
-  // and RPLR, because we might have shifted more packets out of the window
-  // than we have inserted.
-  // Expected window contents: [01] -> [01-GAP-11].
-  AddTransportFeedbackAndValidate(&tracker, base_ + 0x8006, {true, true});
-  ValidatePacketLossStatistics(tracker,
-                               rtc::Optional<float>(),  // Still invalid.
-                               rtc::Optional<float>());
-
-  // Inserting in the middle shows that though some of the elements were
-  // ejected, some were retained.
-  // Expected window contents: [01-GAP-11] -> [01-GAP-1001-GAP-11].
-  AddTransportFeedbackAndValidate(&tracker, base_ + 0x4000,
-                                  {true, false, false, true});
-  ValidatePacketLossStatistics(tracker, 3.0f / 8.0f, 2.0f / 5.0f);
-}
-
 // Sequence number gaps are not gaps in reception. However, gaps in reception
 // are still possible, if a packet which WAS sent on the stream is not acked.
 TEST_P(TransportFeedbackPacketLossTrackerTest, SanityGapsInSequenceNumbers) {
-  TransportFeedbackPacketLossTracker tracker(20, 5, 1);
+  TransportFeedbackPacketLossTracker tracker(500, 5, 1);
 
-  SendPackets(&tracker, {static_cast<uint16_t>(base_),
-                         static_cast<uint16_t>(base_ + 2),
-                         static_cast<uint16_t>(base_ + 4),
-                         static_cast<uint16_t>(base_ + 6),
-                         static_cast<uint16_t>(base_ + 8)});
+  SendPackets(&tracker,
+              {static_cast<uint16_t>(base_),
+               static_cast<uint16_t>(base_ + 2),
+               static_cast<uint16_t>(base_ + 4),
+               static_cast<uint16_t>(base_ + 6),
+               static_cast<uint16_t>(base_ + 8)},
+              10);
 
   // Gaps in sequence numbers not considered as gaps in window, because  only
   // those sequence numbers which were associated with the stream count.
   // Expected window contents: [] -> [11011].
   AddTransportFeedbackAndValidate(
-      // Note: Left packets belong to this stream, odd ones ignored.
+      // Note: Left packets belong to this stream, right ones ignored.
       &tracker, base_, {true, false,
                         true, false,
                         false, false,
@@ -544,88 +464,136 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, SanityGapsInSequenceNumbers) {
   // Create gap by sending [base + 10] but not acking it.
   // Note: Acks for [base + 11] and [base + 13] ignored (other stream).
   // Expected window contents: [11011] -> [11011-GAP-01].
-  SendPackets(&tracker, {static_cast<uint16_t>(base_ + 10),
-                         static_cast<uint16_t>(base_ + 12),
-                         static_cast<uint16_t>(base_ + 14)});
+  SendPackets(&tracker,
+              {static_cast<uint16_t>(base_ + 10),
+               static_cast<uint16_t>(base_ + 12),
+               static_cast<uint16_t>(base_ + 14)},
+              10);
   AddTransportFeedbackAndValidate(&tracker, base_ + 11,
                                   {false, false, false, true, true});
   ValidatePacketLossStatistics(tracker, 2.0f / 7.0f, 2.0f / 5.0f);
 }
 
-// Sending a packet which is less than 0x8000 away from the baseline does
-// not move the window.
-TEST_P(TransportFeedbackPacketLossTrackerTest,
-       UnackedInWindowDoesNotMoveWindow) {
-  TransportFeedbackPacketLossTracker tracker(5, 3, 1);
+// The window cannot span more than 0x8000 in sequence numbers, regardless
+// of time stamps and ack/unacked status.
+TEST_P(TransportFeedbackPacketLossTrackerTest, MaxUnackedPackets) {
+  TransportFeedbackPacketLossTracker tracker(0x10000, 4, 1);
 
-  // Baseline - window has acked messages.
-  // Expected window contents: [] -> [10101].
-  SendPacketRange(&tracker, base_, 5);
+  SendPackets(&tracker, base_, 0x2000, 1, false);
+
+  // Expected window contents: [] -> [10011].
   AddTransportFeedbackAndValidate(&tracker, base_,
-                                  {true, false, true, false, true});
-  ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 2.0f / 4.0f);
+                                  {true, false, false, true, true});
+  ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f);
 
-  // Test - window not moved.
-  // Expected window contents: [10101] -> [10101].
-  SendPackets(&tracker, {static_cast<uint16_t>(base_ + 0x7fff)});
-  ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 2.0f / 4.0f);
+  // Sending more unacked packets, up to 0x7fff from the base, does not
+  // move the window or discard any information.
+  SendPackets(&tracker, static_cast<uint16_t>(base_ + 0x8000 - 0x2000), 0x2000,
+              1, false);
+  ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f);
+
+  // Sending more unacked packets, up to 0x7fff from the base, does not
+  // move the window or discard any information.
+  // Expected window contents: [10011] -> [0011].
+  SendPackets(&tracker, static_cast<uint16_t>(base_ + 0x8000), 1, 1);
+  ValidatePacketLossStatistics(tracker, 2.0f / 4.0f, 1.0f / 3.0f);
 }
 
-// Sending a packet which is at least 0x8000 away from the baseline, but not
-// 0x8000 or more away from the newest packet in the window, moves the window,
-// but does not reset it.
-TEST_P(TransportFeedbackPacketLossTrackerTest, UnackedOutOfWindowMovesWindow) {
-  TransportFeedbackPacketLossTracker tracker(5, 3, 1);
+// The window holds acked packets up until the difference in timestamps between
+// the oldest and newest reaches the configured maximum. Once this maximum
+// is exceeded, old packets are shifted out of window until the maximum is
+// once again observed.
+TEST_P(TransportFeedbackPacketLossTrackerTest, TimeDifferenceMaximumObserved) {
+  constexpr int64_t max_window_size_ms = 500;
+  TransportFeedbackPacketLossTracker tracker(max_window_size_ms, 3, 1);
+
+  // Note: All messages in this test are sent 1ms apart from each other.
+  // Therefore, the delta in sequence numbers equals the timestamps delta.
 
   // Baseline - window has acked messages.
-  // Expected window contents: [] -> [10101].
-  SendPacketRange(&tracker, base_, 5);
-  AddTransportFeedbackAndValidate(&tracker, base_,
-                                  {true, false, true, false, true});
+  // Expected window contents: [] -> [01101].
+  const std::vector<bool> feedback = {false, true, true, false, true};
+  SendPackets(&tracker, base_, feedback.size(), 1);
+  AddTransportFeedbackAndValidate(&tracker, base_, feedback);
   ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 2.0f / 4.0f);
 
-  // 0x8000 from baseline, but only 0x7ffc from newest - window moved.
-  // Expected window contents: [10101] -> [0101].
-  SendPackets(&tracker, {static_cast<uint16_t>(base_ + 0x8000)});
-  ValidatePacketLossStatistics(tracker, 2.0f / 4.0f, 2.0f / 3.0f);
-}
+  // Test - window base not moved.
+  // Expected window contents: [01101] -> [011011].
+  AdvanceClock(max_window_size_ms - feedback.size());
+  SendPackets(&tracker, static_cast<uint16_t>(base_ + feedback.size()), 1, 1);
+  AddTransportFeedbackAndValidate(
+      &tracker, static_cast<uint16_t>(base_ + feedback.size()), {true});
+  ValidatePacketLossStatistics(tracker, 2.0f / 6.0f, 2.0f / 5.0f);
 
-// TODO(elad.alon): More tests possible here, but a CL is in the pipeline which
-// significantly changes the entire class's operation (makes the window
-// time-based), so no sense in writing complicated UTs which will be replaced
-// very soon.
+  // Another packet, sent 1ms later, would already be too late. The window will
+  // be moved, but only after the ACK is received.
+  const uint16_t new_packet_seq_num =
+      static_cast<uint16_t>(base_ + feedback.size() + 1);
+  SendPackets(&tracker, {new_packet_seq_num}, 1);
+  ValidatePacketLossStatistics(tracker, 2.0f / 6.0f, 2.0f / 5.0f);
+  // Expected window contents: [011011] -> [110111].
+  AddTransportFeedbackAndValidate(&tracker, new_packet_seq_num, {true});
+  ValidatePacketLossStatistics(tracker, 1.0f / 6.0f, 1.0f / 5.0f);
+}
 
-// The window is reset by the sending of a packet which is 0x8000 or more
-// away from the newest packet.
-TEST_P(TransportFeedbackPacketLossTrackerTest, WindowResetAfterLongNoSend) {
-  TransportFeedbackPacketLossTracker tracker(10, 2, 1);
+TEST_P(TransportFeedbackPacketLossTrackerTest, RepeatedSeqNumResetsWindow) {
+  TransportFeedbackPacketLossTracker tracker(500, 2, 1);
 
   // Baseline - window has acked messages.
-  // Expected window contents: [] -> [1-GAP-10101].
-  SendPacketRange(&tracker, base_, 1);
-  SendPacketRange(&tracker, base_ + 0x7fff - 4, 5);
-  AddTransportFeedbackAndValidate(&tracker, base_, {true});
-  AddTransportFeedbackAndValidate(&tracker, base_ + 0x7fff - 4,
-                                  {true, false, true, false, true});
-  ValidatePacketLossStatistics(tracker, 2.0f / 6.0f, 2.0f / 5.0f);
+  // Expected window contents: [] -> [01101].
+  SendPackets(&tracker, base_, 5, 10);
+  AddTransportFeedbackAndValidate(&tracker, base_,
+                                  {false, true, true, false, true});
+  ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 2.0f / 4.0f);
 
-  // Sent packet too new - the entire window is reset.
-  // Expected window contents: [1-GAP-10101] -> [].
-  SendPacketRange(&tracker, base_ + 0x7fff + 0x8000, 1);
+  // A reset occurs.
+  SendPackets(&tracker, static_cast<uint16_t>(base_ + 2), 1, 10);
   ValidatePacketLossStatistics(tracker,
                                rtc::Optional<float>(),
                                rtc::Optional<float>());
+}
+
+// The window is reset by the sending of a packet which is 0x8000 or more
+// away from the newest packet acked/unacked packet.
+TEST_P(TransportFeedbackPacketLossTrackerTest,
+       SendAfterLongSuspensionResetsWindow) {
+  TransportFeedbackPacketLossTracker tracker(500, 2, 1);
 
-  // To show it was really reset, prove show that acking the sent packet
-  // still leaves us at unknown, because no acked (or unacked) packets were
-  // left in the window.
-  // Expected window contents: [] -> [1].
-  AddTransportFeedbackAndValidate(&tracker, base_ + 0x7fff + 0x8000, {true});
+  // Baseline - window has acked messages.
+  // Expected window contents: [] -> [01101].
+  SendPackets(&tracker, base_, 5, 10);
+  AddTransportFeedbackAndValidate(&tracker, base_,
+                                  {false, true, true, false, true});
+  ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 2.0f / 4.0f);
+
+  // A reset occurs.
+  SendPackets(&tracker, static_cast<uint16_t>(base_ + 5 + 0x8000), 1, 10);
   ValidatePacketLossStatistics(tracker,
                                rtc::Optional<float>(),
                                rtc::Optional<float>());
 }
 
+#if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
+TEST(TransportFeedbackPacketLossTrackerTest, InvalidConfigMaxWindowSize) {
+  EXPECT_DEATH(TransportFeedbackPacketLossTracker tracker(0, 20, 10), "");
+}
+
+TEST(TransportFeedbackPacketLossTrackerTest, InvalidConfigPlrMinAcked) {
+  EXPECT_DEATH(TransportFeedbackPacketLossTracker tracker(5000, 0, 10), "");
+}
+
+TEST(TransportFeedbackPacketLossTrackerTest, InvalidConfigRplrMinPairs) {
+  EXPECT_DEATH(TransportFeedbackPacketLossTracker tracker(5000, 20, 0), "");
+}
+
+TEST(TransportFeedbackPacketLossTrackerTest, TimeCantFlowBackwards) {
+  TransportFeedbackPacketLossTracker tracker(5000, 2, 1);
+  tracker.OnPacketAdded(100, 0);
+  tracker.OnPacketAdded(101, 2);
+  EXPECT_DEATH(tracker.OnPacketAdded(102, 1), "");
+}
+#endif
+
 // All tests are run multiple times with various baseline sequence number,
 // to weed out potential bugs with wrap-around handling.
 constexpr uint16_t kBases[] = {0x0000, 0x3456, 0xc032, 0xfffe};