From 367bbbf76dbd28ca1e9ff107ec3e83604b65c2dd Mon Sep 17 00:00:00 2001 From: danilchap Date: Wed, 30 Mar 2016 09:43:03 -0700 Subject: [PATCH] [rtcp] ReceiverReport::Parse updated not to use RTCPUtility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BUG=webrtc:5260 R=åsapersson Review URL: https://codereview.webrtc.org/1837213002 Cr-Commit-Position: refs/heads/master@{#12164} --- .../source/rtcp_packet/receiver_report.cc | 20 ++--- .../source/rtcp_packet/receiver_report.h | 7 +- .../rtcp_packet/receiver_report_unittest.cc | 83 ++++++++----------- 3 files changed, 47 insertions(+), 63 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 ef64b4f51b..10744ad939 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report.cc @@ -13,12 +13,10 @@ #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" #include "webrtc/modules/rtp_rtcp/source/byte_io.h" - -using webrtc::RTCPUtility::RtcpCommonHeader; +#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/common_header.h" namespace webrtc { namespace rtcp { - // // RTCP receiver report (RFC 3550). // @@ -31,21 +29,20 @@ namespace rtcp { // +=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+ // | report block(s) | // | .... | -bool ReceiverReport::Parse(const RTCPUtility::RtcpCommonHeader& header, - const uint8_t* payload) { - RTC_DCHECK(header.packet_type == kPacketType); +bool ReceiverReport::Parse(const CommonHeader& packet) { + RTC_DCHECK(packet.type() == kPacketType); - const uint8_t report_blocks_count = header.count_or_format; + const uint8_t report_blocks_count = packet.count(); - if (header.payload_size_bytes < + if (packet.payload_size_bytes() < kRrBaseLength + report_blocks_count * ReportBlock::kLength) { LOG(LS_WARNING) << "Packet is too small to contain all the data."; return false; } - sender_ssrc_ = ByteReader::ReadBigEndian(payload); + sender_ssrc_ = ByteReader::ReadBigEndian(packet.payload()); - const uint8_t* next_report_block = payload + kRrBaseLength; + const uint8_t* next_report_block = packet.payload() + kRrBaseLength; report_blocks_.resize(report_blocks_count); for (ReportBlock& block : report_blocks_) { @@ -53,7 +50,8 @@ bool ReceiverReport::Parse(const RTCPUtility::RtcpCommonHeader& header, next_report_block += ReportBlock::kLength; } - RTC_DCHECK_LE(next_report_block, payload + header.payload_size_bytes); + RTC_DCHECK_EQ(next_report_block - packet.payload(), + static_cast(packet.payload_size_bytes())); return true; } 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 172a84ea2f..237d923cd7 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report.h @@ -16,21 +16,20 @@ #include "webrtc/base/basictypes.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/report_block.h" -#include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h" namespace webrtc { namespace rtcp { +class CommonHeader; class ReceiverReport : public RtcpPacket { public: static const uint8_t kPacketType = 201; ReceiverReport() : sender_ssrc_(0) {} - virtual ~ReceiverReport() {} + ~ReceiverReport() override {} // Parse assumes header is already parsed and validated. - bool Parse(const RTCPUtility::RtcpCommonHeader& header, - const uint8_t* payload); // Size of the payload is in the header. + bool Parse(const CommonHeader& packet); void From(uint32_t ssrc) { sender_ssrc_ = ssrc; } bool WithReportBlock(const ReportBlock& block); 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 d6c7fc1dc4..d2c833b866 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,12 +10,15 @@ #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report.h" +#include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/test/rtcp_packet_parser.h" +using testing::ElementsAreArray; +using testing::IsEmpty; +using testing::make_tuple; using webrtc::rtcp::ReceiverReport; using webrtc::rtcp::ReportBlock; -using webrtc::RTCPUtility::RtcpCommonHeader; -using webrtc::RTCPUtility::RtcpParseCommonHeader; namespace webrtc { namespace { @@ -34,31 +37,11 @@ const uint8_t kPacket[] = {0x81, 201, 0x00, 0x07, 0x12, 0x34, 0x56, 0x78, 0x23, 0x45, 0x67, 0x89, 55, 0x11, 0x12, 0x13, 0x22, 0x23, 0x24, 0x25, 0x33, 0x34, 0x35, 0x36, 0x44, 0x45, 0x46, 0x47, 0x55, 0x56, 0x57, 0x58}; -const size_t kPacketLength = sizeof(kPacket); - -class RtcpPacketReceiverReportTest : public ::testing::Test { - protected: - void BuildPacket() { packet = rr.Build(); } - void ParsePacket() { - RtcpCommonHeader header; - EXPECT_TRUE(RtcpParseCommonHeader(packet.data(), packet.size(), &header)); - EXPECT_EQ(header.BlockSize(), packet.size()); - EXPECT_TRUE(parsed_.Parse( - header, packet.data() + RtcpCommonHeader::kHeaderSizeBytes)); - } +} // namespace +TEST(RtcpPacketReceiverReportTest, ParseWithOneReportBlock) { ReceiverReport rr; - rtc::Buffer packet; - const ReceiverReport& parsed() { return parsed_; } - - private: - ReceiverReport parsed_; -}; - -TEST_F(RtcpPacketReceiverReportTest, Parse) { - RtcpCommonHeader header; - RtcpParseCommonHeader(kPacket, kPacketLength, &header); - EXPECT_TRUE(rr.Parse(header, kPacket + RtcpCommonHeader::kHeaderSizeBytes)); + EXPECT_TRUE(test::ParseSinglePacket(kPacket, &rr)); const ReceiverReport& parsed = rr; EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); @@ -73,14 +56,15 @@ TEST_F(RtcpPacketReceiverReportTest, Parse) { EXPECT_EQ(kDelayLastSr, rb.delay_since_last_sr()); } -TEST_F(RtcpPacketReceiverReportTest, ParseFailsOnIncorrectSize) { - RtcpCommonHeader header; - RtcpParseCommonHeader(kPacket, kPacketLength, &header); - header.count_or_format++; // Damage the packet. - EXPECT_FALSE(rr.Parse(header, kPacket + RtcpCommonHeader::kHeaderSizeBytes)); +TEST(RtcpPacketReceiverReportTest, ParseFailsOnIncorrectSize) { + rtc::Buffer damaged_packet(kPacket); + damaged_packet[0]++; // Damage the packet: increase count field. + ReceiverReport rr; + EXPECT_FALSE(test::ParseSinglePacket(damaged_packet, &rr)); } -TEST_F(RtcpPacketReceiverReportTest, Create) { +TEST(RtcpPacketReceiverReportTest, CreateWithOneReportBlock) { + ReceiverReport rr; rr.From(kSenderSsrc); ReportBlock rb; rb.To(kRemoteSsrc); @@ -92,23 +76,25 @@ TEST_F(RtcpPacketReceiverReportTest, Create) { rb.WithDelayLastSr(kDelayLastSr); rr.WithReportBlock(rb); - BuildPacket(); + rtc::Buffer raw = rr.Build(); - ASSERT_EQ(kPacketLength, packet.size()); - EXPECT_EQ(0, memcmp(kPacket, packet.data(), kPacketLength)); + EXPECT_THAT(make_tuple(raw.data(), raw.size()), ElementsAreArray(kPacket)); } -TEST_F(RtcpPacketReceiverReportTest, WithoutReportBlocks) { +TEST(RtcpPacketReceiverReportTest, CreateAndParseWithoutReportBlocks) { + ReceiverReport rr; rr.From(kSenderSsrc); - BuildPacket(); - ParsePacket(); + rtc::Buffer raw = rr.Build(); + ReceiverReport parsed; + EXPECT_TRUE(test::ParseSinglePacket(raw, &parsed)); - EXPECT_EQ(kSenderSsrc, parsed().sender_ssrc()); - EXPECT_EQ(0u, parsed().report_blocks().size()); + EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); + EXPECT_THAT(parsed.report_blocks(), IsEmpty()); } -TEST_F(RtcpPacketReceiverReportTest, WithTwoReportBlocks) { +TEST(RtcpPacketReceiverReportTest, CreateAndParseWithTwoReportBlocks) { + ReceiverReport rr; ReportBlock rb1; rb1.To(kRemoteSsrc); ReportBlock rb2; @@ -118,16 +104,18 @@ TEST_F(RtcpPacketReceiverReportTest, WithTwoReportBlocks) { EXPECT_TRUE(rr.WithReportBlock(rb1)); EXPECT_TRUE(rr.WithReportBlock(rb2)); - BuildPacket(); - ParsePacket(); + rtc::Buffer raw = rr.Build(); + ReceiverReport parsed; + EXPECT_TRUE(test::ParseSinglePacket(raw, &parsed)); - EXPECT_EQ(kSenderSsrc, parsed().sender_ssrc()); - EXPECT_EQ(2u, parsed().report_blocks().size()); - EXPECT_EQ(kRemoteSsrc, parsed().report_blocks()[0].source_ssrc()); - EXPECT_EQ(kRemoteSsrc + 1, parsed().report_blocks()[1].source_ssrc()); + EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); + EXPECT_EQ(2u, parsed.report_blocks().size()); + EXPECT_EQ(kRemoteSsrc, parsed.report_blocks()[0].source_ssrc()); + EXPECT_EQ(kRemoteSsrc + 1, parsed.report_blocks()[1].source_ssrc()); } -TEST_F(RtcpPacketReceiverReportTest, WithTooManyReportBlocks) { +TEST(RtcpPacketReceiverReportTest, CreateWithTooManyReportBlocks) { + ReceiverReport rr; rr.From(kSenderSsrc); const size_t kMaxReportBlocks = (1 << 5) - 1; ReportBlock rb; @@ -139,5 +127,4 @@ TEST_F(RtcpPacketReceiverReportTest, WithTooManyReportBlocks) { EXPECT_FALSE(rr.WithReportBlock(rb)); } -} // namespace } // namespace webrtc