From 6209dcdeb13558eac8dc4e2a785adbf19606b7ee Mon Sep 17 00:00:00 2001 From: danilchap Date: Tue, 25 Jul 2017 08:07:13 -0700 Subject: [PATCH] Add SetReportBlocks to rtcp Sender/Receive Report classes. BUG=None Review-Url: https://codereview.webrtc.org/2991623002 Cr-Commit-Position: refs/heads/master@{#19136} --- .../source/rtcp_packet/receiver_report.cc | 13 +++++++ .../source/rtcp_packet/receiver_report.h | 4 +- .../rtcp_packet/receiver_report_unittest.cc | 37 +++++++++++++++++-- .../source/rtcp_packet/sender_report.cc | 13 +++++++ .../source/rtcp_packet/sender_report.h | 3 +- .../rtcp_packet/sender_report_unittest.cc | 37 +++++++++++++++++-- 6 files changed, 99 insertions(+), 8 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report.cc index c192807180..fd67600c1b 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report.cc @@ -10,6 +10,8 @@ #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report.h" +#include + #include "webrtc/modules/rtp_rtcp/source/byte_io.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/common_header.h" #include "webrtc/rtc_base/checks.h" @@ -18,6 +20,7 @@ namespace webrtc { namespace rtcp { constexpr uint8_t ReceiverReport::kPacketType; +constexpr size_t ReceiverReport::kMaxNumberOfReportBlocks; // RTCP receiver report (RFC 3550). // // 0 1 2 3 @@ -93,5 +96,15 @@ bool ReceiverReport::AddReportBlock(const ReportBlock& block) { return true; } +bool ReceiverReport::SetReportBlocks(std::vector blocks) { + if (blocks.size() > kMaxNumberOfReportBlocks) { + LOG(LS_WARNING) << "Too many report blocks (" << blocks.size() + << ") for receiver report."; + return false; + } + report_blocks_ = std::move(blocks); + return true; +} + } // namespace rtcp } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report.h b/webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report.h index 85ab8ebd34..1b08f6404c 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report.h @@ -24,6 +24,8 @@ class CommonHeader; class ReceiverReport : public RtcpPacket { public: static constexpr uint8_t kPacketType = 201; + static constexpr size_t kMaxNumberOfReportBlocks = 0x1f; + ReceiverReport(); ~ReceiverReport() override; @@ -32,6 +34,7 @@ class ReceiverReport : public RtcpPacket { void SetSenderSsrc(uint32_t ssrc) { sender_ssrc_ = ssrc; } bool AddReportBlock(const ReportBlock& block); + bool SetReportBlocks(std::vector blocks); uint32_t sender_ssrc() const { return sender_ssrc_; } const std::vector& report_blocks() const { @@ -47,7 +50,6 @@ class ReceiverReport : public RtcpPacket { private: static const size_t kRrBaseLength = 4; - static const size_t kMaxNumberOfReportBlocks = 0x1F; uint32_t sender_ssrc_; std::vector report_blocks_; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report_unittest.cc index 4061db889f..d9b89c8b08 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report_unittest.cc @@ -10,6 +10,8 @@ #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report.h" +#include + #include "webrtc/test/gmock.h" #include "webrtc/test/gtest.h" #include "webrtc/test/rtcp_packet_parser.h" @@ -117,14 +119,43 @@ TEST(RtcpPacketReceiverReportTest, CreateAndParseWithTwoReportBlocks) { TEST(RtcpPacketReceiverReportTest, CreateWithTooManyReportBlocks) { ReceiverReport rr; rr.SetSenderSsrc(kSenderSsrc); - const size_t kMaxReportBlocks = (1 << 5) - 1; ReportBlock rb; - for (size_t i = 0; i < kMaxReportBlocks; ++i) { + for (size_t i = 0; i < ReceiverReport::kMaxNumberOfReportBlocks; ++i) { rb.SetMediaSsrc(kRemoteSsrc + i); EXPECT_TRUE(rr.AddReportBlock(rb)); } - rb.SetMediaSsrc(kRemoteSsrc + kMaxReportBlocks); + rb.SetMediaSsrc(kRemoteSsrc + ReceiverReport::kMaxNumberOfReportBlocks); EXPECT_FALSE(rr.AddReportBlock(rb)); } +TEST(RtcpPacketReceiverReportTest, SetReportBlocksOverwritesOldBlocks) { + ReceiverReport rr; + ReportBlock report_block; + // Use jitter field of the report blocks to distinguish them. + report_block.SetJitter(1001u); + rr.AddReportBlock(report_block); + ASSERT_EQ(rr.report_blocks().size(), 1u); + ASSERT_EQ(rr.report_blocks()[0].jitter(), 1001u); + + std::vector blocks(3u); + blocks[0].SetJitter(2001u); + blocks[1].SetJitter(3001u); + blocks[2].SetJitter(4001u); + EXPECT_TRUE(rr.SetReportBlocks(blocks)); + ASSERT_EQ(rr.report_blocks().size(), 3u); + EXPECT_EQ(rr.report_blocks()[0].jitter(), 2001u); + EXPECT_EQ(rr.report_blocks()[1].jitter(), 3001u); + EXPECT_EQ(rr.report_blocks()[2].jitter(), 4001u); +} + +TEST(RtcpPacketReceiverReportTest, SetReportBlocksMaxLimit) { + ReceiverReport rr; + std::vector max_blocks(ReceiverReport::kMaxNumberOfReportBlocks); + EXPECT_TRUE(rr.SetReportBlocks(std::move(max_blocks))); + + std::vector one_too_many_blocks( + ReceiverReport::kMaxNumberOfReportBlocks + 1); + EXPECT_FALSE(rr.SetReportBlocks(std::move(one_too_many_blocks))); +} + } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/sender_report.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/sender_report.cc index 59c08c3c66..a43ee18b67 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/sender_report.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/sender_report.cc @@ -10,6 +10,8 @@ #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/sender_report.h" +#include + #include "webrtc/modules/rtp_rtcp/source/byte_io.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/common_header.h" #include "webrtc/rtc_base/checks.h" @@ -18,6 +20,7 @@ namespace webrtc { namespace rtcp { constexpr uint8_t SenderReport::kPacketType; +constexpr size_t SenderReport::kMaxNumberOfReportBlocks; // Sender report (SR) (RFC 3550). // 0 1 2 3 // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 @@ -122,5 +125,15 @@ bool SenderReport::AddReportBlock(const ReportBlock& block) { return true; } +bool SenderReport::SetReportBlocks(std::vector blocks) { + if (blocks.size() > kMaxNumberOfReportBlocks) { + LOG(LS_WARNING) << "Too many report blocks (" << blocks.size() + << ") for sender report."; + return false; + } + report_blocks_ = std::move(blocks); + return true; +} + } // namespace rtcp } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/sender_report.h b/webrtc/modules/rtp_rtcp/source/rtcp_packet/sender_report.h index 0e39eb52e6..69b55db12a 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/sender_report.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/sender_report.h @@ -24,6 +24,7 @@ class CommonHeader; class SenderReport : public RtcpPacket { public: static constexpr uint8_t kPacketType = 200; + static constexpr size_t kMaxNumberOfReportBlocks = 0x1f; SenderReport(); ~SenderReport() override; @@ -43,6 +44,7 @@ class SenderReport : public RtcpPacket { sender_octet_count_ = octet_count; } bool AddReportBlock(const ReportBlock& block); + bool SetReportBlocks(std::vector blocks); void ClearReportBlocks() { report_blocks_.clear(); } uint32_t sender_ssrc() const { return sender_ssrc_; } @@ -63,7 +65,6 @@ class SenderReport : public RtcpPacket { RtcpPacket::PacketReadyCallback* callback) const override; private: - static const size_t kMaxNumberOfReportBlocks = 0x1f; const size_t kSenderBaseLength = 24; uint32_t sender_ssrc_; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/sender_report_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/sender_report_unittest.cc index c7576500a4..8e3a926dde 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/sender_report_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/sender_report_unittest.cc @@ -10,6 +10,8 @@ #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/sender_report.h" +#include + #include "webrtc/test/gmock.h" #include "webrtc/test/gtest.h" #include "webrtc/test/rtcp_packet_parser.h" @@ -101,14 +103,43 @@ TEST(RtcpPacketSenderReportTest, CreateAndParseWithTwoReportBlocks) { TEST(RtcpPacketSenderReportTest, CreateWithTooManyReportBlocks) { SenderReport sr; sr.SetSenderSsrc(kSenderSsrc); - const size_t kMaxReportBlocks = (1 << 5) - 1; ReportBlock rb; - for (size_t i = 0; i < kMaxReportBlocks; ++i) { + for (size_t i = 0; i < SenderReport::kMaxNumberOfReportBlocks; ++i) { rb.SetMediaSsrc(kRemoteSsrc + i); EXPECT_TRUE(sr.AddReportBlock(rb)); } - rb.SetMediaSsrc(kRemoteSsrc + kMaxReportBlocks); + rb.SetMediaSsrc(kRemoteSsrc + SenderReport::kMaxNumberOfReportBlocks); EXPECT_FALSE(sr.AddReportBlock(rb)); } +TEST(RtcpPacketSenderReportTest, SetReportBlocksOverwritesOldBlocks) { + SenderReport sr; + ReportBlock report_block; + // Use jitter field of the report blocks to distinguish them. + report_block.SetJitter(1001u); + sr.AddReportBlock(report_block); + ASSERT_EQ(sr.report_blocks().size(), 1u); + ASSERT_EQ(sr.report_blocks()[0].jitter(), 1001u); + + std::vector blocks(3u); + blocks[0].SetJitter(2001u); + blocks[1].SetJitter(3001u); + blocks[2].SetJitter(4001u); + EXPECT_TRUE(sr.SetReportBlocks(blocks)); + ASSERT_EQ(sr.report_blocks().size(), 3u); + EXPECT_EQ(sr.report_blocks()[0].jitter(), 2001u); + EXPECT_EQ(sr.report_blocks()[1].jitter(), 3001u); + EXPECT_EQ(sr.report_blocks()[2].jitter(), 4001u); +} + +TEST(RtcpPacketSenderReportTest, SetReportBlocksMaxLimit) { + SenderReport sr; + std::vector max_blocks(SenderReport::kMaxNumberOfReportBlocks); + EXPECT_TRUE(sr.SetReportBlocks(std::move(max_blocks))); + + std::vector one_too_many_blocks( + SenderReport::kMaxNumberOfReportBlocks + 1); + EXPECT_FALSE(sr.SetReportBlocks(std::move(one_too_many_blocks))); +} + } // namespace webrtc