diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.cc b/modules/rtp_rtcp/source/receive_statistics_impl.cc index 066dc4b7b9..bc742d1ea6 100644 --- a/modules/rtp_rtcp/source/receive_statistics_impl.cc +++ b/modules/rtp_rtcp/source/receive_statistics_impl.cc @@ -48,10 +48,11 @@ StreamStatisticianImpl::StreamStatisticianImpl( last_receive_time_ms_(0), last_received_timestamp_(0), received_seq_first_(0), - received_seq_max_(-1), + received_seq_max_(0), + received_seq_wraps_(0), last_report_inorder_packets_(0), last_report_old_packets_(0), - last_report_seq_max_(-1), + last_report_seq_max_(0), rtcp_callback_(rtcp_callback), rtp_callback_(rtp_callback) {} @@ -63,75 +64,54 @@ void StreamStatisticianImpl::OnRtpPacket(const RtpPacketReceived& packet) { rtp_callback_->DataCountersUpdated(counters, ssrc_); } -bool StreamStatisticianImpl::UpdateOutOfOrder(const RtpPacketReceived& packet, - int64_t sequence_number, - int64_t now_ms) { - RTC_DCHECK_EQ(sequence_number, - seq_unwrapper_.UnwrapWithoutUpdate(packet.SequenceNumber())); - - // Check if |packet| is second packet of a stream restart. - if (received_seq_out_of_order_) { - uint16_t expected_sequence_number = *received_seq_out_of_order_ + 1; - received_seq_out_of_order_ = absl::nullopt; - if (packet.SequenceNumber() == expected_sequence_number) { - // Ignore sequence number gap caused by stream restart for next packet - // loss calculation. - last_report_seq_max_ = sequence_number; - last_report_inorder_packets_ = receive_counters_.transmitted.packets - - receive_counters_.retransmitted.packets; - // As final part of stream restart consider |packet| is not out of order. - return false; - } - } - - if (std::abs(sequence_number - received_seq_max_) > - max_reordering_threshold_) { - // Sequence number gap looks too large, wait until next packet to check - // for a stream restart. - received_seq_out_of_order_ = packet.SequenceNumber(); - return true; - } - - if (sequence_number > received_seq_max_) - return false; - - // Old out of order packet, may be retransmit. - if (enable_retransmit_detection_ && IsRetransmitOfOldPacket(packet, now_ms)) - receive_counters_.retransmitted.AddPacket(packet); - return true; -} - StreamDataCounters StreamStatisticianImpl::UpdateCounters( const RtpPacketReceived& packet) { rtc::CritScope cs(&stream_lock_); RTC_DCHECK_EQ(ssrc_, packet.Ssrc()); + uint16_t sequence_number = packet.SequenceNumber(); + bool in_order = + // First packet is always in order. + last_receive_time_ms_ == 0 || + IsNewerSequenceNumber(sequence_number, received_seq_max_) || + // If we have a restart of the remote side this packet is still in order. + !IsNewerSequenceNumber(sequence_number, + received_seq_max_ - max_reordering_threshold_); int64_t now_ms = clock_->TimeInMilliseconds(); incoming_bitrate_.Update(packet.size(), now_ms); receive_counters_.transmitted.AddPacket(packet); + if (!in_order && enable_retransmit_detection_ && + IsRetransmitOfOldPacket(packet, now_ms)) { + receive_counters_.retransmitted.AddPacket(packet); + } - int64_t sequence_number = - seq_unwrapper_.UnwrapWithoutUpdate(packet.SequenceNumber()); - if (!ReceivedRtpPacket()) { - received_seq_first_ = sequence_number; - last_report_seq_max_ = sequence_number - 1; + if (receive_counters_.transmitted.packets == 1) { + received_seq_first_ = packet.SequenceNumber(); receive_counters_.first_packet_time_ms = now_ms; - } else if (UpdateOutOfOrder(packet, sequence_number, now_ms)) { - return receive_counters_; } - // In order packet. - received_seq_max_ = sequence_number; - seq_unwrapper_.UpdateLast(sequence_number); - // If new time stamp and more than one in-order packet received, calculate - // new jitter statistics. - if (packet.Timestamp() != last_received_timestamp_ && - (receive_counters_.transmitted.packets - - receive_counters_.retransmitted.packets) > 1) { - UpdateJitter(packet, now_ms); + // Count only the new packets received. That is, if packets 1, 2, 3, 5, 4, 6 + // are received, 4 will be ignored. + if (in_order) { + // Wrong if we use RetransmitOfOldPacket. + if (receive_counters_.transmitted.packets > 1 && + received_seq_max_ > packet.SequenceNumber()) { + // Wrap around detected. + received_seq_wraps_++; + } + // New max. + received_seq_max_ = packet.SequenceNumber(); + + // If new time stamp and more than one in-order packet received, calculate + // new jitter statistics. + if (packet.Timestamp() != last_received_timestamp_ && + (receive_counters_.transmitted.packets - + receive_counters_.retransmitted.packets) > 1) { + UpdateJitter(packet, now_ms); + } + last_received_timestamp_ = packet.Timestamp(); + last_receive_time_ms_ = now_ms; } - last_received_timestamp_ = packet.Timestamp(); - last_receive_time_ms_ = now_ms; return receive_counters_; } @@ -183,7 +163,9 @@ bool StreamStatisticianImpl::GetStatistics(RtcpStatistics* statistics, bool reset) { { rtc::CritScope cs(&stream_lock_); - if (!ReceivedRtpPacket()) { + if (received_seq_first_ == 0 && + receive_counters_.transmitted.payload_bytes == 0) { + // We have not received anything. return false; } @@ -214,7 +196,9 @@ bool StreamStatisticianImpl::GetActiveStatisticsAndReset( // Not active. return false; } - if (!ReceivedRtpPacket()) { + if (received_seq_first_ == 0 && + receive_counters_.transmitted.payload_bytes == 0) { + // We have not received anything. return false; } @@ -228,9 +212,19 @@ bool StreamStatisticianImpl::GetActiveStatisticsAndReset( RtcpStatistics StreamStatisticianImpl::CalculateRtcpStatistics() { RtcpStatistics stats; + + if (last_report_inorder_packets_ == 0) { + // First time we send a report. + last_report_seq_max_ = received_seq_first_ - 1; + } + // Calculate fraction lost. - int64_t exp_since_last = received_seq_max_ - last_report_seq_max_; - RTC_DCHECK_GE(exp_since_last, 0); + uint16_t exp_since_last = (received_seq_max_ - last_report_seq_max_); + + if (last_report_seq_max_ > received_seq_max_) { + // Can we assume that the seq_num can't go decrease over a full RTCP period? + exp_since_last = 0; + } // Number of received RTP packets since last report, counts all packets but // not re-transmissions. @@ -267,7 +261,7 @@ RtcpStatistics StreamStatisticianImpl::CalculateRtcpStatistics() { cumulative_loss_ += missing; stats.packets_lost = cumulative_loss_; stats.extended_highest_sequence_number = - static_cast(received_seq_max_); + (received_seq_wraps_ << 16) + received_seq_max_; // Note: internal jitter value is in Q4 and needs to be scaled by 1/16. stats.jitter = jitter_q4_ >> 4; diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.h b/modules/rtp_rtcp/source/receive_statistics_impl.h index 0221d857d2..8153c4400d 100644 --- a/modules/rtp_rtcp/source/receive_statistics_impl.h +++ b/modules/rtp_rtcp/source/receive_statistics_impl.h @@ -17,8 +17,6 @@ #include #include -#include "absl/types/optional.h" -#include "modules/include/module_common_types_public.h" #include "rtc_base/criticalsection.h" #include "rtc_base/rate_statistics.h" #include "rtc_base/thread_annotations.h" @@ -60,18 +58,7 @@ class StreamStatisticianImpl : public StreamStatistician, RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_); void UpdateJitter(const RtpPacketReceived& packet, int64_t receive_time_ms) RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_); - // Updates StreamStatistician for out of order packets. - // Returns true if packet considered to be out of order. - bool UpdateOutOfOrder(const RtpPacketReceived& packet, - int64_t sequence_number, - int64_t now_ms) - RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_); - // Updates StreamStatistician for incoming packets. StreamDataCounters UpdateCounters(const RtpPacketReceived& packet); - // Checks if this StreamStatistician received any rtp packets. - bool ReceivedRtpPacket() const RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_) { - return received_seq_max_ >= 0; - } const uint32_t ssrc_; Clock* const clock_; @@ -87,13 +74,9 @@ class StreamStatisticianImpl : public StreamStatistician, int64_t last_receive_time_ms_ RTC_GUARDED_BY(&stream_lock_); uint32_t last_received_timestamp_ RTC_GUARDED_BY(&stream_lock_); - SequenceNumberUnwrapper seq_unwrapper_ RTC_GUARDED_BY(&stream_lock_); - int64_t received_seq_first_ RTC_GUARDED_BY(&stream_lock_); - int64_t received_seq_max_ RTC_GUARDED_BY(&stream_lock_); - // Assume that the other side restarted when there are two sequential packets - // with large jump from received_seq_max_. - absl::optional received_seq_out_of_order_ - RTC_GUARDED_BY(&stream_lock_); + uint16_t received_seq_first_ RTC_GUARDED_BY(&stream_lock_); + uint16_t received_seq_max_ RTC_GUARDED_BY(&stream_lock_); + uint16_t received_seq_wraps_ RTC_GUARDED_BY(&stream_lock_); // Current counter values. StreamDataCounters receive_counters_ RTC_GUARDED_BY(&stream_lock_); @@ -101,7 +84,7 @@ class StreamStatisticianImpl : public StreamStatistician, // Counter values when we sent the last report. uint32_t last_report_inorder_packets_ RTC_GUARDED_BY(&stream_lock_); uint32_t last_report_old_packets_ RTC_GUARDED_BY(&stream_lock_); - int64_t last_report_seq_max_ RTC_GUARDED_BY(&stream_lock_); + uint16_t last_report_seq_max_ RTC_GUARDED_BY(&stream_lock_); RtcpStatistics last_reported_statistics_ RTC_GUARDED_BY(&stream_lock_); // stream_lock_ shouldn't be held when calling callbacks. diff --git a/modules/rtp_rtcp/source/receive_statistics_unittest.cc b/modules/rtp_rtcp/source/receive_statistics_unittest.cc index 1703ceef41..25393631df 100644 --- a/modules/rtp_rtcp/source/receive_statistics_unittest.cc +++ b/modules/rtp_rtcp/source/receive_statistics_unittest.cc @@ -295,217 +295,6 @@ TEST_F(ReceiveStatisticsTest, RtcpCallbacks) { EXPECT_EQ(177u, statistics.jitter); } -TEST_F(ReceiveStatisticsTest, SimpleLossComputation) { - packet1_.SetSequenceNumber(1); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(3); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(4); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(5); - receive_statistics_->OnRtpPacket(packet1_); - - RtcpStatistics statistics; - receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, - true); - // 20% = 51/255. - EXPECT_EQ(51u, statistics.fraction_lost); - EXPECT_EQ(1, statistics.packets_lost); -} - -TEST_F(ReceiveStatisticsTest, LossComputationWithReordering) { - packet1_.SetSequenceNumber(1); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(3); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(2); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(5); - receive_statistics_->OnRtpPacket(packet1_); - - RtcpStatistics statistics; - receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, - true); - // 20% = 51/255. - EXPECT_EQ(51u, statistics.fraction_lost); - EXPECT_EQ(1, statistics.packets_lost); -} - -TEST_F(ReceiveStatisticsTest, LossComputationWithDuplicates) { - // Lose 2 packets, but also receive 1 duplicate. Should actually count as - // only 1 packet being lost. - packet1_.SetSequenceNumber(1); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(4); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(4); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(5); - receive_statistics_->OnRtpPacket(packet1_); - - RtcpStatistics statistics; - receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, - true); - // 20% = 51/255. - EXPECT_EQ(51u, statistics.fraction_lost); - EXPECT_EQ(1, statistics.packets_lost); -} - -TEST_F(ReceiveStatisticsTest, LossComputationWithSequenceNumberWrapping) { - // First, test loss computation over a period that included a sequence number - // rollover. - packet1_.SetSequenceNumber(0xfffd); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(0); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(0xfffe); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(1); - receive_statistics_->OnRtpPacket(packet1_); - - // Only one packet was actually lost, 0xffff. - RtcpStatistics statistics; - receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, - true); - // 20% = 51/255. - EXPECT_EQ(51u, statistics.fraction_lost); - EXPECT_EQ(1, statistics.packets_lost); - - // Now test losing one packet *after* the rollover. - packet1_.SetSequenceNumber(3); - receive_statistics_->OnRtpPacket(packet1_); - receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, - true); - // 50% = 127/255. - EXPECT_EQ(127u, statistics.fraction_lost); - EXPECT_EQ(2, statistics.packets_lost); -} - -TEST_F(ReceiveStatisticsTest, StreamRestartDoesntCountAsLoss) { - RtcpStatistics statistics; - receive_statistics_->SetMaxReorderingThreshold(200); - - packet1_.SetSequenceNumber(0); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(1); - receive_statistics_->OnRtpPacket(packet1_); - - packet1_.SetSequenceNumber(400); - receive_statistics_->OnRtpPacket(packet1_); - receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, - true); - EXPECT_EQ(0, statistics.fraction_lost); - EXPECT_EQ(0, statistics.packets_lost); - - packet1_.SetSequenceNumber(401); - receive_statistics_->OnRtpPacket(packet1_); - receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, - true); - EXPECT_EQ(0, statistics.fraction_lost); - EXPECT_EQ(0, statistics.packets_lost); -} - -TEST_F(ReceiveStatisticsTest, CountsLossAfterStreamRestart) { - RtcpStatistics statistics; - receive_statistics_->SetMaxReorderingThreshold(200); - - packet1_.SetSequenceNumber(0); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(1); - receive_statistics_->OnRtpPacket(packet1_); - - packet1_.SetSequenceNumber(400); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(401); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(403); - receive_statistics_->OnRtpPacket(packet1_); - - receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, - true); - EXPECT_EQ(1, statistics.packets_lost); -} - -TEST_F(ReceiveStatisticsTest, StreamCanRestartAtSequenceNumberWrapAround) { - RtcpStatistics statistics; - receive_statistics_->SetMaxReorderingThreshold(200); - - packet1_.SetSequenceNumber(0xffff - 401); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(0xffff - 400); - receive_statistics_->OnRtpPacket(packet1_); - - packet1_.SetSequenceNumber(0xffff); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(0); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(2); - receive_statistics_->OnRtpPacket(packet1_); - - receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, - true); - EXPECT_EQ(1, statistics.packets_lost); -} - -TEST_F(ReceiveStatisticsTest, StreamRestartNeedsTwoConsecutivePackets) { - RtcpStatistics statistics; - receive_statistics_->SetMaxReorderingThreshold(200); - - packet1_.SetSequenceNumber(400); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(401); - receive_statistics_->OnRtpPacket(packet1_); - - packet1_.SetSequenceNumber(1); - receive_statistics_->OnRtpPacket(packet1_); - packet1_.SetSequenceNumber(3); - receive_statistics_->OnRtpPacket(packet1_); - receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, - true); - EXPECT_EQ(401u, statistics.extended_highest_sequence_number); - - packet1_.SetSequenceNumber(4); - receive_statistics_->OnRtpPacket(packet1_); - receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, - true); - EXPECT_EQ(4u, statistics.extended_highest_sequence_number); -} - -TEST_F(ReceiveStatisticsTest, WrapsAroundExtendedHighestSequenceNumber) { - RtcpStatistics statistics; - packet1_.SetSequenceNumber(0xffff); - receive_statistics_->OnRtpPacket(packet1_); - receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, - true); - EXPECT_EQ(0xffffu, statistics.extended_highest_sequence_number); - - // Wrap around. - packet1_.SetSequenceNumber(1); - receive_statistics_->OnRtpPacket(packet1_); - receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, - true); - EXPECT_EQ(0x10001u, statistics.extended_highest_sequence_number); - - // Should be treated as out of order; shouldn't increment highest extended - // sequence number. - packet1_.SetSequenceNumber(0x10000 - 6); - receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, - true); - EXPECT_EQ(0x10001u, statistics.extended_highest_sequence_number); - - // Receive a couple packets then wrap around again. - receive_statistics_->SetMaxReorderingThreshold(200); - for (int i = 10; i < 0xffff; i += 150) { - packet1_.SetSequenceNumber(i); - receive_statistics_->OnRtpPacket(packet1_); - } - packet1_.SetSequenceNumber(1); - receive_statistics_->OnRtpPacket(packet1_); - receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, - true); - EXPECT_EQ(0x20001u, statistics.extended_highest_sequence_number); -} - class RtpTestCallback : public StreamDataCountersCallback { public: RtpTestCallback()