Packet Loss Tracker - Stream Separation

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 eb229a09d2a5b2068a9579b7ee4811d9c2510925..1f45702e48246f038936c2386795d3c55e4a8874 100644
--- a/webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc
+++ b/webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc
@@ -27,12 +27,50 @@ namespace webrtc {
 
 namespace {
 
-constexpr size_t kMaxConsecutiveOldReports = 4;
-
 // 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};
 
+void SimulatePacketTransmission(TransportFeedbackPacketLossTracker* tracker,

[minyue-webrtc, 2017/02/14 16:46:59]

"Simulate" may not be needed since the test itself is simulate.

"Transmission" -> "Send" since the former sounds like a network thing

So generally, I would suggest "SendPacket"

[elad.alon_webrtc.org, 2017/02/15 16:47:36]

Done.

+                                uint16_t seq_num,
+                                bool with_validation = true) {
+  tracker->OnPacketAdded(seq_num);
+  if (with_validation) {
+    tracker->Validate();
+  }
+}
+
+void SimulatePacketTransmission(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) {
+      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 SimulatePacketTransmissionRange(
+    TransportFeedbackPacketLossTracker* tracker,
+    uint16_t first_seq_num,
+    size_t num_of_seq_nums,
+    bool validate_all = true) {
+  for (size_t i = 0; i < num_of_seq_nums; i++) {
+    SimulatePacketTransmission(tracker, first_seq_num + i, validate_all);

[minyue-webrtc, 2017/02/14 16:46:59]

can we use SimulatePacketTransmission ver.2 to avoid the need of
SimulatePacketTransmission ver.1

[elad.alon_webrtc.org, 2017/02/15 16:47:36]

Done.

+  }
+  // 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 AddTransportFeedbackAndValidate(
     TransportFeedbackPacketLossTracker* tracker,
     uint16_t base_sequence_num,
@@ -130,6 +168,7 @@ TEST(TransportFeedbackPacketLossTrackerTest, PlrMinimumFilledWindow) {
     // 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].
+    SimulatePacketTransmissionRange(&tracker, base, 5);
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {true, false, false, true, true});
     ValidatePacketLossStatistics(tracker,
@@ -146,6 +185,7 @@ TEST(TransportFeedbackPacketLossTrackerTest, RplrMinimumFilledWindow) {
     // RPLR correctly calculated after minimum pairs reached.
     // PLR not necessarily known at that time (not if min window not reached).
     // Expected window contents: [] -> [10011].
+    SimulatePacketTransmissionRange(&tracker, base, 5);
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {true, false, false, true, true});
     ValidatePacketLossStatistics(tracker,
@@ -159,6 +199,8 @@ TEST(TransportFeedbackPacketLossTrackerTest, ExtendWindow) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(20, 5, 5);
 
+    SimulatePacketTransmissionRange(&tracker, base, 25);
+
     // Expected window contents: [] -> [10011].
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {true, false, false, true, true});
@@ -178,13 +220,14 @@ TEST(TransportFeedbackPacketLossTrackerTest, ExtendWindow) {
   }
 }
 
-// All packets correctly received.
+// Sanity - all packets correctly received.
 TEST(TransportFeedbackPacketLossTrackerTest, AllReceived) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(10, 5, 4);
 
     // PLR and RPLR correctly calculated after minimum window size reached.
     // Expected window contents: [] -> [11111].
+    SimulatePacketTransmissionRange(&tracker, base, 5);
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {true, true, true, true, true});
     ValidatePacketLossStatistics(tracker, 0.0f, 0.0f);
@@ -196,6 +239,8 @@ TEST(TransportFeedbackPacketLossTrackerTest, ReportRepetition) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(10, 5, 4);
 
+    SimulatePacketTransmissionRange(&tracker, base, 5);
+
     // Expected window contents: [] -> [10011].
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {true, false, false, true, true});
@@ -214,6 +259,8 @@ TEST(TransportFeedbackPacketLossTrackerTest, ReportOverlap) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(10, 5, 1);
 
+    SimulatePacketTransmissionRange(&tracker, base, 15);
+
     // Expected window contents: [] -> [10011].
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {true, false, false, true, true});
@@ -231,6 +278,8 @@ TEST(TransportFeedbackPacketLossTrackerTest, ReportConflict) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(10, 5, 4);
 
+    SimulatePacketTransmissionRange(&tracker, base, 15);
+
     // Expected window contents: [] -> [01001].
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {false, true, false, false, true});
@@ -248,6 +297,8 @@ TEST(TransportFeedbackPacketLossTrackerTest, SkippedPackets) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(10, 5, 1);
 
