From 64b17c2aca96492b9398a5e7f1f9f95c1717e90c Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Mon, 25 Jun 2018 16:58:54 +0200 Subject: [PATCH] Remove StreamStatistician::IsPacketInOrder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit this function is now only used in combination with StreamStatistician::IsRetransmitOfOldPacket but IsRetransmitOfOldPacket internally checks if packet is in_order, thus making extra check unnecessary In addition to making code simpler, removing this checks avoids taking two extra CritSection on common code path of incoming rtp packet. Bug: webrtc:8016 Change-Id: I050004e256b5698ce700e3416aa86b55f446a270 Reviewed-on: https://webrtc-review.googlesource.com/85361 Reviewed-by: Niels Moller Reviewed-by: Erik Språng Reviewed-by: Fredrik Solenberg Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#23762} --- audio/channel.cc | 18 ++++------------- audio/channel.h | 3 +-- modules/rtp_rtcp/include/receive_statistics.h | 3 --- .../source/receive_statistics_impl.cc | 5 ----- .../rtp_rtcp/source/receive_statistics_impl.h | 1 - video/rtp_video_stream_receiver.cc | 20 +++++-------------- video/rtp_video_stream_receiver.h | 3 +-- 7 files changed, 11 insertions(+), 42 deletions(-) diff --git a/audio/channel.cc b/audio/channel.cc index e704d7ef06..cb32b9255b 100644 --- a/audio/channel.cc +++ b/audio/channel.cc @@ -872,9 +872,8 @@ void Channel::OnRtpPacket(const RtpPacketReceived& packet) { header.payload_type_frequency = rtp_payload_registry_->GetPayloadTypeFrequency(header.payloadType); if (header.payload_type_frequency >= 0) { - bool in_order = IsPacketInOrder(header); - rtp_receive_statistics_->IncomingPacket( - header, packet.size(), IsPacketRetransmitted(header, in_order)); + rtp_receive_statistics_->IncomingPacket(header, packet.size(), + IsPacketRetransmitted(header)); ReceivePacket(packet.data(), packet.size(), header); } @@ -895,22 +894,13 @@ bool Channel::ReceivePacket(const uint8_t* packet, pl->typeSpecific); } -bool Channel::IsPacketInOrder(const RTPHeader& header) const { - StreamStatistician* statistician = - rtp_receive_statistics_->GetStatistician(header.ssrc); - if (!statistician) - return false; - return statistician->IsPacketInOrder(header.sequenceNumber); -} - -bool Channel::IsPacketRetransmitted(const RTPHeader& header, - bool in_order) const { +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 !in_order && statistician->IsRetransmitOfOldPacket(header); + return statistician->IsRetransmitOfOldPacket(header); } int32_t Channel::ReceivedRTCPPacket(const uint8_t* data, size_t length) { diff --git a/audio/channel.h b/audio/channel.h index 9830a2507d..8d1a26b20e 100644 --- a/audio/channel.h +++ b/audio/channel.h @@ -314,8 +314,7 @@ class Channel bool ReceivePacket(const uint8_t* packet, size_t packet_length, const RTPHeader& header); - bool IsPacketInOrder(const RTPHeader& header) const; - bool IsPacketRetransmitted(const RTPHeader& header, bool in_order) const; + bool IsPacketRetransmitted(const RTPHeader& header) const; int ResendPackets(const uint16_t* sequence_numbers, int length); void UpdatePlayoutTimestamp(bool rtcp); diff --git a/modules/rtp_rtcp/include/receive_statistics.h b/modules/rtp_rtcp/include/receive_statistics.h index 5fff41e797..d372fd362b 100644 --- a/modules/rtp_rtcp/include/receive_statistics.h +++ b/modules/rtp_rtcp/include/receive_statistics.h @@ -60,9 +60,6 @@ class StreamStatistician { // 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; - - // Returns true if |sequence_number| is received in order, false otherwise. - virtual bool IsPacketInOrder(uint16_t sequence_number) const = 0; }; class ReceiveStatistics : public ReceiveStatisticsProvider { diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.cc b/modules/rtp_rtcp/source/receive_statistics_impl.cc index f2ccf22077..d140eb04fe 100644 --- a/modules/rtp_rtcp/source/receive_statistics_impl.cc +++ b/modules/rtp_rtcp/source/receive_statistics_impl.cc @@ -310,11 +310,6 @@ bool StreamStatisticianImpl::IsRetransmitOfOldPacket( return time_diff_ms > rtp_time_stamp_diff_ms + max_delay_ms; } -bool StreamStatisticianImpl::IsPacketInOrder(uint16_t sequence_number) const { - rtc::CritScope cs(&stream_lock_); - return InOrderPacketInternal(sequence_number); -} - bool StreamStatisticianImpl::InOrderPacketInternal( uint16_t sequence_number) const { // First packet is always in order. diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.h b/modules/rtp_rtcp/source/receive_statistics_impl.h index 6bba24ec0e..35869f1f1e 100644 --- a/modules/rtp_rtcp/source/receive_statistics_impl.h +++ b/modules/rtp_rtcp/source/receive_statistics_impl.h @@ -42,7 +42,6 @@ class StreamStatisticianImpl : public StreamStatistician { StreamDataCounters* data_counters) const override; uint32_t BitrateReceived() const override; bool IsRetransmitOfOldPacket(const RTPHeader& header) const override; - bool IsPacketInOrder(uint16_t sequence_number) const override; void IncomingPacket(const RTPHeader& rtp_header, size_t packet_length, diff --git a/video/rtp_video_stream_receiver.cc b/video/rtp_video_stream_receiver.cc index 7f267ef0c2..7634797877 100644 --- a/video/rtp_video_stream_receiver.cc +++ b/video/rtp_video_stream_receiver.cc @@ -286,8 +286,6 @@ void RtpVideoStreamReceiver::OnRtpPacket(const RtpPacketReceived& packet) { header.payload_type_frequency = kVideoPayloadTypeFrequency; - bool in_order = IsPacketInOrder(header); - ReceivePacket(packet.data(), packet.size(), header); // Update receive statistics after ReceivePacket. // Receive statistics will be reset if the payload type changes (make sure @@ -295,8 +293,8 @@ void RtpVideoStreamReceiver::OnRtpPacket(const RtpPacketReceived& packet) { if (!packet.recovered()) { // 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, in_order)); + rtp_receive_statistics_->IncomingPacket(header, packet.size(), + IsPacketRetransmitted(header)); } for (RtpPacketSinkInterface* secondary_sink : secondary_sinks_) { @@ -528,16 +526,8 @@ void RtpVideoStreamReceiver::StopReceive() { receiving_ = false; } -bool RtpVideoStreamReceiver::IsPacketInOrder(const RTPHeader& header) const { - StreamStatistician* statistician = - rtp_receive_statistics_->GetStatistician(header.ssrc); - if (!statistician) - return false; - return statistician->IsPacketInOrder(header.sequenceNumber); -} - -bool RtpVideoStreamReceiver::IsPacketRetransmitted(const RTPHeader& header, - bool in_order) const { +bool RtpVideoStreamReceiver::IsPacketRetransmitted( + const RTPHeader& header) const { // Retransmissions are handled separately if RTX is enabled. if (config_.rtp.rtx_ssrc != 0) return false; @@ -545,7 +535,7 @@ bool RtpVideoStreamReceiver::IsPacketRetransmitted(const RTPHeader& header, rtp_receive_statistics_->GetStatistician(header.ssrc); if (!statistician) return false; - return !in_order && statistician->IsRetransmitOfOldPacket(header); + return statistician->IsRetransmitOfOldPacket(header); } void RtpVideoStreamReceiver::UpdateHistograms() { diff --git a/video/rtp_video_stream_receiver.h b/video/rtp_video_stream_receiver.h index 75dcea5bca..083bb36cb8 100644 --- a/video/rtp_video_stream_receiver.h +++ b/video/rtp_video_stream_receiver.h @@ -150,8 +150,7 @@ class RtpVideoStreamReceiver : public RtpData, const RTPHeader& header); void NotifyReceiverOfEmptyPacket(uint16_t seq_num); void NotifyReceiverOfFecPacket(const RTPHeader& header); - bool IsPacketInOrder(const RTPHeader& header) const; - bool IsPacketRetransmitted(const RTPHeader& header, bool in_order) const; + bool IsPacketRetransmitted(const RTPHeader& header) const; void UpdateHistograms(); bool IsRedEnabled() const; void InsertSpsPpsIntoTracker(uint8_t payload_type);