Packet Loss Tracker - Stream Separation

webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc

 /*
  *  Copyright (c) 2017 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include <limits>
 #include <memory>
+#include <numeric>
 #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;
 using testing::Return;
 using testing::StrictMock;
 
 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 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) {
+      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 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)
@@ skipping 54 matching lines @@
 TEST(TransportFeedbackPacketLossTrackerTest, EmptyWindow) {
   std::unique_ptr<TransportFeedback> feedback;
   TransportFeedbackPacketLossTracker tracker(10, 5, 5);
 
   // PLR and RPLR reported as unknown before reception of first feedback.
   ValidatePacketLossStatistics(tracker,
                                rtc::Optional<float>(),
                                rtc::Optional<float>());
 }
 
+// A feedback received for an empty window has no effect.
+TEST(TransportFeedbackPacketLossTrackerTest, EmptyWindowFeedback) {
+  for (uint16_t base : kBases) {
+    std::unique_ptr<TransportFeedback> feedback;
+    TransportFeedbackPacketLossTracker tracker(3, 3, 2);
+
+    // Feedback doesn't correspond to any packets - ignored.
+    AddTransportFeedbackAndValidate(&tracker, base, {true, false, true});
+    ValidatePacketLossStatistics(tracker,
+                                 rtc::Optional<float>(),
+                                 rtc::Optional<float>());
+
+    // After the packets are transmitted, acking them would have an effect.
+    SendPacketRange(&tracker, base, 3);
+    AddTransportFeedbackAndValidate(&tracker, base, {true, false, true});
+    ValidatePacketLossStatistics(tracker, 1.0 / 3.0, 0.5);
+  }
+}
+
 // Sanity check on partially filled window.
 TEST(TransportFeedbackPacketLossTrackerTest, PlrPartiallyFilledWindow) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(10, 5, 4);
 
     // PLR unknown before minimum window size reached.
     // RPLR unknown before minimum pairs reached.
     // Expected window contents: [] -> [1001].
     AddTransportFeedbackAndValidate(&tracker, base, {true, false, false, true});
     ValidatePacketLossStatistics(tracker,
                                  rtc::Optional<float>(),
                                  rtc::Optional<float>());
   }
 }
 
 // Sanity check on minimum filled window - PLR known, RPLR unknown.
 TEST(TransportFeedbackPacketLossTrackerTest, PlrMinimumFilledWindow) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(10, 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);
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {true, false, false, true, true});
     ValidatePacketLossStatistics(tracker,
                                  rtc::Optional<float>(2.0f / 5.0f),
                                  rtc::Optional<float>());
   }
 }
 
 // Sanity check on minimum filled window - PLR unknown, RPLR known.
 TEST(TransportFeedbackPacketLossTrackerTest, RplrMinimumFilledWindow) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(10, 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);
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {true, false, false, true, true});
     ValidatePacketLossStatistics(tracker,
                                  rtc::Optional<float>(),
                                  rtc::Optional<float>(1.0f / 4.0f));
   }
 }
 
 // Additional reports update PLR and RPLR.
 TEST(TransportFeedbackPacketLossTrackerTest, ExtendWindow) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(20, 5, 5);
 
+    SendPacketRange(&tracker, base, 25);
+
     // Expected window contents: [] -> [10011].
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {true, false, false, true, true});
     ValidatePacketLossStatistics(tracker,
                                  rtc::Optional<float>(2.0f / 5.0f),
                                  rtc::Optional<float>());
 
     // Expected window contents: [10011] -> [1001110101].
     AddTransportFeedbackAndValidate(&tracker, base + 5,
                                     {true, false, true, false, true});
     ValidatePacketLossStatistics(tracker, 4.0f / 10.0f, 3.0f / 9.0f);
 
     // Expected window contents: [1001110101] -> [1001110101-GAP-10001].
     AddTransportFeedbackAndValidate(&tracker, base + 20,
                                     {true, false, false, false, true});
     ValidatePacketLossStatistics(tracker, 7.0f / 15.0f, 4.0f / 13.0f);
   }
 }
 
-// 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].
+    SendPacketRange(&tracker, base, 5);
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {true, true, true, true, true});
     ValidatePacketLossStatistics(tracker, 0.0f, 0.0f);
   }
 }
 
 // Repeated reports are ignored.
 TEST(TransportFeedbackPacketLossTrackerTest, ReportRepetition) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(10, 5, 4);
 
+    SendPacketRange(&tracker, base, 5);
+
     // Expected window contents: [] -> [10011].
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {true, false, false, true, true});
     ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f);
 
     // Repeat entire previous feedback
     // Expected window contents: [10011] -> [10011].
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {true, false, false, true, true});
     ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f);
   }
 }
 
 // Report overlap.
 TEST(TransportFeedbackPacketLossTrackerTest, ReportOverlap) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(10, 5, 1);
 
+    SendPacketRange(&tracker, base, 15);
+
     // Expected window contents: [] -> [10011].
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {true, false, false, true, true});
     ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f);
 
     // Expected window contents: [10011] -> [1001101].
     AddTransportFeedbackAndValidate(&tracker, base + 3,
                                     {true, true, false, true});
     ValidatePacketLossStatistics(tracker, 3.0f / 7.0f, 2.0f / 6.0f);
   }
 }
 
 // Report conflict.
 TEST(TransportFeedbackPacketLossTrackerTest, ReportConflict) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(10, 5, 4);
 
+    SendPacketRange(&tracker, base, 15);
+
     // Expected window contents: [] -> [01001].
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {false, true, false, false, true});
     ValidatePacketLossStatistics(tracker, 3.0f / 5.0f, 2.0f / 4.0f);
 
     // Expected window contents: [01001] -> [11101].
     // While false->true will be applied, true -> false will be ignored.
     AddTransportFeedbackAndValidate(&tracker, base, {true, false, true});
     ValidatePacketLossStatistics(tracker, 1.0f / 5.0f, 1.0f / 4.0f);
   }
 }
 
 // Skipped packets treated as unknown (not lost).
 TEST(TransportFeedbackPacketLossTrackerTest, SkippedPackets) {
   for (uint16_t base : kBases) {
     TransportFeedbackPacketLossTracker tracker(10, 5, 1);
 
+    SendPacketRange(&tracker, base, 200);
+
     // Expected window contents: [] -> [10011].
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {true, false, false, true, true});
     ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f);
 
     // Expected window contents: [10011] -> [10011-GAP-101].
     AddTransportFeedbackAndValidate(&tracker, base + 100, {true, false, true});
     ValidatePacketLossStatistics(tracker, 3.0f / 8.0f, 2.0f / 6.0f);
   }
 }
 
 // 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);
 
+    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 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);
 
+    SendPacketRange(&tracker, base, 300);
+
     // 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);
 
     // 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);
   }
 }
 
-// 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);
 
+    SendPacketRange(&tracker, base, 300);
+
     // Expected window contents: [] -> [10111].
     AddTransportFeedbackAndValidate(&tracker, base,
                                     {true, false, true, true, true});
     ValidatePacketLossStatistics(tracker, 1.0f / 5.0f, 1.0f / 4.0f);
 
     // Expected window contents: [10111] -> [10111-GAP-10101].
     AddTransportFeedbackAndValidate(&tracker, base + 7,
                                     {true, false, true, false, true});
     ValidatePacketLossStatistics(tracker, 3.0f / 10.0f, 3.0f / 8.0f);
 
     // Insert in between, closing the gap completely.
     // Expected window contents: [10111-GAP-10101] -> [101111010101].
     AddTransportFeedbackAndValidate(&tracker, base + 5, {false, true});
     ValidatePacketLossStatistics(tracker, 4.0f / 12.0f, 4.0f / 11.0f);
   }
 }
 
-// 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);
 
+    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);
   }
 }
 
-// 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.
+    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(TransportFeedbackPacketLossTrackerTest, ThirdQuadrantObservesMaxWindow) {
+TEST(TransportFeedbackPacketLossTrackerTest, MaxWindowObservedAfterBaseMoved) {
   for (uint16_t base : kBases) {
     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.
+    // 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);
+
+    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 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].
+    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>(1.0f / 2.0f));
+                                 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);
   }
 }
 
-// Quadrant four reports ignored for up to kMaxConsecutiveOldReports times.
-TEST(TransportFeedbackPacketLossTrackerTest, QuadrantFourInitiallyIgnored) {
+// 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});
+    SendPackets(&tracker, {base, base + 2, base + 4, base + 6, base + 8});
 
-    // 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);
-    }
+    // 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.
+        &tracker, base, {true, false,
+                         true, false,
+                         false, false,
+                         true, false,
+                         true, true});
+    ValidatePacketLossStatistics(tracker, 1.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].
+    SendPackets(&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);
   }
 }
 
-// Receiving a packet from quadrant #1 resets the counter for quadrant #4.
-TEST(TransportFeedbackPacketLossTrackerTest, QuadrantFourCounterResetByQ1) {
+// Sending a packet which is less than 0x8000 away from the baseline does
+// not move the window.
+TEST(TransportFeedbackPacketLossTrackerTest, UnackedInWindowDoesNotMoveWindow) {
   for (uint16_t base : kBases) {
-    TransportFeedbackPacketLossTracker tracker(20, 5, 1);
+    TransportFeedbackPacketLossTracker tracker(5, 3, 1);
 
-    // Expected window contents: [] -> [10011].
+    // Baseline - window has acked messages.
+    // Expected window contents: [] -> [10101].
+    SendPacketRange(&tracker, base, 5);
     AddTransportFeedbackAndValidate(&tracker, base,
-                                    {true, false, false, true, true});
+                                    {true, false, true, false, true});
+    ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 2.0f / 4.0f);
 
-    // 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 #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);
-    }
+    // Test - window not moved.
+    // Expected window contents: [10101] -> [10101].
+    SendPackets(&tracker, {base + 0x7fff});
+    ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 2.0f / 4.0f);
   }
 }
 
-// Receiving a packet from quadrant #2 resets the counter for quadrant #4.
-TEST(TransportFeedbackPacketLossTrackerTest, QuadrantFourCounterResetByQ2) {
+// 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.

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

it looks like _2 include _1, since if a packet that is less than 0x8000 from the
newest does not trigger a reset, why should 0x8000 from baseline trigger a
reset.

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

Please note the difference between baseline (oldest acked/unacked) and the acked
newest packet.
There are three cases here:
* Distance from base <= 0x7fff - no reset. (UnackedOutOfWindowMovesWindow_1)
* Distance from base >= 0x8000, but distance from newest still <= 0x7fff - no
reset. (UnackedOutOfWindowMovesWindow_2)
* Distance from base implicitly >= 0x8000, because distance from newest >=
0x8000. (WindowResetAfterLongNoSend)

+TEST(TransportFeedbackPacketLossTrackerTest, UnackedOutOfWindowMovesWindow_1) {
   for (uint16_t base : kBases) {
-    TransportFeedbackPacketLossTracker tracker(20, 5, 1);
+    TransportFeedbackPacketLossTracker tracker(5, 3, 1);
 
-    // Expected window contents: [] -> [10011].
+    // Baseline - window has acked messages.
+    // Expected window contents: [] -> [10101].
+    SendPacketRange(&tracker, base, 5);
     AddTransportFeedbackAndValidate(&tracker, base,
-                                    {true, false, false, true, true});
+                                    {true, false, true, false, true});
+    ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 2.0f / 4.0f);
 
-    // 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);
-    }
+    // 0x8000 from baseline, but only 0x7ffc from newest - window moved.
+    // Expected window contents: [10101] -> [0101].
+    SendPackets(&tracker, {base + 0x8000});
+    ValidatePacketLossStatistics(tracker, 2.0f / 4.0f, 2.0f / 3.0f);
   }
 }
 
-// Receiving a packet from quadrant #3 resets the counter for quadrant #4.
-TEST(TransportFeedbackPacketLossTrackerTest, QuadrantFourCounterResetByQ3) {
+// Repeats previous test, but this time with the maximum distance from the
+// newest packet, that would still not trigger a reset.
+TEST(TransportFeedbackPacketLossTrackerTest, UnackedOutOfWindowMovesWindow_2) {
   for (uint16_t base : kBases) {
-    TransportFeedbackPacketLossTracker tracker(20, 5, 1);
+    TransportFeedbackPacketLossTracker tracker(10, 2, 1);
 
-    // Expected window contents: [] -> [1001110001].
-    AddTransportFeedbackAndValidate(
-        &tracker, base,
-        {true, false, false, true, true, true, false, false, false, true});
+    // Baseline - window has acked messages.
+    // Expected window contents: [] -> [1-GAP-10101].

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

This test may be made less complicated, how about

[10101] -> base + 5 + 0x8000 -> window moved. (like the previous one, which I
think can be removed, though)

If you find the test not covering enough cases, try divide it into small tests,
each of which is to one point.

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

I think it would be easier to clarify this offline. I'm available for VoIP
whenever is convenient for you.

[minyue-webrtc, 2017/02/20 12:49:06]

let's do

+    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);
 
-    // 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);
-    }
+    // Sending a packet that is less than 0x8000 from the newest does not
+    // reset the window, although it moves it.
+    // Expected window contents: [1-GAP-10101] -> [1].
+    SendPacketRange(&tracker, base + 0x7fff + 0x7fff, 1);
 
-    // 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);
-    }
+    // The does not have enough data for R/PLR to be valid.
+    ValidatePacketLossStatistics(tracker,
+                                 rtc::Optional<float>(),
+                                 rtc::Optional<float>());
+
+    // Adding more data proves the window was not reset; it still had one

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

proves -> to prove

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

I think it should be "proves". The action (singular) proves, not the data
(plural).

[minyue-webrtc, 2017/02/20 12:49:06]

not grammar. I mean the sentence is to indicates the purpose of the following
action. So it is actually "adding more data (in order) to prove ... "

The true problem is that "adding more data" cannot "prove the window was not
reset". It is "adding more data and verifying it still has one ..." that
"proves" ...

[elad.alon_webrtc.org, 2017/02/27 13:18:05]

Acknowledged.

+    // last acked datapoint saved when we sent the previous unacked packet.
+    // Expected window contents: [1] -> [11].
+    AddTransportFeedbackAndValidate(&tracker, base + 0x7fff + 0x7fff, {true});
+    ValidatePacketLossStatistics(tracker, 0, 0);
   }
 }
 
-// Quadrant four reports ignored for up to kMaxConsecutiveOldReports times.
-// After that, the window is reset.
-TEST(TransportFeedbackPacketLossTrackerTest, QuadrantFourReset) {
+// The window is reset by the sending of a packet which is 0x8000 or more
+// away from the newest packet.
+TEST(TransportFeedbackPacketLossTrackerTest, WindowResetByPacketsTooNew) {

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

TooNew may not be good. how about WindowResetAfterLongNoSend?

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

Done.

   for (uint16_t base : kBases) {
-    TransportFeedbackPacketLossTracker tracker(20, 5, 1);
+    TransportFeedbackPacketLossTracker tracker(10, 2, 1);
 
-    // Expected window contents: [] -> [1001110001].
-    AddTransportFeedbackAndValidate(
-        &tracker, base,
-        {true, false, false, true, true, true, false, false, false, true});
+    // 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);
 
-    // Sanity
-    ValidatePacketLossStatistics(tracker, 5.0f / 10.0f, 2.0f / 9.0f);
+    // Sent packet too new - the entire window is reset.
+    // Expected window contents: [1-GAP-10101] -> [].
+    SendPacketRange(&tracker, base + 0x7fff + 0x8000, 1);
+    ValidatePacketLossStatistics(tracker,
+                                 rtc::Optional<float>(),
+                                 rtc::Optional<float>());
 
-    // 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);
+    // 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});
+    ValidatePacketLossStatistics(tracker,
+                                 rtc::Optional<float>(),
+                                 rtc::Optional<float>());
   }
 }
 
-// Feedbacks spanning multiple quadrant are treated correctly (Q1-Q2).
-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);
-  }
-}
-
-// 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);
-  }
-}
-
 }  // namespace webrtc