Change ReceiveStatistics to implement RtpPacketSinkInterface, part 2.

Delete the deprecated IncomingPacket method, and convert implementation
to use RtpPacketReceived rather than RTPHeader.

Part 1 was https://webrtc-review.googlesource.com/c/src/+/100104

Bug: webrtc:7135, webrtc:8016
Change-Id: Ib4840d947870403deea2f9067f847e4b0f182479
Reviewed-on: https://webrtc-review.googlesource.com/c/6762
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25648}

modules/rtp_rtcp/include/receive_statistics.h

@@ -56,13 +56,6 @@ class ReceiveStatistics : public ReceiveStatisticsProvider,
 
   static ReceiveStatistics* Create(Clock* clock);
 
-  // Updates the receive statistics with this packet.
-  // TODO(bugs.webrtc.org/8016): Deprecated. Delete as soon as
-  // downstream code is updated to use OnRtpPacket.
-  RTC_DEPRECATED
-  virtual void IncomingPacket(const RTPHeader& rtp_header,
-                              size_t packet_length) = 0;
-
   // Increment counter for number of FEC packets received.
   virtual void FecPacketReceived(const RtpPacketReceived& packet) = 0;
 

modules/rtp_rtcp/include/rtp_rtcp_defines.cc

@@ -9,6 +9,7 @@
  */
 
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
+#include "modules/rtp_rtcp/source/rtp_packet.h"
 
 #include <ctype.h>
 #include <string.h>
@@ -130,4 +131,11 @@ bool PacketFeedback::operator==(const PacketFeedback& rhs) const {
          payload_size == rhs.payload_size && pacing_info == rhs.pacing_info;
 }
 
+void RtpPacketCounter::AddPacket(const RtpPacket& packet) {
+  ++packets;
+  header_bytes += packet.headers_size();
+  padding_bytes += packet.padding_size();
+  payload_bytes += packet.payload_size();
+}
+
 }  // namespace webrtc

modules/rtp_rtcp/include/rtp_rtcp_defines.h

