Make transport-cc feedback work similarly to libwebrtc

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ### NEXT
 
+- Make transport-cc feedback work similarly to libwebrtc ([PR #1088](https://github.com/versatica/mediasoup/pull/1088) by @penguinol).
 - `TransportListenInfo`: announced ip can also be a hostname ([PR #1322](https://github.com/versatica/mediasoup/pull/1322)).
 
 ### 3.13.17

worker/include/RTC/RTCP/FeedbackRtpTransport.hpp

@@ -212,6 +212,7 @@ namespace RTC
 			~FeedbackRtpTransportPacket() override;
 
 		public:
+			void SetBase(uint16_t sequenceNumber, uint64_t timestamp);
 			AddPacketResult AddPacket(uint16_t sequenceNumber, uint64_t timestamp, size_t maxRtcpPacketLen);
 			// Just for locally generated packets.
 			void Finish();
@@ -317,6 +318,8 @@ namespace RTC
 			void AddPendingChunks();
 
 		private:
+			// Whether baseSequenceNumber has been set.
+			bool baseSet{ false };
 			uint16_t baseSequenceNumber{ 0u };
 			// 24 bits signed integer.
 			int32_t referenceTime{ 0 };

worker/include/RTC/TransportCongestionControlServer.hpp

@@ -6,6 +6,7 @@
 #include "RTC/RTCP/FeedbackRtpTransport.hpp"
 #include "RTC/RTCP/Packet.hpp"
 #include "RTC/RtpPacket.hpp"
+#include "RTC/SeqManager.hpp"
 #include "handles/TimerHandle.hpp"
 #include <libwebrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h>
 #include <deque>
@@ -54,10 +55,12 @@ namespace RTC
 		double GetPacketLoss() const;
 		void IncomingPacket(uint64_t nowMs, const RTC::RtpPacket* packet);
 		void SetMaxIncomingBitrate(uint32_t bitrate);
+		void FillAndSendTransportCcFeedback();
 
 	private:
 		void SendTransportCcFeedback();
-		void MaySendLimitationRembFeedback();
+		void MayDropOldPacketArrivalTimes(uint16_t seqNum, uint64_t nowMs);
+		void MaySendLimitationRembFeedback(uint64_t nowMs);
 		void UpdatePacketLoss(double packetLoss);
 
 		/* Pure virtual methods inherited from webrtc::RemoteBitrateEstimator::Listener. */
@@ -89,6 +92,10 @@ namespace RTC
 		uint8_t unlimitedRembCounter{ 0u };
 		std::deque<double> packetLossHistory;
 		double packetLoss{ 0 };
+		// Whether any packet with transport wide sequence number was received.
+		bool transportWideSeqNumberReceived{ false };
+		uint16_t transportCcFeedbackWideSeqNumStart{ 0u };
+		std::map<uint16_t, uint64_t, RTC::SeqManager<uint16_t>::SeqLowerThan> mapPacketArrivalTimes;
 	};
 } // namespace RTC
 

worker/meson.build

@@ -335,6 +335,7 @@ test_sources = [
   'test/src/RTC/TestSeqManager.cpp',
   'test/src/RTC/TestTrendCalculator.cpp',
   'test/src/RTC/TestRtpEncodingParameters.cpp',
+  'test/src/RTC/TestTransportCongestionControlServer.cpp',
   'test/src/RTC/Codecs/TestVP8.cpp',
   'test/src/RTC/Codecs/TestVP9.cpp',
   'test/src/RTC/Codecs/TestH264.cpp',

worker/src/RTC/RTCP/FeedbackRtpTransport.cpp

@@ -278,24 +278,25 @@ namespace RTC
 			return offset;
 		}
 
+		void FeedbackRtpTransportPacket::SetBase(uint16_t sequenceNumber, uint64_t timestamp)
+		{
+			MS_TRACE();
+
+			this->baseSet              = true;
+			this->baseSequenceNumber   = sequenceNumber;
+			this->referenceTime        = static_cast<int32_t>((timestamp & 0x1FFFFFC0) / 64);
+			this->latestSequenceNumber = sequenceNumber - 1;
+			this->latestTimestamp      = (timestamp >> 6) * 64; // IMPORTANT: Loose precision.
+		}
+
 		FeedbackRtpTransportPacket::AddPacketResult FeedbackRtpTransportPacket::AddPacket(
 		  uint16_t sequenceNumber, uint64_t timestamp, size_t maxRtcpPacketLen)
 		{
 			MS_TRACE();
 
+			MS_ASSERT(baseSet, "base not set");
 			MS_ASSERT(!IsFull(), "packet is full");
 
-			// Let's see if we must set our base.
-			if (this->latestTimestamp == 0u)
-			{
-				this->baseSequenceNumber   = sequenceNumber + 1;
-				this->referenceTime        = static_cast<int32_t>((timestamp & 0x1FFFFFC0) / 64);
-				this->latestSequenceNumber = sequenceNumber;
-				this->latestTimestamp      = (timestamp >> 6) * 64; // IMPORTANT: Loose precision.
-
-				return AddPacketResult::SUCCESS;
-			}
-
 			// If the wide sequence number of the new packet is lower than the latest seen,
 			// ignore it.
 			// NOTE: Not very spec compliant but libwebrtc does it.

worker/src/RTC/TransportCongestionControlServer.cpp

@@ -14,6 +14,7 @@ namespace RTC
 
 	static constexpr uint64_t TransportCcFeedbackSendInterval{ 100u }; // In ms.
 	static constexpr uint64_t LimitationRembInterval{ 1500u };         // In ms.
+	static constexpr uint64_t PacketArrivalTimestampWindow{ 500u };    // In ms.
 	static constexpr uint8_t UnlimitedRembNumPackets{ 4u };
 	static constexpr size_t PacketLossHistogramLength{ 24 };
 
@@ -124,61 +125,32 @@ namespace RTC
 					break;
 				}
 
-				// Update the RTCP media SSRC of the ongoing Transport-CC Feedback packet.
-				this->transportCcFeedbackSenderSsrc = 0u;
-				this->transportCcFeedbackMediaSsrc  = packet->GetSsrc();
-
-				this->transportCcFeedbackPacket->SetSenderSsrc(0u);
-				this->transportCcFeedbackPacket->SetMediaSsrc(this->transportCcFeedbackMediaSsrc);
-
-				// Provide the feedback packet with the RTP packet info. If it fails,
-				// send current feedback and add the packet info to a new one.
-				auto result =
-				  this->transportCcFeedbackPacket->AddPacket(wideSeqNumber, nowMs, this->maxRtcpPacketLen);
-
-				switch (result)
+				// Only insert the packet when receiving it for the first time.
+				if (!this->mapPacketArrivalTimes.try_emplace(wideSeqNumber, nowMs).second)
 				{
-					case RTC::RTCP::FeedbackRtpTransportPacket::AddPacketResult::SUCCESS:
-					{
-						// If the feedback packet is full, send it now.
-						if (this->transportCcFeedbackPacket->IsFull())
-						{
-							MS_DEBUG_DEV("transport-cc feedback packet is full, sending feedback now");
-
-							SendTransportCcFeedback();
-						}
-
-						break;
-					}
-
-					case RTC::RTCP::FeedbackRtpTransportPacket::AddPacketResult::MAX_SIZE_EXCEEDED:
-					{
-						// Send ongoing feedback packet and add the new packet info to the
-						// regenerated one.
-						SendTransportCcFeedback();
+					break;
+				}
 
-						this->transportCcFeedbackPacket->AddPacket(wideSeqNumber, nowMs, this->maxRtcpPacketLen);
+				// We may receive packets with sequence number lower than the one in previous
+				// tcc feedback, these packets may have been reported as lost previously,
+				// therefore we need to reset the start sequence num for the next tcc feedback.
+				if (
+				  !this->transportWideSeqNumberReceived ||
+				  RTC::SeqManager<uint16_t>::IsSeqLowerThan(
+				    wideSeqNumber, this->transportCcFeedbackWideSeqNumStart))
+				{
+					this->transportCcFeedbackWideSeqNumStart = wideSeqNumber;
+				}
 
-						break;
-					}
+				this->transportWideSeqNumberReceived = true;
 
-					case RTC::RTCP::FeedbackRtpTransportPacket::AddPacketResult::FATAL:
-					{
-						// Create a new feedback packet.
-						this->transportCcFeedbackPacket.reset(new RTC::RTCP::FeedbackRtpTransportPacket(
-						  this->transportCcFeedbackSenderSsrc, this->transportCcFeedbackMediaSsrc));
+				MayDropOldPacketArrivalTimes(wideSeqNumber, nowMs);
 
-						// Use current packet count.
-						// NOTE: Do not increment it since the previous ongoing feedback
-						// packet was not sent.
-						this->transportCcFeedbackPacket->SetFeedbackPacketCount(
-						  this->transportCcFeedbackPacketCount);
-
-						break;
-					}
-				}
+				// Update the RTCP media SSRC of the ongoing Transport-CC Feedback packet.
+				this->transportCcFeedbackSenderSsrc = 0u;
+				this->transportCcFeedbackMediaSsrc  = packet->GetSsrc();
 
-				MaySendLimitationRembFeedback();
+				MaySendLimitationRembFeedback(nowMs);
 
 				break;
 			}
