R/PLR calculation - time-based window
BUG=webrtc:7058
Review-Url: https://codereview.webrtc.org/2711473003
Cr-Commit-Position: refs/heads/master@{#17251}
diff --git a/webrtc/test/fuzzers/transport_feedback_packet_loss_tracker_fuzzer.cc b/webrtc/test/fuzzers/transport_feedback_packet_loss_tracker_fuzzer.cc
index 5c3cfcb..c8e2619 100644
--- a/webrtc/test/fuzzers/transport_feedback_packet_loss_tracker_fuzzer.cc
+++ b/webrtc/test/fuzzers/transport_feedback_packet_loss_tracker_fuzzer.cc
@@ -109,16 +109,12 @@
constexpr size_t kSeqNumHalf = 0x8000u;
- // 0x8000 >= max_window_size >= plr_min_num_packets > rplr_min_num_pairs >= 1
- // (The distribution isn't uniform, but it's enough; more would be overkill.)
- const size_t max_window_size = FuzzInRange(data, size, 2, kSeqNumHalf);
- const size_t plr_min_num_packets =
- FuzzInRange(data, size, 2, max_window_size);
- const size_t rplr_min_num_pairs =
- FuzzInRange(data, size, 1, plr_min_num_packets - 1);
+ const int64_t max_window_size_ms = FuzzInRange(data, size, 1, 1 << 16);
+ const size_t plr_min_num_packets = FuzzInRange(data, size, 1, kSeqNumHalf);
+ const size_t rplr_min_num_pairs = FuzzInRange(data, size, 1, kSeqNumHalf - 1);
tracker->reset(new TransportFeedbackPacketLossTracker(
- max_window_size, plr_min_num_packets, rplr_min_num_pairs));
+ max_window_size_ms, plr_min_num_packets, rplr_min_num_pairs));
return true;
}
@@ -140,10 +136,56 @@
return true;
}
+bool FuzzClockAdvancement(const uint8_t** data,
+ size_t* size,
+ int64_t* time_ms) {
+ // Fuzzing 64-bit worth of delta would be extreme overkill, as 32-bit is
+ // already ~49 days long. We'll fuzz deltas up to a smaller value, and this
+ // way also guarantee that wrap-around is impossible, as in real life.
+
+ // Higher likelihood for more likely cases:
+ // 5% chance of delta = 0.
+ // 20% chance of delta in range [1 : 10] (uniformly distributed)
+ // 55% chance of delta in range [11 : 500] (uniformly distributed)
+ // 20% chance of delta in range [501 : 10000] (uniformly distributed)
+ struct ProbabilityDistribution {
+ float probability;
+ size_t lower;
+ size_t higher;
+ };
+ constexpr ProbabilityDistribution clock_probability_distribution[] = {
+ {0.05, 0, 0}, {0.20, 1, 10}, {0.55, 11, 500}, {0.20, 501, 10000}};
+
+ if (*size < sizeof(uint8_t)) {
+ return false;
+ }
+ const float fuzzed = FuzzInput<uint8_t>(data, size) / 256.0f;
+
+ float cumulative_probability = 0;
+ for (const auto& dist : clock_probability_distribution) {
+ cumulative_probability += dist.probability;
+ if (fuzzed < cumulative_probability) {
+ if (dist.lower == dist.higher) {
+ *time_ms += dist.lower;
+ return true;
+ } else if (*size < sizeof(uint16_t)) {
+ return false;
+ } else {
+ *time_ms += FuzzInRange(data, size, dist.lower, dist.higher);
+ return true;
+ }
+ }
+ }
+
+ RTC_NOTREACHED();
+ return false;
+}
+
bool FuzzPacketSendBlock(
std::unique_ptr<TransportFeedbackPacketLossTracker>& tracker,
const uint8_t** data,
- size_t* size) {
+ size_t* size,
+ int64_t* time_ms) {
// We want to test with block lengths between 1 and 2^16, inclusive.
if (*size < sizeof(uint8_t)) {
return false;
@@ -155,16 +197,24 @@
return false;
}
uint16_t seq_num = FuzzInput<uint16_t>(data, size);
- tracker->OnPacketAdded(seq_num);
+ tracker->OnPacketAdded(seq_num, *time_ms);
tracker->Validate();
+ bool may_continue = FuzzClockAdvancement(data, size, time_ms);
+ if (!may_continue) {
+ return false;
+ }
+
for (size_t i = 1; i < packet_block_len; i++) {
uint16_t delta;
- bool may_continue = FuzzSequenceNumberDelta(data, size, &delta);
+ may_continue = FuzzSequenceNumberDelta(data, size, &delta);
+ if (!may_continue)
+ return false;
+ may_continue = FuzzClockAdvancement(data, size, time_ms);
if (!may_continue)
return false;
seq_num += delta;
- tracker->OnPacketAdded(seq_num);
+ tracker->OnPacketAdded(seq_num, *time_ms);
tracker->Validate();
}
@@ -206,8 +256,15 @@
may_continue = Setup(&data, &size, &tracker);
+ // We never expect this to wrap around, so it makes sense to just start with
+ // a sane value, and keep on incrementing by a fuzzed delta.
+ if (size < sizeof(uint32_t)) {
+ return;
+ }
+ int64_t time_ms = FuzzInput<uint32_t>(&data, &size);
+
while (may_continue) {
- may_continue = FuzzPacketSendBlock(tracker, &data, &size);
+ may_continue = FuzzPacketSendBlock(tracker, &data, &size, &time_ms);
if (!may_continue) {
return;
}
diff --git a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc
index ea38f14..6377943 100644
--- a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc
+++ b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc
@@ -28,24 +28,21 @@
--(*counter);
}
}
-
} // namespace
namespace webrtc {
TransportFeedbackPacketLossTracker::TransportFeedbackPacketLossTracker(
- size_t max_acked_packets,
+ int64_t max_window_size_ms,
size_t plr_min_num_acked_packets,
size_t rplr_min_num_acked_pairs)
- : max_acked_packets_(max_acked_packets),
+ : max_window_size_ms_(max_window_size_ms),
ref_packet_status_(packet_status_window_.begin()),
plr_state_(plr_min_num_acked_packets),
rplr_state_(rplr_min_num_acked_pairs) {
+ RTC_DCHECK_GT(max_window_size_ms, 0);
RTC_DCHECK_GT(plr_min_num_acked_packets, 0);
- RTC_DCHECK_GE(max_acked_packets, plr_min_num_acked_packets);
- RTC_DCHECK_LE(max_acked_packets, kSeqNumHalf);
RTC_DCHECK_GT(rplr_min_num_acked_pairs, 0);
- RTC_DCHECK_GT(max_acked_packets, rplr_min_num_acked_pairs);
Reset();
}
@@ -67,7 +64,14 @@
return PreviousPacketStatus(packet_status_window_.end())->first;
}
-void TransportFeedbackPacketLossTracker::OnPacketAdded(uint16_t seq_num) {
+void TransportFeedbackPacketLossTracker::OnPacketAdded(uint16_t seq_num,
+ int64_t send_time_ms) {
+ // Sanity - time can't flow backwards.
+ RTC_DCHECK(
+ packet_status_window_.empty() ||
+ PreviousPacketStatus(packet_status_window_.end())->second.send_time_ms <=
+ send_time_ms);
+
if (packet_status_window_.find(seq_num) != packet_status_window_.end() ||
(!packet_status_window_.empty() &&
ForwardDiff(seq_num, NewestSequenceNumber()) <= kSeqNumHalf)) {
@@ -77,14 +81,16 @@
Reset();
}
- // Shift older packets out of window.
+ // Maintain a window where the newest sequence number is at most 0x7fff away
+ // from the oldest, so that would could still distinguish old/new.
while (!packet_status_window_.empty() &&
ForwardDiff(ref_packet_status_->first, seq_num) >= kSeqNumHalf) {
RemoveOldestPacketStatus();
}
+ SentPacket sent_packet(send_time_ms, PacketStatus::Unacked);
packet_status_window_.insert(packet_status_window_.end(),
- std::make_pair(seq_num, PacketStatus::Unacked));
+ std::make_pair(seq_num, sent_packet));
if (packet_status_window_.size() == 1) {
ref_packet_status_ = packet_status_window_.cbegin();
@@ -93,20 +99,25 @@
void TransportFeedbackPacketLossTracker::OnReceivedTransportFeedback(
const rtcp::TransportFeedback& feedback) {
- const auto& fb_vector = feedback.GetStatusVector();
+ const auto& status_vector = feedback.GetStatusVector();
const uint16_t base_seq_num = feedback.GetBaseSequence();
uint16_t seq_num = base_seq_num;
- for (size_t i = 0; i < fb_vector.size(); ++i, ++seq_num) {
+ for (size_t i = 0; i < status_vector.size(); ++i, ++seq_num) {
const auto& it = packet_status_window_.find(seq_num);
// Packets which aren't at least marked as unacked either do not belong to
// this media stream, or have been shifted out of window.
- if (it != packet_status_window_.end()) {
- const bool received = fb_vector[i] !=
- webrtc::rtcp::TransportFeedback::StatusSymbol::kNotReceived;
- RecordFeedback(it, received);
- }
+ if (it == packet_status_window_.end())
+ continue;
+
+ const bool lost =
+ status_vector[i] ==
+ webrtc::rtcp::TransportFeedback::StatusSymbol::kNotReceived;
+ const PacketStatus packet_status =
+ lost ? PacketStatus::Lost : PacketStatus::Received;
+
+ UpdatePacketStatus(it, packet_status);
}
}
@@ -120,18 +131,20 @@
return rplr_state_.GetMetric();
}
-void TransportFeedbackPacketLossTracker::RecordFeedback(
- PacketStatusMap::iterator it,
- bool received) {
- if (it->second != PacketStatus::Unacked) {
+void TransportFeedbackPacketLossTracker::UpdatePacketStatus(
+ SentPacketStatusMap::iterator it,
+ PacketStatus new_status) {
+ if (it->second.status != PacketStatus::Unacked) {
// Normally, packets are sent (inserted into window as "unacked"), then we
// receive one feedback for them.
// But it is possible that a packet would receive two feedbacks. Then:
- if (it->second == PacketStatus::Lost && received) {
+ if (it->second.status == PacketStatus::Lost &&
+ new_status == PacketStatus::Received) {
// If older status said that the packet was lost but newer one says it
// is received, we take the newer one.
UpdateMetrics(it, false);
- it->second = PacketStatus::Unacked; // For clarity; overwritten shortly.
+ it->second.status =
+ PacketStatus::Unacked; // For clarity; overwritten shortly.
} else {
// If the value is unchanged or if older status said that the packet was
// received but the newer one says it is lost, we ignore it.
@@ -143,15 +156,19 @@
}
// Change from UNACKED to RECEIVED/LOST.
- it->second = received ? PacketStatus::Received : PacketStatus::Lost;
+ it->second.status = new_status;
UpdateMetrics(it, true);
- // Remove packets from the beginning of the window until maximum-acked
- // is observed again. Note that multiple sent-but-unacked packets might
- // be removed before we reach the first acked (whether as received or as
- // lost) packet.
- while (acked_packets_ > max_acked_packets_)
+ // Remove packets from the beginning of the window until we only hold packets,
+ // be they acked or unacked, which are not more than |max_window_size_ms|
+ // older from the newest packet. (If the packet we're now inserting into the
+ // window isn't the newest, it would not trigger any removals; the newest
+ // already removed all relevant.)
+ while (ref_packet_status_ != packet_status_window_.end() &&
+ (it->second.send_time_ms - ref_packet_status_->second.send_time_ms) >
+ max_window_size_ms_) {
RemoveOldestPacketStatus();
+ }
}
void TransportFeedbackPacketLossTracker::RemoveOldestPacketStatus() {
@@ -168,9 +185,9 @@
// Metrics are dependent on feedbacks from the other side. We don't want
// to update the metrics each time a packet is sent, except for the case
// when it shifts old sent-but-unacked-packets out of window.
- RTC_DCHECK(!apply || it->second != PacketStatus::Unacked);
+ RTC_DCHECK(!apply || it->second.status != PacketStatus::Unacked);
- if (it->second != PacketStatus::Unacked) {
+ if (it->second.status != PacketStatus::Unacked) {
UpdateCounter(&acked_packets_, apply);
}
@@ -181,7 +198,7 @@
void TransportFeedbackPacketLossTracker::UpdatePlr(
ConstPacketStatusIterator it,
bool apply /* false = undo */) {
- switch (it->second) {
+ switch (it->second.status) {
case PacketStatus::Unacked:
return;
case PacketStatus::Received:
@@ -198,7 +215,7 @@
void TransportFeedbackPacketLossTracker::UpdateRplr(
ConstPacketStatusIterator it,
bool apply /* false = undo */) {
- if (it->second == PacketStatus::Unacked) {
+ if (it->second.status == PacketStatus::Unacked) {
// Unacked packets cannot compose a pair.
return;
}
@@ -206,10 +223,10 @@
// Previous packet and current packet might compose a pair.
if (it != ref_packet_status_) {
const auto& prev = PreviousPacketStatus(it);
- if (prev->second != PacketStatus::Unacked) {
+ if (prev->second.status != PacketStatus::Unacked) {
UpdateCounter(&rplr_state_.num_acked_pairs_, apply);
- if (prev->second == PacketStatus::Lost &&
- it->second == PacketStatus::Received) {
+ if (prev->second.status == PacketStatus::Lost &&
+ it->second.status == PacketStatus::Received) {
UpdateCounter(
&rplr_state_.num_recoverable_losses_, apply);
}
@@ -219,10 +236,10 @@
// Current packet and next packet might compose a pair.
const auto& next = NextPacketStatus(it);
if (next != packet_status_window_.end() &&
- next->second != PacketStatus::Unacked) {
+ next->second.status != PacketStatus::Unacked) {
UpdateCounter(&rplr_state_.num_acked_pairs_, apply);
- if (it->second == PacketStatus::Lost &&
- next->second == PacketStatus::Received) {
+ if (it->second.status == PacketStatus::Lost &&
+ next->second.status == PacketStatus::Received) {
UpdateCounter(&rplr_state_.num_recoverable_losses_, apply);
}
}
@@ -273,7 +290,6 @@
RTC_CHECK_EQ(plr_state_.num_received_packets_ + plr_state_.num_lost_packets_,
acked_packets_);
RTC_CHECK_LE(acked_packets_, packet_status_window_.size());
- RTC_CHECK_LE(acked_packets_, max_acked_packets_);
RTC_CHECK_LE(rplr_state_.num_recoverable_losses_,
rplr_state_.num_acked_pairs_);
RTC_CHECK_LE(rplr_state_.num_acked_pairs_, acked_packets_ - 1);
@@ -287,7 +303,7 @@
if (!packet_status_window_.empty()) {
ConstPacketStatusIterator it = ref_packet_status_;
do {
- switch (it->second) {
+ switch (it->second.status) {
case PacketStatus::Unacked:
++unacked_packets;
break;
@@ -305,13 +321,17 @@
if (next == packet_status_window_.end())
next = packet_status_window_.begin();
- if (next != ref_packet_status_ &&
- it->second != PacketStatus::Unacked &&
- next->second != PacketStatus::Unacked) {
- ++acked_pairs;
- if (it->second == PacketStatus::Lost &&
- next->second == PacketStatus::Received)
- ++recoverable_losses;
+ if (next != ref_packet_status_) { // If we have a next packet...
+ RTC_CHECK_GE(next->second.send_time_ms, it->second.send_time_ms);
+
+ if (it->second.status != PacketStatus::Unacked &&
+ next->second.status != PacketStatus::Unacked) {
+ ++acked_pairs;
+ if (it->second.status == PacketStatus::Lost &&
+ next->second.status == PacketStatus::Received) {
+ ++recoverable_losses;
+ }
+ }
}
RTC_CHECK_LT(ForwardDiff(ReferenceSequenceNumber(), it->first),
diff --git a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h
index efd0f81..b8fd0cc 100644
--- a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h
+++ b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h
@@ -24,17 +24,17 @@
class TransportFeedbackPacketLossTracker final {
public:
- // * Up to |max_acked_packets| latest packet statuses will be used for
- // calculating the packet loss metrics.
+ // * We count up to |max_window_size_ms| from the sent
+ // time of the latest acked packet for the calculation of the metrics.
// * PLR (packet-loss-rate) is reliably computable once the statuses of
// |plr_min_num_acked_packets| packets are known.
// * RPLR (recoverable-packet-loss-rate) is reliably computable once the
// statuses of |rplr_min_num_acked_pairs| pairs are known.
- TransportFeedbackPacketLossTracker(size_t max_acked_packets,
+ TransportFeedbackPacketLossTracker(int64_t max_window_size_ms,
size_t plr_min_num_acked_packets,
size_t rplr_min_num_acked_pairs);
- void OnPacketAdded(uint16_t seq_num);
+ void OnPacketAdded(uint16_t seq_num, int64_t send_time_ms);
void OnReceivedTransportFeedback(const rtcp::TransportFeedback& feedback);
@@ -56,8 +56,14 @@
// metrics accordingly.
enum class PacketStatus { Unacked = 0, Received = 1, Lost = 2 };
- typedef std::map<uint16_t, PacketStatus> PacketStatusMap;
- typedef PacketStatusMap::const_iterator ConstPacketStatusIterator;
+ struct SentPacket {
+ SentPacket(int64_t send_time_ms, PacketStatus status)
+ : send_time_ms(send_time_ms), status(status) {}
+ int64_t send_time_ms;
+ PacketStatus status;
+ };
+ typedef std::map<uint16_t, SentPacket> SentPacketStatusMap;
+ typedef SentPacketStatusMap::const_iterator ConstPacketStatusIterator;
void Reset();
@@ -68,7 +74,8 @@
// |packet_status_window_|.
uint16_t ReferenceSequenceNumber() const;
uint16_t NewestSequenceNumber() const;
- void RecordFeedback(PacketStatusMap::iterator it, bool received);
+ void UpdatePacketStatus(SentPacketStatusMap::iterator it,
+ PacketStatus new_status);
void RemoveOldestPacketStatus();
void UpdateMetrics(ConstPacketStatusIterator it,
@@ -81,10 +88,10 @@
ConstPacketStatusIterator NextPacketStatus(
ConstPacketStatusIterator it) const;
- const size_t max_acked_packets_;
+ const int64_t max_window_size_ms_;
size_t acked_packets_;
- PacketStatusMap packet_status_window_;
+ SentPacketStatusMap packet_status_window_;
// |ref_packet_status_| points to the oldest item in |packet_status_window_|.
ConstPacketStatusIterator ref_packet_status_;
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 5c9ae21..3bd580c 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()};
+ 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;
+ }
- private:
- RTC_DISALLOW_COPY_AND_ASSIGN(TransportFeedbackPacketLossTrackerTest);
-};
-
-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) {
+ 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 AdvanceClock(int64_t time_delta_ms) {
+ RTC_CHECK_GT(time_delta_ms, 0);
+ time_ms_ += time_delta_ms;
+ }
+
+ 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();
}
-}
-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);
-}
+ // 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);
+ }
-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;
+ 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);
+ }
}
- // 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);
+ // 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));
}
- tracker->OnReceivedTransportFeedback(test_feedback);
- tracker->Validate();
-}
+ uint16_t base_{GetParam()};
-// 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);
- }
+ private:
+ int64_t time_ms_{0};
- 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));
-}
+ 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 @@
// 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 @@
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 @@
// 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 @@
// 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 @@
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 @@
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 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
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);
+// 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_, 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);
-
- 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 @@
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 @@
// 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);
-}
-
-// 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.
-
-// 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);
-
- // 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});
+ // 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);
- // 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>());
+ // 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);
+}
- // 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});
+TEST_P(TransportFeedbackPacketLossTrackerTest, RepeatedSeqNumResetsWindow) {
+ TransportFeedbackPacketLossTracker tracker(500, 2, 1);
+
+ // 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_ + 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);
+
+ // 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};
