diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet.cc index b69f507127..87b086de9d 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet.cc @@ -26,11 +26,6 @@ void AssignUWord16(uint8_t* buffer, size_t* offset, uint16_t value) { } } // namespace -void RtcpPacket::Append(RtcpPacket* packet) { - assert(packet); - appended_packets_.push_back(packet); -} - rtc::Buffer RtcpPacket::Build() const { size_t length = 0; rtc::Buffer packet(IP_PACKET_SIZE); @@ -50,7 +45,7 @@ rtc::Buffer RtcpPacket::Build() const { bool called_; rtc::Buffer* const packet_; } verifier(&packet); - CreateAndAddAppended(packet.data(), &length, packet.capacity(), &verifier); + Create(packet.data(), &length, packet.capacity(), &verifier); OnBufferFull(packet.data(), &length, &verifier); return packet; } @@ -64,24 +59,11 @@ bool RtcpPacket::BuildExternalBuffer(uint8_t* buffer, size_t max_length, PacketReadyCallback* callback) const { size_t index = 0; - if (!CreateAndAddAppended(buffer, &index, max_length, callback)) + if (!Create(buffer, &index, max_length, callback)) return false; return OnBufferFull(buffer, &index, callback); } -bool RtcpPacket::CreateAndAddAppended(uint8_t* packet, - size_t* index, - size_t max_length, - PacketReadyCallback* callback) const { - if (!Create(packet, index, max_length, callback)) - return false; - for (RtcpPacket* appended : appended_packets_) { - if (!appended->CreateAndAddAppended(packet, index, max_length, callback)) - return false; - } - return true; -} - bool RtcpPacket::OnBufferFull(uint8_t* packet, size_t* index, RtcpPacket::PacketReadyCallback* callback) const { diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet.h b/webrtc/modules/rtp_rtcp/source/rtcp_packet.h index fef7258b5a..27ec57aaa9 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet.h @@ -12,17 +12,11 @@ #ifndef WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_H_ #define WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_H_ -#include - #include "webrtc/base/buffer.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h" -#include "webrtc/typedefs.h" namespace webrtc { namespace rtcp { - -static const int kCommonFbFmtLength = 12; - // Class for building RTCP packets. // // Example: @@ -45,16 +39,15 @@ static const int kCommonFbFmtLength = 12; // rtc::Buffer packet = fir.Build(); // Returns a RawPacket holding // // the built rtcp packet. // -// rr.Append(&fir); // Builds a compound RTCP packet with -// rtc::Buffer packet = rr.Build(); // a receiver report, report block -// // and fir message. +// CompoundPacket compound; // Builds a compound RTCP packet with +// compound.Append(&rr); // a receiver report, report block +// compound.Append(&fir); // and fir message. +// rtc::Buffer packet = compound.Build(); class RtcpPacket { public: virtual ~RtcpPacket() {} - void Append(RtcpPacket* packet); - // Callback used to signal that an RTCP packet is ready. Note that this may // not contain all data in this RtcpPacket; if a packet cannot fit in // max_length bytes, it will be fragmented and multiple calls to this @@ -71,27 +64,30 @@ class RtcpPacket { // used, will cause assertion error if fragmentation occurs. rtc::Buffer Build() const; - // Returns true if all calls to Create succeeded. A buffer of size + // Returns true if call to Create succeeded. A buffer of size // IP_PACKET_SIZE will be allocated and reused between calls to callback. bool Build(PacketReadyCallback* callback) const; - // Returns true if all calls to Create succeeded. Provided buffer reference + // Returns true if call to Create succeeded. Provided buffer reference // will be used for all calls to callback. bool BuildExternalBuffer(uint8_t* buffer, size_t max_length, PacketReadyCallback* callback) const; - // Size of this packet in bytes (including headers, excluding nested packets). + // Size of this packet in bytes (including headers). virtual size_t BlockLength() const = 0; - protected: - RtcpPacket() {} - + // Creates packet in the given buffer at the given position. + // Calls PacketReadyCallback::OnPacketReady if remaining buffer is too small + // and assume buffer can be reused after OnPacketReady returns. virtual bool Create(uint8_t* packet, size_t* index, size_t max_length, PacketReadyCallback* callback) const = 0; + protected: + RtcpPacket() {} + static void CreateHeader(uint8_t count_or_format, uint8_t packet_type, size_t block_length, // Size in 32bit words - 1. @@ -105,13 +101,6 @@ class RtcpPacket { size_t HeaderLength() const; static const size_t kHeaderLength = 4; - std::vector appended_packets_; - - private: - bool CreateAndAddAppended(uint8_t* packet, - size_t* index, - size_t max_length, - PacketReadyCallback* callback) const; }; } // namespace rtcp } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet.cc index 8f5afd5dd1..eead45d370 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet.cc @@ -10,18 +10,33 @@ #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet.h" +#include "webrtc/base/checks.h" + namespace webrtc { namespace rtcp { +void CompoundPacket::Append(RtcpPacket* packet) { + RTC_CHECK(packet); + appended_packets_.push_back(packet); +} + bool CompoundPacket::Create(uint8_t* packet, size_t* index, size_t max_length, RtcpPacket::PacketReadyCallback* callback) const { + for (RtcpPacket* appended : appended_packets_) { + if (!appended->Create(packet, index, max_length, callback)) + return false; + } return true; } size_t CompoundPacket::BlockLength() const { - return 0; + size_t block_length = 0; + for (RtcpPacket* appended : appended_packets_) { + block_length += appended->BlockLength(); + } + return block_length; } } // namespace rtcp diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet.h b/webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet.h index f2f49a8ffb..4fb92facc7 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet.h @@ -12,6 +12,8 @@ #ifndef WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_COMPOUND_PACKET_H_ #define WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_COMPOUND_PACKET_H_ +#include + #include "webrtc/base/basictypes.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet.h" @@ -20,17 +22,21 @@ namespace rtcp { class CompoundPacket : public RtcpPacket { public: - CompoundPacket() : RtcpPacket() {} + CompoundPacket() {} + ~CompoundPacket() override {} - virtual ~CompoundPacket() {} + void Append(RtcpPacket* packet); - protected: + // Size of this packet in bytes (i.e. total size of nested packets). + size_t BlockLength() const override; + // Returns true if all calls to Create succeeded. bool Create(uint8_t* packet, size_t* index, size_t max_length, RtcpPacket::PacketReadyCallback* callback) const override; - size_t BlockLength() const override; + protected: + std::vector appended_packets_; private: RTC_DISALLOW_COPY_AND_ASSIGN(CompoundPacket); 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 index 301e131941..f341bad9df 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet_unittest.cc @@ -33,15 +33,17 @@ const uint32_t kRemoteSsrc = 0x23456789; const uint8_t kSeqNo = 13; TEST(RtcpCompoundPacketTest, AppendPacket) { + CompoundPacket compound; Fir fir; fir.WithRequestTo(kRemoteSsrc, kSeqNo); ReportBlock rb; ReceiverReport rr; rr.From(kSenderSsrc); EXPECT_TRUE(rr.WithReportBlock(rb)); - rr.Append(&fir); + compound.Append(&rr); + compound.Append(&fir); - rtc::Buffer packet = rr.Build(); + rtc::Buffer packet = compound.Build(); RtcpPacketParser parser; parser.Parse(packet.data(), packet.size()); EXPECT_EQ(1, parser.receiver_report()->num_packets()); @@ -50,20 +52,9 @@ TEST(RtcpCompoundPacketTest, AppendPacket) { EXPECT_EQ(1, parser.fir()->num_packets()); } -TEST(RtcpCompoundPacketTest, AppendPacketOnEmpty) { - CompoundPacket empty; - ReceiverReport rr; - rr.From(kSenderSsrc); - empty.Append(&rr); - - rtc::Buffer packet = empty.Build(); - RtcpPacketParser parser; - parser.Parse(packet.data(), packet.size()); - EXPECT_EQ(1, parser.receiver_report()->num_packets()); - EXPECT_EQ(0, parser.report_block()->num_packets()); -} - TEST(RtcpCompoundPacketTest, AppendPacketWithOwnAppendedPacket) { + CompoundPacket root; + CompoundPacket leaf; Fir fir; fir.WithRequestTo(kRemoteSsrc, kSeqNo); Bye bye; @@ -71,13 +62,15 @@ TEST(RtcpCompoundPacketTest, AppendPacketWithOwnAppendedPacket) { ReceiverReport rr; EXPECT_TRUE(rr.WithReportBlock(rb)); - rr.Append(&fir); + leaf.Append(&rr); + leaf.Append(&fir); SenderReport sr; - sr.Append(&bye); - sr.Append(&rr); + root.Append(&sr); + root.Append(&bye); + root.Append(&leaf); - rtc::Buffer packet = sr.Build(); + rtc::Buffer packet = root.Build(); RtcpPacketParser parser; parser.Parse(packet.data(), packet.size()); EXPECT_EQ(1, parser.sender_report()->num_packets()); @@ -88,13 +81,15 @@ TEST(RtcpCompoundPacketTest, AppendPacketWithOwnAppendedPacket) { } TEST(RtcpCompoundPacketTest, BuildWithInputBuffer) { + CompoundPacket compound; Fir fir; fir.WithRequestTo(kRemoteSsrc, kSeqNo); ReportBlock rb; ReceiverReport rr; rr.From(kSenderSsrc); EXPECT_TRUE(rr.WithReportBlock(rb)); - rr.Append(&fir); + compound.Append(&rr); + compound.Append(&fir); const size_t kRrLength = 8; const size_t kReportBlockLength = 24; @@ -115,18 +110,20 @@ TEST(RtcpCompoundPacketTest, BuildWithInputBuffer) { } verifier; const size_t kBufferSize = kRrLength + kReportBlockLength + kFirLength; uint8_t buffer[kBufferSize]; - EXPECT_TRUE(rr.BuildExternalBuffer(buffer, kBufferSize, &verifier)); + EXPECT_TRUE(compound.BuildExternalBuffer(buffer, kBufferSize, &verifier)); EXPECT_EQ(1, verifier.packets_created_); } TEST(RtcpCompoundPacketTest, BuildWithTooSmallBuffer_FragmentedSend) { + CompoundPacket compound; Fir fir; fir.WithRequestTo(kRemoteSsrc, kSeqNo); ReportBlock rb; ReceiverReport rr; rr.From(kSenderSsrc); EXPECT_TRUE(rr.WithReportBlock(rb)); - rr.Append(&fir); + compound.Append(&rr); + compound.Append(&fir); const size_t kRrLength = 8; const size_t kReportBlockLength = 24; @@ -157,7 +154,7 @@ TEST(RtcpCompoundPacketTest, BuildWithTooSmallBuffer_FragmentedSend) { } verifier; const size_t kBufferSize = kRrLength + kReportBlockLength; uint8_t buffer[kBufferSize]; - EXPECT_TRUE(rr.BuildExternalBuffer(buffer, kBufferSize, &verifier)); + EXPECT_TRUE(compound.BuildExternalBuffer(buffer, kBufferSize, &verifier)); EXPECT_EQ(2, verifier.packets_created_); } diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index c43bb0e32a..507f835bc3 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -22,6 +22,7 @@ #include "webrtc/modules/rtp_rtcp/source/rtcp_packet.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/extended_jitter_report.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/extended_reports.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/fir.h" @@ -1003,8 +1004,10 @@ TEST_F(RtcpReceiverTest, TmmbrPacketAccepted) { rtcp::SenderReport sr; sr.From(kSenderSsrc); - sr.Append(&tmmbr); - rtc::Buffer packet = sr.Build(); + rtcp::CompoundPacket compound; + compound.Append(&sr); + compound.Append(&tmmbr); + rtc::Buffer packet = compound.Build(); EXPECT_EQ(0, InjectRtcpPacket(packet.data(), packet.size())); EXPECT_EQ(1, rtcp_receiver_->TMMBRReceived(0, 0, nullptr)); @@ -1026,8 +1029,10 @@ TEST_F(RtcpReceiverTest, TmmbrPacketNotForUsIgnored) { rtcp::SenderReport sr; sr.From(kSenderSsrc); - sr.Append(&tmmbr); - rtc::Buffer packet = sr.Build(); + rtcp::CompoundPacket compound; + compound.Append(&sr); + compound.Append(&tmmbr); + rtc::Buffer packet = compound.Build(); std::set ssrcs; ssrcs.insert(kMediaFlowSsrc); @@ -1049,8 +1054,10 @@ TEST_F(RtcpReceiverTest, TmmbrPacketZeroRateIgnored) { rtcp::SenderReport sr; sr.From(kSenderSsrc); - sr.Append(&tmmbr); - rtc::Buffer packet = sr.Build(); + rtcp::CompoundPacket compound; + compound.Append(&sr); + compound.Append(&tmmbr); + rtc::Buffer packet = compound.Build(); EXPECT_EQ(0, InjectRtcpPacket(packet.data(), packet.size())); EXPECT_EQ(0, rtcp_receiver_->TMMBRReceived(0, 0, nullptr)); @@ -1072,8 +1079,10 @@ TEST_F(RtcpReceiverTest, TmmbrThreeConstraintsTimeOut) { rtcp::SenderReport sr; sr.From(ssrc); - sr.Append(&tmmbr); - rtc::Buffer packet = sr.Build(); + rtcp::CompoundPacket compound; + compound.Append(&sr); + compound.Append(&tmmbr); + rtc::Buffer packet = compound.Build(); EXPECT_EQ(0, InjectRtcpPacket(packet.data(), packet.size())); // 5 seconds between each packet. system_clock_.AdvanceTimeMilliseconds(5000); @@ -1209,9 +1218,10 @@ TEST_F(RtcpReceiverTest, HandlesInvalidTransportFeedback) { rtcp::Remb remb; remb.From(kSourceSsrc); remb.WithBitrateBps(kBitrateBps); - packet.Append(&remb); - - rtc::Buffer built_packet = packet.Build(); + rtcp::CompoundPacket compound; + compound.Append(&packet); + compound.Append(&remb); + rtc::Buffer built_packet = compound.Build(); // Modify the TransportFeedback packet so that it is invalid. const size_t kStatusCountOffset = 14;