+    SimulatePacketTransmissionRange(&tracker, base, 200);
+
     // Expected window contents: [] -> [10011].
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {true, false, false, true, true});
@@ -260,11 +311,13 @@ TEST(TransportFeedbackPacketLossTrackerTest, SkippedPackets) {
 }
 
 // The window retain information up to the configured max-window-size, but
-// starts discarding after that.
+// starts discarding after that. (Sent packets are not counted.)
 TEST(TransportFeedbackPacketLossTrackerTest, MaxWindowSize) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(10, 10, 1);
 
+    SimulatePacketTransmissionRange(&tracker, base, 200);
+
     // Up to max-window-size retained.
     // Expected window contents: [] -> [1010100001].
     AddTransportFeedbackAndValidate(
@@ -280,11 +333,13 @@ TEST(TransportFeedbackPacketLossTrackerTest, MaxWindowSize) {
   }
 }
 
-// Inserting into the middle of a full window works correctly.
+// Inserting feedback into the middle of a full window works correctly.
 TEST(TransportFeedbackPacketLossTrackerTest, InsertIntoMiddle) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(10, 5, 1);
 
+    SimulatePacketTransmissionRange(&tracker, base, 300);
+
     // Expected window contents: [] -> [10101].
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {true, false, true, false, true});
@@ -303,11 +358,14 @@ TEST(TransportFeedbackPacketLossTrackerTest, InsertIntoMiddle) {
   }
 }
 
-// Inserting into the middle of a full window works correctly.
+// Inserting feedback into the middle of a full window works correctly - can
+// complete two pairs.
 TEST(TransportFeedbackPacketLossTrackerTest, InsertionCompletesTwoPairs) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(15, 5, 1);
 
+    SimulatePacketTransmissionRange(&tracker, base, 300);
+
     // Expected window contents: [] -> [10111].
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {true, false, true, true, true});
@@ -325,14 +383,14 @@ TEST(TransportFeedbackPacketLossTrackerTest, InsertionCompletesTwoPairs) {
   }
 }
 
-// Entries in the second quadrant treated like those in the first.
-// The sequence number is used in a looped manner. 0xFFFF is followed by 0x0000.
-// In many tests, we divide the circle of sequence number into 4 quadrants, and
-// verify the behavior of TransportFeedbackPacketLossTracker over them.
+// The window can meaningfully hold up to 0x8000 SENT packetts (of which only
+// up to max-window acked messages will be kept and regarded).
 TEST(TransportFeedbackPacketLossTrackerTest, SecondQuadrant) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(20, 5, 1);
 
+    SimulatePacketTransmissionRange(&tracker, base, 0x8000, false);
+
     // Expected window contents: [] -> [10011].
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {true, false, false, true, true});
@@ -351,48 +409,24 @@ TEST(TransportFeedbackPacketLossTrackerTest, SecondQuadrant) {
   }
 }
 
-// Insertion into the third quadrant moves the base of the window.
-TEST(TransportFeedbackPacketLossTrackerTest, ThirdQuadrantMovesBase) {
-  for (uint16_t base : kBases) {
-    TransportFeedbackPacketLossTracker tracker(20, 5, 1);
-
-    // Seed the test.
-    // Expected window contents: [] -> [1001101].
-    AddTransportFeedbackAndValidate(
-        &tracker, base, {true, false, false, true, true, false, true});
-    ValidatePacketLossStatistics(tracker, 3.0f / 7.0f, 2.0f / 6.0f);
-
-    // Quadrant #3 begins at base + 0x8000. It triggers moving the window so
-    // that
-    // at least one (oldest) report shifts out of window.
-    // Expected window contents: [1001101] -> [101-GAP-1001].
-    AddTransportFeedbackAndValidate(&tracker, base + 0x8000,
-                                    {true, false, false, true});
-    ValidatePacketLossStatistics(tracker, 3.0f / 7.0f, 2.0f / 5.0f);
-
-    // The base can move more than once, because the minimum quadrant-1 packets
-    // were dropped out of the window, and some remain.
-    // Expected window contents: [101-GAP-1001] -> [1-GAP-100111].
-    AddTransportFeedbackAndValidate(&tracker, base + 0x8000 + 4, {true, true});
-    ValidatePacketLossStatistics(tracker, 2.0f / 7.0f, 1.0f / 5.0f);
-  }
-}
-
 // After the base has moved due to insertion into the third quadrant, it is