@@ -204,6 +176,75 @@ namespace RTC
 		}
 	}
 
+	void TransportCongestionControlServer::FillAndSendTransportCcFeedback()
+	{
+		MS_TRACE();
+
+		if (!this->transportWideSeqNumberReceived)
+		{
+			return;
+		}
+
+		auto it = this->mapPacketArrivalTimes.lower_bound(this->transportCcFeedbackWideSeqNumStart);
+
+		if (it == this->mapPacketArrivalTimes.end())
+		{
+			return;
+		}
+
+		// Set base sequence num and reference time.
+		this->transportCcFeedbackPacket->SetBase(this->transportCcFeedbackWideSeqNumStart, it->second);
+
+		for (; it != this->mapPacketArrivalTimes.end(); ++it)
+		{
+			auto result =
+			  this->transportCcFeedbackPacket->AddPacket(it->first, it->second, this->maxRtcpPacketLen);
+
+			switch (result)
+			{
+				case RTC::RTCP::FeedbackRtpTransportPacket::AddPacketResult::SUCCESS:
+				{
+					// If the feedback packet is full, send it now.
+					if (this->transportCcFeedbackPacket->IsFull())
+					{
+						MS_DEBUG_DEV("transport-cc feedback packet is full, sending feedback now");
+
+						SendTransportCcFeedback();
+					}
+
+					break;
+				}
+
+				case RTC::RTCP::FeedbackRtpTransportPacket::AddPacketResult::MAX_SIZE_EXCEEDED:
+				{
+					// This should not happen.
+					MS_WARN_DEV("transport-cc feedback packet is exceeded");
+
+					// Create a new feedback packet.
+					this->transportCcFeedbackPacket.reset(new RTC::RTCP::FeedbackRtpTransportPacket(
+					  this->transportCcFeedbackSenderSsrc, this->transportCcFeedbackMediaSsrc));
+				}
+
+				case RTC::RTCP::FeedbackRtpTransportPacket::AddPacketResult::FATAL:
+				{
+					// Create a new feedback packet.
+					this->transportCcFeedbackPacket.reset(new RTC::RTCP::FeedbackRtpTransportPacket(
+					  this->transportCcFeedbackSenderSsrc, this->transportCcFeedbackMediaSsrc));
+
+					// Use current packet count.
+					// NOTE: Do not increment it since the previous ongoing feedback
+					// packet was not sent.
+					this->transportCcFeedbackPacket->SetFeedbackPacketCount(
+					  this->transportCcFeedbackPacketCount);
+
+					break;
+				}
+			}
+		}
+
+		SendTransportCcFeedback();
+	}
+
 	void TransportCongestionControlServer::SetMaxIncomingBitrate(uint32_t bitrate)
 	{
 		MS_TRACE();
@@ -217,7 +258,9 @@ namespace RTC
 			// This is to ensure that we send N REMB packets with bitrate 0 (unlimited).
 			this->unlimitedRembCounter = UnlimitedRembNumPackets;
 
-			MaySendLimitationRembFeedback();
+			auto nowMs = DepLibUV::GetTimeMs();
+
+			MaySendLimitationRembFeedback(nowMs);
 		}
 	}
 
