From 2f7dea164dc49ae8a0322e3c9edb1dd23266c664 Mon Sep 17 00:00:00 2001 From: danilchap Date: Wed, 13 Jan 2016 02:03:04 -0800 Subject: [rtp_rtcp] rtcp::Empty moved into own file and renamed to CompoundPacket on the way MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Class renamed to indicated use of the rtcp::Empty class: it can only be used as container for other rtcp packets. All tests that use Append function moved from rtcp_packet_unittest, even if they did not use Empty class. This is because there is plan to make RtcpPacket class lighter by moving Append functionality to CompoundPacket class. BUG=webrtc:5260 R=åsapersson Review URL: https://codereview.webrtc.org/1582503002 Cr-Commit-Position: refs/heads/master@{#11234} --- webrtc/modules/modules.gyp | 1 + webrtc/modules/rtp_rtcp/BUILD.gn | 2 + webrtc/modules/rtp_rtcp/rtp_rtcp.gypi | 2 + webrtc/modules/rtp_rtcp/source/rtcp_packet.cc | 11 -- webrtc/modules/rtp_rtcp/source/rtcp_packet.h | 18 --- .../rtp_rtcp/source/rtcp_packet/compound_packet.cc | 28 ++++ .../rtp_rtcp/source/rtcp_packet/compound_packet.h | 41 ++++++ .../source/rtcp_packet/compound_packet_unittest.cc | 157 +++++++++++++++++++++ .../rtp_rtcp/source/rtcp_packet_unittest.cc | 126 ----------------- webrtc/modules/rtp_rtcp/source/rtcp_sender.cc | 5 +- 10 files changed, 234 insertions(+), 157 deletions(-) create mode 100644 webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet.cc create mode 100644 webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet.h create mode 100644 webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet_unittest.cc (limited to 'webrtc/modules') diff --git a/webrtc/modules/modules.gyp b/webrtc/modules/modules.gyp index 4fd3408aca..a7dc0ffe53 100644 --- a/webrtc/modules/modules.gyp +++ b/webrtc/modules/modules.gyp @@ -307,6 +307,7 @@ 'rtp_rtcp/source/rtcp_packet_unittest.cc', 'rtp_rtcp/source/rtcp_packet/app_unittest.cc', 'rtp_rtcp/source/rtcp_packet/bye_unittest.cc', + 'rtp_rtcp/source/rtcp_packet/compound_packet_unittest.cc', 'rtp_rtcp/source/rtcp_packet/dlrr_unittest.cc', 'rtp_rtcp/source/rtcp_packet/extended_jitter_report_unittest.cc', 'rtp_rtcp/source/rtcp_packet/nack_unittest.cc', diff --git a/webrtc/modules/rtp_rtcp/BUILD.gn b/webrtc/modules/rtp_rtcp/BUILD.gn index 77f6f65bbe..a3d3403172 100644 --- a/webrtc/modules/rtp_rtcp/BUILD.gn +++ b/webrtc/modules/rtp_rtcp/BUILD.gn @@ -50,6 +50,8 @@ source_set("rtp_rtcp") { "source/rtcp_packet/app.h", "source/rtcp_packet/bye.cc", "source/rtcp_packet/bye.h", + "source/rtcp_packet/compound_packet.cc", + "source/rtcp_packet/compound_packet.h", "source/rtcp_packet/dlrr.cc", "source/rtcp_packet/dlrr.h", "source/rtcp_packet/extended_jitter_report.cc", diff --git a/webrtc/modules/rtp_rtcp/rtp_rtcp.gypi b/webrtc/modules/rtp_rtcp/rtp_rtcp.gypi index 81c5edb3db..d340f746be 100644 --- a/webrtc/modules/rtp_rtcp/rtp_rtcp.gypi +++ b/webrtc/modules/rtp_rtcp/rtp_rtcp.gypi @@ -45,6 +45,8 @@ 'source/rtcp_packet/app.h', 'source/rtcp_packet/bye.cc', 'source/rtcp_packet/bye.h', + 'source/rtcp_packet/compound_packet.cc', + 'source/rtcp_packet/compound_packet.h', 'source/rtcp_packet/dlrr.cc', 'source/rtcp_packet/dlrr.h', 'source/rtcp_packet/extended_jitter_report.cc', diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet.cc index 62371a861c..eef2978371 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet.cc @@ -390,17 +390,6 @@ void RtcpPacket::CreateHeader( AssignUWord16(buffer, pos, length); } -bool Empty::Create(uint8_t* packet, - size_t* index, - size_t max_length, - RtcpPacket::PacketReadyCallback* callback) const { - return true; -} - -size_t Empty::BlockLength() const { - return 0; -} - bool SenderReport::Create(uint8_t* packet, size_t* index, size_t max_length, diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet.h b/webrtc/modules/rtp_rtcp/source/rtcp_packet.h index c2671f0f02..01c97c38ba 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet.h @@ -127,24 +127,6 @@ class RtcpPacket { // TODO(sprang): Move RtcpPacket subclasses out to separate files. -class Empty : public RtcpPacket { - public: - Empty() : RtcpPacket() {} - - virtual ~Empty() {} - - protected: - bool Create(uint8_t* packet, - size_t* index, - size_t max_length, - RtcpPacket::PacketReadyCallback* callback) const override; - - size_t BlockLength() const override; - - private: - RTC_DISALLOW_COPY_AND_ASSIGN(Empty); -}; - // RTCP sender report (RFC 3550). // // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet.cc new file mode 100644 index 0000000000..8f5afd5dd1 --- /dev/null +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet.cc @@ -0,0 +1,28 @@ +/* + * Copyright (c) 2016 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/rtcp_packet/compound_packet.h" + +namespace webrtc { +namespace rtcp { + +bool CompoundPacket::Create(uint8_t* packet, + size_t* index, + size_t max_length, + RtcpPacket::PacketReadyCallback* callback) const { + return true; +} + +size_t CompoundPacket::BlockLength() const { + return 0; +} + +} // namespace rtcp +} // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet.h b/webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet.h new file mode 100644 index 0000000000..f2f49a8ffb --- /dev/null +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet.h @@ -0,0 +1,41 @@ +/* + * Copyright (c) 2016 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. + * + */ + +#ifndef WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_COMPOUND_PACKET_H_ +#define WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_COMPOUND_PACKET_H_ + +#include "webrtc/base/basictypes.h" +#include "webrtc/modules/rtp_rtcp/source/rtcp_packet.h" + +namespace webrtc { +namespace rtcp { + +class CompoundPacket : public RtcpPacket { + public: + CompoundPacket() : RtcpPacket() {} + + virtual ~CompoundPacket() {} + + protected: + bool Create(uint8_t* packet, + size_t* index, + size_t max_length, + RtcpPacket::PacketReadyCallback* callback) const override; + + size_t BlockLength() const override; + + private: + RTC_DISALLOW_COPY_AND_ASSIGN(CompoundPacket); +}; + +} // namespace rtcp +} // namespace webrtc +#endif // WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_COMPOUND_PACKET_H_ diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet_unittest.cc new file mode 100644 index 0000000000..83dc5f6ed3 --- /dev/null +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet_unittest.cc @@ -0,0 +1,157 @@ +/* + * Copyright (c) 2016 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/rtcp_packet/compound_packet.h" + +#include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/modules/rtp_rtcp/source/rtcp_packet.h" +#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/bye.h" +#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report.h" +#include "webrtc/test/rtcp_packet_parser.h" + +using webrtc::rtcp::Bye; +using webrtc::rtcp::CompoundPacket; +using webrtc::rtcp::Fir; +using webrtc::rtcp::RawPacket; +using webrtc::rtcp::ReceiverReport; +using webrtc::rtcp::ReportBlock; +using webrtc::rtcp::SenderReport; +using webrtc::test::RtcpPacketParser; + +namespace webrtc { + +const uint32_t kSenderSsrc = 0x12345678; + +TEST(RtcpCompoundPacketTest, AppendPacket) { + Fir fir; + ReportBlock rb; + ReceiverReport rr; + rr.From(kSenderSsrc); + EXPECT_TRUE(rr.WithReportBlock(rb)); + rr.Append(&fir); + + rtc::scoped_ptr packet(rr.Build()); + RtcpPacketParser parser; + parser.Parse(packet->Buffer(), packet->Length()); + EXPECT_EQ(1, parser.receiver_report()->num_packets()); + EXPECT_EQ(kSenderSsrc, parser.receiver_report()->Ssrc()); + EXPECT_EQ(1, parser.report_block()->num_packets()); + EXPECT_EQ(1, parser.fir()->num_packets()); +} + +TEST(RtcpCompoundPacketTest, AppendPacketOnEmpty) { + CompoundPacket empty; + ReceiverReport rr; + rr.From(kSenderSsrc); + empty.Append(&rr); + + rtc::scoped_ptr packet(empty.Build()); + RtcpPacketParser parser; + parser.Parse(packet->Buffer(), packet->Length()); + EXPECT_EQ(1, parser.receiver_report()->num_packets()); + EXPECT_EQ(0, parser.report_block()->num_packets()); +} + +TEST(RtcpCompoundPacketTest, AppendPacketWithOwnAppendedPacket) { + Fir fir; + Bye bye; + ReportBlock rb; + + ReceiverReport rr; + EXPECT_TRUE(rr.WithReportBlock(rb)); + rr.Append(&fir); + + SenderReport sr; + sr.Append(&bye); + sr.Append(&rr); + + rtc::scoped_ptr packet(sr.Build()); + RtcpPacketParser parser; + parser.Parse(packet->Buffer(), packet->Length()); + EXPECT_EQ(1, parser.sender_report()->num_packets()); + EXPECT_EQ(1, parser.receiver_report()->num_packets()); + EXPECT_EQ(1, parser.report_block()->num_packets()); + EXPECT_EQ(1, parser.bye()->num_packets()); + EXPECT_EQ(1, parser.fir()->num_packets()); +} + +TEST(RtcpCompoundPacketTest, BuildWithInputBuffer) { + Fir fir; + ReportBlock rb; + ReceiverReport rr; + rr.From(kSenderSsrc); + EXPECT_TRUE(rr.WithReportBlock(rb)); + rr.Append(&fir); + + const size_t kRrLength = 8; + const size_t kReportBlockLength = 24; + const size_t kFirLength = 20; + + class Verifier : public rtcp::RtcpPacket::PacketReadyCallback { + public: + void OnPacketReady(uint8_t* data, size_t length) override { + RtcpPacketParser parser; + parser.Parse(data, length); + EXPECT_EQ(1, parser.receiver_report()->num_packets()); + EXPECT_EQ(1, parser.report_block()->num_packets()); + EXPECT_EQ(1, parser.fir()->num_packets()); + ++packets_created_; + } + + int packets_created_ = 0; + } verifier; + const size_t kBufferSize = kRrLength + kReportBlockLength + kFirLength; + uint8_t buffer[kBufferSize]; + EXPECT_TRUE(rr.BuildExternalBuffer(buffer, kBufferSize, &verifier)); + EXPECT_EQ(1, verifier.packets_created_); +} + +TEST(RtcpCompoundPacketTest, BuildWithTooSmallBuffer_FragmentedSend) { + Fir fir; + ReportBlock rb; + ReceiverReport rr; + rr.From(kSenderSsrc); + EXPECT_TRUE(rr.WithReportBlock(rb)); + rr.Append(&fir); + + const size_t kRrLength = 8; + const size_t kReportBlockLength = 24; + + class Verifier : public rtcp::RtcpPacket::PacketReadyCallback { + public: + void OnPacketReady(uint8_t* data, size_t length) override { + RtcpPacketParser parser; + parser.Parse(data, length); + switch (packets_created_++) { + case 0: + EXPECT_EQ(1, parser.receiver_report()->num_packets()); + EXPECT_EQ(1, parser.report_block()->num_packets()); + EXPECT_EQ(0, parser.fir()->num_packets()); + break; + case 1: + EXPECT_EQ(0, parser.receiver_report()->num_packets()); + EXPECT_EQ(0, parser.report_block()->num_packets()); + EXPECT_EQ(1, parser.fir()->num_packets()); + break; + default: + ADD_FAILURE() << "OnPacketReady not expected to be called " + << packets_created_ << " times."; + } + } + + int packets_created_ = 0; + } verifier; + const size_t kBufferSize = kRrLength + kReportBlockLength; + uint8_t buffer[kBufferSize]; + EXPECT_TRUE(rr.BuildExternalBuffer(buffer, kBufferSize, &verifier)); + EXPECT_EQ(2, verifier.packets_created_); +} + +} // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet_unittest.cc index c57fe19e46..22f61f5cab 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet_unittest.cc @@ -24,7 +24,6 @@ using ::testing::ElementsAre; using webrtc::rtcp::App; using webrtc::rtcp::Bye; using webrtc::rtcp::Dlrr; -using webrtc::rtcp::Empty; using webrtc::rtcp::Fir; using webrtc::rtcp::RawPacket; using webrtc::rtcp::ReceiverReport; @@ -305,90 +304,6 @@ TEST(RtcpPacketTest, Fir) { EXPECT_EQ(123U, parser.fir_item()->SeqNum()); } -TEST(RtcpPacketTest, AppendPacket) { - Fir fir; - ReportBlock rb; - ReceiverReport rr; - rr.From(kSenderSsrc); - EXPECT_TRUE(rr.WithReportBlock(rb)); - rr.Append(&fir); - - rtc::scoped_ptr packet(rr.Build()); - RtcpPacketParser parser; - parser.Parse(packet->Buffer(), packet->Length()); - EXPECT_EQ(1, parser.receiver_report()->num_packets()); - EXPECT_EQ(kSenderSsrc, parser.receiver_report()->Ssrc()); - EXPECT_EQ(1, parser.report_block()->num_packets()); - EXPECT_EQ(1, parser.fir()->num_packets()); -} - -TEST(RtcpPacketTest, AppendPacketOnEmpty) { - Empty empty; - ReceiverReport rr; - rr.From(kSenderSsrc); - empty.Append(&rr); - - rtc::scoped_ptr packet(empty.Build()); - RtcpPacketParser parser; - parser.Parse(packet->Buffer(), packet->Length()); - EXPECT_EQ(1, parser.receiver_report()->num_packets()); - EXPECT_EQ(0, parser.report_block()->num_packets()); -} - -TEST(RtcpPacketTest, AppendPacketWithOwnAppendedPacket) { - Fir fir; - Bye bye; - ReportBlock rb; - - ReceiverReport rr; - EXPECT_TRUE(rr.WithReportBlock(rb)); - rr.Append(&fir); - - SenderReport sr; - sr.Append(&bye); - sr.Append(&rr); - - rtc::scoped_ptr packet(sr.Build()); - RtcpPacketParser parser; - parser.Parse(packet->Buffer(), packet->Length()); - EXPECT_EQ(1, parser.sender_report()->num_packets()); - EXPECT_EQ(1, parser.receiver_report()->num_packets()); - EXPECT_EQ(1, parser.report_block()->num_packets()); - EXPECT_EQ(1, parser.bye()->num_packets()); - EXPECT_EQ(1, parser.fir()->num_packets()); -} - -TEST(RtcpPacketTest, BuildWithInputBuffer) { - Fir fir; - ReportBlock rb; - ReceiverReport rr; - rr.From(kSenderSsrc); - EXPECT_TRUE(rr.WithReportBlock(rb)); - rr.Append(&fir); - - const size_t kRrLength = 8; - const size_t kReportBlockLength = 24; - const size_t kFirLength = 20; - - class Verifier : public rtcp::RtcpPacket::PacketReadyCallback { - public: - void OnPacketReady(uint8_t* data, size_t length) override { - RtcpPacketParser parser; - parser.Parse(data, length); - EXPECT_EQ(1, parser.receiver_report()->num_packets()); - EXPECT_EQ(1, parser.report_block()->num_packets()); - EXPECT_EQ(1, parser.fir()->num_packets()); - ++packets_created_; - } - - int packets_created_ = 0; - } verifier; - const size_t kBufferSize = kRrLength + kReportBlockLength + kFirLength; - uint8_t buffer[kBufferSize]; - EXPECT_TRUE(rr.BuildExternalBuffer(buffer, kBufferSize, &verifier)); - EXPECT_EQ(1, verifier.packets_created_); -} - TEST(RtcpPacketTest, BuildWithTooSmallBuffer) { ReportBlock rb; ReceiverReport rr; @@ -409,47 +324,6 @@ TEST(RtcpPacketTest, BuildWithTooSmallBuffer) { EXPECT_FALSE(rr.BuildExternalBuffer(buffer, kBufferSize, &verifier)); } -TEST(RtcpPacketTest, BuildWithTooSmallBuffer_FragmentedSend) { - Fir fir; - ReportBlock rb; - ReceiverReport rr; - rr.From(kSenderSsrc); - EXPECT_TRUE(rr.WithReportBlock(rb)); - rr.Append(&fir); - - const size_t kRrLength = 8; - const size_t kReportBlockLength = 24; - - class Verifier : public rtcp::RtcpPacket::PacketReadyCallback { - public: - void OnPacketReady(uint8_t* data, size_t length) override { - RtcpPacketParser parser; - parser.Parse(data, length); - switch (packets_created_++) { - case 0: - EXPECT_EQ(1, parser.receiver_report()->num_packets()); - EXPECT_EQ(1, parser.report_block()->num_packets()); - EXPECT_EQ(0, parser.fir()->num_packets()); - break; - case 1: - EXPECT_EQ(0, parser.receiver_report()->num_packets()); - EXPECT_EQ(0, parser.report_block()->num_packets()); - EXPECT_EQ(1, parser.fir()->num_packets()); - break; - default: - ADD_FAILURE() << "OnPacketReady not expected to be called " - << packets_created_ << " times."; - } - } - - int packets_created_ = 0; - } verifier; - const size_t kBufferSize = kRrLength + kReportBlockLength; - uint8_t buffer[kBufferSize]; - EXPECT_TRUE(rr.BuildExternalBuffer(buffer, kBufferSize, &verifier)); - EXPECT_EQ(2, verifier.packets_created_); -} - TEST(RtcpPacketTest, Remb) { Remb remb; remb.From(kSenderSsrc); diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc index 7b47ab7c68..848d73b2c4 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc @@ -24,6 +24,7 @@ #include "webrtc/modules/rtp_rtcp/source/byte_io.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/app.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/bye.h" +#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/nack.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/pli.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/receiver_report.h" @@ -79,7 +80,7 @@ RTCPSender::FeedbackState::FeedbackState() has_last_xr_rr(false), module(nullptr) {} -class PacketContainer : public rtcp::Empty, +class PacketContainer : public rtcp::CompoundPacket, public rtcp::RtcpPacket::PacketReadyCallback { public: explicit PacketContainer(Transport* transport) @@ -95,7 +96,7 @@ class PacketContainer : public rtcp::Empty, } size_t SendPackets() { - rtcp::Empty::Build(this); + rtcp::CompoundPacket::Build(this); return bytes_sent_; } -- cgit v1.2.3