From 938c5ddf0b6b4877a78f90e4162bf11686dbc251 Mon Sep 17 00:00:00 2001 From: danilchap Date: Tue, 31 May 2016 04:30:55 -0700 Subject: [PATCH] [rtcp] RapidResyncRequest::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/2020203002 Cr-Commit-Position: refs/heads/master@{#12972} --- .../rtcp_packet/rapid_resync_request.cc | 17 +++++----- .../source/rtcp_packet/rapid_resync_request.h | 8 ++--- .../rapid_resync_request_unittest.cc | 31 +++++++++---------- 3 files changed, 26 insertions(+), 30 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/rapid_resync_request.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/rapid_resync_request.cc index 394b3cf543..12a4444cc9 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/rapid_resync_request.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/rapid_resync_request.cc @@ -12,11 +12,11 @@ #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" - -using webrtc::RTCPUtility::RtcpCommonHeader; +#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/common_header.h" namespace webrtc { namespace rtcp { +constexpr uint8_t RapidResyncRequest::kFeedbackMessageType; // RFC 4585: Feedback format. // Rapid Resynchronisation Request (draft-perkins-avt-rapid-rtp-sync-03). // @@ -29,19 +29,18 @@ namespace rtcp { // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ // | SSRC of media source | // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ -bool RapidResyncRequest::Parse(const RtcpCommonHeader& header, - const uint8_t* payload) { - RTC_CHECK(header.packet_type == kPacketType); - RTC_CHECK(header.count_or_format == kFeedbackMessageType); +bool RapidResyncRequest::Parse(const CommonHeader& packet) { + RTC_DCHECK_EQ(packet.type(), kPacketType); + RTC_DCHECK_EQ(packet.fmt(), kFeedbackMessageType); - if (header.payload_size_bytes != kCommonFeedbackLength) { + if (packet.payload_size_bytes() != kCommonFeedbackLength) { LOG(LS_WARNING) << "Packet payload size should be " << kCommonFeedbackLength - << " instead of " << header.payload_size_bytes + << " instead of " << packet.payload_size_bytes() << " to be a valid Rapid Resynchronisation Request"; return false; } - ParseCommonFeedback(payload); + ParseCommonFeedback(packet.payload()); return true; } diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/rapid_resync_request.h b/webrtc/modules/rtp_rtcp/source/rtcp_packet/rapid_resync_request.h index 8568e7327c..7e2be322ba 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/rapid_resync_request.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/rapid_resync_request.h @@ -14,21 +14,21 @@ #include "webrtc/base/basictypes.h" #include "webrtc/base/constructormagic.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/rtpfb.h" -#include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h" namespace webrtc { namespace rtcp { +class CommonHeader; + // draft-perkins-avt-rapid-rtp-sync-03 class RapidResyncRequest : public Rtpfb { public: - static const uint8_t kFeedbackMessageType = 5; + static constexpr uint8_t kFeedbackMessageType = 5; RapidResyncRequest() {} ~RapidResyncRequest() 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& header); protected: bool Create(uint8_t* packet, diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/rapid_resync_request_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/rapid_resync_request_unittest.cc index 37b1fd9e69..60d46d13c5 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/rapid_resync_request_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/rapid_resync_request_unittest.cc @@ -12,12 +12,11 @@ #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::make_tuple; using webrtc::rtcp::RapidResyncRequest; -using webrtc::RTCPUtility::RtcpCommonHeader; -using webrtc::RTCPUtility::RtcpParseCommonHeader; namespace webrtc { namespace { @@ -27,15 +26,11 @@ const uint32_t kRemoteSsrc = 0x23456789; const uint8_t kPacket[] = {0x85, 205, 0x00, 0x02, 0x12, 0x34, 0x56, 0x78, 0x23, 0x45, 0x67, 0x89}; -const size_t kPacketLength = sizeof(kPacket); } // namespace TEST(RtcpPacketRapidResyncRequestTest, Parse) { - RtcpCommonHeader header; - ASSERT_TRUE(RtcpParseCommonHeader(kPacket, kPacketLength, &header)); RapidResyncRequest mutable_parsed; - EXPECT_TRUE(mutable_parsed.Parse( - header, kPacket + RtcpCommonHeader::kHeaderSizeBytes)); + EXPECT_TRUE(test::ParseSinglePacket(kPacket, &mutable_parsed)); const RapidResyncRequest& parsed = mutable_parsed; EXPECT_EQ(kSenderSsrc, parsed.sender_ssrc()); @@ -53,17 +48,19 @@ TEST(RtcpPacketRapidResyncRequestTest, Create) { ElementsAreArray(kPacket)); } -TEST(RtcpPacketRapidResyncRequestTest, ParseFailsOnWrongSizePacket) { +TEST(RtcpPacketRapidResyncRequestTest, ParseFailsOnTooSmallPacket) { + const uint8_t kTooSmallPacket[] = {0x85, 205, 0x00, 0x01, + 0x12, 0x34, 0x56, 0x78}; RapidResyncRequest parsed; - RtcpCommonHeader header; - ASSERT_TRUE(RtcpParseCommonHeader(kPacket, kPacketLength, &header)); - const size_t kCorrectPayloadSize = header.payload_size_bytes; - const uint8_t* payload = kPacket + RtcpCommonHeader::kHeaderSizeBytes; + EXPECT_FALSE(test::ParseSinglePacket(kTooSmallPacket, &parsed)); +} - header.payload_size_bytes = kCorrectPayloadSize - 1; - EXPECT_FALSE(parsed.Parse(header, payload)); - - header.payload_size_bytes = kCorrectPayloadSize + 1; - EXPECT_FALSE(parsed.Parse(header, payload)); +TEST(RtcpPacketRapidResyncRequestTest, ParseFailsOnTooLargePacket) { + const uint8_t kTooLargePacket[] = {0x85, 205, 0x00, 0x03, + 0x12, 0x34, 0x56, 0x78, + 0x32, 0x21, 0x65, 0x87, + 0x23, 0x45, 0x67, 0x89}; + RapidResyncRequest parsed; + EXPECT_FALSE(test::ParseSinglePacket(kTooLargePacket, &parsed)); } } // namespace webrtc