@@ -27,6 +27,7 @@
 #define IP_PACKET_SIZE 1500  // we assume ethernet
 
 namespace webrtc {
+class RtpPacket;
 namespace rtcp {
 class TransportFeedback;
 }
@@ -449,13 +450,8 @@ struct RtpPacketCounter {
     packets -= other.packets;
   }
 
-  void AddPacket(size_t packet_length, const RTPHeader& header) {
+  // Not inlined, since use of RtpPacket would result in circular includes.
-    ++packets;
+  void AddPacket(const RtpPacket& packet);
-    header_bytes += header.headerLength;
-    padding_bytes += header.paddingLength;
-    payload_bytes +=
-        packet_length - (header.headerLength + header.paddingLength);
-  }
 
   size_t TotalBytes() const {
     return header_bytes + payload_bytes + padding_bytes;

modules/rtp_rtcp/source/receive_statistics_impl.cc

@@ -57,33 +57,31 @@ StreamStatisticianImpl::StreamStatisticianImpl(
 
 StreamStatisticianImpl::~StreamStatisticianImpl() = default;
 
-void StreamStatisticianImpl::IncomingPacket(const RTPHeader& header,
+void StreamStatisticianImpl::OnRtpPacket(const RtpPacketReceived& packet) {
-                                            size_t packet_length) {
   StreamDataCounters counters;
   {
     rtc::CritScope cs(&stream_lock_);
 
     bool retransmitted =
-        enable_retransmit_detection_ && IsRetransmitOfOldPacket(header);
+        enable_retransmit_detection_ && IsRetransmitOfOldPacket(packet);
-    counters = UpdateCounters(header, packet_length, retransmitted);
+    counters = UpdateCounters(packet, retransmitted);
   }
   rtp_callback_->DataCountersUpdated(counters, ssrc_);
 }
 
 StreamDataCounters StreamStatisticianImpl::UpdateCounters(
-    const RTPHeader& header,
+    const RtpPacketReceived& packet,
-    size_t packet_length,
     bool retransmitted) {
-  bool in_order = InOrderPacketInternal(header.sequenceNumber);
+  bool in_order = InOrderPacketInternal(packet.SequenceNumber());
-  RTC_DCHECK_EQ(ssrc_, header.ssrc);
+  RTC_DCHECK_EQ(ssrc_, packet.Ssrc());
-  incoming_bitrate_.Update(packet_length, clock_->TimeInMilliseconds());
+  incoming_bitrate_.Update(packet.size(), clock_->TimeInMilliseconds());
-  receive_counters_.transmitted.AddPacket(packet_length, header);
+  receive_counters_.transmitted.AddPacket(packet);
   if (!in_order && retransmitted) {
-    receive_counters_.retransmitted.AddPacket(packet_length, header);
+    receive_counters_.retransmitted.AddPacket(packet);
   }
 
   if (receive_counters_.transmitted.packets == 1) {
-    received_seq_first_ = header.sequenceNumber;
+    received_seq_first_ = packet.SequenceNumber();
     receive_counters_.first_packet_time_ms = clock_->TimeInMilliseconds();
   }
 
@@ -95,26 +93,26 @@ StreamDataCounters StreamStatisticianImpl::UpdateCounters(
 
     // Wrong if we use RetransmitOfOldPacket.
     if (receive_counters_.transmitted.packets > 1 &&
-        received_seq_max_ > header.sequenceNumber) {
+        received_seq_max_ > packet.SequenceNumber()) {
       // Wrap around detected.
       received_seq_wraps_++;
     }
     // New max.
-    received_seq_max_ = header.sequenceNumber;
+    received_seq_max_ = packet.SequenceNumber();
 
     // If new time stamp and more than one in-order packet received, calculate
     // new jitter statistics.
-    if (header.timestamp != last_received_timestamp_ &&
+    if (packet.Timestamp() != last_received_timestamp_ &&
         (receive_counters_.transmitted.packets -
          receive_counters_.retransmitted.packets) > 1) {
-      UpdateJitter(header, receive_time);
+      UpdateJitter(packet, receive_time);
     }
-    last_received_timestamp_ = header.timestamp;
+    last_received_timestamp_ = packet.Timestamp();
     last_receive_time_ntp_ = receive_time;
     last_receive_time_ms_ = clock_->TimeInMilliseconds();
   }
 
-  size_t packet_oh = header.headerLength + header.paddingLength;
+  size_t packet_oh = packet.headers_size() + packet.padding_size();
 
   // Our measured overhead. Filter from RFC 5104 4.2.1.2:
   // avg_OH (new) = 15/16*avg_OH (old) + 1/16*pckt_OH,
@@ -122,14 +120,14 @@ StreamDataCounters StreamStatisticianImpl::UpdateCounters(
   return receive_counters_;
 }
 
-void StreamStatisticianImpl::UpdateJitter(const RTPHeader& header,
+void StreamStatisticianImpl::UpdateJitter(const RtpPacketReceived& packet,
                                           NtpTime receive_time) {
   uint32_t receive_time_rtp =
-      NtpToRtp(receive_time, header.payload_type_frequency);
+      NtpToRtp(receive_time, packet.payload_type_frequency());
   uint32_t last_receive_time_rtp =
-      NtpToRtp(last_receive_time_ntp_, header.payload_type_frequency);
+      NtpToRtp(last_receive_time_ntp_, packet.payload_type_frequency());
   int32_t time_diff_samples = (receive_time_rtp - last_receive_time_rtp) -
-                              (header.timestamp - last_received_timestamp_);
+                              (packet.Timestamp() - last_received_timestamp_);
 
   time_diff_samples = std::abs(time_diff_samples);
 
@@ -143,12 +141,12 @@ void StreamStatisticianImpl::UpdateJitter(const RTPHeader& header,
   }
 }
 
-void StreamStatisticianImpl::FecPacketReceived(const RTPHeader& header,
+void StreamStatisticianImpl::FecPacketReceived(
-                                               size_t packet_length) {
+    const RtpPacketReceived& packet) {
   StreamDataCounters counters;
   {
     rtc::CritScope cs(&stream_lock_);
-    receive_counters_.fec.AddPacket(packet_length, header);
+    receive_counters_.fec.AddPacket(packet);
     counters = receive_counters_;
   }
   rtp_callback_->DataCountersUpdated(counters, ssrc_);
@@ -312,17 +310,17 @@ uint32_t StreamStatisticianImpl::BitrateReceived() const {
 }
 
 bool StreamStatisticianImpl::IsRetransmitOfOldPacket(
-    const RTPHeader& header) const {
+    const RtpPacketReceived& packet) const {
-  if (InOrderPacketInternal(header.sequenceNumber)) {
+  if (InOrderPacketInternal(packet.SequenceNumber())) {
     return false;
   }
-  uint32_t frequency_khz = header.payload_type_frequency / 1000;
+  uint32_t frequency_khz = packet.payload_type_frequency() / 1000;
   assert(frequency_khz > 0);
 
   int64_t time_diff_ms = clock_->TimeInMilliseconds() - last_receive_time_ms_;
 
   // Diff in time stamp since last received in order.
-  uint32_t timestamp_diff = header.timestamp - last_received_timestamp_;
+  uint32_t timestamp_diff = packet.Timestamp() - last_received_timestamp_;
   uint32_t rtp_time_stamp_diff_ms = timestamp_diff / frequency_khz;
 
   int64_t max_delay_ms = 0;
@@ -374,31 +372,24 @@ ReceiveStatisticsImpl::~ReceiveStatisticsImpl() {
 }
 
 void ReceiveStatisticsImpl::OnRtpPacket(const RtpPacketReceived& packet) {
-  RTPHeader header;
-  packet.GetHeader(&header);
-  IncomingPacket(header, packet.size());
-}
-
-void ReceiveStatisticsImpl::IncomingPacket(const RTPHeader& header,
-                                           size_t packet_length) {
   StreamStatisticianImpl* impl;
   {
     rtc::CritScope cs(&receive_statistics_lock_);
-    auto it = statisticians_.find(header.ssrc);
+    auto it = statisticians_.find(packet.Ssrc());
     if (it != statisticians_.end()) {
       impl = it->second;
     } else {
       impl = new StreamStatisticianImpl(
-          header.ssrc, clock_, /* enable_retransmit_detection = */ false, this,
+          packet.Ssrc(), clock_, /* enable_retransmit_detection = */ false,
-          this);
+          this, this);
-      statisticians_[header.ssrc] = impl;
+      statisticians_[packet.Ssrc()] = impl;
     }
   }
   // StreamStatisticianImpl instance is created once and only destroyed when
   // 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);
+  impl->OnRtpPacket(packet);
 }
 
 void ReceiveStatisticsImpl::FecPacketReceived(const RtpPacketReceived& packet) {
@@ -411,9 +402,7 @@ void ReceiveStatisticsImpl::FecPacketReceived(const RtpPacketReceived& packet) {
       return;
     impl = it->second;
   }
-  RTPHeader header;
+  impl->FecPacketReceived(packet);
-  packet.GetHeader(&header);
-  impl->FecPacketReceived(header, packet.size());
 }
 
 StreamStatistician* ReceiveStatisticsImpl::GetStatistician(

modules/rtp_rtcp/source/receive_statistics_impl.h

@@ -23,7 +23,8 @@
 
 namespace webrtc {
 
-class StreamStatisticianImpl : public StreamStatistician {
+class StreamStatisticianImpl : public StreamStatistician,
+                               public RtpPacketSinkInterface {
  public:
   StreamStatisticianImpl(uint32_t ssrc,
                          Clock* clock,
@@ -41,22 +42,23 @@ class StreamStatisticianImpl : public StreamStatistician {
       StreamDataCounters* data_counters) const override;
   uint32_t BitrateReceived() const override;
 
-  void IncomingPacket(const RTPHeader& rtp_header, size_t packet_length);
+  // Implements RtpPacketSinkInterface
-  void FecPacketReceived(const RTPHeader& header, size_t packet_length);
+  void OnRtpPacket(const RtpPacketReceived& packet) override;
+
+  void FecPacketReceived(const RtpPacketReceived& packet);
   void SetMaxReorderingThreshold(int max_reordering_threshold);
   void EnableRetransmitDetection(bool enable);
 
  private:
-  bool IsRetransmitOfOldPacket(const RTPHeader& header) const
+  bool IsRetransmitOfOldPacket(const RtpPacketReceived& packet) const
       RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_);
   bool InOrderPacketInternal(uint16_t sequence_number) const
       RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_);
   RtcpStatistics CalculateRtcpStatistics()
       RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_);
-  void UpdateJitter(const RTPHeader& header, NtpTime receive_time)
+  void UpdateJitter(const RtpPacketReceived& packet, NtpTime receive_time)
       RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_);
-  StreamDataCounters UpdateCounters(const RTPHeader& rtp_header,
+  StreamDataCounters UpdateCounters(const RtpPacketReceived& packet,
-                                    size_t packet_length,
                                     bool retransmitted)
       RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_);
 
@@ -102,14 +104,13 @@ class ReceiveStatisticsImpl : public ReceiveStatistics,
 
   ~ReceiveStatisticsImpl() override;
 
-  // Implement ReceiveStatisticsProvider.
+  // Implements ReceiveStatisticsProvider.
   std::vector<rtcp::ReportBlock> RtcpReportBlocks(size_t max_blocks) override;
 
-  // Implement RtpPacketSinkInterface
+  // Implements RtpPacketSinkInterface
   void OnRtpPacket(const RtpPacketReceived& packet) override;
 
-  // Implement ReceiveStatistics.
+  // Implements ReceiveStatistics.
-  void IncomingPacket(const RTPHeader& header, size_t packet_length) override;
   void FecPacketReceived(const RtpPacketReceived& packet) override;
   StreamStatistician* GetStatistician(uint32_t ssrc) const override;
   void SetMaxReorderingThreshold(int max_reordering_threshold) override;

modules/rtp_rtcp/source/rtp_sender.cc

@@ -95,14 +95,6 @@ const char* FrameTypeToString(FrameType frame_type) {
   }
   return "";
 }
-
-void CountPacket(RtpPacketCounter* counter, const RtpPacketToSend& packet) {
-  ++counter->packets;
-  counter->header_bytes += packet.headers_size();
-  counter->padding_bytes += packet.padding_size();
-  counter->payload_bytes += packet.payload_size();
-}
-
 }  // namespace
 
 RTPSender::RTPSender(
@@ -883,13 +875,13 @@ void RTPSender::UpdateRtpStats(const RtpPacketToSend& packet,
     counters->first_packet_time_ms = now_ms;
 
   if (IsFecPacket(packet))
-    CountPacket(&counters->fec, packet);
+    counters->fec.AddPacket(packet);
 
   if (is_retransmit) {
-    CountPacket(&counters->retransmitted, packet);
+    counters->retransmitted.AddPacket(packet);
     nack_bitrate_sent_.Update(packet.size(), now_ms);
   }
-  CountPacket(&counters->transmitted, packet);
+  counters->transmitted.AddPacket(packet);
 
   if (rtp_stats_callback_)
     rtp_stats_callback_->DataCountersUpdated(*counters, packet.Ssrc());