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/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(); }