@@ -233,7 +276,6 @@ namespace RTC
 		}
 
 		auto latestWideSeqNumber = this->transportCcFeedbackPacket->GetLatestSequenceNumber();
-		auto latestTimestamp     = this->transportCcFeedbackPacket->GetLatestTimestamp();
 
 		// Notify the listener.
 		this->listener->OnTransportCongestionControlServerSendRtcpPacket(
@@ -262,21 +304,36 @@ namespace RTC
 
 		// Increment packet count.
 		this->transportCcFeedbackPacket->SetFeedbackPacketCount(++this->transportCcFeedbackPacketCount);
+		this->transportCcFeedbackWideSeqNumStart = latestWideSeqNumber + 1;
+	}
+
+	inline void TransportCongestionControlServer::MayDropOldPacketArrivalTimes(
+	  uint16_t seqNum, uint64_t nowMs)
+	{
+		MS_TRACE();
 
-		// Pass the latest packet info (if any) as pre base for the new feedback packet.
-		if (latestTimestamp > 0u)
+		// Ignore nowMs value if it's smaller than PacketArrivalTimestampWindow in
+		// order to avoid negative values (should never happen) and return early if
+		// the condition is met.
+		if (nowMs >= PacketArrivalTimestampWindow)
 		{
-			this->transportCcFeedbackPacket->AddPacket(
-			  latestWideSeqNumber, latestTimestamp, this->maxRtcpPacketLen);
+			auto expiryTimestamp = nowMs - PacketArrivalTimestampWindow;
+			auto it              = this->mapPacketArrivalTimes.begin();
+
+			while (it != this->mapPacketArrivalTimes.end() &&
+			       it->first != this->transportCcFeedbackWideSeqNumStart &&
+			       RTC::SeqManager<uint16_t>::IsSeqLowerThan(it->first, seqNum) &&
+			       it->second <= expiryTimestamp)
+			{
+				it = this->mapPacketArrivalTimes.erase(it);
+			}
 		}
 	}
 
-	inline void TransportCongestionControlServer::MaySendLimitationRembFeedback()
+	inline void TransportCongestionControlServer::MaySendLimitationRembFeedback(uint64_t nowMs)
 	{
 		MS_TRACE();
 
-		auto nowMs = DepLibUV::GetTimeMs();
-
 		// May fix unlimitedRembCounter.
 		if (this->unlimitedRembCounter > 0u && this->maxIncomingBitrate != 0u)
 		{
@@ -389,7 +446,7 @@ namespace RTC
 
 		if (timer == this->transportCcFeedbackSendPeriodicTimer)
 		{
-			SendTransportCcFeedback();
+			FillAndSendTransportCcFeedback();
 		}
 	}
 } // namespace RTC