Revert "Update packetsLost and jitter stats any time a packet is received."
This reverts commit 84916937b70472715efe5682bc273e91c3a72695.
Reason for revert: breaking downstream projects.
Original change's description:
> 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 <deadbeef@webrtc.org>
> Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
> Reviewed-by: Oskar Sundbom <ossu@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#23731}
TBR=danilchap@webrtc.org,deadbeef@webrtc.org,ossu@webrtc.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Landing with NOTRY because ios64_sim_ios10_dbg bot is broken.
Passing all other bots.
NOTRY=True
Bug: webrtc:8804
Change-Id: I07bd6b1206d5a8d211792ad392842f9ed6c505e9
Reviewed-on: https://webrtc-review.googlesource.com/95280
Commit-Queue: Qingsi Wang <qingsi@webrtc.org>
Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24370}diff --git a/audio/channel.cc b/audio/channel.cc
index 9044364..4c3bcaa 100644
--- a/audio/channel.cc
+++ b/audio/channel.cc
@@ -1146,16 +1146,14 @@
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 6c51e85..39482c1 100644
--- a/modules/rtp_rtcp/include/receive_statistics.h
+++ b/modules/rtp_rtcp/include/receive_statistics.h
@@ -36,17 +36,7 @@
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 f6bcddb..362a7cf 100644
--- a/modules/rtp_rtcp/source/receive_statistics_impl.cc
+++ b/modules/rtp_rtcp/source/receive_statistics_impl.cc
@@ -39,12 +39,16 @@
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 @@
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 @@
}
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 @@
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 @@
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<uint8_t>(
- 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<uint8_t>(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::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 35869f1..5559b7c 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 <math.h>
-
#include <algorithm>
#include <map>
#include <vector>
@@ -33,8 +31,8 @@
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 @@
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 @@
// 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 @@
// 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 eedcf46..29fc88d 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 @@
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 {}
-// 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;
+ 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;
+
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 @@
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 {
