From ce25181714243770e4eff3a636ae8d49eca6d92b Mon Sep 17 00:00:00 2001 From: danilchap Date: Mon, 11 Sep 2017 12:24:41 -0700 Subject: [PATCH] Remove RtpPacketToSend::GetHeader as almost unused. Merge rtp::Packet::GetHeader into RtpPacketReceived::GetHeader removing error-prone code where latter shadow former version BUG=None Review-Url: https://codereview.webrtc.org/3012983002 Cr-Commit-Position: refs/heads/master@{#19784} --- webrtc/modules/rtp_rtcp/BUILD.gn | 1 + webrtc/modules/rtp_rtcp/source/rtp_packet.cc | 38 ------------- webrtc/modules/rtp_rtcp/source/rtp_packet.h | 7 +-- .../rtp_rtcp/source/rtp_packet_received.cc | 56 +++++++++++++++++++ .../rtp_rtcp/source/rtp_packet_received.h | 7 +-- .../rtp_rtcp/source/rtp_sender_unittest.cc | 18 +++--- 6 files changed, 68 insertions(+), 59 deletions(-) create mode 100644 webrtc/modules/rtp_rtcp/source/rtp_packet_received.cc diff --git a/webrtc/modules/rtp_rtcp/BUILD.gn b/webrtc/modules/rtp_rtcp/BUILD.gn index 2095aadb28..a5b0d300b6 100644 --- a/webrtc/modules/rtp_rtcp/BUILD.gn +++ b/webrtc/modules/rtp_rtcp/BUILD.gn @@ -118,6 +118,7 @@ rtc_static_library("rtp_rtcp") { "source/rtp_packet.h", "source/rtp_packet_history.cc", "source/rtp_packet_history.h", + "source/rtp_packet_received.cc", "source/rtp_packet_received.h", "source/rtp_packet_to_send.h", "source/rtp_payload_registry.cc", diff --git a/webrtc/modules/rtp_rtcp/source/rtp_packet.cc b/webrtc/modules/rtp_rtcp/source/rtp_packet.cc index 2c9d0aea5a..7df60053fb 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_packet.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_packet.cc @@ -16,7 +16,6 @@ #include "webrtc/common_types.h" #include "webrtc/modules/rtp_rtcp/include/rtp_header_extension_map.h" #include "webrtc/modules/rtp_rtcp/source/byte_io.h" -#include "webrtc/modules/rtp_rtcp/source/rtp_header_extensions.h" #include "webrtc/rtc_base/checks.h" #include "webrtc/rtc_base/logging.h" #include "webrtc/rtc_base/random.h" @@ -145,43 +144,6 @@ std::vector Packet::Csrcs() const { return csrcs; } -void Packet::GetHeader(RTPHeader* header) const { - header->markerBit = Marker(); - header->payloadType = PayloadType(); - header->sequenceNumber = SequenceNumber(); - header->timestamp = Timestamp(); - header->ssrc = Ssrc(); - std::vector csrcs = Csrcs(); - header->numCSRCs = csrcs.size(); - for (size_t i = 0; i < csrcs.size(); ++i) { - header->arrOfCSRCs[i] = csrcs[i]; - } - header->paddingLength = padding_size(); - header->headerLength = headers_size(); - header->payload_type_frequency = 0; - header->extension.hasTransmissionTimeOffset = - GetExtension( - &header->extension.transmissionTimeOffset); - header->extension.hasAbsoluteSendTime = - GetExtension(&header->extension.absoluteSendTime); - header->extension.hasTransportSequenceNumber = - GetExtension( - &header->extension.transportSequenceNumber); - header->extension.hasAudioLevel = GetExtension( - &header->extension.voiceActivity, &header->extension.audioLevel); - header->extension.hasVideoRotation = - GetExtension(&header->extension.videoRotation); - header->extension.hasVideoContentType = - GetExtension( - &header->extension.videoContentType); - header->extension.has_video_timing = - GetExtension(&header->extension.video_timing); - GetExtension(&header->extension.stream_id); - GetExtension(&header->extension.repaired_stream_id); - GetExtension(&header->extension.mid); - GetExtension(&header->extension.playout_delay); -} - size_t Packet::headers_size() const { return payload_offset_; } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_packet.h b/webrtc/modules/rtp_rtcp/source/rtp_packet.h index 8797417834..fb19815b59 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_packet.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_packet.h @@ -18,7 +18,6 @@ #include "webrtc/rtc_base/copyonwritebuffer.h" namespace webrtc { -struct RTPHeader; class RtpHeaderExtensionMap; class Random; @@ -49,10 +48,6 @@ class Packet { uint32_t Ssrc() const; std::vector Csrcs() const; - // TODO(danilchap): Remove this function when all code update to use RtpPacket - // directly. Function is there just for easier backward compatibilty. - void GetHeader(RTPHeader* header) const; - size_t headers_size() const; // Payload. @@ -127,7 +122,7 @@ class Packet { explicit Packet(const ExtensionManager* extensions); Packet(const Packet&); Packet(const ExtensionManager* extensions, size_t capacity); - virtual ~Packet(); + ~Packet(); Packet& operator=(const Packet&) = default; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_packet_received.cc b/webrtc/modules/rtp_rtcp/source/rtp_packet_received.cc new file mode 100644 index 0000000000..82a4f68d69 --- /dev/null +++ b/webrtc/modules/rtp_rtcp/source/rtp_packet_received.cc @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "webrtc/modules/rtp_rtcp/source/rtp_packet_received.h" + +#include + +#include "webrtc/modules/rtp_rtcp/source/rtp_header_extensions.h" + +namespace webrtc { + +void RtpPacketReceived::GetHeader(RTPHeader* header) const { + header->markerBit = Marker(); + header->payloadType = PayloadType(); + header->sequenceNumber = SequenceNumber(); + header->timestamp = Timestamp(); + header->ssrc = Ssrc(); + std::vector csrcs = Csrcs(); + header->numCSRCs = csrcs.size(); + for (size_t i = 0; i < csrcs.size(); ++i) { + header->arrOfCSRCs[i] = csrcs[i]; + } + header->paddingLength = padding_size(); + header->headerLength = headers_size(); + header->payload_type_frequency = payload_type_frequency(); + header->extension.hasTransmissionTimeOffset = + GetExtension( + &header->extension.transmissionTimeOffset); + header->extension.hasAbsoluteSendTime = + GetExtension(&header->extension.absoluteSendTime); + header->extension.hasTransportSequenceNumber = + GetExtension( + &header->extension.transportSequenceNumber); + header->extension.hasAudioLevel = GetExtension( + &header->extension.voiceActivity, &header->extension.audioLevel); + header->extension.hasVideoRotation = + GetExtension(&header->extension.videoRotation); + header->extension.hasVideoContentType = + GetExtension( + &header->extension.videoContentType); + header->extension.has_video_timing = + GetExtension(&header->extension.video_timing); + GetExtension(&header->extension.stream_id); + GetExtension(&header->extension.repaired_stream_id); + GetExtension(&header->extension.mid); + GetExtension(&header->extension.playout_delay); +} + +} // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtp_packet_received.h b/webrtc/modules/rtp_rtcp/source/rtp_packet_received.h index 68c33e9387..14d30c9320 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_packet_received.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_packet_received.h @@ -22,10 +22,9 @@ class RtpPacketReceived : public rtp::Packet { explicit RtpPacketReceived(const ExtensionManager* extensions) : Packet(extensions) {} - void GetHeader(RTPHeader* header) const { - Packet::GetHeader(header); - header->payload_type_frequency = payload_type_frequency(); - } + // TODO(danilchap): Remove this function when all code update to use RtpPacket + // directly. Function is there just for easier backward compatibilty. + void GetHeader(RTPHeader* header) const; // Time in local time base as close as it can to packet arrived on the // network. diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 5a4775959e..860f193d0c 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -500,9 +500,6 @@ TEST_P(RtpSenderTestWithoutPacer, WritesTimestampToTimingExtension) { packet->SetExtension(kVideoTiming); EXPECT_TRUE(rtp_sender_->AssignSequenceNumber(packet.get())); size_t packet_size = packet->size(); - webrtc::RTPHeader rtp_header; - - packet->GetHeader(&rtp_header); const int kStoredTimeInMs = 100; fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); @@ -513,10 +510,10 @@ TEST_P(RtpSenderTestWithoutPacer, WritesTimestampToTimingExtension) { EXPECT_EQ(1, transport_.packets_sent()); EXPECT_EQ(packet_size, transport_.last_sent_packet().size()); - transport_.last_sent_packet().GetHeader(&rtp_header); - EXPECT_TRUE(rtp_header.extension.has_video_timing); - EXPECT_EQ(kStoredTimeInMs, - rtp_header.extension.video_timing.pacer_exit_delta_ms); + VideoSendTiming video_timing; + EXPECT_TRUE(transport_.last_sent_packet().GetExtension( + &video_timing)); + EXPECT_EQ(kStoredTimeInMs, video_timing.pacer_exit_delta_ms); fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, capture_time_ms, false, @@ -525,10 +522,9 @@ TEST_P(RtpSenderTestWithoutPacer, WritesTimestampToTimingExtension) { EXPECT_EQ(2, transport_.packets_sent()); EXPECT_EQ(packet_size, transport_.last_sent_packet().size()); - transport_.last_sent_packet().GetHeader(&rtp_header); - EXPECT_TRUE(rtp_header.extension.has_video_timing); - EXPECT_EQ(kStoredTimeInMs * 2, - rtp_header.extension.video_timing.pacer_exit_delta_ms); + EXPECT_TRUE(transport_.last_sent_packet().GetExtension( + &video_timing)); + EXPECT_EQ(kStoredTimeInMs * 2, video_timing.pacer_exit_delta_ms); } TEST_P(RtpSenderTest, TrafficSmoothingWithExtensions) {