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 5c3cfcba28..c8e261982d 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 @@ bool Setup(const uint8_t** data, 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 @@ bool FuzzSequenceNumberDelta(const uint8_t** data, 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(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& 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 @@ bool FuzzPacketSendBlock( return false; } uint16_t seq_num = FuzzInput(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 @@ void FuzzOneInput(const uint8_t* data, size_t size) { 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(&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 ea38f149da..637794398e 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 @@ void UpdateCounter(size_t* counter, bool increment) { --(*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 @@ uint16_t TransportFeedbackPacketLossTracker::NewestSequenceNumber() const { 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 @@ void TransportFeedbackPacketLossTracker::OnPacketAdded(uint16_t seq_num) { 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::OnPacketAdded(uint16_t seq_num) { 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 @@ TransportFeedbackPacketLossTracker::GetRecoverablePacketLossRate() const { 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 @@ void TransportFeedbackPacketLossTracker::RecordFeedback( } // 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 @@ void TransportFeedbackPacketLossTracker::UpdateMetrics( // 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::UpdateMetrics( 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::UpdatePlr( 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 @@ void TransportFeedbackPacketLossTracker::UpdateRplr( // 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 @@ void TransportFeedbackPacketLossTracker::UpdateRplr( // 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 @@ void TransportFeedbackPacketLossTracker::Validate() const { // Testing only! 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 @@ void TransportFeedbackPacketLossTracker::Validate() const { // Testing only! 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 @@ void TransportFeedbackPacketLossTracker::Validate() const { // Testing only! 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 efd0f81d10..b8fd0cc4bb 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 TransportFeedback; 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 @@ class TransportFeedbackPacketLossTracker final { // metrics accordingly. enum class PacketStatus { Unacked = 0, Received = 1, Lost = 2 }; - typedef std::map 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 SentPacketStatusMap; + typedef SentPacketStatusMap::const_iterator ConstPacketStatusIterator; void Reset(); @@ -68,7 +74,8 @@ class TransportFeedbackPacketLossTracker final { // |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 @@ class TransportFeedbackPacketLossTracker final { 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 5c9ae21c74..3bd580ccef 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 #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 { + using TransportFeedback = webrtc::rtcp::TransportFeedback; + public: TransportFeedbackPacketLossTrackerTest() = default; virtual ~TransportFeedbackPacketLossTrackerTest() = default; protected: - uint16_t base_{GetParam()}; + void SendPackets(TransportFeedbackPacketLossTracker* tracker, + const std::vector& 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& 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 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& 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 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 expected_plr, + rtc::Optional expected_rplr) { + // TODO(elad.alon): Comparing the rtc::Optional 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 plr = tracker.GetPacketLossRate(); + EXPECT_EQ(static_cast(expected_plr), static_cast(plr)); + if (expected_plr && plr) { + EXPECT_EQ(*expected_plr, *plr); + } -void AddTransportFeedbackAndValidate( - TransportFeedbackPacketLossTracker* tracker, - uint16_t base_sequence_num, - const std::vector& 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 rplr = tracker.GetRecoverablePacketLossRate(); + EXPECT_EQ(static_cast(expected_rplr), static_cast(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 constructor is just cumbersome. + void ValidatePacketLossStatistics( + const TransportFeedbackPacketLossTracker& tracker, + float expected_plr, + float expected_rplr) { + ValidatePacketLossStatistics(tracker, rtc::Optional(expected_plr), + rtc::Optional(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 expected_plr, - rtc::Optional expected_rplr) { - // Comparing the rtc::Optional directly would have given concise code, - // but less readable error messages. - rtc::Optional plr = tracker.GetPacketLossRate(); - EXPECT_EQ(static_cast(expected_plr), static_cast(plr)); - if (expected_plr && plr) { - EXPECT_EQ(*expected_plr, *plr); - } + private: + int64_t time_ms_{0}; - rtc::Optional rplr = tracker.GetRecoverablePacketLossRate(); - EXPECT_EQ(static_cast(expected_rplr), static_cast(rplr)); - if (expected_rplr && rplr) { - EXPECT_EQ(*expected_rplr, *rplr); - } -} - -// Convenience function for when both are valid, and explicitly stating -// the rtc::Optional constructor is just cumbersome. -void ValidatePacketLossStatistics( - const TransportFeedbackPacketLossTracker& tracker, - float expected_plr, - float expected_rplr) { - ValidatePacketLossStatistics(tracker, - rtc::Optional(expected_plr), - rtc::Optional(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 @@ TEST(TransportFeedbackPacketLossTrackerTest, EmptyWindow) { // A feedback received for an empty window has no effect. TEST_P(TransportFeedbackPacketLossTrackerTest, EmptyWindowFeedback) { - TransportFeedbackPacketLossTracker tracker(3, 3, 2); + TransportFeedbackPacketLossTracker tracker(5000, 3, 2); // Feedback doesn't correspond to any packets - ignored. AddTransportFeedbackAndValidate(&tracker, base_, {true, false, true}); @@ -149,19 +160,19 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, EmptyWindowFeedback) { rtc::Optional()); // 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(), @@ -170,12 +181,12 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, PartiallyFilledWindow) { // Sanity check on minimum filled window - PLR known, RPLR unknown. TEST_P(TransportFeedbackPacketLossTrackerTest, PlrMinimumFilledWindow) { - TransportFeedbackPacketLossTracker tracker(10, 5, 5); + TransportFeedbackPacketLossTracker tracker(5000, 5, 5); // PLR correctly calculated after minimum window size reached. // RPLR not necessarily known at that time (not if min-pairs not reached). // Expected window contents: [] -> [10011]. - SendPacketRange(&tracker, base_, 5); + SendPackets(&tracker, base_, 5, 10); AddTransportFeedbackAndValidate(&tracker, base_, {true, false, false, true, true}); ValidatePacketLossStatistics(tracker, @@ -185,12 +196,12 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, PlrMinimumFilledWindow) { // Sanity check on minimum filled window - PLR unknown, RPLR known. TEST_P(TransportFeedbackPacketLossTrackerTest, RplrMinimumFilledWindow) { - TransportFeedbackPacketLossTracker tracker(10, 6, 4); + TransportFeedbackPacketLossTracker tracker(5000, 6, 4); // RPLR correctly calculated after minimum pairs reached. // PLR not necessarily known at that time (not if min window not reached). // Expected window contents: [] -> [10011]. - SendPacketRange(&tracker, base_, 5); + SendPackets(&tracker, base_, 5, 10); AddTransportFeedbackAndValidate(&tracker, base_, {true, false, false, true, true}); ValidatePacketLossStatistics(tracker, @@ -198,11 +209,23 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, RplrMinimumFilledWindow) { rtc::Optional(1.0f / 4.0f)); } +// If packets are sent close enough together that the clock reading for both +// is the same, that's handled properly. +TEST_P(TransportFeedbackPacketLossTrackerTest, SameSentTime) { + TransportFeedbackPacketLossTracker tracker(5000, 3, 2); + + // Expected window contents: [] -> [101]. + SendPackets(&tracker, base_, 3, 0); // Note: time interval = 0ms. + AddTransportFeedbackAndValidate(&tracker, base_, {true, false, true}); + + ValidatePacketLossStatistics(tracker, 1.0f / 3.0f, 0.5f); +} + // Additional reports update PLR and RPLR. TEST_P(TransportFeedbackPacketLossTrackerTest, ExtendWindow) { - TransportFeedbackPacketLossTracker tracker(20, 5, 5); + TransportFeedbackPacketLossTracker tracker(5000, 5, 5); - SendPacketRange(&tracker, base_, 25); + SendPackets(&tracker, base_, 25, 10); // Expected window contents: [] -> [10011]. AddTransportFeedbackAndValidate(&tracker, base_, @@ -222,25 +245,63 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, ExtendWindow) { ValidatePacketLossStatistics(tracker, 7.0f / 15.0f, 4.0f / 13.0f); } -// Sanity - all packets correctly received. +// Correct calculation with different packet lengths. +TEST_P(TransportFeedbackPacketLossTrackerTest, DifferentSentIntervals) { + TransportFeedbackPacketLossTracker tracker(5000, 5, 4); + + int64_t now_ms = 0; + int64_t frames[] = {20, 60, 120, 20, 60}; + for (size_t i = 0; i < sizeof(frames) / sizeof(frames[0]); i++) { + tracker.OnPacketAdded(static_cast(base_ + i), now_ms); + now_ms += frames[i]; + } + + // Expected window contents: [] -> [10011]. + AddTransportFeedbackAndValidate(&tracker, base_, + {true, false, false, true, true}); + ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f); +} + +// The window retains information up to sent times that exceed the the max +// window size. The oldest packets get shifted out of window to make room +// for the newer ones. +TEST_P(TransportFeedbackPacketLossTrackerTest, MaxWindowSize) { + TransportFeedbackPacketLossTracker tracker(40, 5, 1); + + SendPackets(&tracker, base_, 6, 10, true); + + // Up to the maximum time-span retained (first + 4 * 10ms). + // Expected window contents: [] -> [01001]. + AddTransportFeedbackAndValidate(&tracker, base_, + {false, true, false, false, true}); + ValidatePacketLossStatistics(tracker, 3.0f / 5.0f, 2.0f / 4.0f); + + // After the maximum time-span, older entries are discarded to accommodate + // newer ones. + // Expected window contents: [01001] -> [10011]. + AddTransportFeedbackAndValidate(&tracker, base_ + 5, {true}); + ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f); +} + +// All packets received. TEST_P(TransportFeedbackPacketLossTrackerTest, AllReceived) { - TransportFeedbackPacketLossTracker tracker(10, 5, 4); + TransportFeedbackPacketLossTracker tracker(5000, 5, 4); // Expected window contents: [] -> [11111]. - SendPacketRange(&tracker, base_, 5); + SendPackets(&tracker, base_, 5, 10); AddTransportFeedbackAndValidate(&tracker, base_, {true, true, true, true, true}); ValidatePacketLossStatistics(tracker, 0.0f, 0.0f); } -// Sanity - all packets lost. +// All packets lost. TEST_P(TransportFeedbackPacketLossTrackerTest, AllLost) { - TransportFeedbackPacketLossTracker tracker(10, 5, 4); + TransportFeedbackPacketLossTracker tracker(5000, 5, 4); + // Note: The last packet in the feedback does not belong to the stream. + // It's only there because we're not allowed to end a feedback with a loss. // Expected window contents: [] -> [00000]. - SendPacketRange(&tracker, base_, 5); - // Note: Last acked packet (the received one) does not belong to the stream, - // and is only there to make sure the feedback message is legal. + SendPackets(&tracker, base_, 5, 10); AddTransportFeedbackAndValidate(&tracker, base_, {false, false, false, false, false, true}); ValidatePacketLossStatistics(tracker, 1.0f, 0.0f); @@ -248,9 +309,9 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, AllLost) { // Repeated reports are ignored. TEST_P(TransportFeedbackPacketLossTrackerTest, ReportRepetition) { - TransportFeedbackPacketLossTracker tracker(10, 5, 4); + TransportFeedbackPacketLossTracker tracker(5000, 5, 4); - SendPacketRange(&tracker, base_, 5); + SendPackets(&tracker, base_, 5, 10); // Expected window contents: [] -> [10011]. AddTransportFeedbackAndValidate(&tracker, base_, @@ -266,9 +327,9 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, ReportRepetition) { // Report overlap. TEST_P(TransportFeedbackPacketLossTrackerTest, ReportOverlap) { - TransportFeedbackPacketLossTracker tracker(10, 5, 1); + TransportFeedbackPacketLossTracker tracker(5000, 5, 1); - SendPacketRange(&tracker, base_, 15); + SendPackets(&tracker, base_, 15, 10); // Expected window contents: [] -> [10011]. AddTransportFeedbackAndValidate(&tracker, base_, @@ -283,9 +344,9 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, ReportOverlap) { // Report conflict. TEST_P(TransportFeedbackPacketLossTrackerTest, ReportConflict) { - TransportFeedbackPacketLossTracker tracker(10, 5, 4); + TransportFeedbackPacketLossTracker tracker(5000, 5, 4); - SendPacketRange(&tracker, base_, 15); + SendPackets(&tracker, base_, 15, 10); // Expected window contents: [] -> [01001]. AddTransportFeedbackAndValidate(&tracker, base_, @@ -300,9 +361,9 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, ReportConflict) { // Skipped packets treated as unknown (not lost). TEST_P(TransportFeedbackPacketLossTrackerTest, SkippedPackets) { - TransportFeedbackPacketLossTracker tracker(10, 5, 1); + TransportFeedbackPacketLossTracker tracker(200 * 10, 5, 1); - SendPacketRange(&tracker, base_, 200); + SendPackets(&tracker, base_, 200, 10); // Expected window contents: [] -> [10011]. AddTransportFeedbackAndValidate(&tracker, base_, @@ -314,56 +375,50 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, SkippedPackets) { ValidatePacketLossStatistics(tracker, 3.0f / 8.0f, 2.0f / 6.0f); } -// The window retains information up to the configured max-window-size, but -// starts discarding after that. (Sent packets are not counted.) -TEST_P(TransportFeedbackPacketLossTrackerTest, MaxWindowSize) { - TransportFeedbackPacketLossTracker tracker(10, 10, 1); +// 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 feedback = {true, false, false, false, true, true}; + AddTransportFeedbackAndValidate(&tracker, moved_oldest_acked, feedback); + ValidatePacketLossStatistics(tracker, 3.0f / 6.0f, 1.0f / 5.0f); - // Insert into the middle of this full window - it discards the older data. - // Expected window contents: [10101-GAP-10001] -> [11111-GAP-10001]. - AddTransportFeedbackAndValidate(&tracker, base_ + 50, - {true, true, true, true, true}); - ValidatePacketLossStatistics(tracker, 3.0f / 10.0f, 1.0f / 8.0f); + // Having acked |feedback.size()| starting with |moved_oldest_acked|, the + // newest of the acked ones is now: + const int64_t moved_newest_acked = moved_oldest_acked + feedback.size() - 1; + + // Messages that *are* more than the span-limit away from the newest + // acked message *are* too old. Acking them would have no effect. + AddTransportFeedbackAndValidate( + &tracker, moved_newest_acked - max_window_size_ms - 1, {true}); + ValidatePacketLossStatistics(tracker, 3.0f / 6.0f, 1.0f / 5.0f); + + // Messages that are *not* more than the span-limit away from the newest + // acked message are *not* too old. Acking them would have an effect. + AddTransportFeedbackAndValidate( + &tracker, moved_newest_acked - max_window_size_ms, {true}); + ValidatePacketLossStatistics(tracker, 3.0f / 7.0f, 1.0f / 5.0f); } -// Inserting feedback into the middle of a full window works correctly - can +// Inserting feedback into the middle of a window works correctly - can // complete two pairs. TEST_P(TransportFeedbackPacketLossTrackerTest, InsertionCompletesTwoPairs) { - TransportFeedbackPacketLossTracker tracker(15, 5, 1); + TransportFeedbackPacketLossTracker tracker(1500, 5, 1); - SendPacketRange(&tracker, base_, 300); + SendPackets(&tracker, base_, 15, 10); // Expected window contents: [] -> [10111]. AddTransportFeedbackAndValidate(&tracker, base_, @@ -376,164 +431,29 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, InsertionCompletesTwoPairs) { ValidatePacketLossStatistics(tracker, 3.0f / 10.0f, 3.0f / 8.0f); // Insert in between, closing the gap completely. - // Expected window contents: [10111-GAP-10101] -> [101111010101]. + // Expected window contents: [10111-GAP-10101] -> [101110110101]. AddTransportFeedbackAndValidate(&tracker, base_ + 5, {false, true}); ValidatePacketLossStatistics(tracker, 4.0f / 12.0f, 4.0f / 11.0f); } -// The window can meaningfully hold up to 0x8000 SENT packets (of which only -// up to max-window acked messages will be kept and regarded). -TEST_P(TransportFeedbackPacketLossTrackerTest, SecondQuadrant) { - TransportFeedbackPacketLossTracker tracker(20, 5, 1); - - SendPacketRange(&tracker, base_, 0x8000, false); - - // Expected window contents: [] -> [10011]. - AddTransportFeedbackAndValidate(&tracker, base_, - {true, false, false, true, true}); - ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f); - - // Window *does* get updated with inputs from quadrant #2. - // Expected window contents: [10011] -> [100111]. - AddTransportFeedbackAndValidate(&tracker, base_ + 0x4321, {true}); - ValidatePacketLossStatistics(tracker, 2.0f / 6.0f, 1.0f / 4.0f); - - // Correct recognition of quadrant #2: up to, but not including, base_ + - // 0x8000 - // Expected window contents: [100111] -> [1001111]. - AddTransportFeedbackAndValidate(&tracker, base_ + 0x7fff, {true}); - ValidatePacketLossStatistics(tracker, 2.0f / 7.0f, 1.0f / 4.0f); -} - -// After the base has moved due to insertion into the third quadrant, it is -// still possible to get feedbacks in the middle of the window and obtain the -// correct PLR and RPLR. Insertion into the middle before the max window size -// has been achieved does not cause older packets to be dropped. -TEST_P(TransportFeedbackPacketLossTrackerTest, InsertIntoMiddleAfterBaseMoved) { - TransportFeedbackPacketLossTracker tracker(20, 5, 1); - - SendPacketRange(&tracker, base_, 20); - SendPacketRange(&tracker, base_ + 0x5000, 20); - - // Expected window contents: [] -> [1001101]. - AddTransportFeedbackAndValidate( - &tracker, base_, {true, false, false, true, true, false, true}); - ValidatePacketLossStatistics(tracker, 3.0f / 7.0f, 2.0f / 6.0f); - - // Expected window contents: [1001101] -> [101-GAP-1001]. - SendPacketRange(&tracker, base_ + 0x8000, 4); - AddTransportFeedbackAndValidate(&tracker, base_ + 0x8000, - {true, false, false, true}); - ValidatePacketLossStatistics(tracker, 3.0f / 7.0f, 2.0f / 5.0f); - - // Inserting into the middle still works after the base has shifted. - // Expected window contents: - // [101-GAP-1001] -> [101-GAP-100101-GAP-1001] - AddTransportFeedbackAndValidate(&tracker, base_ + 0x5000, - {true, false, false, true, false, true}); - ValidatePacketLossStatistics(tracker, 6.0f / 13.0f, 4.0f / 10.0f); - - // The base can keep moving after inserting into the middle. - // Expected window contents: - // [101-GAP-100101-GAP-1001] -> [1-GAP-100101-GAP-100111]. - SendPacketRange(&tracker, base_ + 0x8000 + 4, 2); - AddTransportFeedbackAndValidate(&tracker, base_ + 0x8000 + 4, {true, true}); - ValidatePacketLossStatistics(tracker, 5.0f / 13.0f, 3.0f / 10.0f); -} - -// After moving the base of the window, the max window size is still observed. -TEST_P(TransportFeedbackPacketLossTrackerTest, - MaxWindowObservedAfterBaseMoved) { - TransportFeedbackPacketLossTracker tracker(15, 10, 1); - - // Expected window contents: [] -> [1001110101]. - SendPacketRange(&tracker, base_, 10); - AddTransportFeedbackAndValidate( - &tracker, base_, - {true, false, false, true, true, true, false, true, false, true}); - ValidatePacketLossStatistics(tracker, 4.0f / 10.0f, 3.0f / 9.0f); - - // Create gap (on both sides). - SendPacketRange(&tracker, base_ + 0x4000, 20); - - // Expected window contents: [1001110101] -> [1110101-GAP-101]. - SendPacketRange(&tracker, base_ + 0x8000, 3); - AddTransportFeedbackAndValidate(&tracker, base_ + 0x8000, - {true, false, true}); - ValidatePacketLossStatistics(tracker, 3.0f / 10.0f, 3.0f / 8.0f); - - // Push into middle until max window is reached. The gap is NOT completed. - // Expected window contents: - // [1110101-GAP-101] -> [1110101-GAP-10001-GAP-101] - AddTransportFeedbackAndValidate(&tracker, base_ + 0x4000 + 2, - {true, false, false, false, true}); - ValidatePacketLossStatistics(tracker, 6.0f / 15.0f, 4.0f / 12.0f); - - // Pushing new packets into the middle would discard older packets. - // Expected window contents: - // [1110101-GAP-10001-GAP-101] -> [0101-GAP-10001101-GAP-101] - AddTransportFeedbackAndValidate(&tracker, base_ + 0x4000 + 2 + 5, - {true, false, true}); - ValidatePacketLossStatistics(tracker, 7.0f / 15.0f, 5.0f / 12.0f); -} - -// A packet with a new enough sequence number might shift enough old feedbacks -// out window, that we'd go back to an unknown PLR and RPLR. -TEST_P(TransportFeedbackPacketLossTrackerTest, NewPacketMovesWindowBase) { - TransportFeedbackPacketLossTracker tracker(20, 5, 3); - - SendPacketRange(&tracker, base_, 50); - SendPacketRange(&tracker, base_ + 0x4000 - 1, 6); // Gap - - // Expected window contents: [] -> [1001110101]. - AddTransportFeedbackAndValidate( - &tracker, base_, - {true, false, false, true, true, true, false, true, false, true}); - ValidatePacketLossStatistics(tracker, 4.0f / 10.0f, 3.0f / 9.0f); - - // A new sent packet with a new enough sequence number could shift enough - // acked packets out of window, that we'd go back to an unknown PLR - // and RPLR. This *doesn't* // necessarily mean all of the old ones - // were discarded, though. - // Expected window contents: [1001110101] -> [01]. - SendPacketRange(&tracker, base_ + 0x8006, 2); - ValidatePacketLossStatistics(tracker, - rtc::Optional(), // Still invalid. - rtc::Optional()); - - // 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(), // Still invalid. - rtc::Optional()); - - // 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(base_), - static_cast(base_ + 2), - static_cast(base_ + 4), - static_cast(base_ + 6), - static_cast(base_ + 8)}); + SendPackets(&tracker, + {static_cast(base_), + static_cast(base_ + 2), + static_cast(base_ + 4), + static_cast(base_ + 6), + static_cast(base_ + 8)}, + 10); // Gaps in sequence numbers not considered as gaps in window, because only // those sequence numbers which were associated with the stream count. // Expected window contents: [] -> [11011]. AddTransportFeedbackAndValidate( - // Note: Left packets belong to this stream, odd ones ignored. + // Note: Left packets belong to this stream, right ones ignored. &tracker, base_, {true, false, true, false, false, false, @@ -544,88 +464,136 @@ TEST_P(TransportFeedbackPacketLossTrackerTest, SanityGapsInSequenceNumbers) { // Create gap by sending [base + 10] but not acking it. // Note: Acks for [base + 11] and [base + 13] ignored (other stream). // Expected window contents: [11011] -> [11011-GAP-01]. - SendPackets(&tracker, {static_cast(base_ + 10), - static_cast(base_ + 12), - static_cast(base_ + 14)}); + SendPackets(&tracker, + {static_cast(base_ + 10), + static_cast(base_ + 12), + static_cast(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(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(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(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 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(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(base_ + feedback.size()), 1, 1); + AddTransportFeedbackAndValidate( + &tracker, static_cast(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(), - rtc::Optional()); + // 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(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(base_ + 2), 1, 10); ValidatePacketLossStatistics(tracker, rtc::Optional(), rtc::Optional()); } +// 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(base_ + 5 + 0x8000), 1, 10); + ValidatePacketLossStatistics(tracker, + rtc::Optional(), + rtc::Optional()); +} + +#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};