From 7af935758037dc483581ea701624cdb1a4dc77af Mon Sep 17 00:00:00 2001 From: "elad.alon" Date: Fri, 3 Mar 2017 10:51:35 -0800 Subject: [PATCH] Packet Loss Tracker - Stream Separation 1. Packet reception statistics (PLR and RPLR) calculated on each stream separately. 2. Algorithm changed from considering separate quadrants of the sequence-number-ring to simply looking at the oldest in-window SENT packet. BUG=webrtc:7058 Review-Url: https://codereview.webrtc.org/2632203002 Cr-Commit-Position: refs/heads/master@{#17018} --- ...ort_feedback_packet_loss_tracker_fuzzer.cc | 180 +++++-- .../transport_feedback_packet_loss_tracker.cc | 247 +++++---- .../transport_feedback_packet_loss_tracker.h | 69 +-- ...t_feedback_packet_loss_tracker_unittest.cc | 468 ++++++++---------- 4 files changed, 531 insertions(+), 433 deletions(-) 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 bb2418c81a..5c3cfcba28 100644 --- a/webrtc/test/fuzzers/transport_feedback_packet_loss_tracker_fuzzer.cc +++ b/webrtc/test/fuzzers/transport_feedback_packet_loss_tracker_fuzzer.cc @@ -44,90 +44,174 @@ size_t FuzzInRange(const uint8_t** data, class TransportFeedbackGenerator { public: - explicit TransportFeedbackGenerator(rtc::ArrayView data) - : data_(data), ended_(false), data_idx_(0) {} + explicit TransportFeedbackGenerator(const uint8_t** data, size_t* size) + : data_(data), size_(size) {} - void GetNextTransportFeedback(rtcp::TransportFeedback* feedback) { + bool GetNextTransportFeedback(rtcp::TransportFeedback* feedback) { uint16_t base_seq_num = 0; if (!ReadData(&base_seq_num)) { - return; + return false; } - - const int64_t kBaseTimeUs = 1234; // Irrelevant to this test. + constexpr int64_t kBaseTimeUs = 1234; // Irrelevant to this test. feedback->SetBase(base_seq_num, kBaseTimeUs); - uint16_t num_statuses = 0; - if (!ReadData(&num_statuses)) - return; - num_statuses = std::max(num_statuses, 1); + uint16_t remaining_packets = 0; + if (!ReadData(&remaining_packets)) + return false; + // Range is [0x00001 : 0x10000], but we keep it 0x0000 to 0xffff for now, + // and add the last status as RECEIVED. That is because of a limitation + // that says that the last status cannot be LOST. uint16_t seq_num = base_seq_num; - while (true) { + while (remaining_packets > 0) { uint8_t status_byte = 0; - if (!ReadData(&status_byte)) - return; + if (!ReadData(&status_byte)) { + return false; + } // Each status byte contains 8 statuses. - for (size_t j = 0; j < 8; ++j) { - if (status_byte & 0x01) { + for (size_t i = 0; i < 8 && remaining_packets > 0; ++i) { + const bool received = (status_byte & (0x01 << i)); + if (received) { feedback->AddReceivedPacket(seq_num, kBaseTimeUs); } - seq_num++; - if (seq_num >= base_seq_num + num_statuses) { - feedback->AddReceivedPacket(seq_num, kBaseTimeUs); - return; - } - status_byte >>= 1; + ++seq_num; + --remaining_packets; } } - } - bool ended() const { return ended_; } + // As mentioned above, all feedbacks must report with a received packet. + feedback->AddReceivedPacket(seq_num, kBaseTimeUs); + + return true; + } private: template bool ReadData(T* value) { - RTC_CHECK(!ended_); - if (data_idx_ + sizeof(T) > data_.size()) { - ended_ = true; + if (*size_ < sizeof(T)) { return false; + } else { + *value = FuzzInput(data_, size_); + return true; } - *value = ByteReader::ReadBigEndian(&data_[data_idx_]); - data_idx_ += sizeof(T); - return true; } - const rtc::ArrayView data_; - bool ended_; - size_t data_idx_; + const uint8_t** data_; + size_t* size_; }; -} // namespace - -void FuzzOneInput(const uint8_t* data, size_t size) { - if (size < 3 * sizeof(uint16_t)) { - return; +bool Setup(const uint8_t** data, + size_t* size, + std::unique_ptr* tracker) { + if (*size < 3 * sizeof(uint16_t)) { + return false; } + 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 max_window_size = FuzzInRange(data, size, 2, kSeqNumHalf); const size_t plr_min_num_packets = - FuzzInRange(&data, &size, 2, max_window_size); + FuzzInRange(data, size, 2, max_window_size); const size_t rplr_min_num_pairs = - FuzzInRange(&data, &size, 1, plr_min_num_packets - 1); + FuzzInRange(data, size, 1, plr_min_num_packets - 1); - TransportFeedbackPacketLossTracker tracker( - max_window_size, plr_min_num_packets, rplr_min_num_pairs); + tracker->reset(new TransportFeedbackPacketLossTracker( + max_window_size, plr_min_num_packets, rplr_min_num_pairs)); - TransportFeedbackGenerator feedback_generator( - rtc::ArrayView(data, size)); + return true; +} - while (!feedback_generator.ended()) { +bool FuzzSequenceNumberDelta(const uint8_t** data, + size_t* size, + uint16_t* delta) { + // Fuzz with a higher likelihood for immediately consecutive pairs + // than you would by just fuzzing 1-256. + // Note: Values higher than 256 still possible, but that would be in a new + // packet-sending block. + // * Fuzzed value in [0 : 127] (50% chance) -> delta is 1. + // * Fuzzed value in [128 : 255] (50% chance) -> delta in range [2 : 129]. + if (*size < sizeof(uint8_t)) { + return false; + } + uint8_t fuzzed = FuzzInput(data, size); + *delta = (fuzzed < 128) ? 1 : (fuzzed - 128 + 2); + return true; +} + +bool FuzzPacketSendBlock( + std::unique_ptr& tracker, + const uint8_t** data, + size_t* size) { + // We want to test with block lengths between 1 and 2^16, inclusive. + if (*size < sizeof(uint8_t)) { + return false; + } + size_t packet_block_len = 1 + FuzzInput(data, size); + + // First sent sequence number uniformly selected. + if (*size < sizeof(uint16_t)) { + return false; + } + uint16_t seq_num = FuzzInput(data, size); + tracker->OnPacketAdded(seq_num); + tracker->Validate(); + + for (size_t i = 1; i < packet_block_len; i++) { + uint16_t delta; + bool may_continue = FuzzSequenceNumberDelta(data, size, &delta); + if (!may_continue) + return false; + seq_num += delta; + tracker->OnPacketAdded(seq_num); + tracker->Validate(); + } + + return true; +} + +bool FuzzTransportFeedbackBlock( + std::unique_ptr& tracker, + const uint8_t** data, + size_t* size) { + // Fuzz the number of back-to-back feedbacks. At least one, or this would + // be meaningless - we'd go straight back to fuzzing another packet + // transmission block. + if (*size < sizeof(uint8_t)) { + return false; + } + + size_t feedbacks_num = 1 + (FuzzInput(data, size) & 0x3f); + TransportFeedbackGenerator feedback_generator(data, size); + + for (size_t i = 0; i < feedbacks_num; i++) { rtcp::TransportFeedback feedback; - feedback_generator.GetNextTransportFeedback(&feedback); - tracker.OnReceivedTransportFeedback(feedback); - tracker.Validate(); + bool may_continue = feedback_generator.GetNextTransportFeedback(&feedback); + if (!may_continue) { + return false; + } + tracker->OnReceivedTransportFeedback(feedback); + tracker->Validate(); + } + + return true; +} + +} // namespace + +void FuzzOneInput(const uint8_t* data, size_t size) { + std::unique_ptr tracker; + bool may_continue; + + may_continue = Setup(&data, &size, &tracker); + + while (may_continue) { + may_continue = FuzzPacketSendBlock(tracker, &data, &size); + if (!may_continue) { + return; + } + may_continue = FuzzTransportFeedbackBlock(tracker, &data, &size); } } diff --git a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc index c05b044814..ea38f149da 100644 --- a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc +++ b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc @@ -19,9 +19,6 @@ namespace { constexpr uint16_t kSeqNumHalf = 0x8000u; -constexpr uint16_t kSeqNumQuarter = kSeqNumHalf / 2; -constexpr size_t kMaxConsecutiveOldReports = 4; - void UpdateCounter(size_t* counter, bool increment) { if (increment) { RTC_DCHECK_LT(*counter, std::numeric_limits::max()); @@ -37,25 +34,25 @@ void UpdateCounter(size_t* counter, bool increment) { namespace webrtc { TransportFeedbackPacketLossTracker::TransportFeedbackPacketLossTracker( - size_t max_window_size, - size_t plr_min_num_packets, - size_t rplr_min_num_pairs) - : max_window_size_(max_window_size), + size_t max_acked_packets, + size_t plr_min_num_acked_packets, + size_t rplr_min_num_acked_pairs) + : max_acked_packets_(max_acked_packets), ref_packet_status_(packet_status_window_.begin()), - plr_state_(plr_min_num_packets), - rplr_state_(rplr_min_num_pairs) { - RTC_DCHECK_GT(plr_min_num_packets, 0); - RTC_DCHECK_GE(max_window_size, plr_min_num_packets); - RTC_DCHECK_LE(max_window_size, kSeqNumHalf); - RTC_DCHECK_GT(rplr_min_num_pairs, 0); - RTC_DCHECK_GT(max_window_size, rplr_min_num_pairs); + plr_state_(plr_min_num_acked_packets), + rplr_state_(rplr_min_num_acked_pairs) { + 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(); } void TransportFeedbackPacketLossTracker::Reset() { + acked_packets_ = 0; plr_state_.Reset(); rplr_state_.Reset(); - num_consecutive_old_reports_ = 0; packet_status_window_.clear(); ref_packet_status_ = packet_status_window_.begin(); } @@ -65,13 +62,33 @@ uint16_t TransportFeedbackPacketLossTracker::ReferenceSequenceNumber() const { return ref_packet_status_->first; } -bool TransportFeedbackPacketLossTracker::IsOldSequenceNumber( - uint16_t seq_num) const { - if (packet_status_window_.empty()) { - return false; +uint16_t TransportFeedbackPacketLossTracker::NewestSequenceNumber() const { + RTC_DCHECK(!packet_status_window_.empty()); + return PreviousPacketStatus(packet_status_window_.end())->first; +} + +void TransportFeedbackPacketLossTracker::OnPacketAdded(uint16_t seq_num) { + if (packet_status_window_.find(seq_num) != packet_status_window_.end() || + (!packet_status_window_.empty() && + ForwardDiff(seq_num, NewestSequenceNumber()) <= kSeqNumHalf)) { + // The only way for these two to happen is when the stream lies dormant for + // long enough for the sequence numbers to wrap. Everything in the window in + // such a case would be too old to use. + Reset(); + } + + // Shift older packets out of window. + while (!packet_status_window_.empty() && + ForwardDiff(ref_packet_status_->first, seq_num) >= kSeqNumHalf) { + RemoveOldestPacketStatus(); + } + + packet_status_window_.insert(packet_status_window_.end(), + std::make_pair(seq_num, PacketStatus::Unacked)); + + if (packet_status_window_.size() == 1) { + ref_packet_status_ = packet_status_window_.cbegin(); } - const uint16_t diff = ForwardDiff(ReferenceSequenceNumber(), seq_num); - return diff >= 3 * kSeqNumQuarter; } void TransportFeedbackPacketLossTracker::OnReceivedTransportFeedback( @@ -79,41 +96,16 @@ void TransportFeedbackPacketLossTracker::OnReceivedTransportFeedback( const auto& fb_vector = feedback.GetStatusVector(); const uint16_t base_seq_num = feedback.GetBaseSequence(); - if (IsOldSequenceNumber(base_seq_num)) { - ++num_consecutive_old_reports_; - if (num_consecutive_old_reports_ <= kMaxConsecutiveOldReports) { - // If the number consecutive old reports have not exceed a threshold, we - // consider this packet as a late arrival. We could consider adding it to - // |packet_status_window_|, but in current implementation, we simply - // ignore it. - return; - } - // If we see several consecutive older reports, we assume that we've not - // received reports for an exceedingly long time, and do a reset. - Reset(); - RTC_DCHECK(!IsOldSequenceNumber(base_seq_num)); - } else { - num_consecutive_old_reports_ = 0; - } - uint16_t seq_num = base_seq_num; for (size_t i = 0; i < fb_vector.size(); ++i, ++seq_num) { - // Remove the oldest feedbacks so that the distance between the oldest and - // the packet to be added does not exceed or equal to half of total sequence - // numbers. - while (!packet_status_window_.empty() && - ForwardDiff(ReferenceSequenceNumber(), seq_num) >= kSeqNumHalf) { - RemoveOldestPacketStatus(); - } + const auto& it = packet_status_window_.find(seq_num); - const bool received = - fb_vector[i] != - webrtc::rtcp::TransportFeedback::StatusSymbol::kNotReceived; - InsertPacketStatus(seq_num, received); - - while (packet_status_window_.size() > max_window_size_) { - // Make sure that the window holds at most |max_window_size_| items. - RemoveOldestPacketStatus(); + // 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); } } } @@ -128,25 +120,38 @@ TransportFeedbackPacketLossTracker::GetRecoverablePacketLossRate() const { return rplr_state_.GetMetric(); } -void TransportFeedbackPacketLossTracker::InsertPacketStatus(uint16_t seq_num, - bool received) { - const auto& ret = - packet_status_window_.insert(std::make_pair(seq_num, received)); - if (!ret.second) { - if (!ret.first->second && received) { +void TransportFeedbackPacketLossTracker::RecordFeedback( + PacketStatusMap::iterator it, + bool received) { + if (it->second != 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 older status said that the packet was lost but newer one says it // is received, we take the newer one. - UpdateMetrics(ret.first, false); - ret.first->second = received; + UpdateMetrics(it, false); + it->second = 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. + // The standard allows for previously-reported packets to carry + // no report when the reports overlap, which also looks like the + // packet is being reported as lost. return; } } - UpdateMetrics(ret.first, true); - if (packet_status_window_.size() == 1) - ref_packet_status_ = ret.first; + + // Change from UNACKED to RECEIVED/LOST. + it->second = received ? PacketStatus::Received : PacketStatus::Lost; + 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_) + RemoveOldestPacketStatus(); } void TransportFeedbackPacketLossTracker::RemoveOldestPacketStatus() { @@ -157,34 +162,54 @@ void TransportFeedbackPacketLossTracker::RemoveOldestPacketStatus() { } void TransportFeedbackPacketLossTracker::UpdateMetrics( - PacketStatusIterator it, + ConstPacketStatusIterator it, bool apply /* false = undo */) { RTC_DCHECK(it != packet_status_window_.end()); + // 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); + + if (it->second != PacketStatus::Unacked) { + UpdateCounter(&acked_packets_, apply); + } + UpdatePlr(it, apply); UpdateRplr(it, apply); } void TransportFeedbackPacketLossTracker::UpdatePlr( - PacketStatusIterator it, + ConstPacketStatusIterator it, bool apply /* false = undo */) { - // Record or undo reception status of currently handled packet. - if (it->second) { - UpdateCounter(&plr_state_.num_received_packets_, apply); - } else { - UpdateCounter(&plr_state_.num_lost_packets_, apply); + switch (it->second) { + case PacketStatus::Unacked: + return; + case PacketStatus::Received: + UpdateCounter(&plr_state_.num_received_packets_, apply); + break; + case PacketStatus::Lost: + UpdateCounter(&plr_state_.num_lost_packets_, apply); + break; + default: + RTC_NOTREACHED(); } } void TransportFeedbackPacketLossTracker::UpdateRplr( - PacketStatusIterator it, + ConstPacketStatusIterator it, bool apply /* false = undo */) { - // Previous packet and current packet might compose a known pair. - // If so, the RPLR state needs to be updated accordingly. + if (it->second == PacketStatus::Unacked) { + // Unacked packets cannot compose a pair. + return; + } + + // Previous packet and current packet might compose a pair. if (it != ref_packet_status_) { const auto& prev = PreviousPacketStatus(it); - if (prev->first == static_cast(it->first - 1)) { - UpdateCounter(&rplr_state_.num_known_pairs_, apply); - if (!prev->second && it->second) { + if (prev->second != PacketStatus::Unacked) { + UpdateCounter(&rplr_state_.num_acked_pairs_, apply); + if (prev->second == PacketStatus::Lost && + it->second == PacketStatus::Received) { UpdateCounter( &rplr_state_.num_recoverable_losses_, apply); } @@ -192,20 +217,20 @@ void TransportFeedbackPacketLossTracker::UpdateRplr( } // Current packet and next packet might compose a pair. - // If so, the RPLR state needs to be updated accordingly. const auto& next = NextPacketStatus(it); if (next != packet_status_window_.end() && - next->first == static_cast(it->first + 1)) { - UpdateCounter(&rplr_state_.num_known_pairs_, apply); - if (!it->second && next->second) { + next->second != PacketStatus::Unacked) { + UpdateCounter(&rplr_state_.num_acked_pairs_, apply); + if (it->second == PacketStatus::Lost && + next->second == PacketStatus::Received) { UpdateCounter(&rplr_state_.num_recoverable_losses_, apply); } } } -TransportFeedbackPacketLossTracker::PacketStatusIterator +TransportFeedbackPacketLossTracker::ConstPacketStatusIterator TransportFeedbackPacketLossTracker::PreviousPacketStatus( - PacketStatusIterator it) { + ConstPacketStatusIterator it) const { RTC_DCHECK(it != ref_packet_status_); if (it == packet_status_window_.end()) { // This is to make PreviousPacketStatus(packet_status_window_.end()) point @@ -221,8 +246,9 @@ TransportFeedbackPacketLossTracker::PreviousPacketStatus( return --it; } -TransportFeedbackPacketLossTracker::PacketStatusIterator -TransportFeedbackPacketLossTracker::NextPacketStatus(PacketStatusIterator it) { +TransportFeedbackPacketLossTracker::ConstPacketStatusIterator +TransportFeedbackPacketLossTracker::NextPacketStatus( + ConstPacketStatusIterator it) const { RTC_DCHECK(it != packet_status_window_.end()); ++it; if (it == packet_status_window_.end()) { @@ -244,26 +270,35 @@ TransportFeedbackPacketLossTracker::NextPacketStatus(PacketStatusIterator it) { // error after long period, we can remove the fuzzer test, and move this method // to unit test. void TransportFeedbackPacketLossTracker::Validate() const { // Testing only! - RTC_CHECK_LE(packet_status_window_.size(), max_window_size_); - RTC_CHECK_EQ(packet_status_window_.size(), - plr_state_.num_lost_packets_ + plr_state_.num_received_packets_); + 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_known_pairs_); - RTC_CHECK_LE(rplr_state_.num_known_pairs_, - packet_status_window_.size() - 1); + rplr_state_.num_acked_pairs_); + RTC_CHECK_LE(rplr_state_.num_acked_pairs_, acked_packets_ - 1); + size_t unacked_packets = 0; size_t received_packets = 0; size_t lost_packets = 0; - size_t known_status_pairs = 0; + size_t acked_pairs = 0; size_t recoverable_losses = 0; if (!packet_status_window_.empty()) { - PacketStatusIterator it = ref_packet_status_; + ConstPacketStatusIterator it = ref_packet_status_; do { - if (it->second) { - ++received_packets; - } else { - ++lost_packets; + switch (it->second) { + case PacketStatus::Unacked: + ++unacked_packets; + break; + case PacketStatus::Received: + ++received_packets; + break; + case PacketStatus::Lost: + ++lost_packets; + break; + default: + RTC_NOTREACHED(); } auto next = std::next(it); @@ -271,9 +306,11 @@ void TransportFeedbackPacketLossTracker::Validate() const { // Testing only! next = packet_status_window_.begin(); if (next != ref_packet_status_ && - next->first == static_cast(it->first + 1)) { - ++known_status_pairs; - if (!it->second && next->second) + it->second != PacketStatus::Unacked && + next->second != PacketStatus::Unacked) { + ++acked_pairs; + if (it->second == PacketStatus::Lost && + next->second == PacketStatus::Received) ++recoverable_losses; } @@ -286,14 +323,16 @@ void TransportFeedbackPacketLossTracker::Validate() const { // Testing only! RTC_CHECK_EQ(plr_state_.num_received_packets_, received_packets); RTC_CHECK_EQ(plr_state_.num_lost_packets_, lost_packets); - RTC_CHECK_EQ(rplr_state_.num_known_pairs_, known_status_pairs); + RTC_CHECK_EQ(packet_status_window_.size(), + unacked_packets + received_packets + lost_packets); + RTC_CHECK_EQ(rplr_state_.num_acked_pairs_, acked_pairs); RTC_CHECK_EQ(rplr_state_.num_recoverable_losses_, recoverable_losses); } rtc::Optional TransportFeedbackPacketLossTracker::PlrState::GetMetric() const { const size_t total = num_lost_packets_ + num_received_packets_; - if (total < min_num_packets_) { + if (total < min_num_acked_packets_) { return rtc::Optional(); } else { return rtc::Optional( @@ -303,11 +342,11 @@ TransportFeedbackPacketLossTracker::PlrState::GetMetric() const { rtc::Optional TransportFeedbackPacketLossTracker::RplrState::GetMetric() const { - if (num_known_pairs_ < min_num_pairs_) { + if (num_acked_pairs_ < min_num_acked_pairs_) { return rtc::Optional(); } else { return rtc::Optional( - static_cast(num_recoverable_losses_) / num_known_pairs_); + static_cast(num_recoverable_losses_) / num_acked_pairs_); } } diff --git a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h index 77af2fa7da..efd0f81d10 100644 --- a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h +++ b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h @@ -24,15 +24,17 @@ class TransportFeedback; class TransportFeedbackPacketLossTracker final { public: - // * Up to |max_window_size| latest packet statuses will be used for + // * Up to |max_acked_packets| latest packet statuses will be used for // calculating the packet loss metrics. // * PLR (packet-loss-rate) is reliably computable once the statuses of - // |plr_min_num_packets| packets are known. + // |plr_min_num_acked_packets| packets are known. // * RPLR (recoverable-packet-loss-rate) is reliably computable once the - // statuses of |rplr_min_num_pairs| pairs are known. - TransportFeedbackPacketLossTracker(size_t max_window_size, - size_t plr_min_num_packets, - size_t rplr_min_num_pairs); + // statuses of |rplr_min_num_acked_pairs| pairs are known. + TransportFeedbackPacketLossTracker(size_t max_acked_packets, + size_t plr_min_num_acked_packets, + size_t rplr_min_num_acked_pairs); + + void OnPacketAdded(uint16_t seq_num); void OnReceivedTransportFeedback(const rtcp::TransportFeedback& feedback); @@ -48,11 +50,14 @@ class TransportFeedbackPacketLossTracker final { void Validate() const; private: - // PacketStatus is a map from sequence number to its reception status. The - // status is true if the corresponding packet is received, and false if it is - // lost. Unknown statuses are not present in the map. - typedef std::map PacketStatus; - typedef PacketStatus::const_iterator PacketStatusIterator; + // When a packet is sent, we memorize its association with the stream by + // marking it as (sent-but-so-far-) unacked. If we ever receive a feedback + // that reports it as received/lost, we update the state and + // metrics accordingly. + + enum class PacketStatus { Unacked = 0, Received = 1, Lost = 2 }; + typedef std::map PacketStatusMap; + typedef PacketStatusMap::const_iterator ConstPacketStatusIterator; void Reset(); @@ -62,27 +67,31 @@ class TransportFeedbackPacketLossTracker final { // (2^16 + x - ref_seq_num_) % 2^16 defines its actual position in // |packet_status_window_|. uint16_t ReferenceSequenceNumber() const; - bool IsOldSequenceNumber(uint16_t seq_num) const; - void InsertPacketStatus(uint16_t seq_num, bool received); + uint16_t NewestSequenceNumber() const; + void RecordFeedback(PacketStatusMap::iterator it, bool received); void RemoveOldestPacketStatus(); - void UpdateMetrics(PacketStatusIterator it, bool apply /* false = undo */); - void UpdatePlr(PacketStatusIterator it, bool apply /* false = undo */); - void UpdateRplr(PacketStatusIterator it, bool apply /* false = undo */); + void UpdateMetrics(ConstPacketStatusIterator it, + bool apply /* false = undo */); + void UpdatePlr(ConstPacketStatusIterator it, bool apply /* false = undo */); + void UpdateRplr(ConstPacketStatusIterator it, bool apply /* false = undo */); - PacketStatusIterator PreviousPacketStatus(PacketStatusIterator it); - PacketStatusIterator NextPacketStatus(PacketStatusIterator it); + ConstPacketStatusIterator PreviousPacketStatus( + ConstPacketStatusIterator it) const; + ConstPacketStatusIterator NextPacketStatus( + ConstPacketStatusIterator it) const; - const size_t max_window_size_; + const size_t max_acked_packets_; + size_t acked_packets_; - PacketStatus packet_status_window_; + PacketStatusMap packet_status_window_; // |ref_packet_status_| points to the oldest item in |packet_status_window_|. - PacketStatusIterator ref_packet_status_; + ConstPacketStatusIterator ref_packet_status_; // Packet-loss-rate calculation (lost / all-known-packets). struct PlrState { - explicit PlrState(size_t min_num_packets) - : min_num_packets_(min_num_packets) { + explicit PlrState(size_t min_num_acked_packets) + : min_num_acked_packets_(min_num_acked_packets) { Reset(); } void Reset() { @@ -90,19 +99,19 @@ class TransportFeedbackPacketLossTracker final { num_lost_packets_ = 0; } rtc::Optional GetMetric() const; - const size_t min_num_packets_; + const size_t min_num_acked_packets_; size_t num_received_packets_; size_t num_lost_packets_; } plr_state_; // Recoverable packet loss calculation (first-order-FEC recoverable). struct RplrState { - explicit RplrState(size_t min_num_pairs) - : min_num_pairs_(min_num_pairs) { + explicit RplrState(size_t min_num_acked_pairs) + : min_num_acked_pairs_(min_num_acked_pairs) { Reset(); } void Reset() { - num_known_pairs_ = 0; + num_acked_pairs_ = 0; num_recoverable_losses_ = 0; } rtc::Optional GetMetric() const; @@ -111,12 +120,10 @@ class TransportFeedbackPacketLossTracker final { // the data from the former (lost) packet could be recovered. // The RPLR is calculated as the fraction of such pairs (lost-received) out // of all pairs of consecutive acked packets. - const size_t min_num_pairs_; - size_t num_known_pairs_; + const size_t min_num_acked_pairs_; + size_t num_acked_pairs_; size_t num_recoverable_losses_; } rplr_state_; - - size_t num_consecutive_old_reports_; // TODO(elad.alon): Upcoming CL removes. }; } // namespace webrtc 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 eb229a09d2..02719e3969 100644 --- a/webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc +++ b/webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc @@ -10,6 +10,7 @@ #include #include +#include #include #include "webrtc/base/checks.h" @@ -27,12 +28,36 @@ namespace webrtc { namespace { -constexpr size_t kMaxConsecutiveOldReports = 4; - // All tests are run multiple times with various baseline sequence number, // to weed out potential bugs with wrap-around handling. constexpr uint16_t kBases[] = {0x0000, 0x3456, 0xc032, 0xfffe}; +void SendPackets(TransportFeedbackPacketLossTracker* tracker, + const std::vector& seq_nums, + bool validate_all = true) { + for (uint16_t seq_num : seq_nums) { + tracker->OnPacketAdded(seq_num); + if (validate_all) { + tracker->Validate(); + } + } + + // We've either validated after each packet, or, for making sure the UT + // doesn't run too long, we might validate only at the end of the range. + if (!validate_all) { + tracker->Validate(); + } +} + +void SendPacketRange(TransportFeedbackPacketLossTracker* tracker, + uint16_t first_seq_num, + size_t num_of_seq_nums, + bool validate_all = true) { + std::vector seq_nums(num_of_seq_nums); + std::iota(seq_nums.begin(), seq_nums.end(), first_seq_num); + SendPackets(tracker, seq_nums, validate_all); +} + void AddTransportFeedbackAndValidate( TransportFeedbackPacketLossTracker* tracker, uint16_t base_sequence_num, @@ -98,7 +123,6 @@ void ValidatePacketLossStatistics( // Sanity check on an empty window. TEST(TransportFeedbackPacketLossTrackerTest, EmptyWindow) { - std::unique_ptr feedback; TransportFeedbackPacketLossTracker tracker(10, 5, 5); // PLR and RPLR reported as unknown before reception of first feedback. @@ -107,14 +131,33 @@ TEST(TransportFeedbackPacketLossTrackerTest, EmptyWindow) { rtc::Optional()); } +// A feedback received for an empty window has no effect. +TEST(TransportFeedbackPacketLossTrackerTest, EmptyWindowFeedback) { + for (uint16_t base : kBases) { + TransportFeedbackPacketLossTracker tracker(3, 3, 2); + + // Feedback doesn't correspond to any packets - ignored. + AddTransportFeedbackAndValidate(&tracker, base, {true, false, true}); + ValidatePacketLossStatistics(tracker, + rtc::Optional(), + rtc::Optional()); + + // After the packets are transmitted, acking them would have an effect. + SendPacketRange(&tracker, base, 3); + AddTransportFeedbackAndValidate(&tracker, base, {true, false, true}); + ValidatePacketLossStatistics(tracker, 1.0f / 3.0f, 0.5f); + } +} + // Sanity check on partially filled window. -TEST(TransportFeedbackPacketLossTrackerTest, PlrPartiallyFilledWindow) { +TEST(TransportFeedbackPacketLossTrackerTest, PartiallyFilledWindow) { for (uint16_t base : kBases) { TransportFeedbackPacketLossTracker tracker(10, 5, 4); // PLR unknown before minimum window size reached. // RPLR unknown before minimum pairs reached. // Expected window contents: [] -> [1001]. + SendPacketRange(&tracker, base, 3); AddTransportFeedbackAndValidate(&tracker, base, {true, false, false, true}); ValidatePacketLossStatistics(tracker, rtc::Optional(), @@ -130,6 +173,7 @@ TEST(TransportFeedbackPacketLossTrackerTest, PlrMinimumFilledWindow) { // PLR correctly calculated after minimum window size reached. // RPLR not necessarily known at that time (not if min-pairs not reached). // Expected window contents: [] -> [10011]. + SendPacketRange(&tracker, base, 5); AddTransportFeedbackAndValidate(&tracker, base, {true, false, false, true, true}); ValidatePacketLossStatistics(tracker, @@ -146,6 +190,7 @@ TEST(TransportFeedbackPacketLossTrackerTest, RplrMinimumFilledWindow) { // RPLR correctly calculated after minimum pairs reached. // PLR not necessarily known at that time (not if min window not reached). // Expected window contents: [] -> [10011]. + SendPacketRange(&tracker, base, 5); AddTransportFeedbackAndValidate(&tracker, base, {true, false, false, true, true}); ValidatePacketLossStatistics(tracker, @@ -159,6 +204,8 @@ TEST(TransportFeedbackPacketLossTrackerTest, ExtendWindow) { for (uint16_t base : kBases) { TransportFeedbackPacketLossTracker tracker(20, 5, 5); + SendPacketRange(&tracker, base, 25); + // Expected window contents: [] -> [10011]. AddTransportFeedbackAndValidate(&tracker, base, {true, false, false, true, true}); @@ -178,24 +225,41 @@ TEST(TransportFeedbackPacketLossTrackerTest, ExtendWindow) { } } -// All packets correctly received. +// Sanity - all packets correctly received. TEST(TransportFeedbackPacketLossTrackerTest, AllReceived) { for (uint16_t base : kBases) { TransportFeedbackPacketLossTracker tracker(10, 5, 4); - // PLR and RPLR correctly calculated after minimum window size reached. // Expected window contents: [] -> [11111]. + SendPacketRange(&tracker, base, 5); AddTransportFeedbackAndValidate(&tracker, base, {true, true, true, true, true}); ValidatePacketLossStatistics(tracker, 0.0f, 0.0f); } } +// Sanity - all packets lost. +TEST(TransportFeedbackPacketLossTrackerTest, AllLost) { + for (uint16_t base : kBases) { + TransportFeedbackPacketLossTracker tracker(10, 5, 4); + + // 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. + AddTransportFeedbackAndValidate(&tracker, base, + {false, false, false, false, false, true}); + ValidatePacketLossStatistics(tracker, 1.0f, 0.0f); + } +} + // Repeated reports are ignored. TEST(TransportFeedbackPacketLossTrackerTest, ReportRepetition) { for (uint16_t base : kBases) { TransportFeedbackPacketLossTracker tracker(10, 5, 4); + SendPacketRange(&tracker, base, 5); + // Expected window contents: [] -> [10011]. AddTransportFeedbackAndValidate(&tracker, base, {true, false, false, true, true}); @@ -214,6 +278,8 @@ TEST(TransportFeedbackPacketLossTrackerTest, ReportOverlap) { for (uint16_t base : kBases) { TransportFeedbackPacketLossTracker tracker(10, 5, 1); + SendPacketRange(&tracker, base, 15); + // Expected window contents: [] -> [10011]. AddTransportFeedbackAndValidate(&tracker, base, {true, false, false, true, true}); @@ -231,6 +297,8 @@ TEST(TransportFeedbackPacketLossTrackerTest, ReportConflict) { for (uint16_t base : kBases) { TransportFeedbackPacketLossTracker tracker(10, 5, 4); + SendPacketRange(&tracker, base, 15); + // Expected window contents: [] -> [01001]. AddTransportFeedbackAndValidate(&tracker, base, {false, true, false, false, true}); @@ -248,6 +316,8 @@ TEST(TransportFeedbackPacketLossTrackerTest, SkippedPackets) { for (uint16_t base : kBases) { TransportFeedbackPacketLossTracker tracker(10, 5, 1); + SendPacketRange(&tracker, base, 200); + // Expected window contents: [] -> [10011]. AddTransportFeedbackAndValidate(&tracker, base, {true, false, false, true, true}); @@ -259,12 +329,14 @@ TEST(TransportFeedbackPacketLossTrackerTest, SkippedPackets) { } } -// The window retain information up to the configured max-window-size, but -// starts discarding after that. +// The window retains information up to the configured max-window-size, but +// starts discarding after that. (Sent packets are not counted.) TEST(TransportFeedbackPacketLossTrackerTest, MaxWindowSize) { for (uint16_t base : kBases) { TransportFeedbackPacketLossTracker tracker(10, 10, 1); + SendPacketRange(&tracker, base, 200); + // Up to max-window-size retained. // Expected window contents: [] -> [1010100001]. AddTransportFeedbackAndValidate( @@ -280,11 +352,13 @@ TEST(TransportFeedbackPacketLossTrackerTest, MaxWindowSize) { } } -// Inserting into the middle of a full window works correctly. +// Inserting a feedback into the middle of a full window works correctly. TEST(TransportFeedbackPacketLossTrackerTest, InsertIntoMiddle) { for (uint16_t base : kBases) { TransportFeedbackPacketLossTracker tracker(10, 5, 1); + SendPacketRange(&tracker, base, 300); + // Expected window contents: [] -> [10101]. AddTransportFeedbackAndValidate(&tracker, base, {true, false, true, false, true}); @@ -303,11 +377,14 @@ TEST(TransportFeedbackPacketLossTrackerTest, InsertIntoMiddle) { } } -// Inserting into the middle of a full window works correctly. +// Inserting feedback into the middle of a full window works correctly - can +// complete two pairs. TEST(TransportFeedbackPacketLossTrackerTest, InsertionCompletesTwoPairs) { for (uint16_t base : kBases) { TransportFeedbackPacketLossTracker tracker(15, 5, 1); + SendPacketRange(&tracker, base, 300); + // Expected window contents: [] -> [10111]. AddTransportFeedbackAndValidate(&tracker, base, {true, false, true, true, true}); @@ -325,14 +402,14 @@ TEST(TransportFeedbackPacketLossTrackerTest, InsertionCompletesTwoPairs) { } } -// Entries in the second quadrant treated like those in the first. -// The sequence number is used in a looped manner. 0xFFFF is followed by 0x0000. -// In many tests, we divide the circle of sequence number into 4 quadrants, and -// verify the behavior of TransportFeedbackPacketLossTracker over them. +// The window can meaningfully hold up to 0x8000 SENT packets (of which only +// up to max-window acked messages will be kept and regarded). TEST(TransportFeedbackPacketLossTrackerTest, SecondQuadrant) { for (uint16_t base : kBases) { TransportFeedbackPacketLossTracker tracker(20, 5, 1); + SendPacketRange(&tracker, base, 0x8000, false); + // Expected window contents: [] -> [10011]. AddTransportFeedbackAndValidate(&tracker, base, {true, false, false, true, true}); @@ -351,48 +428,24 @@ TEST(TransportFeedbackPacketLossTrackerTest, SecondQuadrant) { } } -// Insertion into the third quadrant moves the base of the window. -TEST(TransportFeedbackPacketLossTrackerTest, ThirdQuadrantMovesBase) { - for (uint16_t base : kBases) { - TransportFeedbackPacketLossTracker tracker(20, 5, 1); - - // Seed the test. - // Expected window contents: [] -> [1001101]. - AddTransportFeedbackAndValidate( - &tracker, base, {true, false, false, true, true, false, true}); - ValidatePacketLossStatistics(tracker, 3.0f / 7.0f, 2.0f / 6.0f); - - // Quadrant #3 begins at base + 0x8000. It triggers moving the window so - // that - // at least one (oldest) report shifts out of window. - // Expected window contents: [1001101] -> [101-GAP-1001]. - AddTransportFeedbackAndValidate(&tracker, base + 0x8000, - {true, false, false, true}); - ValidatePacketLossStatistics(tracker, 3.0f / 7.0f, 2.0f / 5.0f); - - // The base can move more than once, because the minimum quadrant-1 packets - // were dropped out of the window, and some remain. - // Expected window contents: [101-GAP-1001] -> [1-GAP-100111]. - AddTransportFeedbackAndValidate(&tracker, base + 0x8000 + 4, {true, true}); - ValidatePacketLossStatistics(tracker, 2.0f / 7.0f, 1.0f / 5.0f); - } -} - // After the base has moved due to insertion into the third quadrant, it is -// still possible to insert into the middle of the window and obtain the correct -// PLR and RPLR. Insertion into the middle before the max window size has been -// achieved does not cause older packets to be dropped. -TEST(TransportFeedbackPacketLossTrackerTest, InsertIntoMiddleAfterBaseMove) { +// still possible to get feedbacks in the middle of the window and obtain the +// correct PLR and RPLR. Insertion into the middle before the max window size +// has been achieved does not cause older packets to be dropped. +TEST(TransportFeedbackPacketLossTrackerTest, InsertIntoMiddleAfterBaseMoved) { for (uint16_t base : kBases) { TransportFeedbackPacketLossTracker tracker(20, 5, 1); - // Seed the test. + 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); @@ -407,48 +460,57 @@ TEST(TransportFeedbackPacketLossTrackerTest, InsertIntoMiddleAfterBaseMove) { // The base can keep moving after inserting into the middle. // Expected window contents: // [101-GAP-100101-GAP-1001] -> [1-GAP-100101-GAP-100111]. + SendPacketRange(&tracker, base + 0x8000 + 4, 2); AddTransportFeedbackAndValidate(&tracker, base + 0x8000 + 4, {true, true}); ValidatePacketLossStatistics(tracker, 5.0f / 13.0f, 3.0f / 10.0f); } } // After moving the base of the window, the max window size is still observed. -TEST(TransportFeedbackPacketLossTrackerTest, ThirdQuadrantObservesMaxWindow) { +TEST(TransportFeedbackPacketLossTrackerTest, MaxWindowObservedAfterBaseMoved) { for (uint16_t base : kBases) { TransportFeedbackPacketLossTracker tracker(15, 10, 1); // Expected window contents: [] -> [1001110101]. + SendPacketRange(&tracker, base, 10); AddTransportFeedbackAndValidate( &tracker, base, {true, false, false, true, true, true, false, true, false, true}); ValidatePacketLossStatistics(tracker, 4.0f / 10.0f, 3.0f / 9.0f); + // Create gap (on both sides). + SendPacketRange(&tracker, base + 0x4000, 20); + // Expected window contents: [1001110101] -> [1110101-GAP-101]. + SendPacketRange(&tracker, base + 0x8000, 3); AddTransportFeedbackAndValidate(&tracker, base + 0x8000, {true, false, true}); ValidatePacketLossStatistics(tracker, 3.0f / 10.0f, 3.0f / 8.0f); - // Push into middle until max window is reached. + // Push into middle until max window is reached. The gap is NOT completed. // Expected window contents: // [1110101-GAP-101] -> [1110101-GAP-10001-GAP-101] - AddTransportFeedbackAndValidate(&tracker, base + 0x4000, + AddTransportFeedbackAndValidate(&tracker, base + 0x4000 + 2, {true, false, false, false, true}); ValidatePacketLossStatistics(tracker, 6.0f / 15.0f, 4.0f / 12.0f); // Pushing new packets into the middle would discard older packets. // Expected window contents: // [1110101-GAP-10001-GAP-101] -> [0101-GAP-10001101-GAP-101] - AddTransportFeedbackAndValidate(&tracker, base + 0x4000 + 5, + AddTransportFeedbackAndValidate(&tracker, base + 0x4000 + 2 + 5, {true, false, true}); ValidatePacketLossStatistics(tracker, 7.0f / 15.0f, 5.0f / 12.0f); } } -// A new feedback in quadrant #3 might shift enough old feedbacks out of window, -// that we'd go back to an unknown PLR and RPLR. -TEST(TransportFeedbackPacketLossTrackerTest, QuadrantThreeMovedBaseMinWindow) { +// A packet with a new enough sequence number might shift enough old feedbacks +// out window, that we'd go back to an unknown PLR and RPLR. +TEST(TransportFeedbackPacketLossTrackerTest, NewPacketMovesWindowBase) { for (uint16_t base : kBases) { - TransportFeedbackPacketLossTracker tracker(20, 5, 1); + TransportFeedbackPacketLossTracker tracker(20, 5, 3); + + SendPacketRange(&tracker, base, 50); + SendPacketRange(&tracker, base + 0x4000 - 1, 6); // Gap // Expected window contents: [] -> [1001110101]. AddTransportFeedbackAndValidate( @@ -456,14 +518,24 @@ TEST(TransportFeedbackPacketLossTrackerTest, QuadrantThreeMovedBaseMinWindow) { {true, false, false, true, true, true, false, true, false, true}); ValidatePacketLossStatistics(tracker, 4.0f / 10.0f, 3.0f / 9.0f); - // A new feedback in quadrant #3 might shift enough old feedbacks out of - // window, that we'd go back to an unknown PLR and RPLR. This *doesn't* - // necessarily mean all of the old ones were discarded, though. - // Expected window contents: [1001110101] -> [01-GAP-11]. + // A new sent packet with a new enough sequence number could shift enough + // acked packets out of window, that we'd go back to an unknown PLR + // and RPLR. This *doesn't* // necessarily mean all of the old ones + // were discarded, though. + // Expected window contents: [1001110101] -> [01]. + 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(1.0f / 2.0f)); + rtc::Optional()); // Inserting in the middle shows that though some of the elements were // ejected, some were retained. @@ -474,222 +546,118 @@ TEST(TransportFeedbackPacketLossTrackerTest, QuadrantThreeMovedBaseMinWindow) { } } -// Quadrant four reports ignored for up to kMaxConsecutiveOldReports times. -TEST(TransportFeedbackPacketLossTrackerTest, QuadrantFourInitiallyIgnored) { +// Sequence number gaps are not gaps in reception. However, gaps in reception +// are still possible, if a packet which WAS sent on the stream is not acked. +TEST(TransportFeedbackPacketLossTrackerTest, SanityGapsInSequenceNumbers) { for (uint16_t base : kBases) { TransportFeedbackPacketLossTracker tracker(20, 5, 1); - // Expected window contents: [] -> [10011]. - AddTransportFeedbackAndValidate(&tracker, base, - {true, false, false, true, true}); + SendPackets(&tracker, {static_cast(base), + static_cast(base + 2), + static_cast(base + 4), + static_cast(base + 6), + static_cast(base + 8)}); - // Feedbacks in quadrant #4 are discarded (up to kMaxConsecutiveOldReports - // consecutive reports). - // Expected window contents: [10011] -> [10011]. - for (size_t i = 0; i < kMaxConsecutiveOldReports; i++) { - AddTransportFeedbackAndValidate(&tracker, base + 0xc000, {true, true}); - ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f); - } - } -} - -// Receiving a packet from quadrant #1 resets the counter for quadrant #4. -TEST(TransportFeedbackPacketLossTrackerTest, QuadrantFourCounterResetByQ1) { - for (uint16_t base : kBases) { - TransportFeedbackPacketLossTracker tracker(20, 5, 1); - - // Expected window contents: [] -> [10011]. - AddTransportFeedbackAndValidate(&tracker, base, - {true, false, false, true, true}); - - // Feedbacks in quadrant #4 are discarded (up to kMaxConsecutiveOldReports - // consecutive reports). - // Expected window contents: [10011] -> [10011]. - for (size_t i = 0; i < kMaxConsecutiveOldReports; i++) { - AddTransportFeedbackAndValidate(&tracker, base + 0xc000, {true, true}); - ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f); - } - - // If we receive a feedback in quadrant #1, the above counter is reset. - // Expected window contents: [10011] -> [100111]. - AddTransportFeedbackAndValidate(&tracker, base + 5, {true}); - for (size_t i = 0; i < kMaxConsecutiveOldReports; i++) { - // Note: though the feedback message reports three packets, it only gets - // counted once. - AddTransportFeedbackAndValidate(&tracker, base + 0xc000, - {true, false, true}); - ValidatePacketLossStatistics(tracker, 2.0f / 6.0f, 1.0f / 5.0f); - } - - // The same is true for reports which create a gap - they still reset. - // Expected window contents: [10011] -> [100111-GAP-01]. - AddTransportFeedbackAndValidate(&tracker, base + 0x00ff, {false, true}); - for (size_t i = 0; i < kMaxConsecutiveOldReports; i++) { - // Note: though the feedback message reports three packets, it only gets - // counted once. - AddTransportFeedbackAndValidate(&tracker, base + 0xc000, - {true, false, true}); - ValidatePacketLossStatistics(tracker, 3.0f / 8.0f, 2.0f / 6.0f); - } - } -} - -// Receiving a packet from quadrant #2 resets the counter for quadrant #4. -TEST(TransportFeedbackPacketLossTrackerTest, QuadrantFourCounterResetByQ2) { - for (uint16_t base : kBases) { - TransportFeedbackPacketLossTracker tracker(20, 5, 1); - - // Expected window contents: [] -> [10011]. - AddTransportFeedbackAndValidate(&tracker, base, - {true, false, false, true, true}); - - // Feedbacks in quadrant #4 are discarded (up to kMaxConsecutiveOldReports - // consecutive reports). - // Expected window contents: [10011] -> [10011]. - for (size_t i = 0; i < kMaxConsecutiveOldReports; i++) { - AddTransportFeedbackAndValidate(&tracker, base + 0xc000, {true, true}); - ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f); - } - - // If we receive a feedback in quadrant #2, the above counter is reset. - // Expected window contents: [10011] -> [10011-GAP-11]. - AddTransportFeedbackAndValidate(&tracker, base + 0x400f, {true, true}); - for (size_t i = 0; i < kMaxConsecutiveOldReports; i++) { - // Note: though the feedback message reports three packets, it only gets - // counted once. - AddTransportFeedbackAndValidate(&tracker, base + 0xc000, - {true, false, true}); - ValidatePacketLossStatistics(tracker, 2.0f / 7.0f, 1.0f / 5.0f); - } - } -} - -// Receiving a packet from quadrant #3 resets the counter for quadrant #4. -TEST(TransportFeedbackPacketLossTrackerTest, QuadrantFourCounterResetByQ3) { - for (uint16_t base : kBases) { - TransportFeedbackPacketLossTracker tracker(20, 5, 1); - - // Expected window contents: [] -> [1001110001]. + // Gaps in sequence numbers not considered as gaps in window, because only + // those sequence numbers which were associated with the stream count. + // Expected window contents: [] -> [11011]. AddTransportFeedbackAndValidate( - &tracker, base, - {true, false, false, true, true, true, false, false, false, true}); + // Note: Left packets belong to this stream, odd ones ignored. + &tracker, base, {true, false, + true, false, + false, false, + true, false, + true, true}); + ValidatePacketLossStatistics(tracker, 1.0f / 5.0f, 1.0f / 4.0f); - // Feedbacks in quadrant #4 are discarded (up to kMaxConsecutiveOldReports - // consecutive reports). - // Expected window contents: [1001110001] -> [1001110001]. - for (size_t i = 0; i < kMaxConsecutiveOldReports; i++) { - AddTransportFeedbackAndValidate(&tracker, base + 0xc000, {true, true}); - ValidatePacketLossStatistics(tracker, 5.0f / 10.0f, 2.0f / 9.0f); - } - - // If we receive a feedback in quadrant #1, the above counter is reset. - // Expected window contents: [1001110001] -> [1110001-GAP-111]. - AddTransportFeedbackAndValidate(&tracker, base + 0x8000, - {true, true, true}); - for (size_t i = 0; i < kMaxConsecutiveOldReports; i++) { - // Note: though the feedback message reports three packets, it only gets - // counted once. - AddTransportFeedbackAndValidate(&tracker, base + 0xc000 + 10, - {true, false, true}); - ValidatePacketLossStatistics(tracker, 3.0f / 10.0f, 1.0f / 8.0f); - } + // 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)}); + AddTransportFeedbackAndValidate(&tracker, base + 11, + {false, false, false, true, true}); + ValidatePacketLossStatistics(tracker, 2.0f / 7.0f, 2.0f / 5.0f); } } -// Quadrant four reports ignored for up to kMaxConsecutiveOldReports times. -// After that, the window is reset. -TEST(TransportFeedbackPacketLossTrackerTest, QuadrantFourReset) { +// Sending a packet which is less than 0x8000 away from the baseline does +// not move the window. +TEST(TransportFeedbackPacketLossTrackerTest, UnackedInWindowDoesNotMoveWindow) { for (uint16_t base : kBases) { - TransportFeedbackPacketLossTracker tracker(20, 5, 1); + TransportFeedbackPacketLossTracker tracker(5, 3, 1); - // Expected window contents: [] -> [1001110001]. - AddTransportFeedbackAndValidate( - &tracker, base, - {true, false, false, true, true, true, false, false, false, true}); - - // Sanity - ValidatePacketLossStatistics(tracker, 5.0f / 10.0f, 2.0f / 9.0f); - - // The first kMaxConsecutiveOldReports quadrant #4 reports are ignored. - // It doesn't matter that they consist of multiple packets - each report - // is only counted once. - for (size_t i = 0; i < kMaxConsecutiveOldReports; i++) { - // Expected window contents: [1001110001] -> [1001110001]. - AddTransportFeedbackAndValidate(&tracker, base + 0xc000, - {true, true, false, true}); - ValidatePacketLossStatistics(tracker, 5.0f / 10.0f, 2.0f / 9.0f); - } - - // One additional feedback in quadrant #4 brings us over - // kMaxConsecutiveOldReports consecutive "old" reports, resetting the - // window. - // The new window is not completely empty - it's been seeded with the - // packets reported in the feedback that has triggered the reset. - // Note: The report doesn't have to be the same as the previous ones. - // Expected window contents: [1001110001] -> [10011]. - AddTransportFeedbackAndValidate(&tracker, base + 0xc000, - {true, false, false, true, true}); - ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f); - } -} - -// Feedbacks spanning multiple quadrant are treated correctly (Q1-Q2). -TEST(TransportFeedbackPacketLossTrackerTest, MultiQuadrantQ1Q2) { - for (uint16_t base : kBases) { - TransportFeedbackPacketLossTracker tracker(20, 5, 1); - - // Expected window contents: [] -> [10011]. + // Baseline - window has acked messages. + // Expected window contents: [] -> [10101]. + SendPacketRange(&tracker, base, 5); AddTransportFeedbackAndValidate(&tracker, base, - {true, false, false, true, true}); - ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f); + {true, false, true, false, true}); + ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 2.0f / 4.0f); - // A feedback with entries in both quadrant #1 and #2 gets both counted: - // Expected window contents: [10011] -> [10011-GAP-1001]. - AddTransportFeedbackAndValidate(&tracker, base + 0x3ffe, - {true, false, false, true}); - ValidatePacketLossStatistics(tracker, 4.0f / 9.0f, 2.0f / 7.0f); + // 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); } } -// Feedbacks spanning multiple quadrant are treated correctly (Q2-Q3). -TEST(TransportFeedbackPacketLossTrackerTest, MultiQuadrantQ2Q3) { +// 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(TransportFeedbackPacketLossTrackerTest, UnackedOutOfWindowMovesWindow) { for (uint16_t base : kBases) { - TransportFeedbackPacketLossTracker tracker(20, 5, 1); + TransportFeedbackPacketLossTracker tracker(5, 3, 1); - // Expected window contents: [] -> [1001100001]. - AddTransportFeedbackAndValidate( - &tracker, base, - {true, false, false, true, true, false, false, false, false, true}); - ValidatePacketLossStatistics(tracker, 6.0f / 10.0f, 2.0f / 9.0f); + // Baseline - window has acked messages. + // Expected window contents: [] -> [10101]. + SendPacketRange(&tracker, base, 5); + AddTransportFeedbackAndValidate(&tracker, base, + {true, false, true, false, true}); + ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 2.0f / 4.0f); - // A feedback with entries in both quadrant #2 and #3 gets both counted, - // but only those from #3 trigger throwing out old entries from quadrant #1: - // Expected window contents: [1001100001] -> [01100001-GAP-1001]. - AddTransportFeedbackAndValidate(&tracker, base + 0x7ffe, - {true, false, false, true}); - ValidatePacketLossStatistics(tracker, 7.0f / 12.0f, 3.0f / 10.0f); + // 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); } } -// Feedbacks spanning multiple quadrant are treated correctly (Q3-Q4). -TEST(TransportFeedbackPacketLossTrackerTest, MultiQuadrantQ3Q4) { +// 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(TransportFeedbackPacketLossTrackerTest, WindowResetAfterLongNoSend) { for (uint16_t base : kBases) { - TransportFeedbackPacketLossTracker tracker(20, 5, 1); + TransportFeedbackPacketLossTracker tracker(10, 2, 1); - // Expected window contents: [] -> [1001100001]. - AddTransportFeedbackAndValidate( - &tracker, base, - {true, false, false, true, true, false, false, false, false, true}); - ValidatePacketLossStatistics(tracker, 6.0f / 10.0f, 2.0f / 9.0f); + // Baseline - window has acked messages. + // Expected window contents: [] -> [1-GAP-10101]. + SendPacketRange(&tracker, base, 1); + SendPacketRange(&tracker, base + 0x7fff - 4, 5); + AddTransportFeedbackAndValidate(&tracker, base, {true}); + AddTransportFeedbackAndValidate(&tracker, base + 0x7fff - 4, + {true, false, true, false, true}); + ValidatePacketLossStatistics(tracker, 2.0f / 6.0f, 2.0f / 5.0f); - // A feedback with entries in both quadrant #3 and #4 would have the entries - // from quadrant #3 shift enough quadrant #1 entries out of window, that - // by the time the #4 packets are examined, the moving baseline has made - // them into quadrant #3 packets. - // Expected window contents: [1001100001] -> [10011]. - AddTransportFeedbackAndValidate(&tracker, base + 0xbfff, - {true, false, false, true, true}); - ValidatePacketLossStatistics(tracker, 2.0f / 5.0f, 1.0f / 4.0f); + // 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()); + + // To show it was really reset, prove show that acking the sent packet + // still leaves us at unknown, because no acked (or unacked) packets were + // left in the window. + // Expected window contents: [] -> [1]. + AddTransportFeedbackAndValidate(&tracker, base + 0x7fff + 0x8000, {true}); + ValidatePacketLossStatistics(tracker, + rtc::Optional(), + rtc::Optional()); } }