-// still possible to insert into 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(TransportFeedbackPacketLossTrackerTest, InsertIntoMiddleAfterBaseMove) {
+// 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(TransportFeedbackPacketLossTrackerTest, InsertIntoMiddleAfterBaseMoved) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(20, 5, 1);
 
-    // Seed the test.
+    SimulatePacketTransmissionRange(&tracker, base, 20);
+    SimulatePacketTransmissionRange(&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].
+    SimulatePacketTransmissionRange(&tracker, base + 0x8000, 4);
     AddTransportFeedbackAndValidate(&tracker, base + 0x8000,
                                     {true, false, false, true});
     ValidatePacketLossStatistics(tracker, 3.0f / 7.0f, 2.0f / 5.0f);
@@ -407,48 +441,57 @@ TEST(TransportFeedbackPacketLossTrackerTest, InsertIntoMiddleAfterBaseMove) {
     // The base can keep moving after inserting into the middle.
     // Expected window contents:
     // [101-GAP-100101-GAP-1001] -> [1-GAP-100101-GAP-100111].
+    SimulatePacketTransmissionRange(&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(TransportFeedbackPacketLossTrackerTest, ThirdQuadrantObservesMaxWindow) {
+TEST(TransportFeedbackPacketLossTrackerTest, MaxWindowObservedAfterBaseMoved) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(15, 10, 1);
 
     // Expected window contents: [] -> [1001110101].
+    SimulatePacketTransmissionRange(&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).
+    SimulatePacketTransmissionRange(&tracker, base + 0x4000, 20);
+
     // Expected window contents: [1001110101] -> [1110101-GAP-101].
+    SimulatePacketTransmissionRange(&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.
+    // 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,
+    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 + 5,
+    AddTransportFeedbackAndValidate(&tracker, base + 0x4000 + 2 + 5,
                                     {true, false, true});
     ValidatePacketLossStatistics(tracker, 7.0f / 15.0f, 5.0f / 12.0f);
   }
 }
 
-// A new feedback in quadrant #3 might shift enough old feedbacks out of window,
-// that we'd go back to an unknown PLR and RPLR.
-TEST(TransportFeedbackPacketLossTrackerTest, QuadrantThreeMovedBaseMinWindow) {
+// 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(TransportFeedbackPacketLossTrackerTest, NewPacketMovesWindowBase) {
   for (uint16_t base : kBases) {
-    TransportFeedbackPacketLossTracker tracker(20, 5, 1);
+    TransportFeedbackPacketLossTracker tracker(20, 5, 3);
+
+    SimulatePacketTransmissionRange(&tracker, base, 50);
+    SimulatePacketTransmissionRange(&tracker, base + 0x4000 - 1, 6);  // Gap
 
     // Expected window contents: [] -> [1001110101].
     AddTransportFeedbackAndValidate(
@@ -456,14 +499,24 @@ TEST(TransportFeedbackPacketLossTrackerTest, QuadrantThreeMovedBaseMinWindow) {
         {true, false, false, true, true, true, false, true, false, true});
     ValidatePacketLossStatistics(tracker, 4.0f / 10.0f, 3.0f / 9.0f);
 
-    // A new feedback in quadrant #3 might shift enough old feedbacks 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-GAP-11].
+    // 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].
+    SimulatePacketTransmissionRange(&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>(1.0f / 2.0f));
+                                 rtc::Optional<float>());
 
     // Inserting in the middle shows that though some of the elements were
     // ejected, some were retained.
@@ -474,222 +527,35 @@ TEST(TransportFeedbackPacketLossTrackerTest, QuadrantThreeMovedBaseMinWindow) {
   }
 }
 
-// Quadrant four reports ignored for up to kMaxConsecutiveOldReports times.
-TEST(TransportFeedbackPacketLossTrackerTest, QuadrantFourInitiallyIgnored) {
-  for (uint16_t base : kBases) {
-    TransportFeedbackPacketLossTracker tracker(20, 5, 1);
-
-    // Expected window contents: [] -> [10011].
-    AddTransportFeedbackAndValidate(&tracker, base,
-                                    {true, false, false, true, true});
-
-    // Feedbacks in quadrant #4 are discarded (up to kMaxConsecutiveOldReports
-    // consecutive reports).
-    // Expected window contents: [10011] -> [10011].
-    for (size_t i = 0; i < kMaxConsecutiveOldReports; i++) {
-      AddTransportFeedbackAndValidate(&tracker, base + 0xc000, {true, true});
-      ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f);
-    }
-  }
-}
-
-// Receiving a packet from quadrant #1 resets the counter for quadrant #4.
-TEST(TransportFeedbackPacketLossTrackerTest, QuadrantFourCounterResetByQ1) {
+// 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(TransportFeedbackPacketLossTrackerTest, SanityGapsInSequenceNumbers) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(20, 5, 1);
 
-    // Expected window contents: [] -> [10011].
-    AddTransportFeedbackAndValidate(&tracker, base,
-                                    {true, false, false, true, true});
-
-    // Feedbacks in quadrant #4 are discarded (up to kMaxConsecutiveOldReports
-    // consecutive reports).
-    // Expected window contents: [10011] -> [10011].
-    for (size_t i = 0; i < kMaxConsecutiveOldReports; i++) {
-      AddTransportFeedbackAndValidate(&tracker, base + 0xc000, {true, true});
-      ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f);
-    }
+    SimulatePacketTransmission(
+        &tracker, {base, base + 2, base + 4, base + 6, base + 8});
 
-    // If we receive a feedback in quadrant #1, the above counter is reset.
-    // Expected window contents: [10011] -> [100111].
-    AddTransportFeedbackAndValidate(&tracker, base + 5, {true});
-    for (size_t i = 0; i < kMaxConsecutiveOldReports; i++) {
-      // Note: though the feedback message reports three packets, it only gets
-      // counted once.
-      AddTransportFeedbackAndValidate(&tracker, base + 0xc000,
-                                      {true, false, true});
-      ValidatePacketLossStatistics(tracker, 2.0f / 6.0f, 1.0f / 5.0f);
-    }
-
-    // The same is true for reports which create a gap - they still reset.
-    // Expected window contents: [10011] -> [100111-GAP-01].
-    AddTransportFeedbackAndValidate(&tracker, base + 0x00ff, {false, true});
-    for (size_t i = 0; i < kMaxConsecutiveOldReports; i++) {
-      // Note: though the feedback message reports three packets, it only gets
-      // counted once.
-      AddTransportFeedbackAndValidate(&tracker, base + 0xc000,
-                                      {true, false, true});
-      ValidatePacketLossStatistics(tracker, 3.0f / 8.0f, 2.0f / 6.0f);
-    }
-  }
-}
-
-// Receiving a packet from quadrant #2 resets the counter for quadrant #4.
-TEST(TransportFeedbackPacketLossTrackerTest, QuadrantFourCounterResetByQ2) {
-  for (uint16_t base : kBases) {
-    TransportFeedbackPacketLossTracker tracker(20, 5, 1);
-
-    // Expected window contents: [] -> [10011].
-    AddTransportFeedbackAndValidate(&tracker, base,
-                                    {true, false, false, true, true});
-
-    // Feedbacks in quadrant #4 are discarded (up to kMaxConsecutiveOldReports
-    // consecutive reports).
-    // Expected window contents: [10011] -> [10011].
-    for (size_t i = 0; i < kMaxConsecutiveOldReports; i++) {
-      AddTransportFeedbackAndValidate(&tracker, base + 0xc000, {true, true});
-      ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f);
-    }
-
-    // If we receive a feedback in quadrant #2, the above counter is reset.
-    // Expected window contents: [10011] -> [10011-GAP-11].
-    AddTransportFeedbackAndValidate(&tracker, base + 0x400f, {true, true});
-    for (size_t i = 0; i < kMaxConsecutiveOldReports; i++) {
-      // Note: though the feedback message reports three packets, it only gets
-      // counted once.
-      AddTransportFeedbackAndValidate(&tracker, base + 0xc000,
-                                      {true, false, true});
-      ValidatePacketLossStatistics(tracker, 2.0f / 7.0f, 1.0f / 5.0f);
-    }
-  }
-}
-
-// Receiving a packet from quadrant #3 resets the counter for quadrant #4.
-TEST(TransportFeedbackPacketLossTrackerTest, QuadrantFourCounterResetByQ3) {
-  for (uint16_t base : kBases) {
-    TransportFeedbackPacketLossTracker tracker(20, 5, 1);
-
-    // Expected window contents: [] -> [1001110001].
-    AddTransportFeedbackAndValidate(
-        &tracker, base,
-        {true, false, false, true, true, true, false, false, false, true});
-
-    // Feedbacks in quadrant #4 are discarded (up to kMaxConsecutiveOldReports
-    // consecutive reports).
-    // Expected window contents: [1001110001] -> [1001110001].
-    for (size_t i = 0; i < kMaxConsecutiveOldReports; i++) {
-      AddTransportFeedbackAndValidate(&tracker, base + 0xc000, {true, true});
-      ValidatePacketLossStatistics(tracker, 5.0f / 10.0f, 2.0f / 9.0f);
-    }
-
-    // If we receive a feedback in quadrant #1, the above counter is reset.
-    // Expected window contents: [1001110001] -> [1110001-GAP-111].
-    AddTransportFeedbackAndValidate(&tracker, base + 0x8000,
-                                    {true, true, true});
-    for (size_t i = 0; i < kMaxConsecutiveOldReports; i++) {
-      // Note: though the feedback message reports three packets, it only gets
-      // counted once.
-      AddTransportFeedbackAndValidate(&tracker, base + 0xc000 + 10,
-                                      {true, false, true});
-      ValidatePacketLossStatistics(tracker, 3.0f / 10.0f, 1.0f / 8.0f);
-    }
-  }
-}
-
-// Quadrant four reports ignored for up to kMaxConsecutiveOldReports times.
-// After that, the window is reset.
-TEST(TransportFeedbackPacketLossTrackerTest, QuadrantFourReset) {
-  for (uint16_t base : kBases) {
-    TransportFeedbackPacketLossTracker tracker(20, 5, 1);
-
-    // Expected window contents: [] -> [1001110001].
+    // 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(
-        &tracker, base,
-        {true, false, false, true, true, true, false, false, false, true});
-
-    // Sanity
-    ValidatePacketLossStatistics(tracker, 5.0f / 10.0f, 2.0f / 9.0f);
-
-    // The first kMaxConsecutiveOldReports quadrant #4 reports are ignored.
-    // It doesn't matter that they consist of multiple packets - each report
-    // is only counted once.
-    for (size_t i = 0; i < kMaxConsecutiveOldReports; i++) {
-      // Expected window contents: [1001110001] -> [1001110001].
-      AddTransportFeedbackAndValidate(&tracker, base + 0xc000,
-                                      {true, true, false, true});
-      ValidatePacketLossStatistics(tracker, 5.0f / 10.0f, 2.0f / 9.0f);
-    }
-
-    // One additional feedback in quadrant #4 brings us over
-    // kMaxConsecutiveOldReports consecutive "old" reports, resetting the
-    // window.
-    // The new window is not completely empty - it's been seeded with the
-    // packets reported in the feedback that has triggered the reset.
-    // Note: The report doesn't have to be the same as the previous ones.
-    // Expected window contents: [1001110001] -> [10011].
-    AddTransportFeedbackAndValidate(&tracker, base + 0xc000,
-                                    {true, false, false, true, true});
-    ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f);
-  }
-}
-
-// Feedbacks spanning multiple quadrant are treated correctly (Q1-Q2).

