Packet Loss Tracker - Stream Separation

webrtc/voice_engine/transport_feedback_packet_loss_tracker.h

Index: webrtc/voice_engine/transport_feedback_packet_loss_tracker.h
diff --git a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h
index 77af2fa7da30c33b5b3a490a19977fb8863b3ca0..56d691d5ace665c877c2936c6e6e987a2b23ab2c 100644
--- a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h
+++ b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h
@@ -24,15 +24,17 @@ class TransportFeedback;
 
 class TransportFeedbackPacketLossTracker final {
  public:
-  // * Up to |max_window_size| latest packet statuses will be used for
+  // * Up to |max_acked_packets| latest packet statuses will be used for
   //   calculating the packet loss metrics.
   // * PLR (packet-loss-rate) is reliably computable once the statuses of
-  //   |plr_min_num_packets| packets are known.
+  //   |plr_min_num_acked_packets| packets are known.
   // * RPLR (recoverable-packet-loss-rate) is reliably computable once the
-  //   statuses of |rplr_min_num_pairs| pairs are known.
-  TransportFeedbackPacketLossTracker(size_t max_window_size,
-                                     size_t plr_min_num_packets,
-                                     size_t rplr_min_num_pairs);
+  //   statuses of |rplr_min_num_acked_pairs| pairs are known.
+  TransportFeedbackPacketLossTracker(size_t max_acked_packets,
+                                     size_t plr_min_num_acked_packets,
+                                     size_t rplr_min_num_acked_pairs);
+
+  void OnPacketAdded(uint16_t seq_num);
 
   void OnReceivedTransportFeedback(const rtcp::TransportFeedback& feedback);
 
@@ -48,11 +50,14 @@ class TransportFeedbackPacketLossTracker final {
   void Validate() const;
 
  private:
-  // PacketStatus is a map from sequence number to its reception status. The
-  // status is true if the corresponding packet is received, and false if it is
-  // lost. Unknown statuses are not present in the map.
-  typedef std::map<uint16_t, bool> PacketStatus;
-  typedef PacketStatus::const_iterator PacketStatusIterator;
+  // When a packet is sent, we memorize its association with the stream by
+  // marking it as (sent-but-so-far-) unacked. If we ever receive a feedback
+  // that reports it as received/lost, we update the state and

[minyue-webrtc, 2017/02/14 16:46:59]

nit:  line break after "metrics"

[elad.alon_webrtc.org, 2017/02/15 16:47:36]

Done.

+  // metrics accordingly.
+  enum class PacketStatus { Unacked = 0, Received = 1, Lost = 2 };
+  typedef std::map<uint16_t, PacketStatus> PacketStatusMap;
+  typedef PacketStatusMap::iterator PacketStatusIterator;
+  typedef PacketStatusMap::const_iterator ConstPacketStatusIterator;
 
   void Reset();
 
@@ -62,27 +67,32 @@ class TransportFeedbackPacketLossTracker final {
   // (2^16 + x - ref_seq_num_) % 2^16 defines its actual position in
   // |packet_status_window_|.
   uint16_t ReferenceSequenceNumber() const;
-  bool IsOldSequenceNumber(uint16_t seq_num) const;
-  void InsertPacketStatus(uint16_t seq_num, bool received);
+  uint16_t NewestSequenceNumber() const;
+  void RecordPacketStatus(PacketStatusIterator it,

[minyue-webrtc, 2017/02/14 16:46:59]

Is it really necessary to define PacketStatusIterator

[elad.alon_webrtc.org, 2017/02/15 16:47:36]

We have ConstPacketStatusIterator (earlier named "PacketStatusIterator"). Imho,
we either get rid of both, or of neither. Wdys?

[minyue-webrtc, 2017/02/16 09:51:44]

I like getting rid of both. But I am not sure if we can still achieve the same
efficiency. Since ConstPacketStatusIterator was used to avoid duplicating
"find".

I found that PacketStatusIterator can be avoid without introducing more find(),
and const iterator feels safer because you know the function cannot change the
window.

[elad.alon_webrtc.org, 2017/02/16 15:51:40]

Answered in .cc file and offline.

+                          PacketStatus new_packet_status);
   void RemoveOldestPacketStatus();
 
-  void UpdateMetrics(PacketStatusIterator it, bool apply /* false = undo */);
-  void UpdatePlr(PacketStatusIterator it, bool apply /* false = undo */);
-  void UpdateRplr(PacketStatusIterator it, bool apply /* false = undo */);
+  void UpdateMetrics(ConstPacketStatusIterator it,
+                     bool apply /* false = undo */);
+  void UpdatePlr(ConstPacketStatusIterator it, bool apply /* false = undo */);
+  void UpdateRplr(ConstPacketStatusIterator it, bool apply /* false = undo */);
 
-  PacketStatusIterator PreviousPacketStatus(PacketStatusIterator it);
-  PacketStatusIterator NextPacketStatus(PacketStatusIterator it);
+  ConstPacketStatusIterator PreviousPacketStatus(
+      ConstPacketStatusIterator it) const;
+  ConstPacketStatusIterator NextPacketStatus(
+      ConstPacketStatusIterator it) const;
 
-  const size_t max_window_size_;
+  const size_t max_acked_packets_;
+  size_t acked_packets_;
 
-  PacketStatus packet_status_window_;
+  PacketStatusMap packet_status_window_;
   // |ref_packet_status_| points to the oldest item in |packet_status_window_|.
-  PacketStatusIterator ref_packet_status_;
+  ConstPacketStatusIterator ref_packet_status_;
 
   // Packet-loss-rate calculation (lost / all-known-packets).
   struct PlrState {
-    explicit PlrState(size_t min_num_packets)
-        : min_num_packets_(min_num_packets) {
+    explicit PlrState(size_t min_num_acked_packets)
+        : min_num_acked_packets_(min_num_acked_packets) {
       Reset();
     }
     void Reset() {
@@ -90,19 +100,19 @@ class TransportFeedbackPacketLossTracker final {
       num_lost_packets_ = 0;
     }
     rtc::Optional<float> GetMetric() const;
-    const size_t min_num_packets_;
+    const size_t min_num_acked_packets_;
     size_t num_received_packets_;
     size_t num_lost_packets_;
   } plr_state_;
 
   // Recoverable packet loss calculation (first-order-FEC recoverable).
   struct RplrState {
-    explicit RplrState(size_t min_num_pairs)
-        : min_num_pairs_(min_num_pairs) {
+    explicit RplrState(size_t min_num_acked_pairs)
+        : min_num_acked_pairs_(min_num_acked_pairs) {
       Reset();
     }
     void Reset() {
-      num_known_pairs_ = 0;
+      num_acked_pairs_ = 0;
       num_recoverable_losses_ = 0;
     }
     rtc::Optional<float> GetMetric() const;
@@ -111,12 +121,10 @@ class TransportFeedbackPacketLossTracker final {
     // the data from the former (lost) packet could be recovered.
     // The RPLR is calculated as the fraction of such pairs (lost-received) out
     // of all pairs of consecutive acked packets.
-    const size_t min_num_pairs_;
-    size_t num_known_pairs_;
+    const size_t min_num_acked_pairs_;
+    size_t num_acked_pairs_;
     size_t num_recoverable_losses_;
   } rplr_state_;
-
-  size_t num_consecutive_old_reports_;  // TODO(elad.alon): Upcoming CL removes.
 };
 
 }  // namespace webrtc