diff --git a/audio/channel.cc b/audio/channel.cc index 90443645ae..4c3bcaaf36 100644 --- a/audio/channel.cc +++ b/audio/channel.cc @@ -1146,16 +1146,14 @@ int Channel::GetRemoteRTCPReportBlocks( int Channel::GetRTPStatistics(CallStatistics& stats) { // --- RtcpStatistics - // Jitter, cumulative loss, and extended max sequence number is updated for - // each received RTP packet. + // The jitter statistics is updated for each received RTP packet and is + // based on received packets. RtcpStatistics statistics; StreamStatistician* statistician = rtp_receive_statistics_->GetStatistician(remote_ssrc_); if (statistician) { - // 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); + statistician->GetStatistics(&statistics, + _rtpRtcpModule->RTCP() == RtcpMode::kOff); } stats.fractionLost = statistics.fraction_lost; diff --git a/modules/rtp_rtcp/include/receive_statistics.h b/modules/rtp_rtcp/include/receive_statistics.h index 6c51e8502b..39482c17ec 100644 --- a/modules/rtp_rtcp/include/receive_statistics.h +++ b/modules/rtp_rtcp/include/receive_statistics.h @@ -36,17 +36,7 @@ class StreamStatistician { public: virtual ~StreamStatistician(); - // 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 bool GetStatistics(RtcpStatistics* statistics, bool reset) = 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 f6bcddb5c8..362a7cfb4d 100644 --- a/modules/rtp_rtcp/source/receive_statistics_impl.cc +++ b/modules/rtp_rtcp/source/receive_statistics_impl.cc @@ -39,12 +39,16 @@ 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) {} @@ -53,24 +57,15 @@ StreamStatisticianImpl::~StreamStatisticianImpl() = default; void StreamStatisticianImpl::IncomingPacket(const RTPHeader& header, size_t packet_length, bool 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); - } - + auto counters = UpdateCounters(header, packet_length, retransmitted); 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()); @@ -158,7 +153,7 @@ void StreamStatisticianImpl::SetMaxReorderingThreshold( } bool StreamStatisticianImpl::GetStatistics(RtcpStatistics* statistics, - bool update_fraction_lost) { + bool reset) { { rtc::CritScope cs(&stream_lock_); if (received_seq_first_ == 0 && @@ -167,12 +162,20 @@ bool StreamStatisticianImpl::GetStatistics(RtcpStatistics* statistics, return false; } - *statistics = CalculateRtcpStatistics(update_fraction_lost); + if (!reset) { + if (last_report_inorder_packets_ == 0) { + // No report. + return false; + } + // Just get last report. + *statistics = last_reported_statistics_; + return true; + } + + *statistics = CalculateRtcpStatistics(); } - if (update_fraction_lost) { - rtcp_callback_->StatisticsUpdated(*statistics, ssrc_); - } + rtcp_callback_->StatisticsUpdated(*statistics, ssrc_); return true; } @@ -191,71 +194,84 @@ bool StreamStatisticianImpl::GetActiveStatisticsAndReset( return false; } - *statistics = CalculateRtcpStatistics(/*update_fraction_lost=*/true); + *statistics = CalculateRtcpStatistics(); } rtcp_callback_->StatisticsUpdated(*statistics, ssrc_); return true; } -RtcpStatistics StreamStatisticianImpl::CalculateRtcpStatistics( - bool update_fraction_lost) { - RtcpStatistics statistics; +RtcpStatistics StreamStatisticianImpl::CalculateRtcpStatistics() { + RtcpStatistics stats; - 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; + if (last_report_inorder_packets_ == 0) { + // First time we send a report. + last_report_seq_max_ = received_seq_first_ - 1; } - 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); - // TODO(bugs.webrtc.org/9598): This packets_lost should be signed according to - // RFC3550. However, old WebRTC implementations reads it as unsigned. - // Therefore we limit this to 0. - statistics.packets_lost = std::max(statistics.packets_lost, 0); - statistics.extended_highest_sequence_number = extended_seq_max; - // Note: internal jitter value is in Q4 and needs to be scaled by 1/16. - statistics.jitter = jitter_q4_ >> 4; + // 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_; + // Note: internal jitter value is in Q4 and needs to be scaled by 1/16. + stats.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(), - statistics.packets_lost, ssrc_); + cumulative_loss_, ssrc_); BWE_TEST_LOGGING_PLOT_WITH_SSRC( 1, "received_seq_max_pkts", clock_->TimeInMilliseconds(), (received_seq_max_ - received_seq_first_), ssrc_); - return statistics; + return stats; } void StreamStatisticianImpl::GetDataCounters(size_t* bytes_received, @@ -316,7 +332,7 @@ bool StreamStatisticianImpl::IsRetransmitOfOldPacket( bool StreamStatisticianImpl::InOrderPacketInternal( uint16_t sequence_number) const { // First packet is always in order. - if (receive_counters_.transmitted.packets == 0) + if (last_receive_time_ms_ == 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 35869f1f1e..5559b7c287 100644 --- a/modules/rtp_rtcp/source/receive_statistics_impl.h +++ b/modules/rtp_rtcp/source/receive_statistics_impl.h @@ -13,8 +13,6 @@ #include "modules/rtp_rtcp/include/receive_statistics.h" -#include - #include #include #include @@ -33,8 +31,8 @@ class StreamStatisticianImpl : public StreamStatistician { StreamDataCountersCallback* rtp_callback); ~StreamStatisticianImpl() override; - bool GetStatistics(RtcpStatistics* statistics, - bool update_fraction_lost) override; + // |reset| here and in next method restarts calculation of fraction_lost stat. + bool GetStatistics(RtcpStatistics* statistics, bool reset) override; bool GetActiveStatisticsAndReset(RtcpStatistics* statistics); void GetDataCounters(size_t* bytes_received, uint32_t* packets_received) const override; @@ -50,15 +48,13 @@ class StreamStatisticianImpl : public StreamStatistician { void SetMaxReorderingThreshold(int max_reordering_threshold); private: - bool InOrderPacketInternal(uint16_t sequence_number) const - RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_); - RtcpStatistics CalculateRtcpStatistics(bool update_fraction_lost) + bool InOrderPacketInternal(uint16_t sequence_number) const; + RtcpStatistics CalculateRtcpStatistics() 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) - RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_); + bool retransmitted); const uint32_t ssrc_; Clock* const clock_; @@ -68,6 +64,7 @@ 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_; @@ -78,12 +75,13 @@ class StreamStatisticianImpl : public StreamStatistician { // Current counter values. size_t received_packet_overhead_; - StreamDataCounters receive_counters_ RTC_GUARDED_BY(stream_lock_); + StreamDataCounters receive_counters_; - // 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; + // 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_; // 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 eedcf46df7..29fc88daac 100644 --- a/modules/rtp_rtcp/source/receive_statistics_unittest.cc +++ b/modules/rtp_rtcp/source/receive_statistics_unittest.cc @@ -19,8 +19,6 @@ namespace webrtc { namespace { -using ::testing::_; -using ::testing::SaveArg; using ::testing::SizeIs; using ::testing::UnorderedElementsAre; @@ -190,82 +188,29 @@ TEST_F(ReceiveStatisticsTest, GetReceiveStreamDataCounters) { EXPECT_EQ(2u, counters.transmitted.packets); } -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_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; -// 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); @@ -283,384 +228,53 @@ TEST_F(ReceiveStatisticsTest, clock_.AdvanceTimeMilliseconds(11); header1_.timestamp += 17; receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + ++header1_.sequenceNumber; - // 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); + EXPECT_EQ(0u, callback.num_calls_); - 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. + // Call GetStatistics, simulating a timed rtcp sender thread. RtcpStatistics statistics; - receive_statistics_->GetStatistician(kSsrc1)->GetStatistics( - &statistics, /*update_fraction_lost=*/false); - EXPECT_EQ(0u, stats_from_callback.fraction_lost); + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, + true); - // 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(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); EXPECT_EQ(1, statistics.packets_lost); -} + EXPECT_EQ(5u, statistics.extended_highest_sequence_number); + EXPECT_EQ(4u, statistics.jitter); -// 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); + receive_statistics_->RegisterRtcpStatisticsCallback(NULL); - 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); - // TODO(bugs.webrtc.org/9598): Since old WebRTC implementations reads this - // value as unsigned we currently limit it to 0. - EXPECT_EQ(0, 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); - // TODO(bugs.webrtc.org/9598): Since old WebRTC implementations reads this - // value as unsigned we currently limit it to 0. - EXPECT_EQ(0, 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. + // Add some more data. header1_.sequenceNumber = 1; clock_.AdvanceTimeMilliseconds(7); header1_.timestamp += 3; 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; + header1_.sequenceNumber += 2; + clock_.AdvanceTimeMilliseconds(9); + header1_.timestamp += 9; receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); - EXPECT_EQ(0u, stats_from_callback.jitter); - - // Difference of 16ms, divided by 16 yields exactly 1. - ++header1_.sequenceNumber; - clock_.AdvanceTimeMilliseconds(32); - header1_.timestamp += 16; + --header1_.sequenceNumber; + clock_.AdvanceTimeMilliseconds(13); + header1_.timestamp += 47; receive_statistics_->IncomingPacket(header1_, kPacketSize1, true); - EXPECT_EQ(1u, stats_from_callback.jitter); - - // (90 + 1 * 15) / 16 = 6.5625; should round down to 6. - // TODO(deadbeef): Why don't we round to the nearest integer? + header1_.sequenceNumber += 3; + clock_.AdvanceTimeMilliseconds(11); + header1_.timestamp += 17; + receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); ++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); -} + receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, + true); -// 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); + // Should not have been called after deregister. + EXPECT_EQ(1u, callback.num_calls_); } class RtpTestCallback : public StreamDataCountersCallback {