GetTransportFeedbackVector() includes unreceived packets, sorted by seq-num

webrtc/modules/congestion_controller/transport_feedback_adapter.cc

Index: webrtc/modules/congestion_controller/transport_feedback_adapter.cc
diff --git a/webrtc/modules/congestion_controller/transport_feedback_adapter.cc b/webrtc/modules/congestion_controller/transport_feedback_adapter.cc
index 4d502c0e874afd38f1ab479dc5ff0acbb0fd15eb..2c0d3a7d7ef6ced2d7995d3f8ec8063509f84160 100644
--- a/webrtc/modules/congestion_controller/transport_feedback_adapter.cc
+++ b/webrtc/modules/congestion_controller/transport_feedback_adapter.cc
@@ -15,6 +15,7 @@
 
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
+#include "webrtc/base/mod_ops.h"
 #include "webrtc/logging/rtc_event_log/rtc_event_log.h"
 #include "webrtc/modules/bitrate_controller/include/bitrate_controller.h"
 #include "webrtc/modules/congestion_controller/delay_based_bwe.h"
@@ -30,17 +31,6 @@ const int64_t kBaseTimestampScaleFactor =
     rtcp::TransportFeedback::kDeltaScaleFactor * (1 << 8);
 const int64_t kBaseTimestampRangeSizeUs = kBaseTimestampScaleFactor * (1 << 24);
 
-class PacketFeedbackComparator {
- public:
-  inline bool operator()(const PacketFeedback& lhs, const PacketFeedback& rhs) {
-    if (lhs.arrival_time_ms != rhs.arrival_time_ms)
-      return lhs.arrival_time_ms < rhs.arrival_time_ms;
-    if (lhs.send_time_ms != rhs.send_time_ms)
-      return lhs.send_time_ms < rhs.send_time_ms;
-    return lhs.sequence_number < rhs.sequence_number;
-  }
-};
-
 TransportFeedbackAdapter::TransportFeedbackAdapter(
     RtcEventLog* event_log,
     Clock* clock,
@@ -135,26 +125,48 @@ std::vector<PacketFeedback> TransportFeedbackAdapter::GetPacketFeedbackVector(
 
   auto received_packets = feedback.GetReceivedPackets();
   std::vector<PacketFeedback> packet_feedback_vector;
-  packet_feedback_vector.reserve(received_packets.size());
   if (received_packets.empty()) {
     LOG(LS_INFO) << "Empty transport feedback packet received.";
     return packet_feedback_vector;
   }
+  const uint16_t last_sequence_number =
+      received_packets[received_packets.size() - 1].sequence_number();

[stefan-webrtc, 2017/03/03 20:11:13]

received_packets.back().sequence_number()

[elad.alon_webrtc.org, 2017/03/06 12:24:58]

Will do.

+  const size_t packet_count =
+      1 + ForwardDiff(feedback.GetBaseSequence(), last_sequence_number);
+  packet_feedback_vector.reserve(packet_count);
+  // feedback.GetStatusVector().size() is a less efficient way to reach what
+  // should be the same value.
+  RTC_DCHECK_EQ(packet_count, feedback.GetStatusVector().size());
+
   {
     rtc::CritScope cs(&lock_);
     size_t failed_lookups = 0;
     int64_t offset_us = 0;
     int64_t timestamp_ms = 0;
-    for (const auto& packet : feedback.GetReceivedPackets()) {
+    uint16_t seq_no = feedback.GetBaseSequence();

[stefan-webrtc, 2017/03/03 20:11:13]

seq_num is typically used throughout the code base

[elad.alon_webrtc.org, 2017/03/06 12:24:58]

Will change (I also prefer seq_num). FYI, I lifted this from
webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.cc, so you might
want some changes there.

+    for (const auto& packet : received_packets) {
+      // Insert into the vector those unreceived packets which precede this
+      // iteration's received packet.
+      for (; seq_no != packet.sequence_number(); ++seq_no) {
+        PacketFeedback packet_feedback(PacketFeedback::kNotReceived, seq_no);
+        // Note: Element not removed from history because it might be reported
+        // as received by another feedback.
+        if (!send_time_history_.GetFeedback(&packet_feedback, false))
+          ++failed_lookups;
+        packet_feedback_vector.push_back(packet_feedback);
+      }
+
+      // Handle this iteration's received packet.
       offset_us += packet.delta_us();
       timestamp_ms = current_offset_ms_ + (offset_us / 1000);
       PacketFeedback packet_feedback(timestamp_ms, packet.sequence_number());
       if (!send_time_history_.GetFeedback(&packet_feedback, true))
         ++failed_lookups;
       packet_feedback_vector.push_back(packet_feedback);
+
+      ++seq_no;
     }
-    std::sort(packet_feedback_vector.begin(), packet_feedback_vector.end(),
-              PacketFeedbackComparator());
+
     if (failed_lookups > 0) {
       LOG(LS_WARNING) << "Failed to lookup send time for " << failed_lookups
                       << " packet" << (failed_lookups > 1 ? "s" : "")