TransportFeedbackPacketLossTracker to receive std::vector<PacketFeedback> in place of the entire feedback

BUG=webrtc:7058

Review-Url: https://codereview.webrtc.org/2754373002
Cr-Commit-Position: refs/heads/master@{#17322}

webrtc/test/fuzzers/transport_feedback_packet_loss_tracker_fuzzer.cc

@@ -11,6 +11,7 @@
 #include <algorithm>
 
 #include "webrtc/base/array_view.h"
+#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "webrtc/modules/rtp_rtcp/source/byte_io.h"
 #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
 #include "webrtc/voice_engine/transport_feedback_packet_loss_tracker.h"
@@ -47,22 +48,24 @@ class TransportFeedbackGenerator {
   explicit TransportFeedbackGenerator(const uint8_t** data, size_t* size)
       : data_(data), size_(size) {}
 
-  bool GetNextTransportFeedback(rtcp::TransportFeedback* feedback) {
-    uint16_t base_seq_num = 0;
-    if (!ReadData<uint16_t>(&base_seq_num)) {
-      return false;
-    }
-    constexpr int64_t kBaseTimeUs = 1234;  // Irrelevant to this test.
-    feedback->SetBase(base_seq_num, kBaseTimeUs);
+  bool GetNextTransportFeedbackVector(
+      std::vector<PacketFeedback>* feedback_vector) {
+    RTC_CHECK(feedback_vector->empty());
 
     uint16_t remaining_packets = 0;
-    if (!ReadData<uint16_t>(&remaining_packets))
+    if (!ReadData<uint16_t>(&remaining_packets)) {
       return false;
-    // Range is [0x00001 : 0x10000], but we keep it 0x0000 to 0xffff for now,
-    // and add the last status as RECEIVED. That is because of a limitation
-    // that says that the last status cannot be LOST.
+    }
+
+    if (remaining_packets == 0) {
+      return true;
+    }
+
+    uint16_t seq_num;
+    if (!ReadData<uint16_t>(&seq_num)) {  // Fuzz base sequence number.
+      return false;
+    }
 
-    uint16_t seq_num = base_seq_num;
     while (remaining_packets > 0) {
       uint8_t status_byte = 0;
       if (!ReadData<uint8_t>(&status_byte)) {
@@ -70,18 +73,18 @@ class TransportFeedbackGenerator {
       }
       // Each status byte contains 8 statuses.
       for (size_t i = 0; i < 8 && remaining_packets > 0; ++i) {
+        // Any positive integer signals reception. kNotReceived signals loss.
+        // Other values are just illegal.
+        constexpr int64_t kArrivalTimeMs = 1234;
+
         const bool received = (status_byte & (0x01 << i));
-        if (received) {
-          feedback->AddReceivedPacket(seq_num, kBaseTimeUs);
-        }
-        ++seq_num;
+        feedback_vector->emplace_back(PacketFeedback(
+            received ? kArrivalTimeMs : PacketFeedback::kNotReceived,
+            seq_num++));
         --remaining_packets;
       }
     }
 
-    // As mentioned above, all feedbacks must report with a received packet.
-    feedback->AddReceivedPacket(seq_num, kBaseTimeUs);
-
     return true;
   }
 
@@ -236,12 +239,13 @@ bool FuzzTransportFeedbackBlock(
   TransportFeedbackGenerator feedback_generator(data, size);
 
   for (size_t i = 0; i < feedbacks_num; i++) {
-    rtcp::TransportFeedback feedback;
-    bool may_continue = feedback_generator.GetNextTransportFeedback(&feedback);
+    std::vector<PacketFeedback> feedback_vector;
+    bool may_continue =
+        feedback_generator.GetNextTransportFeedbackVector(&feedback_vector);
     if (!may_continue) {
       return false;
     }
-    tracker->OnReceivedTransportFeedback(feedback);
+    tracker->OnNewTransportFeedbackVector(feedback_vector);
     tracker->Validate();
   }
 

webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc

@@ -15,6 +15,7 @@
 
 #include "webrtc/base/checks.h"
 #include "webrtc/base/mod_ops.h"
+#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
 
 namespace {
@@ -97,23 +98,17 @@ void TransportFeedbackPacketLossTracker::OnPacketAdded(uint16_t seq_num,
   }
 }
 
-void TransportFeedbackPacketLossTracker::OnReceivedTransportFeedback(
-    const rtcp::TransportFeedback& feedback) {
-  const auto& status_vector = feedback.GetStatusVector();
-  const uint16_t base_seq_num = feedback.GetBaseSequence();
-
-  uint16_t seq_num = base_seq_num;
-  for (size_t i = 0; i < status_vector.size(); ++i, ++seq_num) {
-    const auto& it = packet_status_window_.find(seq_num);
+void TransportFeedbackPacketLossTracker::OnNewTransportFeedbackVector(
+    const std::vector<PacketFeedback>& packet_feedback_vector) {
+  for (const PacketFeedback& packet : packet_feedback_vector) {
+    const auto& it = packet_status_window_.find(packet.sequence_number);
 
     // Packets which aren't at least marked as unacked either do not belong to
     // this media stream, or have been shifted out of window.
     if (it == packet_status_window_.end())
       continue;
 
-    const bool lost =
-        status_vector[i] ==
-        webrtc::rtcp::TransportFeedback::StatusSymbol::kNotReceived;
+    const bool lost = packet.arrival_time_ms == PacketFeedback::kNotReceived;
     const PacketStatus packet_status =
         lost ? PacketStatus::Lost : PacketStatus::Received;
 

webrtc/voice_engine/transport_feedback_packet_loss_tracker.h

@@ -22,6 +22,8 @@ namespace rtcp {
 class TransportFeedback;
 }
 
+struct PacketFeedback;
+
 class TransportFeedbackPacketLossTracker final {
  public:
   // * We count up to |max_window_size_ms| from the sent
@@ -36,7 +38,8 @@ class TransportFeedbackPacketLossTracker final {
 
   void OnPacketAdded(uint16_t seq_num, int64_t send_time_ms);
 
-  void OnReceivedTransportFeedback(const rtcp::TransportFeedback& feedback);
+  void OnNewTransportFeedbackVector(
+      const std::vector<PacketFeedback>& packet_feedbacks_vector);
 
   // Returns the packet loss rate, if the window has enough packet statuses to
   // reliably compute it. Otherwise, returns empty.

webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc

@@ -14,6 +14,7 @@
 #include <vector>
 
 #include "webrtc/base/checks.h"
+#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
 #include "webrtc/test/gmock.h"
 #include "webrtc/test/gtest.h"
@@ -28,8 +29,6 @@ constexpr int64_t kDefaultMaxWindowSizeMs = 500 * kDefaultSendIntervalMs;
 
 class TransportFeedbackPacketLossTrackerTest
     : public ::testing::TestWithParam<uint16_t> {
-  using TransportFeedback = webrtc::rtcp::TransportFeedback;
-
  public:
   TransportFeedbackPacketLossTrackerTest() = default;
   virtual ~TransportFeedbackPacketLossTrackerTest() = default;
@@ -75,28 +74,19 @@ class TransportFeedbackPacketLossTrackerTest
       TransportFeedbackPacketLossTracker* tracker,
       uint16_t base_sequence_num,
       const std::vector<bool>& reception_status_vec) {
-    const int64_t kBaseTimeUs = 1234;  // Irrelevant to this test.
-    TransportFeedback test_feedback;
-    test_feedback.SetBase(base_sequence_num, kBaseTimeUs);
-    uint16_t sequence_num = base_sequence_num;
-    for (bool status : reception_status_vec) {
-      if (status)
-        test_feedback.AddReceivedPacket(sequence_num, kBaseTimeUs);
-      ++sequence_num;
+    // Any positive integer signals reception. kNotReceived signals loss.
+    // Other values are just illegal.
+    constexpr int64_t kArrivalTimeMs = 1234;
+
+    std::vector<PacketFeedback> packet_feedback_vector;
+    uint16_t seq_num = base_sequence_num;
+    for (bool received : reception_status_vec) {
+      packet_feedback_vector.emplace_back(PacketFeedback(
+          received ? kArrivalTimeMs : PacketFeedback::kNotReceived, seq_num));
+      ++seq_num;
     }
 
-    // TransportFeedback imposes some limitations on what constitutes a legal
-    // status vector. For instance, the vector cannot terminate in a lost
-    // packet. Make sure all limitations are abided by.
-    RTC_CHECK_EQ(base_sequence_num, test_feedback.GetBaseSequence());
-    const auto& vec = test_feedback.GetStatusVector();
-    RTC_CHECK_EQ(reception_status_vec.size(), vec.size());
-    for (size_t i = 0; i < reception_status_vec.size(); i++) {
-      RTC_CHECK_EQ(reception_status_vec[i],
-                   vec[i] != TransportFeedback::StatusSymbol::kNotReceived);
-    }
-
-    tracker->OnReceivedTransportFeedback(test_feedback);
+    tracker->OnNewTransportFeedbackVector(packet_feedback_vector);
     tracker->Validate();
   }