First-order-FEC recoverability calculation

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 4ce78a1932351005e83ef61d455bd836cfeac04e..a8ea85a5e19630ce644755d868b8118a29dcc1a2 100644
--- a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc
+++ b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc
@@ -27,20 +27,25 @@ namespace webrtc {
 
 TransportFeedbackPacketLossTracker::TransportFeedbackPacketLossTracker(
     size_t min_window_size,
-    size_t max_window_size)
+    size_t max_window_size,
+    size_t min_pairs_num_for_rplr)
     : min_window_size_(min_window_size),
       max_window_size_(max_window_size),
+      min_pairs_num_for_rplr_(min_pairs_num_for_rplr),
       ref_packet_status_(packet_status_window_.begin()) {
   RTC_DCHECK_GT(min_window_size, 0u);
   RTC_DCHECK_GE(max_window_size_, min_window_size_);
   RTC_DCHECK_LE(max_window_size_, kSeqNumHalf);
+  RTC_DCHECK_GT(min_pairs_num_for_rplr, 0);
+  RTC_DCHECK_GT(max_window_size, min_pairs_num_for_rplr);
   Reset();
 }
 
 void TransportFeedbackPacketLossTracker::Reset() {
   num_received_packets_ = 0;
   num_lost_packets_ = 0;
-  num_consecutive_losses_ = 0;
+  num_known_status_pairs_ = 0;
+  num_loss_followed_by_reception_pairs_ = 0;
   num_consecutive_old_reports_ = 0;
   packet_status_window_.clear();
   ref_packet_status_ = packet_status_window_.begin();
@@ -103,16 +108,25 @@ void TransportFeedbackPacketLossTracker::OnReceivedTransportFeedback(
   }
 }
 
