From 84916937b70472715efe5682bc273e91c3a72695 Mon Sep 17 00:00:00 2001 From: Taylor Brandstetter Date: Mon, 25 Jun 2018 15:50:26 -0700 Subject: [PATCH] Update packetsLost and jitter stats any time a packet is received. Before this CL, the packetsLost and jitter stats (as returned by GetStats, at the API level) were only being updated when an RTCP SR or RR is generated. According to the stats spec, "local" stats like this should be updated any time a packet is received. This CL also fixes some minor issues with the calculation of packetsLost (and fractionLost): * Packets weren't being count as lost if lost over a sequence number rollover. * Temporary periods of "negative" loss (caused by duplicate or out of order packets) weren't being accumulated into the cumulative loss counter. Example: Period 1: Received packets 1, 2, 4 Loss over that period: 1 (expected 4 packets, got 3) Reported cumulative loss: 1 Period 2: Received packets 3, 5 Loss over that period: -1 (expected 1 packet, got 2) Reported cumulative loss: 1 (should be 0!) Landing with NOTRY because Android compile bots are broken for an unrelated reason. NOTRY=True Bug: webrtc:8804 Change-Id: I840ba34de8957b1276f6bdaf93718f805629f5c8 Reviewed-on: https://webrtc-review.googlesource.com/50020 Commit-Queue: Taylor Brandstetter Reviewed-by: Danil Chapovalov Reviewed-by: Oskar Sundbom Cr-Commit-Position: refs/heads/master@{#23731} --- audio/channel.cc | 10 +- modules/rtp_rtcp/include/receive_statistics.h | 12 +- .../source/receive_statistics_impl.cc | 143 +++-- .../rtp_rtcp/source/receive_statistics_impl.h | 26 +- .../source/receive_statistics_unittest.cc | 488 ++++++++++++++++-- 5 files changed, 528 insertions(+), 151 deletions(-) diff --git a/audio/channel.cc b/audio/channel.cc index 7eafd52a4a..e704d7ef06 100644 --- a/audio/channel.cc +++ b/audio/channel.cc @@ -1154,14 +1154,16 @@ int Channel::GetRemoteRTCPReportBlocks( int Channel::GetRTPStatistics(CallStatistics& stats) { // --- RtcpStatistics - // The jitter statistics is updated for each received RTP packet and is - // based on received packets. + // Jitter, cumulative loss, and extended max sequence number is updated for + // each received RTP packet. RtcpStatistics statistics; StreamStatistician* statistician = rtp_receive_statistics_->GetStatistician(rtp_receiver_->SSRC()); if (statistician) { - statistician->GetStatistics(&statistics, - _rtpRtcpModule->RTCP() == RtcpMode::kOff); + // Recompute |fraction_lost| only if RTCP is off. If it's on, then + // |fraction_lost| should only be recomputed when an RTCP SR or RR is sent. + bool update_fraction_lost = _rtpRtcpModule->RTCP() == RtcpMode::kOff; + statistician->GetStatistics(&statistics, update_fraction_lost); } stats.fractionLost = statistics.fraction_lost; diff --git a/modules/rtp_rtcp/include/receive_statistics.h b/modules/rtp_rtcp/include/receive_statistics.h index 0adc79bcd0..5fff41e797 100644 --- a/modules/rtp_rtcp/include/receive_statistics.h +++ b/modules/rtp_rtcp/include/receive_statistics.h @@ -37,7 +37,17 @@ class StreamStatistician { public: virtual ~StreamStatistician(); - virtual bool GetStatistics(RtcpStatistics* statistics, bool reset) = 0; + // If |update_fraction_lost| is true, |fraction_lost| will be recomputed + // between now and the last time |update_fraction_lost| was true. Otherwise + // the last-computed value of |fraction_lost| will be returned. + // + // |update_fraction_lost| should be true any time an RTCP SR or RR is being + // generated, since RFC3550 defines it as the fraction of packets lost since + // the previous SR or RR packet was sent. + // + // Aside from |fraction_lost|, every other value will be freshly computed. + virtual bool GetStatistics(RtcpStatistics* statistics, + bool update_fraction_lost) = 0; virtual void GetDataCounters(size_t* bytes_received, uint32_t* packets_received) const = 0; diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.cc b/modules/rtp_rtcp/source/receive_statistics_impl.cc index 888fa929e7..f2ccf22077 100644 --- a/modules/rtp_rtcp/source/receive_statistics_impl.cc +++ b/modules/rtp_rtcp/source/receive_statistics_impl.cc @@ -39,16 +39,12 @@ StreamStatisticianImpl::StreamStatisticianImpl( RateStatistics::kBpsScale), max_reordering_threshold_(kDefaultMaxReorderingThreshold), jitter_q4_(0), - cumulative_loss_(0), last_receive_time_ms_(0), last_received_timestamp_(0), received_seq_first_(0), received_seq_max_(0), received_seq_wraps_(0), received_packet_overhead_(12), - last_report_inorder_packets_(0), - last_report_old_packets_(0), - last_report_seq_max_(0), rtcp_callback_(rtcp_callback), rtp_callback_(rtp_callback) {} @@ -57,15 +53,24 @@ StreamStatisticianImpl::~StreamStatisticianImpl() = default; void StreamStatisticianImpl::IncomingPacket(const RTPHeader& header, size_t packet_length, bool retransmitted) { - auto counters = UpdateCounters(header, packet_length, retransmitted); + StreamDataCounters counters; + RtcpStatistics rtcp_stats; + { + rtc::CritScope cs(&stream_lock_); + counters = UpdateCounters(header, packet_length, retransmitted); + // We only want to recalculate |fraction_lost| when sending an RTCP SR or + // RR. + rtcp_stats = CalculateRtcpStatistics(/*update_fraction_lost=*/false); + } + rtp_callback_->DataCountersUpdated(counters, ssrc_); + rtcp_callback_->StatisticsUpdated(rtcp_stats, ssrc_); } StreamDataCounters StreamStatisticianImpl::UpdateCounters( const RTPHeader& header, size_t packet_length, bool retransmitted) { - rtc::CritScope cs(&stream_lock_); bool in_order = InOrderPacketInternal(header.sequenceNumber); RTC_DCHECK_EQ(ssrc_, header.ssrc); incoming_bitrate_.Update(packet_length, clock_->TimeInMilliseconds()); @@ -153,7 +158,7 @@ void StreamStatisticianImpl::SetMaxReorderingThreshold( } bool StreamStatisticianImpl::GetStatistics(RtcpStatistics* statistics, - bool reset) { + bool update_fraction_lost) { { rtc::CritScope cs(&stream_lock_); if (received_seq_first_ == 0 && @@ -162,20 +167,12 @@ bool StreamStatisticianImpl::GetStatistics(RtcpStatistics* statistics, return false; } - if (!reset) { - if (last_report_inorder_packets_ == 0) { - // No report. - return false; - } - // Just get last report. - *statistics = last_reported_statistics_; - return true; - } - - *statistics = CalculateRtcpStatistics(); + *statistics = CalculateRtcpStatistics(update_fraction_lost); } - rtcp_callback_->StatisticsUpdated(*statistics, ssrc_); + if (update_fraction_lost) { + rtcp_callback_->StatisticsUpdated(*statistics, ssrc_); + } return true; } @@ -194,84 +191,68 @@ bool StreamStatisticianImpl::GetActiveStatisticsAndReset( return false; } - *statistics = CalculateRtcpStatistics(); + *statistics = CalculateRtcpStatistics(/*update_fraction_lost=*/true); } rtcp_callback_->StatisticsUpdated(*statistics, ssrc_); return true; } -RtcpStatistics StreamStatisticianImpl::CalculateRtcpStatistics() { - RtcpStatistics stats; +RtcpStatistics StreamStatisticianImpl::CalculateRtcpStatistics( + bool update_fraction_lost) { + RtcpStatistics statistics; - if (last_report_inorder_packets_ == 0) { - // First time we send a report. - last_report_seq_max_ = received_seq_first_ - 1; + uint32_t extended_seq_max = (received_seq_wraps_ << 16) + received_seq_max_; + + if (update_fraction_lost) { + if (last_report_received_packets_ == 0) { + // First time we're calculating fraction lost. + last_report_extended_seq_max_ = received_seq_first_ - 1; + } + + uint32_t exp_since_last = + (extended_seq_max - last_report_extended_seq_max_); + + // Number of received RTP packets since last report; counts all packets + // including retransmissions. + uint32_t rec_since_last = + receive_counters_.transmitted.packets - last_report_received_packets_; + + // Calculate fraction lost according to RFC3550 Appendix A.3. Snap to 0 if + // negative (which is possible with duplicate packets). + uint8_t local_fraction_lost = 0; + if (exp_since_last > rec_since_last) { + // Scale 0 to 255, where 255 is 100% loss. + local_fraction_lost = static_cast( + 255 * (exp_since_last - rec_since_last) / exp_since_last); + } + + last_fraction_lost_ = local_fraction_lost; + last_report_received_packets_ = receive_counters_.transmitted.packets; + last_report_extended_seq_max_ = extended_seq_max; } - // Calculate fraction lost. - 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. - uint32_t rec_since_last = (receive_counters_.transmitted.packets - - receive_counters_.retransmitted.packets) - - last_report_inorder_packets_; - - // With NACK we don't know the expected retransmissions during the last - // second. We know how many "old" packets we have received. We just count - // the number of old received to estimate the loss, but it still does not - // guarantee an exact number since we run this based on time triggered by - // sending of an RTP packet. This should have a minimum effect. - - // With NACK we don't count old packets as received since they are - // re-transmitted. We use RTT to decide if a packet is re-ordered or - // re-transmitted. - uint32_t retransmitted_packets = - receive_counters_.retransmitted.packets - last_report_old_packets_; - rec_since_last += retransmitted_packets; - - int32_t missing = 0; - if (exp_since_last > rec_since_last) { - missing = (exp_since_last - rec_since_last); - } - uint8_t local_fraction_lost = 0; - if (exp_since_last) { - // Scale 0 to 255, where 255 is 100% loss. - local_fraction_lost = static_cast(255 * missing / exp_since_last); - } - stats.fraction_lost = local_fraction_lost; - - // We need a counter for cumulative loss too. - // TODO(danilchap): Ensure cumulative loss is below maximum value of 2^24. - cumulative_loss_ += missing; - stats.packets_lost = cumulative_loss_; - stats.extended_highest_sequence_number = - (received_seq_wraps_ << 16) + received_seq_max_; + statistics.fraction_lost = last_fraction_lost_; + // Calculate cumulative loss, according to RFC3550 Appendix A.3. + uint32_t total_expected_packets = extended_seq_max - received_seq_first_ + 1; + statistics.packets_lost = + total_expected_packets - receive_counters_.transmitted.packets; + // Since cumulative loss is carried in a signed 24-bit field in RTCP, we may + // need to clamp it. + statistics.packets_lost = std::min(statistics.packets_lost, 0x7fffff); + statistics.packets_lost = std::max(statistics.packets_lost, -0x800000); + statistics.extended_highest_sequence_number = extended_seq_max; // Note: internal jitter value is in Q4 and needs to be scaled by 1/16. - stats.jitter = jitter_q4_ >> 4; + statistics.jitter = jitter_q4_ >> 4; - // Store this report. - last_reported_statistics_ = stats; - - // Only for report blocks in RTCP SR and RR. - last_report_inorder_packets_ = receive_counters_.transmitted.packets - - receive_counters_.retransmitted.packets; - last_report_old_packets_ = receive_counters_.retransmitted.packets; - last_report_seq_max_ = received_seq_max_; BWE_TEST_LOGGING_PLOT_WITH_SSRC(1, "cumulative_loss_pkts", clock_->TimeInMilliseconds(), - cumulative_loss_, ssrc_); + statistics.packets_lost, ssrc_); BWE_TEST_LOGGING_PLOT_WITH_SSRC( 1, "received_seq_max_pkts", clock_->TimeInMilliseconds(), (received_seq_max_ - received_seq_first_), ssrc_); - return stats; + return statistics; } void StreamStatisticianImpl::GetDataCounters(size_t* bytes_received, @@ -337,7 +318,7 @@ bool StreamStatisticianImpl::IsPacketInOrder(uint16_t sequence_number) const { bool StreamStatisticianImpl::InOrderPacketInternal( uint16_t sequence_number) const { // First packet is always in order. - if (last_receive_time_ms_ == 0) + if (receive_counters_.transmitted.packets == 0) return true; if (IsNewerSequenceNumber(sequence_number, received_seq_max_)) { diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.h b/modules/rtp_rtcp/source/receive_statistics_impl.h index d0cd865764..6bba24ec0e 100644 --- a/modules/rtp_rtcp/source/receive_statistics_impl.h +++ b/modules/rtp_rtcp/source/receive_statistics_impl.h @@ -13,6 +13,8 @@ #include "modules/rtp_rtcp/include/receive_statistics.h" +#include + #include #include #include @@ -31,8 +33,8 @@ class StreamStatisticianImpl : public StreamStatistician { StreamDataCountersCallback* rtp_callback); ~StreamStatisticianImpl() override; - // |reset| here and in next method restarts calculation of fraction_lost stat. - bool GetStatistics(RtcpStatistics* statistics, bool reset) override; + bool GetStatistics(RtcpStatistics* statistics, + bool update_fraction_lost) override; bool GetActiveStatisticsAndReset(RtcpStatistics* statistics); void GetDataCounters(size_t* bytes_received, uint32_t* packets_received) const override; @@ -49,13 +51,15 @@ class StreamStatisticianImpl : public StreamStatistician { void SetMaxReorderingThreshold(int max_reordering_threshold); private: - bool InOrderPacketInternal(uint16_t sequence_number) const; - RtcpStatistics CalculateRtcpStatistics() + bool InOrderPacketInternal(uint16_t sequence_number) const + RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_); + RtcpStatistics CalculateRtcpStatistics(bool update_fraction_lost) RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_); void UpdateJitter(const RTPHeader& header, NtpTime receive_time); StreamDataCounters UpdateCounters(const RTPHeader& rtp_header, size_t packet_length, - bool retransmitted); + bool retransmitted) + RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_); const uint32_t ssrc_; Clock* const clock_; @@ -65,7 +69,6 @@ class StreamStatisticianImpl : public StreamStatistician { // Stats on received RTP packets. uint32_t jitter_q4_; - uint32_t cumulative_loss_; int64_t last_receive_time_ms_; NtpTime last_receive_time_ntp_; @@ -76,13 +79,12 @@ class StreamStatisticianImpl : public StreamStatistician { // Current counter values. size_t received_packet_overhead_; - StreamDataCounters receive_counters_; + StreamDataCounters receive_counters_ RTC_GUARDED_BY(stream_lock_); - // Counter values when we sent the last report. - uint32_t last_report_inorder_packets_; - uint32_t last_report_old_packets_; - uint16_t last_report_seq_max_; - RtcpStatistics last_reported_statistics_; + // Used to calculate fraction_lost between reports. + uint32_t last_report_received_packets_ = 0; + uint32_t last_report_extended_seq_max_ = 0; + uint8_t last_fraction_lost_ = 0; // stream_lock_ shouldn't be held when calling callbacks. RtcpStatisticsCallback* const rtcp_callback_; diff --git a/modules/rtp_rtcp/source/receive_statistics_unittest.cc b/modules/rtp_rtcp/source/receive_statistics_unittest.cc index 29fc88daac..3c7fb6f83c 100644 --- a/modules/rtp_rtcp/source/receive_statistics_unittest.cc +++ b/modules/rtp_rtcp/source/receive_statistics_unittest.cc @@ -19,6 +19,8 @@ namespace webrtc { namespace { +using ::testing::_; +using ::testing::SaveArg; using ::testing::SizeIs; using ::testing::UnorderedElementsAre; @@ -188,29 +190,82 @@ TEST_F(ReceiveStatisticsTest, GetReceiveStreamDataCounters) { EXPECT_EQ(2u, counters.transmitted.packets); } -TEST_F(ReceiveStatisticsTest, RtcpCallbacks) { - class TestCallback : public RtcpStatisticsCallback { - public: - TestCallback() - : RtcpStatisticsCallback(), num_calls_(0), ssrc_(0), stats_() {} - ~TestCallback() override {} - - void StatisticsUpdated(const RtcpStatistics& statistics, - uint32_t ssrc) override { - ssrc_ = ssrc; - stats_ = statistics; - ++num_calls_; - } - - void CNameChanged(const char* cname, uint32_t ssrc) override {} - - uint32_t num_calls_; - uint32_t ssrc_; - RtcpStatistics stats_; - } callback; +class MockRtcpCallback : public RtcpStatisticsCallback { + public: + MOCK_METHOD2(StatisticsUpdated, + void(const RtcpStatistics& statistics, uint32_t ssrc)); + MOCK_METHOD2(CNameChanged, void(const char* cname, uint32_t ssrc)); +}; +// Test that the RTCP statistics callback is invoked every time a packet is +// received (so that at the application level, GetStats will return up-to-date +// stats, not just stats from the last generated RTCP SR or RR). +TEST_F(ReceiveStatisticsTest, + RtcpStatisticsCallbackInvokedForEveryPacketReceived) { + MockRtcpCallback callback; receive_statistics_->RegisterRtcpStatisticsCallback(&callback); + // Just receive the same packet multiple times; doesn't really matter for the + // purposes of this test. + EXPECT_CALL(callback, StatisticsUpdated(_, _)).Times(3); + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); +} + +// The callback should also be invoked when |fraction_lost| is updated due to +// GetStatistics being called. +TEST_F(ReceiveStatisticsTest, + RtcpStatisticsCallbackInvokedWhenFractionLostUpdated) { + MockRtcpCallback callback; + receive_statistics_->RegisterRtcpStatisticsCallback(&callback); + + EXPECT_CALL(callback, StatisticsUpdated(_, _)).Times(2); + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + + // This just returns the current statistics without updating anything, so no + // need to invoke the callback. + RtcpStatistics statistics; + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics( + &statistics, /*update_fraction_lost=*/false); + + // Update fraction lost, expecting a new callback. + EXPECT_CALL(callback, StatisticsUpdated(_, _)).Times(1); + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics( + &statistics, /*update_fraction_lost=*/true); +} + +TEST_F(ReceiveStatisticsTest, + RtcpStatisticsCallbackNotInvokedAfterDeregistered) { + // Register the callback and receive a couple packets. + MockRtcpCallback callback; + receive_statistics_->RegisterRtcpStatisticsCallback(&callback); + EXPECT_CALL(callback, StatisticsUpdated(_, _)).Times(2); + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + + // Dereigster the callback. Neither receiving a packet nor generating a + // report (calling GetStatistics) should result in another callback. + receive_statistics_->RegisterRtcpStatisticsCallback(nullptr); + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + RtcpStatistics statistics; + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics( + &statistics, /*update_fraction_lost=*/true); +} + +// Test that the RtcpStatisticsCallback sees the exact same values as returned +// from GetStatistics. +TEST_F(ReceiveStatisticsTest, + RtcpStatisticsFromCallbackMatchThoseFromGetStatistics) { + MockRtcpCallback callback; + RtcpStatistics stats_from_callback; + EXPECT_CALL(callback, StatisticsUpdated(_, _)) + .WillRepeatedly(SaveArg<0>(&stats_from_callback)); + receive_statistics_->RegisterRtcpStatisticsCallback(&callback); + + // Using units of milliseconds. + header1_.payload_type_frequency = 1000; // Add some arbitrary data, with loss and jitter. header1_.sequenceNumber = 1; clock_.AdvanceTimeMilliseconds(7); @@ -228,53 +283,380 @@ TEST_F(ReceiveStatisticsTest, RtcpCallbacks) { clock_.AdvanceTimeMilliseconds(11); header1_.timestamp += 17; receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); - ++header1_.sequenceNumber; - EXPECT_EQ(0u, callback.num_calls_); + // The stats from the last callback due to IncomingPacket should match + // those returned by GetStatistics afterwards. + RtcpStatistics stats_from_getstatistics; + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics( + &stats_from_getstatistics, /*update_fraction_lost=*/false); - // Call GetStatistics, simulating a timed rtcp sender thread. + EXPECT_EQ(stats_from_getstatistics.packets_lost, + stats_from_callback.packets_lost); + EXPECT_EQ(stats_from_getstatistics.extended_highest_sequence_number, + stats_from_callback.extended_highest_sequence_number); + EXPECT_EQ(stats_from_getstatistics.fraction_lost, + stats_from_callback.fraction_lost); + EXPECT_EQ(stats_from_getstatistics.jitter, stats_from_callback.jitter); + + // Now update fraction lost, and check that we got matching values from the + // new callback. + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics( + &stats_from_getstatistics, /*update_fraction_lost=*/true); + EXPECT_EQ(stats_from_getstatistics.packets_lost, + stats_from_callback.packets_lost); + EXPECT_EQ(stats_from_getstatistics.extended_highest_sequence_number, + stats_from_callback.extended_highest_sequence_number); + EXPECT_EQ(stats_from_getstatistics.fraction_lost, + stats_from_callback.fraction_lost); + EXPECT_EQ(stats_from_getstatistics.jitter, stats_from_callback.jitter); +} + +// Test that |fraction_lost| is only updated when a report is generated (when +// GetStatistics is called with |update_fraction_lost| set to true). Meaning +// that it will always represent a value computed between two RTCP SR or RRs. +TEST_F(ReceiveStatisticsTest, FractionLostOnlyUpdatedWhenReportGenerated) { + MockRtcpCallback callback; + RtcpStatistics stats_from_callback; + EXPECT_CALL(callback, StatisticsUpdated(_, _)) + .WillRepeatedly(SaveArg<0>(&stats_from_callback)); + receive_statistics_->RegisterRtcpStatisticsCallback(&callback); + + // Simulate losing one packet. + header1_.sequenceNumber = 1; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + header1_.sequenceNumber = 2; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + header1_.sequenceNumber = 4; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + // Haven't generated a report yet, so |fraction_lost| should still be 0. + EXPECT_EQ(0u, stats_from_callback.fraction_lost); + + // Call GetStatistics with |update_fraction_lost| set to false; should be a + // no-op. RtcpStatistics statistics; - receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, - true); + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics( + &statistics, /*update_fraction_lost=*/false); + EXPECT_EQ(0u, stats_from_callback.fraction_lost); - EXPECT_EQ(1u, callback.num_calls_); - EXPECT_EQ(callback.ssrc_, kSsrc1); - EXPECT_EQ(statistics.packets_lost, callback.stats_.packets_lost); - EXPECT_EQ(statistics.extended_highest_sequence_number, - callback.stats_.extended_highest_sequence_number); - EXPECT_EQ(statistics.fraction_lost, callback.stats_.fraction_lost); - EXPECT_EQ(statistics.jitter, callback.stats_.jitter); - EXPECT_EQ(51, statistics.fraction_lost); + // Call GetStatistics with |update_fraction_lost| set to true, simulating a + // report being generated. + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics( + &statistics, /*update_fraction_lost=*/true); + // 25% = 63/255. + EXPECT_EQ(63u, stats_from_callback.fraction_lost); + + // Lose another packet. + header1_.sequenceNumber = 6; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + // Should return same value as before since we haven't generated a new report + // yet. + EXPECT_EQ(63u, stats_from_callback.fraction_lost); + + // Simulate another report being generated. + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics( + &statistics, /*update_fraction_lost=*/true); + // 50% = 127/255. + EXPECT_EQ(127, stats_from_callback.fraction_lost); +} + +// Simple test for fraction/cumulative loss computation, with only loss, no +// duplicates or reordering. +TEST_F(ReceiveStatisticsTest, SimpleLossComputation) { + header1_.sequenceNumber = 1; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + header1_.sequenceNumber = 3; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + header1_.sequenceNumber = 4; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + header1_.sequenceNumber = 5; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + + RtcpStatistics statistics; + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics( + &statistics, /*update_fraction_lost=*/true); + // 20% = 51/255. + EXPECT_EQ(51u, statistics.fraction_lost); EXPECT_EQ(1, statistics.packets_lost); - EXPECT_EQ(5u, statistics.extended_highest_sequence_number); - EXPECT_EQ(4u, statistics.jitter); +} - receive_statistics_->RegisterRtcpStatisticsCallback(NULL); +// Test that fraction/cumulative loss is computed correctly when there's some +// reordering. +TEST_F(ReceiveStatisticsTest, LossComputationWithReordering) { + header1_.sequenceNumber = 1; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + header1_.sequenceNumber = 3; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + header1_.sequenceNumber = 2; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + header1_.sequenceNumber = 5; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); - // Add some more data. + RtcpStatistics statistics; + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics( + &statistics, /*update_fraction_lost=*/true); + // 20% = 51/255. + EXPECT_EQ(51u, statistics.fraction_lost); +} + +// Somewhat unintuitively, duplicate packets count against lost packets +// according to RFC3550. +TEST_F(ReceiveStatisticsTest, LossComputationWithDuplicates) { + // Lose 2 packets, but also receive 1 duplicate. Should actually count as + // only 1 packet being lost. + header1_.sequenceNumber = 1; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + header1_.sequenceNumber = 4; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + header1_.sequenceNumber = 4; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + header1_.sequenceNumber = 5; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + + RtcpStatistics statistics; + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics( + &statistics, /*update_fraction_lost=*/true); + // 20% = 51/255. + EXPECT_EQ(51u, statistics.fraction_lost); + EXPECT_EQ(1, statistics.packets_lost); +} + +// Test that sequence numbers wrapping around doesn't screw up loss +// computations. +TEST_F(ReceiveStatisticsTest, LossComputationWithSequenceNumberWrapping) { + // First, test loss computation over a period that included a sequence number + // rollover. + header1_.sequenceNumber = 65533; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + header1_.sequenceNumber = 0; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + header1_.sequenceNumber = 65534; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + header1_.sequenceNumber = 1; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + + // Only one packet was actually lost, 65535. + RtcpStatistics statistics; + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics( + &statistics, /*update_fraction_lost=*/true); + // 20% = 51/255. + EXPECT_EQ(51u, statistics.fraction_lost); + EXPECT_EQ(1, statistics.packets_lost); + + // Now test losing one packet *after* the rollover. + header1_.sequenceNumber = 3; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics( + &statistics, /*update_fraction_lost=*/true); + // 50% = 127/255. + EXPECT_EQ(127u, statistics.fraction_lost); + EXPECT_EQ(2, statistics.packets_lost); +} + +// Somewhat unintuitively, since duplicate packets count against loss, you can +// actually end up with negative loss. |fraction_lost| should be clamped to +// zero in this case, since it's signed, while |packets_lost| is signed so it +// should be negative. +TEST_F(ReceiveStatisticsTest, NegativeLoss) { + // Receive one packet and simulate a report being generated by calling + // GetStatistics, to establish a baseline for |fraction_lost|. + header1_.sequenceNumber = 1; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + RtcpStatistics statistics; + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics( + &statistics, /*update_fraction_lost=*/true); + + // Receive some duplicate packets. Results in "negative" loss, since + // "expected packets since last report" is 3 and "received" is 4, and 3 minus + // 4 is -1. See RFC3550 Appendix A.3. + header1_.sequenceNumber = 4; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + header1_.sequenceNumber = 2; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + header1_.sequenceNumber = 2; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + header1_.sequenceNumber = 2; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics( + &statistics, /*update_fraction_lost=*/true); + EXPECT_EQ(0u, statistics.fraction_lost); + EXPECT_EQ(-1, statistics.packets_lost); + + // Lose 2 packets; now cumulative loss should become positive again. + header1_.sequenceNumber = 7; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics( + &statistics, /*update_fraction_lost=*/true); + // 66% = 170/255. + EXPECT_EQ(170u, statistics.fraction_lost); + EXPECT_EQ(1, statistics.packets_lost); +} + +// Since cumulative loss is carried in a signed 24-bit field, it should be +// clamped to 0x7fffff in the positive direction, 0x800000 in the negative +// direction. +TEST_F(ReceiveStatisticsTest, PositiveCumulativeLossClamped) { + header1_.sequenceNumber = 1; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + + // Lose 2^23 packets, expecting loss to be clamped to 2^23-1. + for (int i = 0; i < 0x800000; ++i) { + header1_.sequenceNumber = (header1_.sequenceNumber + 2 % 65536); + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + } + RtcpStatistics statistics; + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics( + &statistics, /*update_fraction_lost=*/false); + EXPECT_EQ(0x7fffff, statistics.packets_lost); +} + +TEST_F(ReceiveStatisticsTest, NegativeCumulativeLossClamped) { + header1_.sequenceNumber = 1; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + + // Receive 2^23+1 duplicate packets (counted as negative loss), expecting + // loss to be clamped to -2^23. + for (int i = 0; i < 0x800001; ++i) { + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + } + RtcpStatistics statistics; + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics( + &statistics, /*update_fraction_lost=*/false); + EXPECT_EQ(-0x800000, statistics.packets_lost); +} + +// Test that the extended highest sequence number is computed correctly when +// sequence numbers wrap around or packets are received out of order. +TEST_F(ReceiveStatisticsTest, ExtendedHighestSequenceNumberComputation) { + MockRtcpCallback callback; + RtcpStatistics stats_from_callback; + EXPECT_CALL(callback, StatisticsUpdated(_, _)) + .WillRepeatedly(SaveArg<0>(&stats_from_callback)); + receive_statistics_->RegisterRtcpStatisticsCallback(&callback); + + header1_.sequenceNumber = 65535; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + EXPECT_EQ(65535u, stats_from_callback.extended_highest_sequence_number); + + // Wrap around. + header1_.sequenceNumber = 1; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + EXPECT_EQ(65536u + 1u, stats_from_callback.extended_highest_sequence_number); + + // Should be treated as out of order; shouldn't increment highest extended + // sequence number. + header1_.sequenceNumber = 65530; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + EXPECT_EQ(65536u + 1u, stats_from_callback.extended_highest_sequence_number); + + // Receive a couple packets then wrap around again. + // TODO(bugs.webrtc.org/9445): With large jumps like this, RFC3550 suggests + // for the receiver to assume the other side restarted, and reset all its + // sequence number counters. Why aren't we doing this? + header1_.sequenceNumber = 30000; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + EXPECT_EQ(65536u + 30000u, + stats_from_callback.extended_highest_sequence_number); + + header1_.sequenceNumber = 50000; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + EXPECT_EQ(65536u + 50000u, + stats_from_callback.extended_highest_sequence_number); + + header1_.sequenceNumber = 10000; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + EXPECT_EQ(2 * 65536u + 10000u, + stats_from_callback.extended_highest_sequence_number); + + // If a packet is received more than "MaxReorderingThreshold" packets out of + // order (defaults to 50), it's assumed to be in order. + // TODO(bugs.webrtc.org/9445): RFC3550 would recommend treating this as a + // restart as mentioned above. + header1_.sequenceNumber = 9900; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + EXPECT_EQ(3 * 65536u + 9900u, + stats_from_callback.extended_highest_sequence_number); +} + +// Test jitter computation with no loss/reordering/etc. +TEST_F(ReceiveStatisticsTest, SimpleJitterComputation) { + MockRtcpCallback callback; + RtcpStatistics stats_from_callback; + EXPECT_CALL(callback, StatisticsUpdated(_, _)) + .WillRepeatedly(SaveArg<0>(&stats_from_callback)); + receive_statistics_->RegisterRtcpStatisticsCallback(&callback); + + // Using units of milliseconds. + header1_.payload_type_frequency = 1000; + + // Regardless of initial timestamps, jitter should start at 0. header1_.sequenceNumber = 1; clock_.AdvanceTimeMilliseconds(7); header1_.timestamp += 3; receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); - header1_.sequenceNumber += 2; - clock_.AdvanceTimeMilliseconds(9); - header1_.timestamp += 9; - receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); - --header1_.sequenceNumber; - clock_.AdvanceTimeMilliseconds(13); - header1_.timestamp += 47; - receive_statistics_->IncomingPacket(header1_, kPacketSize1, true); - header1_.sequenceNumber += 3; - clock_.AdvanceTimeMilliseconds(11); - header1_.timestamp += 17; - receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + EXPECT_EQ(0u, stats_from_callback.jitter); + + // Incrementing timestamps by the same amount shouldn't increase jitter. ++header1_.sequenceNumber; + clock_.AdvanceTimeMilliseconds(50); + header1_.timestamp += 50; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + EXPECT_EQ(0u, stats_from_callback.jitter); - receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, - true); + // Difference of 16ms, divided by 16 yields exactly 1. + ++header1_.sequenceNumber; + clock_.AdvanceTimeMilliseconds(32); + header1_.timestamp += 16; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, true); + EXPECT_EQ(1u, stats_from_callback.jitter); - // Should not have been called after deregister. - EXPECT_EQ(1u, callback.num_calls_); + // (90 + 1 * 15) / 16 = 6.5625; should round down to 6. + // TODO(deadbeef): Why don't we round to the nearest integer? + ++header1_.sequenceNumber; + clock_.AdvanceTimeMilliseconds(10); + header1_.timestamp += 100; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, true); + EXPECT_EQ(6u, stats_from_callback.jitter); + + // (30 + 6.5625 * 15) / 16 = 8.0273; should round down to 8. + ++header1_.sequenceNumber; + clock_.AdvanceTimeMilliseconds(50); + header1_.timestamp += 20; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, true); + EXPECT_EQ(8u, stats_from_callback.jitter); +} + +// TODO(deadbeef): Why do we do this? It goes against RFC3550, which explicitly +// says the calculation should be based on order of arrival and packets may not +// necessarily arrive in sequence. +TEST_F(ReceiveStatisticsTest, JitterComputationIgnoresReorderedPackets) { + MockRtcpCallback callback; + RtcpStatistics stats_from_callback; + EXPECT_CALL(callback, StatisticsUpdated(_, _)) + .WillRepeatedly(SaveArg<0>(&stats_from_callback)); + receive_statistics_->RegisterRtcpStatisticsCallback(&callback); + + // Using units of milliseconds. + header1_.payload_type_frequency = 1000; + + // Regardless of initial timestamps, jitter should start at 0. + header1_.sequenceNumber = 1; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + EXPECT_EQ(0u, stats_from_callback.jitter); + + // This should be ignored, even though there's a difference of 70 here. + header1_.sequenceNumber = 0; + clock_.AdvanceTimeMilliseconds(50); + header1_.timestamp -= 20; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + EXPECT_EQ(0u, stats_from_callback.jitter); + + // Relative to the first packet there's a difference of 181ms in arrival + // time, 20ms in timestamp, so jitter should be 161/16 = 10. + header1_.sequenceNumber = 2; + clock_.AdvanceTimeMilliseconds(131); + header1_.timestamp += 40; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + EXPECT_EQ(10u, stats_from_callback.jitter); } class RtpTestCallback : public StreamDataCountersCallback {