From 96c1587551551b062ec9bd4617a3035c81a049d9 Mon Sep 17 00:00:00 2001 From: danilchap Date: Mon, 21 Nov 2016 01:35:29 -0800 Subject: [PATCH] RtpPacket::payload() return rtc::ArrayView instead of raw pointer BUG=webrtc:5261 Review-Url: https://codereview.webrtc.org/2506373004 Cr-Commit-Position: refs/heads/master@{#15162} --- .../rtp_rtcp/source/flexfec_receiver.cc | 6 ++-- webrtc/modules/rtp_rtcp/source/rtp_packet.cc | 4 +-- webrtc/modules/rtp_rtcp/source/rtp_packet.h | 3 +- .../rtp_rtcp/source/rtp_packet_unittest.cc | 6 ++-- webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 3 +- .../rtp_rtcp/source/rtp_sender_unittest.cc | 34 ++++++------------- .../rtp_rtcp/source/rtp_sender_video.cc | 6 ++-- 7 files changed, 25 insertions(+), 37 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/flexfec_receiver.cc b/webrtc/modules/rtp_rtcp/source/flexfec_receiver.cc index c84da55208..2c97475491 100644 --- a/webrtc/modules/rtp_rtcp/source/flexfec_receiver.cc +++ b/webrtc/modules/rtp_rtcp/source/flexfec_receiver.cc @@ -97,9 +97,9 @@ bool FlexfecReceiver::AddReceivedPacket(const uint8_t* packet, // TODO(brandtr): Remove this memcpy when the FEC packet classes // are using COW buffers internally. received_packet->pkt = rtc::scoped_refptr(new Packet()); - memcpy(received_packet->pkt->data, parsed_packet.payload(), - parsed_packet.payload_size()); - received_packet->pkt->length = parsed_packet.payload_size(); + auto payload = parsed_packet.payload(); + memcpy(received_packet->pkt->data, payload.data(), payload.size()); + received_packet->pkt->length = payload.size(); } else { // This is a media packet, or a FlexFEC packet belonging to some // other FlexFEC stream. diff --git a/webrtc/modules/rtp_rtcp/source/rtp_packet.cc b/webrtc/modules/rtp_rtcp/source/rtp_packet.cc index 2362ff2393..245ea8b352 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_packet.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_packet.cc @@ -172,8 +172,8 @@ size_t Packet::padding_size() const { return padding_size_; } -const uint8_t* Packet::payload() const { - return data() + payload_offset_; +rtc::ArrayView Packet::payload() const { + return rtc::MakeArrayView(data() + payload_offset_, payload_size_); } rtc::CopyOnWriteBuffer Packet::Buffer() const { diff --git a/webrtc/modules/rtp_rtcp/source/rtp_packet.h b/webrtc/modules/rtp_rtcp/source/rtp_packet.h index 6981eabc25..3f4d5769ba 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_packet.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_packet.h @@ -12,6 +12,7 @@ #include +#include "webrtc/base/array_view.h" #include "webrtc/base/basictypes.h" #include "webrtc/base/copyonwritebuffer.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" @@ -56,7 +57,7 @@ class Packet { // Payload. size_t payload_size() const; size_t padding_size() const; - const uint8_t* payload() const; + rtc::ArrayView payload() const; // Buffer. rtc::CopyOnWriteBuffer Buffer() const; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_packet_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_packet_unittest.cc index a1f57ac156..a84e5f87f1 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_packet_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_packet_unittest.cc @@ -219,8 +219,7 @@ TEST(RtpPacketTest, ParseWithInvalidSizedExtension) { EXPECT_FALSE(packet.GetExtension(&time_offset)); // But shouldn't prevent reading payload. - EXPECT_THAT(make_tuple(packet.payload(), packet.payload_size()), - ElementsAreArray(kPayload)); + EXPECT_THAT(packet.payload(), ElementsAreArray(kPayload)); } TEST(RtpPacketTest, ParseWithOverSizedExtension) { @@ -274,8 +273,7 @@ TEST(RtpPacketTest, ParseWithAllFeatures) { EXPECT_EQ(kTimestamp, packet.Timestamp()); EXPECT_EQ(kSsrc, packet.Ssrc()); EXPECT_THAT(packet.Csrcs(), ElementsAreArray(kCsrcs)); - EXPECT_THAT(make_tuple(packet.payload(), packet.payload_size()), - ElementsAreArray(kPayload)); + EXPECT_THAT(packet.payload(), ElementsAreArray(kPayload)); EXPECT_EQ(kPacketPaddingSize, packet.padding_size()); int32_t time_offset; EXPECT_TRUE(packet.GetExtension(&time_offset)); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index 71ee0b6002..04c742651f 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -1216,7 +1216,8 @@ std::unique_ptr RTPSender::BuildRtxPacket( ByteWriter::WriteBigEndian(rtx_payload, packet.SequenceNumber()); // Add original payload data. - memcpy(rtx_payload + kRtxHeaderSize, packet.payload(), packet.payload_size()); + auto payload = packet.payload(); + memcpy(rtx_payload + kRtxHeaderSize, payload.data(), payload.size()); return rtx_packet; } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index d8f0baf9a6..929f5b2097 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -878,16 +878,11 @@ TEST_F(RtpSenderTestWithoutPacer, SendGenericVideo) { 4321, payload, sizeof(payload), nullptr, nullptr, nullptr)); - const uint8_t* payload_data = transport_.last_sent_packet().payload(); - uint8_t generic_header = *payload_data++; - - ASSERT_EQ(sizeof(payload) + sizeof(generic_header), - transport_.last_sent_packet().payload_size()); - + auto sent_payload = transport_.last_sent_packet().payload(); + uint8_t generic_header = sent_payload[0]; EXPECT_TRUE(generic_header & RtpFormatVideoGeneric::kKeyFrameBit); EXPECT_TRUE(generic_header & RtpFormatVideoGeneric::kFirstPacketBit); - - EXPECT_EQ(0, memcmp(payload, payload_data, sizeof(payload))); + EXPECT_THAT(sent_payload.subview(1), ElementsAreArray(payload)); // Send delta frame payload[0] = 13; @@ -898,16 +893,11 @@ TEST_F(RtpSenderTestWithoutPacer, SendGenericVideo) { kVideoFrameDelta, payload_type, 1234, 4321, payload, sizeof(payload), nullptr, nullptr, nullptr)); - payload_data = transport_.last_sent_packet().payload(); - generic_header = *payload_data++; - + sent_payload = transport_.last_sent_packet().payload(); + generic_header = sent_payload[0]; EXPECT_FALSE(generic_header & RtpFormatVideoGeneric::kKeyFrameBit); EXPECT_TRUE(generic_header & RtpFormatVideoGeneric::kFirstPacketBit); - - ASSERT_EQ(sizeof(payload) + sizeof(generic_header), - transport_.last_sent_packet().payload_size()); - - EXPECT_EQ(0, memcmp(payload, payload_data, sizeof(payload))); + EXPECT_THAT(sent_payload.subview(1), ElementsAreArray(payload)); } TEST_F(RtpSenderTest, SendFlexfecPackets) { @@ -1258,10 +1248,8 @@ TEST_F(RtpSenderAudioTest, SendAudio) { kAudioFrameCN, payload_type, 1234, 4321, payload, sizeof(payload), nullptr, nullptr, nullptr)); - const uint8_t* payload_data = transport_.last_sent_packet().payload(); - - ASSERT_EQ(sizeof(payload), transport_.last_sent_packet().payload_size()); - EXPECT_EQ(0, memcmp(payload, payload_data, sizeof(payload))); + auto sent_payload = transport_.last_sent_packet().payload(); + EXPECT_THAT(sent_payload, ElementsAreArray(payload)); } TEST_F(RtpSenderAudioTest, SendAudioWithAudioLevelExtension) { @@ -1279,10 +1267,8 @@ TEST_F(RtpSenderAudioTest, SendAudioWithAudioLevelExtension) { kAudioFrameCN, payload_type, 1234, 4321, payload, sizeof(payload), nullptr, nullptr, nullptr)); - const uint8_t* payload_data = transport_.last_sent_packet().payload(); - - ASSERT_EQ(sizeof(payload), transport_.last_sent_packet().payload_size()); - EXPECT_EQ(0, memcmp(payload, payload_data, sizeof(payload))); + auto sent_payload = transport_.last_sent_packet().payload(); + EXPECT_THAT(sent_payload, ElementsAreArray(payload)); // Verify AudioLevel extension. bool voice_activity; uint8_t audio_level; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc index 2c0267d367..22bf1079d0 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -39,8 +39,10 @@ void BuildRedPayload(const RtpPacketToSend& media_packet, kRedForFecHeaderLength + media_packet.payload_size()); RTC_DCHECK(red_payload); red_payload[0] = media_packet.PayloadType(); - memcpy(&red_payload[kRedForFecHeaderLength], media_packet.payload(), - media_packet.payload_size()); + + auto media_payload = media_packet.payload(); + memcpy(&red_payload[kRedForFecHeaderLength], media_payload.data(), + media_payload.size()); } } // namespace