[minyue-webrtc, 2017/02/14 16:46:59]

why the tests below here also become obsolete?

[minyue-webrtc, 2017/02/16 09:51:44]

missed this comment?

[elad.alon_webrtc.org, 2017/02/16 15:51:40]

Yes, I'd missed the comment; thanks for pointing it out again.
These tests are no longer relevant.
* Before, we had no notion of unacked message in the window. When a new feedback
arrived, it could trigger moving the window in a way that would get old messages
forgotten (independently of the window being full or not).
* Now, we have the notion of unacked windows in the packet, and it's the
addition of new packets that could get old ones discarded. That being said, yes,
I see the sense in adding a new test where we get 0x8000 or more in between
unacked messages, and therefore reset the window.

[minyue-webrtc, 2017/02/17 11:09:14]

Ok. then, as you said, the removing of old packets for OnPacketAdded (which I
think it still be OnSentPacket) should be tested.

[elad.alon_webrtc.org, 2017/02/17 12:24:42]

Yes. The following UTs have been added (when reviewing, please note that they're
not all in one block, because one of them belongs elsewhere):
* EmptyWindowFeedback
* UnackedInWindowDoesNotMoveWindow
* UnackedOutOfWindowMovesWindow_1
* UnackedOutOfWindowMovesWindow_2
* WindowResetByPacketsTooNew

