From 2d3aae5a845c18e882792bb1fdb78d5627d89be4 Mon Sep 17 00:00:00 2001 From: danilchap Date: Mon, 30 May 2016 08:50:53 -0700 Subject: [PATCH] [rtcp] ExtendedReports::Parse updated not to use RTCPUtility BUG=webrtc:5260 Review-Url: https://codereview.webrtc.org/2006313010 Cr-Commit-Position: refs/heads/master@{#12962} --- .../source/rtcp_packet/extended_reports.cc | 20 +++++----- .../source/rtcp_packet/extended_reports.h | 9 ++--- .../rtcp_packet/extended_reports_unittest.cc | 40 ++++++------------- 3 files changed, 27 insertions(+), 42 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports.cc index 243e4b743f..9e704b12da 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports.cc @@ -13,11 +13,11 @@ #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 { +constexpr uint8_t ExtendedReports::kPacketType; // From RFC 3611: RTP Control Protocol Extended Reports (RTCP XR). // // Format for XR packets: @@ -43,23 +43,23 @@ namespace rtcp { ExtendedReports::ExtendedReports() : sender_ssrc_(0) {} ExtendedReports::~ExtendedReports() {} -bool ExtendedReports::Parse(const RtcpCommonHeader& header, - const uint8_t* payload) { - RTC_CHECK(header.packet_type == kPacketType); +bool ExtendedReports::Parse(const CommonHeader& packet) { + RTC_DCHECK_EQ(packet.type(), kPacketType); - if (header.payload_size_bytes < kXrBaseLength) { + if (packet.payload_size_bytes() < kXrBaseLength) { LOG(LS_WARNING) << "Packet is too small to be an ExtendedReports packet."; return false; } - sender_ssrc_ = ByteReader::ReadBigEndian(payload); + sender_ssrc_ = ByteReader::ReadBigEndian(packet.payload()); rrtr_blocks_.clear(); dlrr_blocks_.clear(); voip_metric_blocks_.clear(); - const uint8_t* current_block = payload + kXrBaseLength; - const uint8_t* const packet_end = payload + header.payload_size_bytes; - const size_t kBlockHeaderSizeBytes = 4; + const uint8_t* current_block = packet.payload() + kXrBaseLength; + const uint8_t* const packet_end = + packet.payload() + packet.payload_size_bytes(); + constexpr size_t kBlockHeaderSizeBytes = 4; while (current_block + kBlockHeaderSizeBytes <= packet_end) { uint8_t block_type = ByteReader::ReadBigEndian(current_block); uint16_t block_length = diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports.h b/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports.h index d7e715bc9e..b15d416583 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports.h @@ -18,22 +18,21 @@ #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/dlrr.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/rrtr.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/voip_metric.h" -#include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h" namespace webrtc { namespace rtcp { +class CommonHeader; // From RFC 3611: RTP Control Protocol Extended Reports (RTCP XR). class ExtendedReports : public RtcpPacket { public: - static const uint8_t kPacketType = 207; + static constexpr uint8_t kPacketType = 207; ExtendedReports(); ~ExtendedReports() 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; } @@ -59,7 +58,7 @@ class ExtendedReports : public RtcpPacket { static const size_t kMaxNumberOfRrtrBlocks = 50; static const size_t kMaxNumberOfDlrrBlocks = 50; static const size_t kMaxNumberOfVoipMetricBlocks = 50; - static const size_t kXrBaseLength = 4; + static constexpr size_t kXrBaseLength = 4; size_t BlockLength() const override { return kHeaderLength + kXrBaseLength + RrtrLength() + DlrrLength() + diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports_unittest.cc index 8d7bbfd4f6..133e840144 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports_unittest.cc @@ -13,6 +13,7 @@ #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "webrtc/base/random.h" +#include "webrtc/test/rtcp_packet_parser.h" using testing::ElementsAre; using testing::ElementsAreArray; @@ -23,8 +24,6 @@ using webrtc::rtcp::ExtendedReports; using webrtc::rtcp::ReceiveTimeInfo; using webrtc::rtcp::Rrtr; using webrtc::rtcp::VoipMetric; -using webrtc::RTCPUtility::RtcpCommonHeader; -using webrtc::RTCPUtility::RtcpParseCommonHeader; namespace webrtc { // Define comparision operators that shouldn't be needed in production, @@ -74,22 +73,9 @@ bool operator==(const VoipMetric& metric1, const VoipMetric& metric2) { } // namespace rtcp namespace { - const uint32_t kSenderSsrc = 0x12345678; - const uint8_t kEmptyPacket[8] = {0x80, 207, 0x00, 0x01, - 0x12, 0x34, 0x56, 0x78}; - - bool Parse(const uint8_t* buffer, - size_t length, - ExtendedReports* packet) { - RtcpCommonHeader header; - EXPECT_TRUE(RtcpParseCommonHeader(buffer, length, &header)); - EXPECT_EQ(length, header.BlockSize()); - return packet->Parse(header, buffer + RtcpCommonHeader::kHeaderSizeBytes); - } - - bool Parse(const rtc::Buffer& buffer, ExtendedReports* packet) { - return Parse(buffer.data(), buffer.size(), packet); - } +constexpr uint32_t kSenderSsrc = 0x12345678; +constexpr uint8_t kEmptyPacket[] = {0x80, 207, 0x00, 0x01, + 0x12, 0x34, 0x56, 0x78}; } // namespace class RtcpPacketExtendedReportsTest : public ::testing::Test { @@ -174,7 +160,7 @@ TEST_F(RtcpPacketExtendedReportsTest, CreateWithoutReportBlocks) { TEST_F(RtcpPacketExtendedReportsTest, ParseWithoutReportBlocks) { ExtendedReports parsed; - EXPECT_TRUE(Parse(kEmptyPacket, sizeof(kEmptyPacket), &parsed)); + EXPECT_TRUE(test::ParseSinglePacket(kEmptyPacket, &parsed)); EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); EXPECT_THAT(parsed.rrtrs(), IsEmpty()); EXPECT_THAT(parsed.dlrrs(), IsEmpty()); @@ -189,7 +175,7 @@ TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithOneRrtrBlock) { rtc::Buffer packet = xr.Build(); ExtendedReports mparsed; - EXPECT_TRUE(Parse(packet, &mparsed)); + EXPECT_TRUE(test::ParseSinglePacket(packet, &mparsed)); const ExtendedReports& parsed = mparsed; EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); @@ -207,7 +193,7 @@ TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithTwoRrtrBlocks) { rtc::Buffer packet = xr.Build(); ExtendedReports mparsed; - EXPECT_TRUE(Parse(packet, &mparsed)); + EXPECT_TRUE(test::ParseSinglePacket(packet, &mparsed)); const ExtendedReports& parsed = mparsed; EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); @@ -224,7 +210,7 @@ TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithDlrrWithOneSubBlock) { rtc::Buffer packet = xr.Build(); ExtendedReports mparsed; - EXPECT_TRUE(Parse(packet, &mparsed)); + EXPECT_TRUE(test::ParseSinglePacket(packet, &mparsed)); const ExtendedReports& parsed = mparsed; EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); @@ -242,7 +228,7 @@ TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithDlrrWithTwoSubBlocks) { rtc::Buffer packet = xr.Build(); ExtendedReports mparsed; - EXPECT_TRUE(Parse(packet, &mparsed)); + EXPECT_TRUE(test::ParseSinglePacket(packet, &mparsed)); const ExtendedReports& parsed = mparsed; EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); @@ -262,7 +248,7 @@ TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithTwoDlrrBlocks) { rtc::Buffer packet = xr.Build(); ExtendedReports mparsed; - EXPECT_TRUE(Parse(packet, &mparsed)); + EXPECT_TRUE(test::ParseSinglePacket(packet, &mparsed)); const ExtendedReports& parsed = mparsed; EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); @@ -279,7 +265,7 @@ TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithVoipMetric) { rtc::Buffer packet = xr.Build(); ExtendedReports mparsed; - EXPECT_TRUE(Parse(packet, &mparsed)); + EXPECT_TRUE(test::ParseSinglePacket(packet, &mparsed)); const ExtendedReports& parsed = mparsed; EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); @@ -300,7 +286,7 @@ TEST_F(RtcpPacketExtendedReportsTest, CreateAndParseWithMultipleReportBlocks) { rtc::Buffer packet = xr.Build(); ExtendedReports mparsed; - EXPECT_TRUE(Parse(packet, &mparsed)); + EXPECT_TRUE(test::ParseSinglePacket(packet, &mparsed)); const ExtendedReports& parsed = mparsed; EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); @@ -322,7 +308,7 @@ TEST_F(RtcpPacketExtendedReportsTest, DlrrWithoutItemNotIncludedInPacket) { rtc::Buffer packet = xr.Build(); ExtendedReports mparsed; - EXPECT_TRUE(Parse(packet, &mparsed)); + EXPECT_TRUE(test::ParseSinglePacket(packet, &mparsed)); const ExtendedReports& parsed = mparsed; EXPECT_THAT(parsed.rrtrs(), ElementsAre(rrtr));