Add stats for duplicate sent and received NACK requests.

R=mflodman@webrtc.org, stefan@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/27799004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@7559 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/common_types.h b/webrtc/common_types.h
index e607ddc..8a20e5e 100644
--- a/webrtc/common_types.h
+++ b/webrtc/common_types.h
@@ -202,17 +202,31 @@
   RtcpPacketTypeCounter()
     : nack_packets(0),
       fir_packets(0),
-      pli_packets(0) {}
+      pli_packets(0),
+      nack_requests(0),
+      unique_nack_requests(0) {}
 
   void Add(const RtcpPacketTypeCounter& other) {
     nack_packets += other.nack_packets;
     fir_packets += other.fir_packets;
     pli_packets += other.pli_packets;
+    nack_requests += other.nack_requests;
+    unique_nack_requests += other.unique_nack_requests;
   }
 
-  uint32_t nack_packets;
-  uint32_t fir_packets;
-  uint32_t pli_packets;
+  int UniqueNackRequestsInPercent() const {
+    if (nack_requests == 0) {
+      return 0;
+    }
+    return static_cast<int>(
+        (unique_nack_requests * 100.0f / nack_requests) + 0.5f);
+  }
+
+  uint32_t nack_packets;          // Number of RTCP NACK packets.
+  uint32_t fir_packets;           // Number of RTCP FIR packets.
+  uint32_t pli_packets;           // Number of RTCP PLI packets.
+  uint32_t nack_requests;         // Number of NACKed RTP packets.
+  uint32_t unique_nack_requests;  // Number of unique NACKed RTP packets.
 };
 
 // Data usage statistics for a (rtp) stream
diff --git a/webrtc/modules/modules.gyp b/webrtc/modules/modules.gyp
index 12200fe..f1d9df9 100644
--- a/webrtc/modules/modules.gyp
+++ b/webrtc/modules/modules.gyp
@@ -213,6 +213,7 @@
             'rtp_rtcp/source/rtcp_packet_unittest.cc',
             'rtp_rtcp/source/rtcp_receiver_unittest.cc',
             'rtp_rtcp/source/rtcp_sender_unittest.cc',
+            'rtp_rtcp/source/rtcp_utility_unittest.cc',
             'rtp_rtcp/source/rtp_fec_unittest.cc',
             'rtp_rtcp/source/rtp_format_h264_unittest.cc',
             'rtp_rtcp/source/rtp_format_vp8_unittest.cc',
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc
index a934314..1d1e291 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc
@@ -833,6 +833,8 @@
 
     if (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpNack) {
       ++packet_type_counter_.nack_packets;
+      packet_type_counter_.nack_requests = nack_stats_.requests();
+      packet_type_counter_.unique_nack_requests = nack_stats_.unique_requests();
     }
 }
 
@@ -842,6 +844,7 @@
                              RTCPPacketInformation& rtcpPacketInformation)
 {
     rtcpPacketInformation.AddNACKPacket(rtcpPacket.NACKItem.PacketID);
+    nack_stats_.ReportRequest(rtcpPacket.NACKItem.PacketID);
 
     uint16_t bitMask = rtcpPacket.NACKItem.BitMask;
     if(bitMask)
@@ -851,6 +854,7 @@
             if(bitMask & 0x01)
             {
                 rtcpPacketInformation.AddNACKPacket(rtcpPacket.NACKItem.PacketID + i);
+                nack_stats_.ReportRequest(rtcpPacket.NACKItem.PacketID + i);
             }
             bitMask = bitMask >>1;
         }
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h
index 84eb24c..087722c 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h
@@ -270,6 +270,8 @@
   RtcpStatisticsCallback* stats_callback_;
 
   RtcpPacketTypeCounter packet_type_counter_;
+
+  RTCPUtility::NackStats nack_stats_;
 };
 }  // namespace webrtc
 #endif // WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_RECEIVER_H_
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
index b3a9d09..2a61573 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc
@@ -592,13 +592,6 @@
   EXPECT_FALSE(rtcp_packet_info_.xr_dlrr_item);
 }
 
