Consolidate FEC book-keeping

Number of received FEC bytes is used for the
WebRTC.Video.FecBitrateReceivedInKbps UMA histogram. Before this cl,
that value is based on a FEC packet counter updated by
ReceiveStatistics::FecPacketReceived. This cl deletes that method, and
instead adds a byte count to the FecPacketCounter struct, which is
maintained by the UlpFecReceiver and used for other FEC-related stats.

Bug: webrtc:10917
Change-Id: I24bd494b6909a2fe109d28e2b71ca8f413d05911
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/150533
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28976}

modules/rtp_rtcp/include/receive_statistics.h

@@ -64,9 +64,6 @@ class ReceiveStatistics : public ReceiveStatisticsProvider,
 
   static std::unique_ptr<ReceiveStatistics> Create(Clock* clock);
 
-  // Increment counter for number of FEC packets received.
-  virtual void FecPacketReceived(const RtpPacketReceived& packet) = 0;
-
   // Returns a pointer to the statistician of an ssrc.
   virtual StreamStatistician* GetStatistician(uint32_t ssrc) const = 0;
 

modules/rtp_rtcp/include/ulpfec_receiver.h

@@ -20,16 +20,13 @@
 namespace webrtc {
 
 struct FecPacketCounter {
-  FecPacketCounter()
-      : num_packets(0),
-        num_fec_packets(0),
-        num_recovered_packets(0),
-        first_packet_time_ms(-1) {}
-
-  size_t num_packets;            // Number of received packets.
-  size_t num_fec_packets;        // Number of received FEC packets.
-  size_t num_recovered_packets;  // Number of recovered media packets using FEC.
-  int64_t first_packet_time_ms;  // Time when first packet is received.
+  FecPacketCounter() = default;
+  size_t num_packets = 0;  // Number of received packets.
+  size_t num_bytes = 0;
+  size_t num_fec_packets = 0;  // Number of received FEC packets.
+  size_t num_recovered_packets =
+      0;  // Number of recovered media packets using FEC.
+  int64_t first_packet_time_ms = -1;  // Time when first packet is received.
 };
 
 class UlpfecReceiver {

modules/rtp_rtcp/source/receive_statistics_impl.cc

@@ -149,12 +149,6 @@ void StreamStatisticianImpl::UpdateJitter(const RtpPacketReceived& packet,
   }
 }
 
-void StreamStatisticianImpl::FecPacketReceived(
-    const RtpPacketReceived& packet) {
-  rtc::CritScope cs(&stream_lock_);
-  receive_counters_.fec.AddPacket(packet);
-}
-
 void StreamStatisticianImpl::SetMaxReorderingThreshold(
     int max_reordering_threshold) {
   rtc::CritScope cs(&stream_lock_);
@@ -358,14 +352,6 @@ void ReceiveStatisticsImpl::OnRtpPacket(const RtpPacketReceived& packet) {
   GetOrCreateStatistician(packet.Ssrc())->OnRtpPacket(packet);
 }
 
-void ReceiveStatisticsImpl::FecPacketReceived(const RtpPacketReceived& packet) {
-  StreamStatisticianImpl* impl = GetStatistician(packet.Ssrc());
-  // Ignore FEC if it is the first packet.
-  if (impl) {
-    impl->FecPacketReceived(packet);
-  }
-}
-
 StreamStatisticianImpl* ReceiveStatisticsImpl::GetStatistician(
     uint32_t ssrc) const {
   rtc::CritScope cs(&receive_statistics_lock_);

modules/rtp_rtcp/source/receive_statistics_impl.h

@@ -44,7 +44,6 @@ class StreamStatisticianImpl : public StreamStatistician,
   // Implements RtpPacketSinkInterface
   void OnRtpPacket(const RtpPacketReceived& packet) override;
 
-  void FecPacketReceived(const RtpPacketReceived& packet);
   void SetMaxReorderingThreshold(int max_reordering_threshold);
   void EnableRetransmitDetection(bool enable);
 
@@ -114,7 +113,6 @@ class ReceiveStatisticsImpl : public ReceiveStatistics {
   void OnRtpPacket(const RtpPacketReceived& packet) override;
 
   // Implements ReceiveStatistics.
-  void FecPacketReceived(const RtpPacketReceived& packet) override;
   // Note: More specific return type for use in the implementation.
   StreamStatisticianImpl* GetStatistician(uint32_t ssrc) const override;
   void SetMaxReorderingThreshold(int max_reordering_threshold) override;

modules/rtp_rtcp/source/receive_statistics_unittest.cc

@@ -552,20 +552,6 @@ TEST_F(ReceiveStatisticsTest, StreamDataCounters) {
   EXPECT_EQ(counters.retransmitted.header_bytes, kHeaderLength);
   EXPECT_EQ(counters.retransmitted.padding_bytes, kPaddingLength);
   EXPECT_EQ(counters.retransmitted.packets, 1u);
-
-  // One FEC packet.
-  packet1.SetSequenceNumber(packet2.SequenceNumber() + 1);
-  clock_.AdvanceTimeMilliseconds(5);
-  receive_statistics_->OnRtpPacket(packet1);
-  receive_statistics_->FecPacketReceived(packet1);
-  counters = receive_statistics_->GetStatistician(kSsrc1)
-                 ->GetReceiveStreamDataCounters();
-  EXPECT_EQ(counters.transmitted.payload_bytes, kPacketSize1 * 4);
-  EXPECT_EQ(counters.transmitted.header_bytes, kHeaderLength * 4);
-  EXPECT_EQ(counters.transmitted.packets, 4u);
-  EXPECT_EQ(counters.fec.payload_bytes, kPacketSize1);
-  EXPECT_EQ(counters.fec.header_bytes, kHeaderLength);
-  EXPECT_EQ(counters.fec.packets, 1u);
 }
 
 TEST_F(ReceiveStatisticsTest, LastPacketReceivedTimestamp) {
@@ -585,37 +571,5 @@ TEST_F(ReceiveStatisticsTest, LastPacketReceivedTimestamp) {
   EXPECT_EQ(45, counters.last_packet_received_timestamp_ms);
 }
 
-TEST_F(ReceiveStatisticsTest, FecFirst) {
-  receive_statistics_ = ReceiveStatistics::Create(&clock_);
-
-  const uint32_t kHeaderLength = 20;
-  RtpPacketReceived packet =
-      CreateRtpPacket(kSsrc1, kHeaderLength, kPacketSize1, 0);
-  // If first packet is FEC, ignore it.
-  receive_statistics_->FecPacketReceived(packet);
-
-  EXPECT_EQ(receive_statistics_->GetStatistician(kSsrc1), nullptr);
-
-  receive_statistics_->OnRtpPacket(packet);
-  StreamDataCounters counters = receive_statistics_->GetStatistician(kSsrc1)
-                                    ->GetReceiveStreamDataCounters();
-  EXPECT_EQ(counters.transmitted.payload_bytes, kPacketSize1);
-  EXPECT_EQ(counters.transmitted.header_bytes, kHeaderLength);
-  EXPECT_EQ(counters.transmitted.padding_bytes, 0u);
-  EXPECT_EQ(counters.transmitted.packets, 1u);
-  EXPECT_EQ(counters.fec.packets, 0u);
-
-  receive_statistics_->FecPacketReceived(packet);
-  counters = receive_statistics_->GetStatistician(kSsrc1)
-                 ->GetReceiveStreamDataCounters();
-  EXPECT_EQ(counters.transmitted.payload_bytes, kPacketSize1);
-  EXPECT_EQ(counters.transmitted.header_bytes, kHeaderLength);
-  EXPECT_EQ(counters.transmitted.padding_bytes, 0u);
-  EXPECT_EQ(counters.transmitted.packets, 1u);
-  EXPECT_EQ(counters.fec.payload_bytes, kPacketSize1);
-  EXPECT_EQ(counters.fec.header_bytes, kHeaderLength);
-  EXPECT_EQ(counters.fec.packets, 1u);
-}
-
 }  // namespace
 }  // namespace webrtc

modules/rtp_rtcp/source/ulpfec_receiver_impl.cc

@@ -122,6 +122,7 @@ int32_t UlpfecReceiverImpl::AddReceivedRedPacket(
   }
 
   ++packet_counter_.num_packets;
+  packet_counter_.num_bytes += packet_length;
   if (packet_counter_.first_packet_time_ms == -1) {
     packet_counter_.first_packet_time_ms = rtc::TimeMillis();
   }

video/receive_statistics_proxy.cc

@@ -432,12 +432,6 @@ void ReceiveStatisticsProxy::UpdateHistograms(
           static_cast<int>(rtx_stats->transmitted.TotalBytes() * 8 /
                            elapsed_sec / 1000));
     }
-    if (config_.rtp.ulpfec_payload_type != -1) {
-      RTC_HISTOGRAM_COUNTS_10000(
-          "WebRTC.Video.FecBitrateReceivedInKbps",
-          static_cast<int>(rtp_rtx_stats.fec.TotalBytes() * 8 / elapsed_sec /
-                           1000));
-    }
     const RtcpPacketTypeCounter& counters = stats_.rtcp_packet_type_counts;
     RTC_HISTOGRAM_COUNTS_10000("WebRTC.Video.NackPacketsSentPerMinute",
                                counters.nack_packets * 60 / elapsed_sec);

video/rtp_video_stream_receiver.cc

@@ -725,7 +725,6 @@ void RtpVideoStreamReceiver::ParseAndHandleEncapsulatingHeader(
   if (packet.PayloadType() == config_.rtp.red_payload_type &&
       packet.payload_size() > 0) {
     if (packet.payload()[0] == config_.rtp.ulpfec_payload_type) {
-      rtp_receive_statistics_->FecPacketReceived(packet);
       // Notify video_receiver about received FEC packets to avoid NACKing these
       // packets.
       NotifyReceiverOfEmptyPacket(packet.SequenceNumber());
@@ -866,6 +865,11 @@ void RtpVideoStreamReceiver::UpdateHistograms() {
                              static_cast<int>(counter.num_recovered_packets *
                                               100 / counter.num_fec_packets));
   }
+  if (config_.rtp.ulpfec_payload_type != -1) {
+    RTC_HISTOGRAM_COUNTS_10000(
+        "WebRTC.Video.FecBitrateReceivedInKbps",
+        static_cast<int>(counter.num_bytes * 8 / elapsed_sec / 1000));
+  }
 }
 
 void RtpVideoStreamReceiver::InsertSpsPpsIntoTracker(uint8_t payload_type) {