From 5304a32a94b3d81367067b19372a374bcbd53589 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Mon, 27 Aug 2018 13:27:05 +0200 Subject: [PATCH] Delete StreamStatistician::IsRetransmitOfOldPacket MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Return value always passed as the |retransmitted| argument to ReceiveStatistics::IncomingPacket. The implementation of this method, StreamStatisticianImpl::IncomingPacket, can call its own IsRetransmitOfOldPacket, which is demoted to a private method. Bug: webrtc:7135 Change-Id: I904db676738689c7a1db4caa588f70e64e3c357d Reviewed-on: https://webrtc-review.googlesource.com/95649 Reviewed-by: Åsa Persson Reviewed-by: Fredrik Solenberg Reviewed-by: Danil Chapovalov Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/master@{#24494} --- audio/channel.cc | 13 +-- audio/channel.h | 1 - call/flexfec_receive_stream_impl.cc | 5 +- call/rtx_receive_stream.cc | 3 +- .../test/estimators/nada.cc | 2 +- .../test/estimators/remb.cc | 2 +- modules/rtp_rtcp/include/receive_statistics.h | 20 +++-- .../source/receive_statistics_impl.cc | 37 ++++++-- .../rtp_rtcp/source/receive_statistics_impl.h | 15 ++-- .../source/receive_statistics_unittest.cc | 84 +++++++++++-------- .../rtp_rtcp/source/rtcp_sender_unittest.cc | 2 +- .../rtp_rtcp/source/rtp_rtcp_impl_unittest.cc | 2 +- modules/rtp_rtcp/test/testAPI/test_api.cc | 2 +- video/rtp_video_stream_receiver.cc | 15 +--- video/rtp_video_stream_receiver.h | 1 - video/video_receive_stream.cc | 3 + 16 files changed, 112 insertions(+), 95 deletions(-) diff --git a/audio/channel.cc b/audio/channel.cc index 4c3bcaaf36..d884e03173 100644 --- a/audio/channel.cc +++ b/audio/channel.cc @@ -537,6 +537,7 @@ Channel::Channel(ProcessThread* module_process_thread, _outputAudioLevel.Clear(); + rtp_receive_statistics_->EnableRetransmitDetection(remote_ssrc_, true); RtpRtcp::Configuration configuration; configuration.audio = true; configuration.outgoing_transport = this; @@ -876,8 +877,7 @@ void Channel::OnRtpPacket(const RtpPacketReceived& packet) { return; header.payload_type_frequency = it->second; - rtp_receive_statistics_->IncomingPacket(header, packet.size(), - IsPacketRetransmitted(header)); + rtp_receive_statistics_->IncomingPacket(header, packet.size()); ReceivePacket(packet.data(), packet.size(), header); } @@ -900,15 +900,6 @@ bool Channel::ReceivePacket(const uint8_t* packet, &webrtc_rtp_header); } -bool Channel::IsPacketRetransmitted(const RTPHeader& header) const { - StreamStatistician* statistician = - rtp_receive_statistics_->GetStatistician(header.ssrc); - if (!statistician) - return false; - // Check if this is a retransmission. - return statistician->IsRetransmitOfOldPacket(header); -} - int32_t Channel::ReceivedRTCPPacket(const uint8_t* data, size_t length) { // Store playout timestamp for the received RTCP packet UpdatePlayoutTimestamp(true); diff --git a/audio/channel.h b/audio/channel.h index 98f2613492..562e79ef04 100644 --- a/audio/channel.h +++ b/audio/channel.h @@ -313,7 +313,6 @@ class Channel bool ReceivePacket(const uint8_t* packet, size_t packet_length, const RTPHeader& header); - bool IsPacketRetransmitted(const RTPHeader& header) const; int ResendPackets(const uint16_t* sequence_numbers, int length); void UpdatePlayoutTimestamp(bool rtcp); diff --git a/call/flexfec_receive_stream_impl.cc b/call/flexfec_receive_stream_impl.cc index 973df66164..0512ceeca6 100644 --- a/call/flexfec_receive_stream_impl.cc +++ b/call/flexfec_receive_stream_impl.cc @@ -179,10 +179,7 @@ void FlexfecReceiveStreamImpl::OnRtpPacket(const RtpPacketReceived& packet) { if (packet.Ssrc() == config_.remote_ssrc) { RTPHeader header; packet.GetHeader(&header); - // FlexFEC packets are never retransmitted. - const bool kNotRetransmitted = false; - rtp_receive_statistics_->IncomingPacket(header, packet.size(), - kNotRetransmitted); + rtp_receive_statistics_->IncomingPacket(header, packet.size()); } } diff --git a/call/rtx_receive_stream.cc b/call/rtx_receive_stream.cc index 5d5fec19bd..3e2934fda8 100644 --- a/call/rtx_receive_stream.cc +++ b/call/rtx_receive_stream.cc @@ -38,8 +38,7 @@ void RtxReceiveStream::OnRtpPacket(const RtpPacketReceived& rtx_packet) { if (rtp_receive_statistics_) { RTPHeader header; rtx_packet.GetHeader(&header); - rtp_receive_statistics_->IncomingPacket(header, rtx_packet.size(), - false /* retransmitted */); + rtp_receive_statistics_->IncomingPacket(header, rtx_packet.size()); } rtc::ArrayView payload = rtx_packet.payload(); diff --git a/modules/remote_bitrate_estimator/test/estimators/nada.cc b/modules/remote_bitrate_estimator/test/estimators/nada.cc index 0d81878c7c..30d36f8d3d 100644 --- a/modules/remote_bitrate_estimator/test/estimators/nada.cc +++ b/modules/remote_bitrate_estimator/test/estimators/nada.cc @@ -58,7 +58,7 @@ void NadaBweReceiver::ReceivePacket(int64_t arrival_time_ms, clock_.AdvanceTimeMilliseconds(arrival_time_ms - clock_.TimeInMilliseconds()); recv_stats_->IncomingPacket(media_packet.header(), - media_packet.payload_size(), false); + media_packet.payload_size()); // Refered as x_n. int64_t delay_ms = arrival_time_ms - media_packet.sender_timestamp_ms(); diff --git a/modules/remote_bitrate_estimator/test/estimators/remb.cc b/modules/remote_bitrate_estimator/test/estimators/remb.cc index 34e60a6fa5..94416fe6fe 100644 --- a/modules/remote_bitrate_estimator/test/estimators/remb.cc +++ b/modules/remote_bitrate_estimator/test/estimators/remb.cc @@ -84,7 +84,7 @@ RembReceiver::~RembReceiver() {} void RembReceiver::ReceivePacket(int64_t arrival_time_ms, const MediaPacket& media_packet) { recv_stats_->IncomingPacket(media_packet.header(), - media_packet.payload_size(), false); + media_packet.payload_size()); latest_estimate_bps_ = -1; diff --git a/modules/rtp_rtcp/include/receive_statistics.h b/modules/rtp_rtcp/include/receive_statistics.h index 39482c17ec..fe72ada419 100644 --- a/modules/rtp_rtcp/include/receive_statistics.h +++ b/modules/rtp_rtcp/include/receive_statistics.h @@ -18,6 +18,7 @@ #include "modules/include/module_common_types.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/rtcp_packet/report_block.h" +#include "rtc_base/deprecation.h" namespace webrtc { @@ -45,10 +46,6 @@ class StreamStatistician { StreamDataCounters* data_counters) const = 0; virtual uint32_t BitrateReceived() const = 0; - - // Returns true if the packet with RTP header |header| is likely to be a - // retransmitted packet, false otherwise. - virtual bool IsRetransmitOfOldPacket(const RTPHeader& header) const = 0; }; class ReceiveStatistics : public ReceiveStatisticsProvider { @@ -59,8 +56,16 @@ class ReceiveStatistics : public ReceiveStatisticsProvider { // Updates the receive statistics with this packet. virtual void IncomingPacket(const RTPHeader& rtp_header, - size_t packet_length, - bool retransmitted) = 0; + size_t packet_length) = 0; + + // TODO(nisse): Wrapper for backwards compatibility. Delete as soon as + // downstream callers are updated. + RTC_DEPRECATED + void IncomingPacket(const RTPHeader& rtp_header, + size_t packet_length, + bool retransmitted) { + IncomingPacket(rtp_header, packet_length); + } // Increment counter for number of FEC packets received. virtual void FecPacketReceived(const RTPHeader& header, @@ -71,6 +76,9 @@ class ReceiveStatistics : public ReceiveStatisticsProvider { // Sets the max reordering threshold in number of packets. virtual void SetMaxReorderingThreshold(int max_reordering_threshold) = 0; + // Detect retransmissions, enabling updates of the retransmitted counters. The + // default is false. + virtual void EnableRetransmitDetection(uint32_t ssrc, bool enable) = 0; // Called on new RTCP stats creation. virtual void RegisterRtcpStatisticsCallback( diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.cc b/modules/rtp_rtcp/source/receive_statistics_impl.cc index dd3ee55088..80b8c9b0a2 100644 --- a/modules/rtp_rtcp/source/receive_statistics_impl.cc +++ b/modules/rtp_rtcp/source/receive_statistics_impl.cc @@ -31,6 +31,7 @@ StreamStatistician::~StreamStatistician() {} StreamStatisticianImpl::StreamStatisticianImpl( uint32_t ssrc, Clock* clock, + bool enable_retransmit_detection, RtcpStatisticsCallback* rtcp_callback, StreamDataCountersCallback* rtp_callback) : ssrc_(ssrc), @@ -38,6 +39,7 @@ StreamStatisticianImpl::StreamStatisticianImpl( incoming_bitrate_(kStatisticsProcessIntervalMs, RateStatistics::kBpsScale), max_reordering_threshold_(kDefaultMaxReorderingThreshold), + enable_retransmit_detection_(enable_retransmit_detection), jitter_q4_(0), cumulative_loss_(0), last_receive_time_ms_(0), @@ -55,12 +57,13 @@ StreamStatisticianImpl::StreamStatisticianImpl( StreamStatisticianImpl::~StreamStatisticianImpl() = default; void StreamStatisticianImpl::IncomingPacket(const RTPHeader& header, - size_t packet_length, - bool retransmitted) { + size_t packet_length) { StreamDataCounters counters; { rtc::CritScope cs(&stream_lock_); + bool retransmitted = + enable_retransmit_detection_ && IsRetransmitOfOldPacket(header); counters = UpdateCounters(header, packet_length, retransmitted); } rtp_callback_->DataCountersUpdated(counters, ssrc_); @@ -156,6 +159,11 @@ void StreamStatisticianImpl::SetMaxReorderingThreshold( max_reordering_threshold_ = max_reordering_threshold; } +void StreamStatisticianImpl::EnableRetransmitDetection(bool enable) { + rtc::CritScope cs(&stream_lock_); + enable_retransmit_detection_ = enable; +} + bool StreamStatisticianImpl::GetStatistics(RtcpStatistics* statistics, bool reset) { { @@ -304,7 +312,6 @@ uint32_t StreamStatisticianImpl::BitrateReceived() const { bool StreamStatisticianImpl::IsRetransmitOfOldPacket( const RTPHeader& header) const { - rtc::CritScope cs(&stream_lock_); if (InOrderPacketInternal(header.sequenceNumber)) { return false; } @@ -366,8 +373,7 @@ ReceiveStatisticsImpl::~ReceiveStatisticsImpl() { } void ReceiveStatisticsImpl::IncomingPacket(const RTPHeader& header, - size_t packet_length, - bool retransmitted) { + size_t packet_length) { StreamStatisticianImpl* impl; { rtc::CritScope cs(&receive_statistics_lock_); @@ -375,7 +381,9 @@ void ReceiveStatisticsImpl::IncomingPacket(const RTPHeader& header, if (it != statisticians_.end()) { impl = it->second; } else { - impl = new StreamStatisticianImpl(header.ssrc, clock_, this, this); + impl = new StreamStatisticianImpl( + header.ssrc, clock_, /* enable_retransmit_detection = */ false, this, + this); statisticians_[header.ssrc] = impl; } } @@ -383,7 +391,7 @@ void ReceiveStatisticsImpl::IncomingPacket(const RTPHeader& header, // this whole ReceiveStatisticsImpl is destroyed. StreamStatisticianImpl has // it's own locking so don't hold receive_statistics_lock_ (potential // deadlock). - impl->IncomingPacket(header, packet_length, retransmitted); + impl->IncomingPacket(header, packet_length); } void ReceiveStatisticsImpl::FecPacketReceived(const RTPHeader& header, @@ -417,6 +425,21 @@ void ReceiveStatisticsImpl::SetMaxReorderingThreshold( } } +void ReceiveStatisticsImpl::EnableRetransmitDetection(uint32_t ssrc, + bool enable) { + StreamStatisticianImpl* impl; + { + rtc::CritScope cs(&receive_statistics_lock_); + StreamStatisticianImpl*& impl_ref = statisticians_[ssrc]; + if (impl_ref == nullptr) { // new element + impl_ref = new StreamStatisticianImpl(ssrc, clock_, enable, this, this); + return; + } + impl = impl_ref; + } + impl->EnableRetransmitDetection(enable); +} + void ReceiveStatisticsImpl::RegisterRtcpStatisticsCallback( RtcpStatisticsCallback* callback) { rtc::CritScope cs(&receive_statistics_lock_); diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.h b/modules/rtp_rtcp/source/receive_statistics_impl.h index b0235ed88b..f94256ade6 100644 --- a/modules/rtp_rtcp/source/receive_statistics_impl.h +++ b/modules/rtp_rtcp/source/receive_statistics_impl.h @@ -27,6 +27,7 @@ class StreamStatisticianImpl : public StreamStatistician { public: StreamStatisticianImpl(uint32_t ssrc, Clock* clock, + bool enable_retransmit_detection, RtcpStatisticsCallback* rtcp_callback, StreamDataCountersCallback* rtp_callback); ~StreamStatisticianImpl() override; @@ -39,15 +40,15 @@ class StreamStatisticianImpl : public StreamStatistician { void GetReceiveStreamDataCounters( StreamDataCounters* data_counters) const override; uint32_t BitrateReceived() const override; - bool IsRetransmitOfOldPacket(const RTPHeader& header) const override; - void IncomingPacket(const RTPHeader& rtp_header, - size_t packet_length, - bool retransmitted); + void IncomingPacket(const RTPHeader& rtp_header, size_t packet_length); void FecPacketReceived(const RTPHeader& header, size_t packet_length); void SetMaxReorderingThreshold(int max_reordering_threshold); + void EnableRetransmitDetection(bool enable); private: + bool IsRetransmitOfOldPacket(const RTPHeader& header) const + RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_); bool InOrderPacketInternal(uint16_t sequence_number) const RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_); RtcpStatistics CalculateRtcpStatistics() @@ -65,6 +66,7 @@ class StreamStatisticianImpl : public StreamStatistician { RateStatistics incoming_bitrate_ RTC_GUARDED_BY(&stream_lock_); // In number of packets or sequence numbers. int max_reordering_threshold_ RTC_GUARDED_BY(&stream_lock_); + bool enable_retransmit_detection_ RTC_GUARDED_BY(&stream_lock_); // Stats on received RTP packets. uint32_t jitter_q4_ RTC_GUARDED_BY(&stream_lock_); @@ -104,13 +106,12 @@ class ReceiveStatisticsImpl : public ReceiveStatistics, std::vector RtcpReportBlocks(size_t max_blocks) override; // Implement ReceiveStatistics. - void IncomingPacket(const RTPHeader& header, - size_t packet_length, - bool retransmitted) override; + void IncomingPacket(const RTPHeader& header, size_t packet_length) override; void FecPacketReceived(const RTPHeader& header, size_t packet_length) override; StreamStatistician* GetStatistician(uint32_t ssrc) const override; void SetMaxReorderingThreshold(int max_reordering_threshold) override; + void EnableRetransmitDetection(uint32_t ssrc, bool enable) override; void RegisterRtcpStatisticsCallback( RtcpStatisticsCallback* callback) override; diff --git a/modules/rtp_rtcp/source/receive_statistics_unittest.cc b/modules/rtp_rtcp/source/receive_statistics_unittest.cc index 29fc88daac..b721b034f5 100644 --- a/modules/rtp_rtcp/source/receive_statistics_unittest.cc +++ b/modules/rtp_rtcp/source/receive_statistics_unittest.cc @@ -34,6 +34,7 @@ RTPHeader CreateRtpHeader(uint32_t ssrc) { memset(&header, 0, sizeof(header)); header.ssrc = ssrc; header.sequenceNumber = 100; + header.payload_type_frequency = 90000; return header; } @@ -53,14 +54,14 @@ class ReceiveStatisticsTest : public ::testing::Test { }; TEST_F(ReceiveStatisticsTest, TwoIncomingSsrcs) { - receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1); ++header1_.sequenceNumber; - receive_statistics_->IncomingPacket(header2_, kPacketSize2, false); + receive_statistics_->IncomingPacket(header2_, kPacketSize2); ++header2_.sequenceNumber; clock_.AdvanceTimeMilliseconds(100); - receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1); ++header1_.sequenceNumber; - receive_statistics_->IncomingPacket(header2_, kPacketSize2, false); + receive_statistics_->IncomingPacket(header2_, kPacketSize2); ++header2_.sequenceNumber; StreamStatistician* statistician = @@ -83,9 +84,9 @@ TEST_F(ReceiveStatisticsTest, TwoIncomingSsrcs) { EXPECT_EQ(2u, receive_statistics_->RtcpReportBlocks(3).size()); // Add more incoming packets and verify that they are registered in both // access methods. - receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1); ++header1_.sequenceNumber; - receive_statistics_->IncomingPacket(header2_, kPacketSize2, false); + receive_statistics_->IncomingPacket(header2_, kPacketSize2); ++header2_.sequenceNumber; receive_statistics_->GetStatistician(kSsrc1)->GetDataCounters( @@ -103,9 +104,9 @@ TEST_F(ReceiveStatisticsTest, RTPHeader header1 = CreateRtpHeader(kSsrc1); RTPHeader header2 = CreateRtpHeader(kSsrc2); RTPHeader header3 = CreateRtpHeader(kSsrc3); - receive_statistics_->IncomingPacket(header1, kPacketSize1, false); - receive_statistics_->IncomingPacket(header2, kPacketSize1, false); - receive_statistics_->IncomingPacket(header3, kPacketSize1, false); + receive_statistics_->IncomingPacket(header1, kPacketSize1); + receive_statistics_->IncomingPacket(header2, kPacketSize1); + receive_statistics_->IncomingPacket(header3, kPacketSize1); EXPECT_THAT(receive_statistics_->RtcpReportBlocks(2), SizeIs(2)); EXPECT_THAT(receive_statistics_->RtcpReportBlocks(2), SizeIs(2)); @@ -118,10 +119,10 @@ TEST_F(ReceiveStatisticsTest, RTPHeader header2 = CreateRtpHeader(kSsrc2); RTPHeader header3 = CreateRtpHeader(kSsrc3); RTPHeader header4 = CreateRtpHeader(kSsrc4); - receive_statistics_->IncomingPacket(header1, kPacketSize1, false); - receive_statistics_->IncomingPacket(header2, kPacketSize1, false); - receive_statistics_->IncomingPacket(header3, kPacketSize1, false); - receive_statistics_->IncomingPacket(header4, kPacketSize1, false); + receive_statistics_->IncomingPacket(header1, kPacketSize1); + receive_statistics_->IncomingPacket(header2, kPacketSize1); + receive_statistics_->IncomingPacket(header3, kPacketSize1); + receive_statistics_->IncomingPacket(header4, kPacketSize1); std::vector observed_ssrcs; std::vector report_blocks = @@ -140,10 +141,10 @@ TEST_F(ReceiveStatisticsTest, } TEST_F(ReceiveStatisticsTest, ActiveStatisticians) { - receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1); ++header1_.sequenceNumber; clock_.AdvanceTimeMilliseconds(1000); - receive_statistics_->IncomingPacket(header2_, kPacketSize2, false); + receive_statistics_->IncomingPacket(header2_, kPacketSize2); ++header2_.sequenceNumber; // Nothing should time out since only 1000 ms has passed since the first // packet came in. @@ -157,7 +158,7 @@ TEST_F(ReceiveStatisticsTest, ActiveStatisticians) { // kSsrc2 should have timed out. EXPECT_EQ(0u, receive_statistics_->RtcpReportBlocks(3).size()); - receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1); ++header1_.sequenceNumber; // kSsrc1 should be active again and the data counters should have survived. EXPECT_EQ(1u, receive_statistics_->RtcpReportBlocks(3).size()); @@ -171,8 +172,19 @@ TEST_F(ReceiveStatisticsTest, ActiveStatisticians) { EXPECT_EQ(2u, packets_received); } +TEST_F(ReceiveStatisticsTest, + DoesntCreateRtcpReportBlockUntilFirstReceivedPacketForSsrc) { + // Creates a statistician object for the ssrc. + receive_statistics_->EnableRetransmitDetection(kSsrc1, true); + EXPECT_TRUE(receive_statistics_->GetStatistician(kSsrc1) != nullptr); + EXPECT_EQ(0u, receive_statistics_->RtcpReportBlocks(3).size()); + // Receive first packet + receive_statistics_->IncomingPacket(header1_, kPacketSize1); + EXPECT_EQ(1u, receive_statistics_->RtcpReportBlocks(3).size()); +} + TEST_F(ReceiveStatisticsTest, GetReceiveStreamDataCounters) { - receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1); StreamStatistician* statistician = receive_statistics_->GetStatistician(kSsrc1); ASSERT_TRUE(statistician != NULL); @@ -182,7 +194,7 @@ TEST_F(ReceiveStatisticsTest, GetReceiveStreamDataCounters) { EXPECT_GT(counters.first_packet_time_ms, -1); EXPECT_EQ(1u, counters.transmitted.packets); - receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1); statistician->GetReceiveStreamDataCounters(&counters); EXPECT_GT(counters.first_packet_time_ms, -1); EXPECT_EQ(2u, counters.transmitted.packets); @@ -210,24 +222,25 @@ TEST_F(ReceiveStatisticsTest, RtcpCallbacks) { } callback; receive_statistics_->RegisterRtcpStatisticsCallback(&callback); + receive_statistics_->EnableRetransmitDetection(kSsrc1, true); // Add some arbitrary data, with loss and jitter. header1_.sequenceNumber = 1; clock_.AdvanceTimeMilliseconds(7); header1_.timestamp += 3; - receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1); header1_.sequenceNumber += 2; clock_.AdvanceTimeMilliseconds(9); header1_.timestamp += 9; - receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1); --header1_.sequenceNumber; clock_.AdvanceTimeMilliseconds(13); header1_.timestamp += 47; - receive_statistics_->IncomingPacket(header1_, kPacketSize1, true); + receive_statistics_->IncomingPacket(header1_, kPacketSize1); header1_.sequenceNumber += 3; clock_.AdvanceTimeMilliseconds(11); header1_.timestamp += 17; - receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1); ++header1_.sequenceNumber; EXPECT_EQ(0u, callback.num_calls_); @@ -247,7 +260,7 @@ TEST_F(ReceiveStatisticsTest, RtcpCallbacks) { 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); + EXPECT_EQ(177u, statistics.jitter); receive_statistics_->RegisterRtcpStatisticsCallback(NULL); @@ -255,19 +268,19 @@ TEST_F(ReceiveStatisticsTest, RtcpCallbacks) { header1_.sequenceNumber = 1; clock_.AdvanceTimeMilliseconds(7); header1_.timestamp += 3; - receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1); header1_.sequenceNumber += 2; clock_.AdvanceTimeMilliseconds(9); header1_.timestamp += 9; - receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1); --header1_.sequenceNumber; clock_.AdvanceTimeMilliseconds(13); header1_.timestamp += 47; - receive_statistics_->IncomingPacket(header1_, kPacketSize1, true); + receive_statistics_->IncomingPacket(header1_, kPacketSize1); header1_.sequenceNumber += 3; clock_.AdvanceTimeMilliseconds(11); header1_.timestamp += 17; - receive_statistics_->IncomingPacket(header1_, kPacketSize1, false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1); ++header1_.sequenceNumber; receive_statistics_->GetStatistician(kSsrc1)->GetStatistics(&statistics, @@ -316,14 +329,14 @@ class RtpTestCallback : public StreamDataCountersCallback { TEST_F(ReceiveStatisticsTest, RtpCallbacks) { RtpTestCallback callback; receive_statistics_->RegisterRtpStatisticsCallback(&callback); + receive_statistics_->EnableRetransmitDetection(kSsrc1, true); const size_t kHeaderLength = 20; const size_t kPaddingLength = 9; // One packet of size kPacketSize1. header1_.headerLength = kHeaderLength; - receive_statistics_->IncomingPacket(header1_, kPacketSize1 + kHeaderLength, - false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1 + kHeaderLength); StreamDataCounters expected; expected.transmitted.payload_bytes = kPacketSize1; expected.transmitted.header_bytes = kHeaderLength; @@ -341,7 +354,7 @@ TEST_F(ReceiveStatisticsTest, RtpCallbacks) { header1_.paddingLength = 9; // Another packet of size kPacketSize1 with 9 bytes padding. receive_statistics_->IncomingPacket( - header1_, kPacketSize1 + kHeaderLength + kPaddingLength, false); + header1_, kPacketSize1 + kHeaderLength + kPaddingLength); expected.transmitted.payload_bytes = kPacketSize1 * 2; expected.transmitted.header_bytes = kHeaderLength * 2; expected.transmitted.padding_bytes = kPaddingLength; @@ -351,7 +364,7 @@ TEST_F(ReceiveStatisticsTest, RtpCallbacks) { clock_.AdvanceTimeMilliseconds(5); // Retransmit last packet. receive_statistics_->IncomingPacket( - header1_, kPacketSize1 + kHeaderLength + kPaddingLength, true); + header1_, kPacketSize1 + kHeaderLength + kPaddingLength); expected.transmitted.payload_bytes = kPacketSize1 * 3; expected.transmitted.header_bytes = kHeaderLength * 3; expected.transmitted.padding_bytes = kPaddingLength * 2; @@ -366,8 +379,7 @@ TEST_F(ReceiveStatisticsTest, RtpCallbacks) { ++header1_.sequenceNumber; clock_.AdvanceTimeMilliseconds(5); // One FEC packet. - receive_statistics_->IncomingPacket(header1_, kPacketSize1 + kHeaderLength, - false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1 + kHeaderLength); receive_statistics_->FecPacketReceived(header1_, kPacketSize1 + kHeaderLength); expected.transmitted.payload_bytes = kPacketSize1 * 4; @@ -383,8 +395,7 @@ TEST_F(ReceiveStatisticsTest, RtpCallbacks) { // New stats, but callback should not be called. ++header1_.sequenceNumber; clock_.AdvanceTimeMilliseconds(5); - receive_statistics_->IncomingPacket(header1_, kPacketSize1 + kHeaderLength, - true); + receive_statistics_->IncomingPacket(header1_, kPacketSize1 + kHeaderLength); callback.Matches(5, kSsrc1, expected); } @@ -400,8 +411,7 @@ TEST_F(ReceiveStatisticsTest, RtpCallbacksFecFirst) { kPacketSize1 + kHeaderLength); EXPECT_EQ(0u, callback.num_calls_); - receive_statistics_->IncomingPacket(header1_, kPacketSize1 + kHeaderLength, - false); + receive_statistics_->IncomingPacket(header1_, kPacketSize1 + kHeaderLength); StreamDataCounters expected; expected.transmitted.payload_bytes = kPacketSize1; expected.transmitted.header_bytes = kHeaderLength; diff --git a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index 974f19f5ee..bccf96df5e 100644 --- a/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -99,7 +99,7 @@ class RtcpSenderTest : public ::testing::Test { header.timestamp = 12345; header.headerLength = 12; size_t kPacketLength = 100; - receive_statistics_->IncomingPacket(header, kPacketLength, false); + receive_statistics_->IncomingPacket(header, kPacketLength); } test::RtcpPacketParser* parser() { return &test_transport_.parser_; } diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc index 5071eeeecf..2de6175bc8 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc @@ -327,7 +327,7 @@ TEST_F(RtpRtcpImplTest, Rtt) { header.sequenceNumber = 123; header.ssrc = kSenderSsrc; header.headerLength = 12; - receiver_.receive_statistics_->IncomingPacket(header, 100, false); + receiver_.receive_statistics_->IncomingPacket(header, 100); // Send Frame before sending an SR. SendFrame(&sender_, kBaseLayerTid); diff --git a/modules/rtp_rtcp/test/testAPI/test_api.cc b/modules/rtp_rtcp/test/testAPI/test_api.cc index 2fd4464b3c..b51daa3053 100644 --- a/modules/rtp_rtcp/test/testAPI/test_api.cc +++ b/modules/rtp_rtcp/test/testAPI/test_api.cc @@ -56,7 +56,7 @@ bool LoopBackTransport::SendRtp(const uint8_t* data, const uint8_t* payload = data + header.headerLength; RTC_CHECK_GE(len, header.headerLength); const size_t payload_length = len - header.headerLength; - receive_statistics_->IncomingPacket(header, len, false); + receive_statistics_->IncomingPacket(header, len); return rtp_receiver_->IncomingRtpPacket(header, payload, payload_length, pl->typeSpecific); } diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index b2ec0c701e..46563477e2 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -314,8 +314,7 @@ void RtpVideoStreamReceiver::OnRtpPacket(const RtpPacketReceived& packet) { packet.GetHeader(&header); // TODO(nisse): We should pass a recovered flag to stats, to aid // fixing bug bugs.webrtc.org/6339. - rtp_receive_statistics_->IncomingPacket(header, packet.size(), - IsPacketRetransmitted(header)); + rtp_receive_statistics_->IncomingPacket(header, packet.size()); } for (RtpPacketSinkInterface* secondary_sink : secondary_sinks_) { @@ -584,18 +583,6 @@ void RtpVideoStreamReceiver::StopReceive() { receiving_ = false; } -bool RtpVideoStreamReceiver::IsPacketRetransmitted( - const RTPHeader& header) const { - // Retransmissions are handled separately if RTX is enabled. - if (config_.rtp.rtx_ssrc != 0) - return false; - StreamStatistician* statistician = - rtp_receive_statistics_->GetStatistician(header.ssrc); - if (!statistician) - return false; - return statistician->IsRetransmitOfOldPacket(header); -} - void RtpVideoStreamReceiver::UpdateHistograms() { FecPacketCounter counter = ulpfec_receiver_->GetPacketCounter(); if (counter.first_packet_time_ms == -1) diff --git a/video/rtp_video_stream_receiver.h b/video/rtp_video_stream_receiver.h index 20463b9fe8..a351c92eaf 100644 --- a/video/rtp_video_stream_receiver.h +++ b/video/rtp_video_stream_receiver.h @@ -144,7 +144,6 @@ class RtpVideoStreamReceiver : public RtpData, size_t packet_length, const RTPHeader& header); void NotifyReceiverOfEmptyPacket(uint16_t seq_num); - bool IsPacketRetransmitted(const RTPHeader& header) const; void UpdateHistograms(); bool IsRedEnabled() const; void InsertSpsPpsIntoTracker(uint8_t payload_type); diff --git a/video/video_receive_stream.cc b/video/video_receive_stream.cc index bd61881583..477bf40314 100644 --- a/video/video_receive_stream.cc +++ b/video/video_receive_stream.cc @@ -145,6 +145,9 @@ VideoReceiveStream::VideoReceiveStream( config_.rtp.remote_ssrc, rtp_receive_statistics_.get()); rtx_receiver_ = receiver_controller->CreateReceiver( config_.rtp.rtx_ssrc, rtx_receive_stream_.get()); + } else { + rtp_receive_statistics_->EnableRetransmitDetection(config.rtp.remote_ssrc, + true); } }