(I think the comments make the difference between
UnackedOutOfWindowMovesWindow_1 and UnackedOutOfWindowMovesWindow_2 clear, but
let me know if you'd prefer something else.)

-TEST(TransportFeedbackPacketLossTrackerTest, MultiQuadrantQ1Q2) {
-  for (uint16_t base : kBases) {
-    TransportFeedbackPacketLossTracker tracker(20, 5, 1);
-
-    // Expected window contents: [] -> [10011].
-    AddTransportFeedbackAndValidate(&tracker, base,
-                                    {true, false, false, true, true});
-    ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f);
-
-    // A feedback with entries in both quadrant #1 and #2 gets both counted:
-    // Expected window contents: [10011] -> [10011-GAP-1001].
-    AddTransportFeedbackAndValidate(&tracker, base + 0x3ffe,
-                                    {true, false, false, true});
-    ValidatePacketLossStatistics(tracker, 4.0f / 9.0f, 2.0f / 7.0f);
-  }
-}
-
-// Feedbacks spanning multiple quadrant are treated correctly (Q2-Q3).
-TEST(TransportFeedbackPacketLossTrackerTest, MultiQuadrantQ2Q3) {
-  for (uint16_t base : kBases) {
-    TransportFeedbackPacketLossTracker tracker(20, 5, 1);
-
-    // Expected window contents: [] -> [1001100001].
-    AddTransportFeedbackAndValidate(
-        &tracker, base,
-        {true, false, false, true, true, false, false, false, false, true});
-    ValidatePacketLossStatistics(tracker, 6.0f / 10.0f, 2.0f / 9.0f);
-
-    // A feedback with entries in both quadrant #2 and #3 gets both counted,
-    // but only those from #3 trigger throwing out old entries from quadrant #1:
-    // Expected window contents: [1001100001] -> [01100001-GAP-1001].
-    AddTransportFeedbackAndValidate(&tracker, base + 0x7ffe,
-                                    {true, false, false, true});
-    ValidatePacketLossStatistics(tracker, 7.0f / 12.0f, 3.0f / 10.0f);
-  }
-}
+        // Note: Left packets belong to this stream, odd ones ignored.
+        &tracker, base, {true, false,
+                         true, false,
+                         false, false,
+                         true, false,
+                         true, true});
+    ValidatePacketLossStatistics(tracker, 1.0f / 5.0f, 1.0f / 4.0f);
 
