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

webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc

Index: webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc
diff --git a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc
index 637794398e2fe8edffc1a9c2e3bdbd34bd970df5..99f5753826d0662895097bd9fe30250e8a696ca4 100644
--- a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc
+++ b/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::OnNewTransportFeedbacks(
+    const std::vector<PacketFeedback>& packet_feedbacks) {
+  for (const PacketFeedback& packet : packet_feedbacks) {
+    const auto& it = packet_status_window_.find(packet.sequence_number);

[stefan-webrtc, 2017/03/20 18:14:44]

Can this be more efficient if we avoid start searching from the beginning each
time?

[elad.alon_webrtc.org, 2017/03/20 18:21:51]

Maybe, although I am not sure if the complexity of the code (*) would be worth
the savings in search-time. How about we do that in a separate CL? (This was not
introduced in this CL, so no degradation here.)

---
(*) First lookup, wrap-around of sequence number, sequence numbers in the vector
which aren't in the map, etc.

[elad.alon_webrtc.org, 2017/03/20 18:33:43]

(By the way, an earlier more efficient deque-based implementation was replaced
with this map-based implementation for readability and maintainability reasons.)

[stefan-webrtc, 2017/03/20 18:36:21]

I guess it depends on the size of the window in packets. Let's leave it for a
future discussion, as you suggest.

 
     // 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;