Merge pull request #4 from satoren/unique_ptr

refactor: use unique_ptr instead of raw pointer for CompoundPacket
nazar-pc · Nov 21, 2021 · f4189c2 · f4189c2
2 parents 5833d50 + ee5e195
commit f4189c2
Show file tree

Hide file tree

Showing 14 changed files with 47 additions and 38 deletions.
diff --git a/worker/include/RTC/Consumer.hpp b/worker/include/RTC/Consumer.hpp
@@ -138,13 +138,14 @@ namespace RTC
 		{
 			this->externallyManagedBitrate = true;
 		}
-		virtual uint8_t GetBitratePriority() const                                                = 0;
-		virtual uint32_t IncreaseLayer(uint32_t bitrate, bool considerLoss)                       = 0;
-		virtual void ApplyLayers()                                                                = 0;
-		virtual uint32_t GetDesiredBitrate() const                                                = 0;
-		virtual void SendRtpPacket(RTC::RtpPacket* packet, RTC::RtpPacket** clonedPacket)         = 0;
-		virtual const std::vector<RTC::RtpStreamSend*>& GetRtpStreams() const                     = 0;
-		virtual RTC::RTCP::CompoundPacket* GetRtcp(RTC::RtpStreamSend* rtpStream, uint64_t nowMs) = 0;
+		virtual uint8_t GetBitratePriority() const                                        = 0;
+		virtual uint32_t IncreaseLayer(uint32_t bitrate, bool considerLoss)               = 0;
+		virtual void ApplyLayers()                                                        = 0;
+		virtual uint32_t GetDesiredBitrate() const                                        = 0;
+		virtual void SendRtpPacket(RTC::RtpPacket* packet, RTC::RtpPacket** clonedPacket) = 0;
+		virtual const std::vector<RTC::RtpStreamSend*>& GetRtpStreams() const             = 0;
+		virtual RTC::RTCP::CompoundPacket::UniquePtr GetRtcp(
+		  RTC::RtpStreamSend* rtpStream, uint64_t nowMs) = 0;
 		virtual void NeedWorstRemoteFractionLost(uint32_t mappedSsrc, uint8_t& worstRemoteFractionLost) = 0;
 		virtual void ReceiveNack(RTC::RTCP::FeedbackRtpNackPacket* nackPacket) = 0;
 		virtual void ReceiveKeyFrameRequest(

diff --git a/worker/include/RTC/PipeConsumer.hpp b/worker/include/RTC/PipeConsumer.hpp
@@ -31,7 +31,7 @@ namespace RTC
 		void ApplyLayers() override;
 		uint32_t GetDesiredBitrate() const override;
 		void SendRtpPacket(RTC::RtpPacket* packet, RTC::RtpPacket** clonedPacket) override;
-		RTC::RTCP::CompoundPacket* GetRtcp(RTC::RtpStreamSend* rtpStream, uint64_t nowMs) override;
+		RTC::RTCP::CompoundPacket::UniquePtr GetRtcp(RTC::RtpStreamSend* rtpStream, uint64_t nowMs) override;
 		const std::vector<RTC::RtpStreamSend*>& GetRtpStreams() const override
 		{
 			return this->rtpStreams;

diff --git a/worker/include/RTC/Producer.hpp b/worker/include/RTC/Producer.hpp
@@ -123,7 +123,7 @@ namespace RTC
 		ReceiveRtpPacketResult ReceiveRtpPacket(RTC::RtpPacket* packet);
 		void ReceiveRtcpSenderReport(RTC::RTCP::SenderReport* report);
 		void ReceiveRtcpXrDelaySinceLastRr(RTC::RTCP::DelaySinceLastRr::SsrcInfo* ssrcInfo);
-		RTC::RTCP::CompoundPacket* GetRtcp(uint64_t nowMs);
+		RTC::RTCP::CompoundPacket::UniquePtr GetRtcp(uint64_t nowMs);
 		void RequestKeyFrame(uint32_t mappedSsrc);
 
 	private:

diff --git a/worker/include/RTC/RTCP/CompoundPacket.hpp b/worker/include/RTC/RTCP/CompoundPacket.hpp
@@ -15,8 +15,8 @@ namespace RTC
 		class CompoundPacket
 		{
 		public:
-			static CompoundPacket* Create();
-			static void ReturnIntoPool(CompoundPacket* packet);
+			using UniquePtr = std::unique_ptr<CompoundPacket>;
+			static UniquePtr Create();
 
 		public:
 			const uint8_t* GetData() const
@@ -60,6 +60,9 @@ namespace RTC
 			// Use `CompoundPacket::ReturnIntoPool()` instead
 			~CompoundPacket() = default;
 
+			friend struct std::default_delete<RTC::RTCP::CompoundPacket>;
+			static void ReturnIntoPool(CompoundPacket* packet);
+
 		private:
 			uint8_t* header{ nullptr };
 			size_t size{ 0 };
@@ -71,4 +74,15 @@ namespace RTC
 	} // namespace RTCP
 } // namespace RTC
 
+namespace std
+{
+	template<>
+	struct default_delete<RTC::RTCP::CompoundPacket>
+	{
+		void operator()(RTC::RTCP::CompoundPacket* ptr) const
+		{
+			RTC::RTCP::CompoundPacket::ReturnIntoPool(ptr);
+		}
+	};
+}; // namespace std
 #endif
diff --git a/worker/include/RTC/SimpleConsumer.hpp b/worker/include/RTC/SimpleConsumer.hpp
@@ -45,7 +45,7 @@ namespace RTC
 		{
 			return this->rtpStreams;
 		}
-		RTC::RTCP::CompoundPacket* GetRtcp(RTC::RtpStreamSend* rtpStream, uint64_t nowMs) override;
+		RTC::RTCP::CompoundPacket::UniquePtr GetRtcp(RTC::RtpStreamSend* rtpStream, uint64_t nowMs) override;
 		void NeedWorstRemoteFractionLost(uint32_t mappedSsrc, uint8_t& worstRemoteFractionLost) override;
 		void ReceiveNack(RTC::RTCP::FeedbackRtpNackPacket* nackPacket) override;
 		void ReceiveKeyFrameRequest(RTC::RTCP::FeedbackPs::MessageType messageType, uint32_t ssrc) override;

diff --git a/worker/include/RTC/SimulcastConsumer.hpp b/worker/include/RTC/SimulcastConsumer.hpp
@@ -57,7 +57,7 @@ namespace RTC
 		void ApplyLayers() override;
 		uint32_t GetDesiredBitrate() const override;
 		void SendRtpPacket(RTC::RtpPacket* packet, RTC::RtpPacket** clonedPacket) override;
-		RTC::RTCP::CompoundPacket* GetRtcp(RTC::RtpStreamSend* rtpStream, uint64_t nowMs) override;
+		RTC::RTCP::CompoundPacket::UniquePtr GetRtcp(RTC::RtpStreamSend* rtpStream, uint64_t nowMs) override;
 		const std::vector<RTC::RtpStreamSend*>& GetRtpStreams() const override
 		{
 			return this->rtpStreams;

diff --git a/worker/include/RTC/SvcConsumer.hpp b/worker/include/RTC/SvcConsumer.hpp
@@ -52,7 +52,7 @@ namespace RTC
 		void ApplyLayers() override;
 		uint32_t GetDesiredBitrate() const override;
 		void SendRtpPacket(RTC::RtpPacket* packet, RTC::RtpPacket** clonedPacket) override;
-		RTC::RTCP::CompoundPacket* GetRtcp(RTC::RtpStreamSend* rtpStream, uint64_t nowMs) override;
+		RTC::RTCP::CompoundPacket::UniquePtr GetRtcp(RTC::RtpStreamSend* rtpStream, uint64_t nowMs) override;
 		const std::vector<RTC::RtpStreamSend*>& GetRtpStreams() const override
 		{
 			return this->rtpStreams;

diff --git a/worker/src/RTC/PipeConsumer.cpp b/worker/src/RTC/PipeConsumer.cpp
@@ -279,7 +279,7 @@ namespace RTC
 		packet->SetSequenceNumber(origSeq);
 	}
 
-	RTC::RTCP::CompoundPacket* PipeConsumer::GetRtcp(RTC::RtpStreamSend* rtpStream, uint64_t nowMs)
+	RTC::RTCP::CompoundPacket::UniquePtr PipeConsumer::GetRtcp(RTC::RtpStreamSend* rtpStream, uint64_t nowMs)
 	{
 		MS_TRACE();
 
@@ -304,7 +304,7 @@ namespace RTC
 		if (!report)
 			return nullptr;
 
-		auto* packet = RTC::RTCP::CompoundPacket::Create();
+		auto packet = RTC::RTCP::CompoundPacket::Create();
 
 		packet->AddSenderReport(report);
 

diff --git a/worker/src/RTC/Producer.cpp b/worker/src/RTC/Producer.cpp
@@ -752,14 +752,14 @@ namespace RTC
 		rtpStream->ReceiveRtcpXrDelaySinceLastRr(ssrcInfo);
 	}
 
-	RTC::RTCP::CompoundPacket* Producer::GetRtcp(uint64_t nowMs)
+	RTC::RTCP::CompoundPacket::UniquePtr Producer::GetRtcp(uint64_t nowMs)
 	{
 		MS_TRACE();
 
 		if (static_cast<float>((nowMs - this->lastRtcpSentTime) * 1.15) < this->maxRtcpInterval)
 			return nullptr;
 
-		auto* packet = RTC::RTCP::CompoundPacket::Create();
+		auto packet = RTC::RTCP::CompoundPacket::Create();
 
 		for (auto& kv : this->mapSsrcRtpStream)
 		{

diff --git a/worker/src/RTC/RTCP/CompoundPacket.cpp b/worker/src/RTC/RTCP/CompoundPacket.cpp
@@ -12,12 +12,11 @@ namespace RTC
 
 		/* Instance methods. */
 
-		CompoundPacket* CompoundPacket::Create()
+		CompoundPacket::UniquePtr CompoundPacket::Create()
 		{
 			auto* packet = CompoundPacketPool.Allocate();
-			new (packet) CompoundPacket();
 
-			return packet;
+			return UniquePtr(new (packet) CompoundPacket());
 		}
 
 		void CompoundPacket::ReturnIntoPool(CompoundPacket* packet)

diff --git a/worker/src/RTC/SimpleConsumer.cpp b/worker/src/RTC/SimpleConsumer.cpp
@@ -316,7 +316,8 @@ namespace RTC
 		packet->SetSequenceNumber(origSeq);
 	}
 
-	RTC::RTCP::CompoundPacket* SimpleConsumer::GetRtcp(RTC::RtpStreamSend* rtpStream, uint64_t nowMs)
+	RTC::RTCP::CompoundPacket::UniquePtr SimpleConsumer::GetRtcp(
+	  RTC::RtpStreamSend* rtpStream, uint64_t nowMs)
 	{
 		MS_TRACE();
 
@@ -330,7 +331,7 @@ namespace RTC
 		if (!report)
 			return nullptr;
 
-		auto* packet = RTC::RTCP::CompoundPacket::Create();
+		auto packet = RTC::RTCP::CompoundPacket::Create();
 
 		packet->AddSenderReport(report);
 

diff --git a/worker/src/RTC/SimulcastConsumer.cpp b/worker/src/RTC/SimulcastConsumer.cpp
@@ -924,7 +924,8 @@ namespace RTC
 		packet->RestorePayload();
 	}
 
-	RTC::RTCP::CompoundPacket* SimulcastConsumer::GetRtcp(RTC::RtpStreamSend* rtpStream, uint64_t nowMs)
+	RTC::RTCP::CompoundPacket::UniquePtr SimulcastConsumer::GetRtcp(
+	  RTC::RtpStreamSend* rtpStream, uint64_t nowMs)
 	{
 		MS_TRACE();
 
@@ -938,7 +939,7 @@ namespace RTC
 		if (!report)
 			return nullptr;
 
-		auto* packet = RTC::RTCP::CompoundPacket::Create();
+		auto packet = RTC::RTCP::CompoundPacket::Create();
 
 		packet->AddSenderReport(report);
 

diff --git a/worker/src/RTC/SvcConsumer.cpp b/worker/src/RTC/SvcConsumer.cpp
@@ -654,7 +654,7 @@ namespace RTC
 		packet->RestorePayload();
 	}
 
-	RTC::RTCP::CompoundPacket* SvcConsumer::GetRtcp(RTC::RtpStreamSend* rtpStream, uint64_t nowMs)
+	RTC::RTCP::CompoundPacket::UniquePtr SvcConsumer::GetRtcp(RTC::RtpStreamSend* rtpStream, uint64_t nowMs)
 	{
 		MS_TRACE();
 
@@ -668,7 +668,7 @@ namespace RTC
 		if (!report)
 			return nullptr;
 
-		auto* packet = RTC::RTCP::CompoundPacket::Create();
+		auto packet = RTC::RTCP::CompoundPacket::Create();
 
 		packet->AddSenderReport(report);
 

diff --git a/worker/src/RTC/Transport.cpp b/worker/src/RTC/Transport.cpp
@@ -2312,56 +2312,49 @@ namespace RTC
 	{
 		MS_TRACE();
 
-		RTC::RTCP::CompoundPacket* packet{ nullptr };
+		RTC::RTCP::CompoundPacket::UniquePtr packet(nullptr);
 
 		for (auto& kv : this->mapConsumers)
 		{
 			auto* consumer = kv.second;
 
 			for (const auto& rtpStream : consumer->GetRtpStreams())
 			{
-				RTC::RTCP::CompoundPacket::ReturnIntoPool(packet);
 				packet = consumer->GetRtcp(rtpStream, nowMs);
 
 				// Send the RTCP compound packet if there is a sender report.
 				if (packet != nullptr && packet->HasSenderReport())
 				{
 					packet->Serialize(RTC::RTCP::Buffer);
-					SendRtcpCompoundPacket(packet);
+					SendRtcpCompoundPacket(packet.get());
 				}
 			}
 		}
 
 		// Reset the Compound packet.
-		RTC::RTCP::CompoundPacket::ReturnIntoPool(packet);
 		packet = nullptr;
 
 		for (auto& kv : this->mapProducers)
 		{
 			auto* producer = kv.second;
 
-			RTC::RTCP::CompoundPacket::ReturnIntoPool(packet);
 			packet = producer->GetRtcp(nowMs);
 
 			// One more RR would exceed the MTU, send the compound packet now.
 			if (packet != nullptr && packet->GetSize() + sizeof(RTCP::ReceiverReport::Header) > RTC::MtuSize)
 			{
 				packet->Serialize(RTC::RTCP::Buffer);
-				SendRtcpCompoundPacket(packet);
+				SendRtcpCompoundPacket(packet.get());
 
-				// Reset the Compound packet.
-				RTC::RTCP::CompoundPacket::ReturnIntoPool(packet);
 				packet = nullptr;
 			}
 		}
 
 		if (packet != nullptr && packet->GetReceiverReportCount() != 0u)
 		{
 			packet->Serialize(RTC::RTCP::Buffer);
-			SendRtcpCompoundPacket(packet);
+			SendRtcpCompoundPacket(packet.get());
 		}
-
-		RTC::RTCP::CompoundPacket::ReturnIntoPool(packet);
 	}
 
 	void Transport::DistributeAvailableOutgoingBitrate()