-// Feedbacks spanning multiple quadrant are treated correctly (Q3-Q4).
-TEST(TransportFeedbackPacketLossTrackerTest, MultiQuadrantQ3Q4) {
-  for (uint16_t base : kBases) {
-    TransportFeedbackPacketLossTracker tracker(20, 5, 1);
 
-    // Expected window contents: [] -> [1001100001].
-    AddTransportFeedbackAndValidate(
-        &tracker, base,
-        {true, false, false, true, true, false, false, false, false, true});
-    ValidatePacketLossStatistics(tracker, 6.0f / 10.0f, 2.0f / 9.0f);
-
-    // A feedback with entries in both quadrant #3 and #4 would have the entries
-    // from quadrant #3 shift enough quadrant #1 entries out of window, that
-    // by the time the #4 packets are examined, the moving baseline has made
-    // them into quadrant #3 packets.
-    // Expected window contents: [1001100001] -> [10011].
-    AddTransportFeedbackAndValidate(&tracker, base + 0xbfff,
-                                    {true, false, false, true, true});
-    ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f);
+    // 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].
+    SimulatePacketTransmission(&tracker, {base + 10, base + 12, base + 14});
+    AddTransportFeedbackAndValidate(&tracker, base + 11,
+                                    {false, false, false, true, true});
+    ValidatePacketLossStatistics(tracker, 2.0f / 7.0f, 2.0f / 5.0f);
   }
 }