diff --git a/modules/rtp_rtcp/source/rtcp_packet/compound_packet.cc b/modules/rtp_rtcp/source/rtcp_packet/compound_packet.cc index 5e762335ea..54f3555fc6 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/compound_packet.cc +++ b/modules/rtp_rtcp/source/rtcp_packet/compound_packet.cc @@ -10,6 +10,9 @@ #include "modules/rtp_rtcp/source/rtcp_packet/compound_packet.h" +#include +#include + #include "rtc_base/checks.h" namespace webrtc { @@ -19,16 +22,16 @@ CompoundPacket::CompoundPacket() = default; CompoundPacket::~CompoundPacket() = default; -void CompoundPacket::Append(RtcpPacket* packet) { +void CompoundPacket::Append(std::unique_ptr packet) { RTC_CHECK(packet); - appended_packets_.push_back(packet); + appended_packets_.push_back(std::move(packet)); } bool CompoundPacket::Create(uint8_t* packet, size_t* index, size_t max_length, PacketReadyCallback callback) const { - for (RtcpPacket* appended : appended_packets_) { + for (const auto& appended : appended_packets_) { if (!appended->Create(packet, index, max_length, callback)) return false; } @@ -37,7 +40,7 @@ bool CompoundPacket::Create(uint8_t* packet, size_t CompoundPacket::BlockLength() const { size_t block_length = 0; - for (RtcpPacket* appended : appended_packets_) { + for (const auto& appended : appended_packets_) { block_length += appended->BlockLength(); } return block_length; diff --git a/modules/rtp_rtcp/source/rtcp_packet/compound_packet.h b/modules/rtp_rtcp/source/rtcp_packet/compound_packet.h index f521c7f921..ca70c0ee20 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/compound_packet.h +++ b/modules/rtp_rtcp/source/rtcp_packet/compound_packet.h @@ -12,6 +12,7 @@ #ifndef MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_COMPOUND_PACKET_H_ #define MODULES_RTP_RTCP_SOURCE_RTCP_PACKET_COMPOUND_PACKET_H_ +#include #include #include "modules/rtp_rtcp/source/rtcp_packet.h" @@ -25,7 +26,14 @@ class CompoundPacket : public RtcpPacket { CompoundPacket(); ~CompoundPacket() override; - void Append(RtcpPacket* packet); + void Append(std::unique_ptr packet); + + // Fallback for call-sites that have not yet migrated to passing a unique_ptr. + // TODO(bugs.webrtc.org/11925): Remove when all usage is gone. + template + void Append(T* packet) { + Append(std::make_unique(*packet)); + } // Size of this packet in bytes (i.e. total size of nested packets). size_t BlockLength() const override; @@ -36,7 +44,7 @@ class CompoundPacket : public RtcpPacket { PacketReadyCallback callback) const override; protected: - std::vector appended_packets_; + std::vector> appended_packets_; private: RTC_DISALLOW_COPY_AND_ASSIGN(CompoundPacket); diff --git a/modules/rtp_rtcp/source/rtcp_packet/compound_packet_unittest.cc b/modules/rtp_rtcp/source/rtcp_packet/compound_packet_unittest.cc index 02a4f11ac2..9348aee7e4 100644 --- a/modules/rtp_rtcp/source/rtcp_packet/compound_packet_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_packet/compound_packet_unittest.cc @@ -10,6 +10,9 @@ #include "modules/rtp_rtcp/source/rtcp_packet/compound_packet.h" +#include +#include + #include "modules/rtp_rtcp/source/rtcp_packet.h" #include "modules/rtp_rtcp/source/rtcp_packet/bye.h" #include "modules/rtp_rtcp/source/rtcp_packet/fir.h" @@ -38,14 +41,14 @@ const uint8_t kSeqNo = 13; TEST(RtcpCompoundPacketTest, AppendPacket) { CompoundPacket compound; - Fir fir; - fir.AddRequestTo(kRemoteSsrc, kSeqNo); + auto fir = std::make_unique(); + fir->AddRequestTo(kRemoteSsrc, kSeqNo); ReportBlock rb; - ReceiverReport rr; - rr.SetSenderSsrc(kSenderSsrc); - EXPECT_TRUE(rr.AddReportBlock(rb)); - compound.Append(&rr); - compound.Append(&fir); + auto rr = std::make_unique(); + rr->SetSenderSsrc(kSenderSsrc); + EXPECT_TRUE(rr->AddReportBlock(rb)); + compound.Append(std::move(rr)); + compound.Append(std::move(fir)); rtc::Buffer packet = compound.Build(); RtcpPacketParser parser; @@ -58,21 +61,22 @@ TEST(RtcpCompoundPacketTest, AppendPacket) { TEST(RtcpCompoundPacketTest, AppendPacketWithOwnAppendedPacket) { CompoundPacket root; - CompoundPacket leaf; - Fir fir; - fir.AddRequestTo(kRemoteSsrc, kSeqNo); - Bye bye; + auto leaf = std::make_unique(); + + auto fir = std::make_unique(); + fir->AddRequestTo(kRemoteSsrc, kSeqNo); + auto bye = std::make_unique(); ReportBlock rb; - ReceiverReport rr; - EXPECT_TRUE(rr.AddReportBlock(rb)); - leaf.Append(&rr); - leaf.Append(&fir); + auto rr = std::make_unique(); + EXPECT_TRUE(rr->AddReportBlock(rb)); + leaf->Append(std::move(rr)); + leaf->Append(std::move(fir)); - SenderReport sr; - root.Append(&sr); - root.Append(&bye); - root.Append(&leaf); + auto sr = std::make_unique(); + root.Append(std::move(sr)); + root.Append(std::move(bye)); + root.Append(std::move(leaf)); rtc::Buffer packet = root.Build(); RtcpPacketParser parser; @@ -86,14 +90,14 @@ TEST(RtcpCompoundPacketTest, AppendPacketWithOwnAppendedPacket) { TEST(RtcpCompoundPacketTest, BuildWithInputBuffer) { CompoundPacket compound; - Fir fir; - fir.AddRequestTo(kRemoteSsrc, kSeqNo); + auto fir = std::make_unique(); + fir->AddRequestTo(kRemoteSsrc, kSeqNo); ReportBlock rb; - ReceiverReport rr; - rr.SetSenderSsrc(kSenderSsrc); - EXPECT_TRUE(rr.AddReportBlock(rb)); - compound.Append(&rr); - compound.Append(&fir); + auto rr = std::make_unique(); + rr->SetSenderSsrc(kSenderSsrc); + EXPECT_TRUE(rr->AddReportBlock(rb)); + compound.Append(std::move(rr)); + compound.Append(std::move(fir)); const size_t kRrLength = 8; const size_t kReportBlockLength = 24; @@ -115,14 +119,14 @@ TEST(RtcpCompoundPacketTest, BuildWithInputBuffer) { TEST(RtcpCompoundPacketTest, BuildWithTooSmallBuffer_FragmentedSend) { CompoundPacket compound; - Fir fir; - fir.AddRequestTo(kRemoteSsrc, kSeqNo); + auto fir = std::make_unique(); + fir->AddRequestTo(kRemoteSsrc, kSeqNo); ReportBlock rb; - ReceiverReport rr; - rr.SetSenderSsrc(kSenderSsrc); - EXPECT_TRUE(rr.AddReportBlock(rb)); - compound.Append(&rr); - compound.Append(&fir); + auto rr = std::make_unique(); + rr->SetSenderSsrc(kSenderSsrc); + EXPECT_TRUE(rr->AddReportBlock(rb)); + compound.Append(std::move(rr)); + compound.Append(std::move(fir)); const size_t kRrLength = 8; const size_t kReportBlockLength = 24; diff --git a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index a384d71913..0506aedadd 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -11,6 +11,7 @@ #include "modules/rtp_rtcp/source/rtcp_receiver.h" #include +#include #include "api/array_view.h" #include "api/units/timestamp.h" @@ -1200,14 +1201,14 @@ TEST(RtcpReceiverTest, TmmbrPacketAccepted) { receiver.SetRemoteSSRC(kSenderSsrc); const uint32_t kBitrateBps = 30000; - rtcp::Tmmbr tmmbr; - tmmbr.SetSenderSsrc(kSenderSsrc); - tmmbr.AddTmmbr(rtcp::TmmbItem(kReceiverMainSsrc, kBitrateBps, 0)); - rtcp::SenderReport sr; - sr.SetSenderSsrc(kSenderSsrc); + auto tmmbr = std::make_unique(); + tmmbr->SetSenderSsrc(kSenderSsrc); + tmmbr->AddTmmbr(rtcp::TmmbItem(kReceiverMainSsrc, kBitrateBps, 0)); + auto sr = std::make_unique(); + sr->SetSenderSsrc(kSenderSsrc); rtcp::CompoundPacket compound; - compound.Append(&sr); - compound.Append(&tmmbr); + compound.Append(std::move(sr)); + compound.Append(std::move(tmmbr)); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); EXPECT_CALL(mocks.rtp_rtcp_impl, SetTmmbn(SizeIs(1))); @@ -1228,15 +1229,15 @@ TEST(RtcpReceiverTest, TmmbrPacketNotForUsIgnored) { receiver.SetRemoteSSRC(kSenderSsrc); const uint32_t kBitrateBps = 30000; - rtcp::Tmmbr tmmbr; - tmmbr.SetSenderSsrc(kSenderSsrc); - tmmbr.AddTmmbr(rtcp::TmmbItem(kNotToUsSsrc, kBitrateBps, 0)); + auto tmmbr = std::make_unique(); + tmmbr->SetSenderSsrc(kSenderSsrc); + tmmbr->AddTmmbr(rtcp::TmmbItem(kNotToUsSsrc, kBitrateBps, 0)); - rtcp::SenderReport sr; - sr.SetSenderSsrc(kSenderSsrc); + auto sr = std::make_unique(); + sr->SetSenderSsrc(kSenderSsrc); rtcp::CompoundPacket compound; - compound.Append(&sr); - compound.Append(&tmmbr); + compound.Append(std::move(sr)); + compound.Append(std::move(tmmbr)); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); @@ -1251,14 +1252,14 @@ TEST(RtcpReceiverTest, TmmbrPacketZeroRateIgnored) { RTCPReceiver receiver(DefaultConfiguration(&mocks), &mocks.rtp_rtcp_impl); receiver.SetRemoteSSRC(kSenderSsrc); - rtcp::Tmmbr tmmbr; - tmmbr.SetSenderSsrc(kSenderSsrc); - tmmbr.AddTmmbr(rtcp::TmmbItem(kReceiverMainSsrc, 0, 0)); - rtcp::SenderReport sr; - sr.SetSenderSsrc(kSenderSsrc); + auto tmmbr = std::make_unique(); + tmmbr->SetSenderSsrc(kSenderSsrc); + tmmbr->AddTmmbr(rtcp::TmmbItem(kReceiverMainSsrc, 0, 0)); + auto sr = std::make_unique(); + sr->SetSenderSsrc(kSenderSsrc); rtcp::CompoundPacket compound; - compound.Append(&sr); - compound.Append(&tmmbr); + compound.Append(std::move(sr)); + compound.Append(std::move(tmmbr)); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); EXPECT_CALL(mocks.bandwidth_observer, OnReceivedRtcpReceiverReport); @@ -1276,14 +1277,14 @@ TEST(RtcpReceiverTest, TmmbrThreeConstraintsTimeOut) { // Inject 3 packets "from" kSenderSsrc, kSenderSsrc+1, kSenderSsrc+2. // The times of arrival are starttime + 0, starttime + 5 and starttime + 10. for (uint32_t ssrc = kSenderSsrc; ssrc < kSenderSsrc + 3; ++ssrc) { - rtcp::Tmmbr tmmbr; - tmmbr.SetSenderSsrc(ssrc); - tmmbr.AddTmmbr(rtcp::TmmbItem(kReceiverMainSsrc, 30000, 0)); - rtcp::SenderReport sr; - sr.SetSenderSsrc(ssrc); + auto tmmbr = std::make_unique(); + tmmbr->SetSenderSsrc(ssrc); + tmmbr->AddTmmbr(rtcp::TmmbItem(kReceiverMainSsrc, 30000, 0)); + auto sr = std::make_unique(); + sr->SetSenderSsrc(ssrc); rtcp::CompoundPacket compound; - compound.Append(&sr); - compound.Append(&tmmbr); + compound.Append(std::move(sr)); + compound.Append(std::move(tmmbr)); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedRtcpReportBlocks); EXPECT_CALL(mocks.rtp_rtcp_impl, SetTmmbn); @@ -1598,19 +1599,19 @@ TEST(RtcpReceiverTest, HandlesInvalidTransportFeedback) { receiver.SetRemoteSSRC(kSenderSsrc); // Send a compound packet with a TransportFeedback followed by something else. - rtcp::TransportFeedback packet; - packet.SetMediaSsrc(kReceiverMainSsrc); - packet.SetSenderSsrc(kSenderSsrc); - packet.SetBase(1, 1000); - packet.AddReceivedPacket(1, 1000); + auto packet = std::make_unique(); + packet->SetMediaSsrc(kReceiverMainSsrc); + packet->SetSenderSsrc(kSenderSsrc); + packet->SetBase(1, 1000); + packet->AddReceivedPacket(1, 1000); static uint32_t kBitrateBps = 50000; - rtcp::Remb remb; - remb.SetSenderSsrc(kSenderSsrc); - remb.SetBitrateBps(kBitrateBps); + auto remb = std::make_unique(); + remb->SetSenderSsrc(kSenderSsrc); + remb->SetBitrateBps(kBitrateBps); rtcp::CompoundPacket compound; - compound.Append(&packet); - compound.Append(&remb); + compound.Append(std::move(packet)); + compound.Append(std::move(remb)); rtc::Buffer built_packet = compound.Build(); // Modify the TransportFeedback packet so that it is invalid. @@ -1639,10 +1640,10 @@ TEST(RtcpReceiverTest, Nack) { nack_set.insert(std::begin(kNackList1), std::end(kNackList1)); nack_set.insert(std::begin(kNackList23), std::end(kNackList23)); - rtcp::Nack nack1; - nack1.SetSenderSsrc(kSenderSsrc); - nack1.SetMediaSsrc(kReceiverMainSsrc); - nack1.SetPacketIds(kNackList1, arraysize(kNackList1)); + auto nack1 = std::make_unique(); + nack1->SetSenderSsrc(kSenderSsrc); + nack1->SetMediaSsrc(kReceiverMainSsrc); + nack1->SetPacketIds(kNackList1, arraysize(kNackList1)); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedNack(ElementsAreArray(kNackList1))); @@ -1653,21 +1654,21 @@ TEST(RtcpReceiverTest, Nack) { arraysize(kNackList1)), Field(&RtcpPacketTypeCounter::unique_nack_requests, arraysize(kNackList1))))); - receiver.IncomingPacket(nack1.Build()); + receiver.IncomingPacket(nack1->Build()); - rtcp::Nack nack2; - nack2.SetSenderSsrc(kSenderSsrc); - nack2.SetMediaSsrc(kReceiverMainSsrc); - nack2.SetPacketIds(kNackList23, kNackListLength2); + auto nack2 = std::make_unique(); + nack2->SetSenderSsrc(kSenderSsrc); + nack2->SetMediaSsrc(kReceiverMainSsrc); + nack2->SetPacketIds(kNackList23, kNackListLength2); - rtcp::Nack nack3; - nack3.SetSenderSsrc(kSenderSsrc); - nack3.SetMediaSsrc(kReceiverMainSsrc); - nack3.SetPacketIds(kNackList23 + kNackListLength2, kNackListLength3); + auto nack3 = std::make_unique(); + nack3->SetSenderSsrc(kSenderSsrc); + nack3->SetMediaSsrc(kReceiverMainSsrc); + nack3->SetPacketIds(kNackList23 + kNackListLength2, kNackListLength3); rtcp::CompoundPacket two_nacks; - two_nacks.Append(&nack2); - two_nacks.Append(&nack3); + two_nacks.Append(std::move(nack2)); + two_nacks.Append(std::move(nack3)); EXPECT_CALL(mocks.rtp_rtcp_impl, OnReceivedNack(ElementsAreArray(kNackList23))); diff --git a/modules/rtp_rtcp/source/rtcp_sender.cc b/modules/rtp_rtcp/source/rtcp_sender.cc index fae635e1bc..5d2c9a2304 100644 --- a/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/modules/rtp_rtcp/source/rtcp_sender.cc @@ -55,10 +55,6 @@ class PacketContainer : public rtcp::CompoundPacket { public: PacketContainer(Transport* transport, RtcEventLog* event_log) : transport_(transport), event_log_(event_log) {} - ~PacketContainer() override { - for (RtcpPacket* packet : appended_packets_) - delete packet; - } size_t SendPackets(size_t max_payload_length) { size_t bytes_sent = 0; @@ -792,14 +788,14 @@ absl::optional RTCPSender::ComputeCompoundRTCPPacket( if (builder_it->first == kRtcpBye) { packet_bye = std::move(packet); } else { - out_packet->Append(packet.release()); + out_packet->Append(std::move(packet)); } } } // Append the BYE now at the end if (packet_bye) { - out_packet->Append(packet_bye.release()); + out_packet->Append(std::move(packet_bye)); } if (packet_type_counter_observer_ != nullptr) { diff --git a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc index 9c4c5adf79..b7694df1e8 100644 --- a/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc +++ b/modules/rtp_rtcp/source/rtcp_transceiver_impl_unittest.cc @@ -10,6 +10,8 @@ #include "modules/rtp_rtcp/source/rtcp_transceiver_impl.h" +#include +#include #include #include "absl/memory/memory.h" @@ -677,12 +679,12 @@ TEST(RtcpTransceiverImplTest, CallsObserverOnByeBehindSenderReport) { rtcp_transceiver.AddMediaReceiverRtcpObserver(kRemoteSsrc, &observer); CompoundPacket compound; - SenderReport sr; - sr.SetSenderSsrc(kRemoteSsrc); - compound.Append(&sr); - Bye bye; - bye.SetSenderSsrc(kRemoteSsrc); - compound.Append(&bye); + auto sr = std::make_unique(); + sr->SetSenderSsrc(kRemoteSsrc); + compound.Append(std::move(sr)); + auto bye = std::make_unique(); + bye->SetSenderSsrc(kRemoteSsrc); + compound.Append(std::move(bye)); auto raw_packet = compound.Build(); EXPECT_CALL(observer, OnBye(kRemoteSsrc)); @@ -698,11 +700,11 @@ TEST(RtcpTransceiverImplTest, CallsObserverOnByeBehindUnknownRtcpPacket) { CompoundPacket compound; // Use Application-Defined rtcp packet as unknown. - webrtc::rtcp::App app; - compound.Append(&app); - Bye bye; - bye.SetSenderSsrc(kRemoteSsrc); - compound.Append(&bye); + auto app = std::make_unique(); + compound.Append(std::move(app)); + auto bye = std::make_unique(); + bye->SetSenderSsrc(kRemoteSsrc); + compound.Append(std::move(bye)); auto raw_packet = compound.Build(); EXPECT_CALL(observer, OnBye(kRemoteSsrc));