Remove StreamStatistician::IsPacketInOrder

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 <[email protected]> Reviewed-by: Erik Språng <[email protected]> Reviewed-by: Fredrik Solenberg <[email protected]> Commit-Queue: Danil Chapovalov <[email protected]> Cr-Commit-Position: refs/heads/master@{#23762}
luckystarzj · Jun 28, 2018 · 64b17c2 · 64b17c2
1 parent 968b1dd
commit 64b17c2
Show file tree

Hide file tree

Showing 7 changed files with 11 additions and 42 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -286,17 +286,15 @@ 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
   // that the first packet is included in the stats).
   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,24 +526,16 @@ 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;
   StreamStatistician* statistician =
       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
@@ -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);