-bool TransportFeedbackPacketLossTracker::GetPacketLossRates(
-    float* packet_loss_rate,
-    float* consecutive_packet_loss_rate) const {
+rtc::Optional<float>
+TransportFeedbackPacketLossTracker::GetPacketLossRate() const {
   const size_t total = num_lost_packets_ + num_received_packets_;
   if (total < min_window_size_)
-    return false;
-  *packet_loss_rate = static_cast<float>(num_lost_packets_) / total;
-  *consecutive_packet_loss_rate =
-      static_cast<float>(num_consecutive_losses_) / total;
-  return true;
+    return rtc::Optional<float>();
+  else
+    return rtc::Optional<float>(static_cast<float>(num_lost_packets_) / total);
+}
+
+// Returns the first-order-FEC recoverable packet loss rate, if the window
+// has enough data to reliably compute it.
+rtc::Optional<float>
+TransportFeedbackPacketLossTracker::GetRecoverablePacketLossRate() const {
+  if (num_known_status_pairs_ < min_pairs_num_for_rplr_)
+    return rtc::Optional<float>();
+  else
+    return rtc::Optional<float>(
+        static_cast<float>(num_loss_followed_by_reception_pairs_) /
+        num_known_status_pairs_);
 }
 
 void TransportFeedbackPacketLossTracker::InsertPacketStatus(uint16_t seq_num,
@@ -146,51 +160,67 @@ void TransportFeedbackPacketLossTracker::RemoveOldestPacketStatus() {
 void TransportFeedbackPacketLossTracker::ApplyPacketStatus(

[elad.alon_webrtc.org, 2017/01/14 14:05:55]

Style question:

ApplyPacketStatus and UndoPacketStatus are pretty much the same, except for one
incrementing while the other decrementing. It looks like merging them would be
the way to go, but the function which would do both -
ModifyPacketStatus(PacketStatusIterator it, bool addition) - would need to add
-1/+1 to an unsigned type (size_t), meaning unwanted conversions, etc. The way
out of that is to also add a convenience function, Update(size_t& val, bool
increment), which would ++ or -- as needed. However, that already gets a bit
unsightly, and I am not sure if I'll have created a bigger mess than I had set
out to clean.

Thoughts?

     PacketStatusIterator it) {
   RTC_DCHECK(it != packet_status_window_.end());
+
+  // Record recetion status of currently handled packet.
   if (it->second) {
     ++num_received_packets_;
   } else {
     ++num_lost_packets_;
-    const auto& next = NextPacketStatus(it);
-    if (next != packet_status_window_.end() &&
-        next->first == static_cast<uint16_t>(it->first + 1) && !next->second) {
-      // Feedback shows that the next packet has been lost. Since this
-      // packet is lost, we increase the consecutive loss counter.
-      ++num_consecutive_losses_;
-    }
-    if (it != ref_packet_status_) {
-      const auto& pre = PreviousPacketStatus(it);
-      if (pre->first == static_cast<uint16_t>(it->first - 1) && !pre->second) {
-        // Feedback shows that the previous packet has been lost. Since this
-        // packet is lost, we increase the consecutive loss counter.
-        ++num_consecutive_losses_;
+  }
+
+  // Previous packet and current packet might compose a pair.
+  if (it != ref_packet_status_) {
+    const auto& prev = PreviousPacketStatus(it);
+    if (prev->first == static_cast<uint16_t>(it->first - 1)) {
+      ++num_known_status_pairs_;
+      if (!prev->second && it->second) {
+        ++num_loss_followed_by_reception_pairs_;
       }
     }
   }
+
+  // Current packet and next packet might compose a pair.
+  const auto& next = NextPacketStatus(it);
+  if (next != packet_status_window_.end() &&
+      next->first == static_cast<uint16_t>(it->first + 1)) {
+    ++num_known_status_pairs_;
+    if (!it->second && next->second) {
+      ++num_loss_followed_by_reception_pairs_;
+    }
+  }
 }
 
 void TransportFeedbackPacketLossTracker::UndoPacketStatus(
     PacketStatusIterator it) {
   RTC_DCHECK(it != packet_status_window_.end());
+
+  // Undo recetion status of currently handled packet.
   if (it->second) {
-    RTC_DCHECK_GT(num_received_packets_, 0u);
     --num_received_packets_;
   } else {
-    RTC_DCHECK_GT(num_lost_packets_, 0u);
     --num_lost_packets_;
-    const auto& next = NextPacketStatus(it);
-    if (next != packet_status_window_.end() &&
-        next->first == static_cast<uint16_t>(it->first + 1) && !next->second) {
-      RTC_DCHECK_GT(num_consecutive_losses_, 0u);
-      --num_consecutive_losses_;
-    }
-    if (it != ref_packet_status_) {
-      const auto& pre = PreviousPacketStatus(it);
-      if (pre->first == static_cast<uint16_t>(it->first - 1) && !pre->second) {
-        RTC_DCHECK_GT(num_consecutive_losses_, 0u);
-        --num_consecutive_losses_;
+  }
+
+  // Previous packet and current packet might have composed a pair.
+  if (it != ref_packet_status_) {
+    const auto& prev = PreviousPacketStatus(it);
+    if (prev->first == static_cast<uint16_t>(it->first - 1)) {
+      --num_known_status_pairs_;
+      if (!prev->second && it->second) {
+        --num_loss_followed_by_reception_pairs_;
       }
     }
   }
+
+  // Current packet and next packet might have composed a pair.
+  const auto& next = NextPacketStatus(it);
+  if (next != packet_status_window_.end() &&
+      next->first == static_cast<uint16_t>(it->first + 1)) {
+    --num_known_status_pairs_;
+    if (!it->second && next->second) {
+      --num_loss_followed_by_reception_pairs_;
+    }
+  }
 }
 
 TransportFeedbackPacketLossTracker::PacketStatusIterator
@@ -235,13 +265,15 @@ TransportFeedbackPacketLossTracker::NextPacketStatus(PacketStatusIterator it) {
 // to unit test.
 void TransportFeedbackPacketLossTracker::Validate() const {  // Testing only!
   RTC_CHECK_LE(packet_status_window_.size(), max_window_size_);
-  RTC_CHECK_GE(num_lost_packets_, num_consecutive_losses_);
   RTC_CHECK_EQ(packet_status_window_.size(),
                num_lost_packets_ + num_received_packets_);
+  RTC_CHECK_LE(num_loss_followed_by_reception_pairs_, num_known_status_pairs_);
+  RTC_CHECK_LE(num_known_status_pairs_, packet_status_window_.size() - 1);
 
   size_t received_packets = 0;
   size_t lost_packets = 0;
-  size_t consecutive_losses = 0;
+  size_t known_status_pairs = 0;
+  size_t loss_followed_by_reception_pairs = 0;
 
   if (!packet_status_window_.empty()) {
     PacketStatusIterator it = ref_packet_status_;
@@ -252,8 +284,17 @@ void TransportFeedbackPacketLossTracker::Validate() const {  // Testing only!
         ++received_packets;
       } else {
         ++lost_packets;
-        if (pre_lost && pre_seq_num == static_cast<uint16_t>(it->first - 1))
-          ++consecutive_losses;
+      }
+
+      auto next = std::next(it);
+      if (next == packet_status_window_.end())
+        next = packet_status_window_.begin();
+
+      if (next != ref_packet_status_ &&
+          static_cast<uint16_t>(it->first + 1) == next->first) {
+        ++known_status_pairs;
+        if (!it->second && next->second)
+          ++loss_followed_by_reception_pairs;
       }
 
       RTC_CHECK_LT(ForwardDiff(ReferenceSequenceNumber(), it->first),
@@ -262,15 +303,15 @@ void TransportFeedbackPacketLossTracker::Validate() const {  // Testing only!
       pre_lost = !it->second;
       pre_seq_num = it->first;
 
-      ++it;
-      if (it == packet_status_window_.end())
-        it = packet_status_window_.begin();
+      it = next;
     } while (it != ref_packet_status_);
   }
 
   RTC_CHECK_EQ(num_received_packets_, received_packets);
   RTC_CHECK_EQ(num_lost_packets_, lost_packets);
-  RTC_CHECK_EQ(num_consecutive_losses_, consecutive_losses);
+  RTC_CHECK_EQ(num_known_status_pairs_, known_status_pairs);
+  RTC_CHECK_EQ(num_loss_followed_by_reception_pairs_,
+               loss_followed_by_reception_pairs);
 }
 
 }  // namespace webrtc