-TEST(RtcpUtilityTest, MidNtp) {
-  const uint32_t kNtpSec = 0x12345678;
-  const uint32_t kNtpFrac = 0x23456789;
-  const uint32_t kNtpMid = 0x56782345;
-  EXPECT_EQ(kNtpMid, RTCPUtility::MidNtp(kNtpSec, kNtpFrac));
-}
-
 TEST_F(RtcpReceiverTest, TestXrRrRttInitiallyFalse) {
   uint16_t rtt_ms;
   EXPECT_FALSE(rtcp_receiver_->GetAndResetXrRrRtt(&rtt_ms));
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
index 1edbee4..afe8019 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc
@@ -1354,7 +1354,6 @@
     RtpUtility::AssignUWord32ToBuffer(rtcpbuffer + pos, _remoteSSRC);
     pos += 4;
 
-    NACKStringBuilder stringBuilder;
     // Build NACK bitmasks and write them to the RTCP message.
     // The nack list should be sorted and not contain duplicates if one
     // wants to build the smallest rtcp nack packet.
@@ -1363,13 +1362,11 @@
                                       (IP_PACKET_SIZE - pos) / 4);
     int i = 0;
     while (i < nackSize && numOfNackFields < maxNackFields) {
-      stringBuilder.PushNACK(nackList[i]);
       uint16_t nack = nackList[i++];
       uint16_t bitmask = 0;
       while (i < nackSize) {
         int shift = static_cast<uint16_t>(nackList[i] - nack) - 1;
         if (shift >= 0 && shift <= 15) {
-          stringBuilder.PushNACK(nackList[i]);
           bitmask |= (1 << shift);
           ++i;
         } else {
@@ -1384,11 +1381,21 @@
       pos += 2;
       numOfNackFields++;
     }
-    if (i != nackSize) {
-      LOG(LS_WARNING) << "Nack list to large for one packet.";
-    }
     rtcpbuffer[nackSizePos] = static_cast<uint8_t>(2 + numOfNackFields);
+
+    if (i != nackSize) {
+      LOG(LS_WARNING) << "Nack list too large for one packet.";
+    }
+
+    // Report stats.
+    NACKStringBuilder stringBuilder;
+    for (int idx = 0; idx < i; ++idx) {
+      stringBuilder.PushNACK(nackList[idx]);
+      nack_stats_.ReportRequest(nackList[idx]);
+    }
     *nackString = stringBuilder.GetResult();
+    packet_type_counter_.nack_requests = nack_stats_.requests();
+    packet_type_counter_.unique_nack_requests = nack_stats_.unique_requests();
     return 0;
 }
 
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.h b/webrtc/modules/rtp_rtcp/source/rtcp_sender.h
index c0b8ebd..06c373b 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.h
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.h
@@ -360,6 +360,8 @@
 
     RtcpPacketTypeCounter packet_type_counter_
         GUARDED_BY(_criticalSectionRTCPSender);
+
+    RTCPUtility::NackStats nack_stats_ GUARDED_BY(_criticalSectionRTCPSender);
 };
 }  // namespace webrtc
 
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_utility.cc b/webrtc/modules/rtp_rtcp/source/rtcp_utility.cc
index 9acab73..0423389 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_utility.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_utility.cc
@@ -17,6 +17,23 @@
 namespace webrtc {
 
 namespace RTCPUtility {
+
+NackStats::NackStats()
+    : max_sequence_number_(0),
+      requests_(0),
+      unique_requests_(0) {}
+
+NackStats::~NackStats() {}
+
+void NackStats::ReportRequest(uint16_t sequence_number) {
+  if (requests_ == 0 ||
+      webrtc::IsNewerSequenceNumber(sequence_number, max_sequence_number_)) {
+    max_sequence_number_ =  sequence_number;
+    ++unique_requests_;
+  }
+  ++requests_;
+}
+
 uint32_t MidNtp(uint32_t ntp_sec, uint32_t ntp_frac) {
   return (ntp_sec << 16) + (ntp_frac >> 16);
 }  // end RTCPUtility
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_utility.h b/webrtc/modules/rtp_rtcp/source/rtcp_utility.h
index f0867a7..804e8b9 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_utility.h
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_utility.h
@@ -19,6 +19,29 @@
 
 namespace webrtc {
 namespace RTCPUtility {
+
+class NackStats {
+ public:
+  NackStats();
+  ~NackStats();
+
+  // Updates stats with requested sequence number.
+  // This function should be called for each NACK request to calculate the
+  // number of unique NACKed RTP packets.
+  void ReportRequest(uint16_t sequence_number);
+
+  // Gets the number of NACKed RTP packets.
+  uint32_t requests() const { return requests_; }
+
+  // Gets the number of unique NACKed RTP packets.
+  uint32_t unique_requests() const { return unique_requests_; }
+
+ private:
+  uint16_t max_sequence_number_;
+  uint32_t requests_;
+  uint32_t unique_requests_;
+};
+
     uint32_t MidNtp(uint32_t ntp_sec, uint32_t ntp_frac);
 
     // CNAME
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_utility_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_utility_unittest.cc
new file mode 100644
index 0000000..275b007
--- /dev/null
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_utility_unittest.cc
@@ -0,0 +1,72 @@
+/*
+ *  Copyright (c) 2014 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+#include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h"
+
+namespace webrtc {
+
+TEST(RtcpUtilityTest, MidNtp) {
+  const uint32_t kNtpSec = 0x12345678;
+  const uint32_t kNtpFrac = 0x23456789;
+  const uint32_t kNtpMid = 0x56782345;
+  EXPECT_EQ(kNtpMid, RTCPUtility::MidNtp(kNtpSec, kNtpFrac));
+}
+
+TEST(RtcpUtilityTest, NackRequests) {
+  RTCPUtility::NackStats stats;
+  EXPECT_EQ(0U, stats.unique_requests());
+  EXPECT_EQ(0U, stats.requests());
+  stats.ReportRequest(10);
+  EXPECT_EQ(1U, stats.unique_requests());
+  EXPECT_EQ(1U, stats.requests());
+
+  stats.ReportRequest(10);
+  EXPECT_EQ(1U, stats.unique_requests());
+  stats.ReportRequest(11);
+  EXPECT_EQ(2U, stats.unique_requests());
+
+  stats.ReportRequest(11);
+  EXPECT_EQ(2U, stats.unique_requests());
+  stats.ReportRequest(13);
+  EXPECT_EQ(3U, stats.unique_requests());
+
+  stats.ReportRequest(11);
+  EXPECT_EQ(3U, stats.unique_requests());
+  EXPECT_EQ(6U, stats.requests());
+}
+
+TEST(RtcpUtilityTest, NackRequestsWithWrap) {
+  RTCPUtility::NackStats stats;
+  stats.ReportRequest(65534);
+  EXPECT_EQ(1U, stats.unique_requests());
+
+  stats.ReportRequest(65534);
+  EXPECT_EQ(1U, stats.unique_requests());
+  stats.ReportRequest(65535);
+  EXPECT_EQ(2U, stats.unique_requests());
+
+  stats.ReportRequest(65535);
+  EXPECT_EQ(2U, stats.unique_requests());
+  stats.ReportRequest(0);
+  EXPECT_EQ(3U, stats.unique_requests());
+
+  stats.ReportRequest(65535);
+  EXPECT_EQ(3U, stats.unique_requests());
+  stats.ReportRequest(0);
+  EXPECT_EQ(3U, stats.unique_requests());
+  stats.ReportRequest(1);
+  EXPECT_EQ(4U, stats.unique_requests());
+  EXPECT_EQ(8U, stats.requests());
+}
+
+}  // namespace webrtc
+
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
index 4868b71..24ff227 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
@@ -18,8 +18,10 @@
 #include "webrtc/modules/rtp_rtcp/source/rtcp_packet.h"
 #include "webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h"
 #include "webrtc/system_wrappers/interface/scoped_vector.h"
+#include "webrtc/test/rtcp_packet_parser.h"
 
 using ::testing::_;
+using ::testing::ElementsAre;
 using ::testing::NiceMock;
 using ::testing::Return;
 using ::testing::SaveArg;
@@ -76,6 +78,10 @@
     return len;
   }
   virtual int SendRTCPPacket(int /*ch*/, const void *data, int len) OVERRIDE {
+    test::RtcpPacketParser parser;
+    parser.Parse(static_cast<const uint8_t*>(data), len);
+    last_nack_list_ = parser.nack_item()->last_nack_list();
+
     if (clock_) {
       clock_->AdvanceTimeMilliseconds(delay_ms_);
     }
@@ -89,6 +95,7 @@
   uint32_t delay_ms_;
   int rtp_packets_sent_;
   RTPHeader last_rtp_header_;
+  std::vector<uint16_t> last_nack_list_;
 };
 
 class RtpRtcpModule {
@@ -129,6 +136,9 @@
   uint16_t LastRtpSequenceNumber() {
     return transport_.last_rtp_header_.sequenceNumber;
   }
+  std::vector<uint16_t> LastNackListSent() {
+    return transport_.last_nack_list_;
+  }
 };
 }  // namespace
 
@@ -344,6 +354,46 @@
   EXPECT_EQ(1U, sender_.RtcpReceived().pli_packets);
 }
 
+TEST_F(RtpRtcpImplTest, UniqueNackRequests) {
+  receiver_.transport_.SimulateNetworkDelay(0, &clock_);
+  EXPECT_EQ(0U, receiver_.RtcpSent().nack_packets);
+  EXPECT_EQ(0U, receiver_.RtcpSent().nack_requests);
+  EXPECT_EQ(0U, receiver_.RtcpSent().unique_nack_requests);
+  EXPECT_EQ(0, receiver_.RtcpSent().UniqueNackRequestsInPercent());
+
+  // Receive module sends NACK request.
+  const uint16_t kNackLength = 4;
+  uint16_t nack_list[kNackLength] = {10, 11, 13, 18};
+  EXPECT_EQ(0, receiver_.impl_->SendNACK(nack_list, kNackLength));
+  EXPECT_EQ(1U, receiver_.RtcpSent().nack_packets);
+  EXPECT_EQ(4U, receiver_.RtcpSent().nack_requests);
+  EXPECT_EQ(4U, receiver_.RtcpSent().unique_nack_requests);
+  EXPECT_THAT(receiver_.LastNackListSent(), ElementsAre(10, 11, 13, 18));
+
+  // Send module receives the request.
+  EXPECT_EQ(1U, sender_.RtcpReceived().nack_packets);
+  EXPECT_EQ(4U, sender_.RtcpReceived().nack_requests);
+  EXPECT_EQ(4U, sender_.RtcpReceived().unique_nack_requests);
+  EXPECT_EQ(100, sender_.RtcpReceived().UniqueNackRequestsInPercent());
+
+  // Receive module sends new request with duplicated packets.
+  const int kStartupRttMs = 100;
+  clock_.AdvanceTimeMilliseconds(kStartupRttMs + 1);
+  const uint16_t kNackLength2 = 4;
+  uint16_t nack_list2[kNackLength2] = {11, 18, 20, 21};
+  EXPECT_EQ(0, receiver_.impl_->SendNACK(nack_list2, kNackLength2));
+  EXPECT_EQ(2U, receiver_.RtcpSent().nack_packets);
+  EXPECT_EQ(8U, receiver_.RtcpSent().nack_requests);
+  EXPECT_EQ(6U, receiver_.RtcpSent().unique_nack_requests);
+  EXPECT_THAT(receiver_.LastNackListSent(), ElementsAre(11, 18, 20, 21));
+
+  // Send module receives the request.
+  EXPECT_EQ(2U, sender_.RtcpReceived().nack_packets);
+  EXPECT_EQ(8U, sender_.RtcpReceived().nack_requests);
+  EXPECT_EQ(6U, sender_.RtcpReceived().unique_nack_requests);
+  EXPECT_EQ(75, sender_.RtcpReceived().UniqueNackRequestsInPercent());
+}
+
 class RtpSendingTestTransport : public Transport {
  public:
   void ResetCounters() { bytes_received_.clear(); }