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 {