From cfe202b979c954df8589a10211792d10a5f25417 Mon Sep 17 00:00:00 2001 From: penguinol Date: Fri, 26 May 2023 13:30:38 +0800 Subject: [PATCH 01/27] fix tcc-feedback --- .../RTC/TransportCongestionControlServer.hpp | 7 + .../RTC/TransportCongestionControlServer.cpp | 143 +++++++++++------- 2 files changed, 94 insertions(+), 56 deletions(-) diff --git a/worker/include/RTC/TransportCongestionControlServer.hpp b/worker/include/RTC/TransportCongestionControlServer.hpp index 5747d13d03..67af10ae62 100644 --- a/worker/include/RTC/TransportCongestionControlServer.hpp +++ b/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/Timer.hpp" #include #include @@ -57,6 +58,8 @@ namespace RTC private: void SendTransportCcFeedback(); + void MayDropOldPacketArrivalTimes(uint16_t seqNum, uint64_t nowMs); + void FillAndSendTransportCcFeedback(); void MaySendLimitationRembFeedback(); void UpdatePacketLoss(double packetLoss); @@ -89,6 +92,10 @@ namespace RTC uint8_t unlimitedRembCounter{ 0u }; std::deque packetLossHistory; double packetLoss{ 0 }; + bool receivedTransportWideSeqNumber{ false }; + uint16_t transportCcFeedbackStartSeqNum{ 0u }; + std::map::SeqLowerThan> mapPacketArrivalTimes; + }; } // namespace RTC diff --git a/worker/src/RTC/TransportCongestionControlServer.cpp b/worker/src/RTC/TransportCongestionControlServer.cpp index 8c7a016949..3c67b16778 100644 --- a/worker/src/RTC/TransportCongestionControlServer.cpp +++ b/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,60 +125,23 @@ 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) + if (!this->receivedTransportWideSeqNumber || + RTC::SeqManager::IsSeqLowerThan(wideSeqNumber, this->transportCcFeedbackStartSeqNum)) { - 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(); - - this->transportCcFeedbackPacket->AddPacket(wideSeqNumber, nowMs, this->maxRtcpPacketLen); - - break; - } - - 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); + this->transportCcFeedbackStartSeqNum = wideSeqNumber; + } + this->receivedTransportWideSeqNumber = true; - break; - } + auto result = this->mapPacketArrivalTimes.emplace(wideSeqNumber, nowMs); + if (result.second) + { + MayDropOldPacketArrivalTimes(wideSeqNumber, nowMs); } + // Update the RTCP media SSRC of the ongoing Transport-CC Feedback packet. + this->transportCcFeedbackSenderSsrc = 0u; + this->transportCcFeedbackMediaSsrc = packet->GetSsrc(); + MaySendLimitationRembFeedback(); break; @@ -204,6 +168,61 @@ namespace RTC } } + inline void TransportCongestionControlServer::FillAndSendTransportCcFeedback() + { + MS_TRACE(); + + for (auto it = this->mapPacketArrivalTimes.find(this->transportCcFeedbackStartSeqNum); + 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_DEBUG_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(); @@ -233,7 +252,6 @@ namespace RTC } auto latestWideSeqNumber = this->transportCcFeedbackPacket->GetLatestSequenceNumber(); - auto latestTimestamp = this->transportCcFeedbackPacket->GetLatestTimestamp(); // Notify the listener. this->listener->OnTransportCongestionControlServerSendRtcpPacket( @@ -262,12 +280,25 @@ namespace RTC // Increment packet count. this->transportCcFeedbackPacket->SetFeedbackPacketCount(++this->transportCcFeedbackPacketCount); + this->transportCcFeedbackStartSeqNum = latestWideSeqNumber; + } + + 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) + 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->transportCcFeedbackStartSeqNum && + RTC::SeqManager::IsSeqLowerThan(it->first, seqNum) && + it->second <= expiryTimestamp) + { + it = this->mapPacketArrivalTimes.erase(it); + } } } @@ -389,7 +420,7 @@ namespace RTC if (timer == this->transportCcFeedbackSendPeriodicTimer) { - SendTransportCcFeedback(); + FillAndSendTransportCcFeedback(); } } } // namespace RTC From 08d14a92adf1c693ec0fb438ed01c39d7bf3661a Mon Sep 17 00:00:00 2001 From: penguinol Date: Fri, 26 May 2023 13:33:48 +0800 Subject: [PATCH 02/27] update format --- worker/src/RTC/TransportCongestionControlServer.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/worker/src/RTC/TransportCongestionControlServer.cpp b/worker/src/RTC/TransportCongestionControlServer.cpp index 3c67b16778..5a91918ca7 100644 --- a/worker/src/RTC/TransportCongestionControlServer.cpp +++ b/worker/src/RTC/TransportCongestionControlServer.cpp @@ -125,8 +125,10 @@ namespace RTC break; } - if (!this->receivedTransportWideSeqNumber || - RTC::SeqManager::IsSeqLowerThan(wideSeqNumber, this->transportCcFeedbackStartSeqNum)) + if ( + !this->receivedTransportWideSeqNumber || + RTC::SeqManager::IsSeqLowerThan( + wideSeqNumber, this->transportCcFeedbackStartSeqNum)) { this->transportCcFeedbackStartSeqNum = wideSeqNumber; } @@ -283,7 +285,8 @@ namespace RTC this->transportCcFeedbackStartSeqNum = latestWideSeqNumber; } - inline void TransportCongestionControlServer::MayDropOldPacketArrivalTimes(uint16_t seqNum, uint64_t nowMs) + inline void TransportCongestionControlServer::MayDropOldPacketArrivalTimes( + uint16_t seqNum, uint64_t nowMs) { MS_TRACE(); From 601736f59ac4a93ded53ab2a4ad2c2ee347a2117 Mon Sep 17 00:00:00 2001 From: penguinol Date: Fri, 26 May 2023 13:39:34 +0800 Subject: [PATCH 03/27] Update TransportCongestionControlServer.cpp --- worker/src/RTC/TransportCongestionControlServer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/worker/src/RTC/TransportCongestionControlServer.cpp b/worker/src/RTC/TransportCongestionControlServer.cpp index 5a91918ca7..f3b4ef6e4b 100644 --- a/worker/src/RTC/TransportCongestionControlServer.cpp +++ b/worker/src/RTC/TransportCongestionControlServer.cpp @@ -286,7 +286,7 @@ namespace RTC } inline void TransportCongestionControlServer::MayDropOldPacketArrivalTimes( - uint16_t seqNum, uint64_t nowMs) + uint16_t seqNum, uint64_t nowMs) { MS_TRACE(); From 86676f6d0ac58080980896392ebaae50b63c7e64 Mon Sep 17 00:00:00 2001 From: penguinol Date: Fri, 26 May 2023 15:44:08 +0800 Subject: [PATCH 04/27] Update TransportCongestionControlServer.hpp --- worker/include/RTC/TransportCongestionControlServer.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/worker/include/RTC/TransportCongestionControlServer.hpp b/worker/include/RTC/TransportCongestionControlServer.hpp index 67af10ae62..02af54c637 100644 --- a/worker/include/RTC/TransportCongestionControlServer.hpp +++ b/worker/include/RTC/TransportCongestionControlServer.hpp @@ -95,7 +95,6 @@ namespace RTC bool receivedTransportWideSeqNumber{ false }; uint16_t transportCcFeedbackStartSeqNum{ 0u }; std::map::SeqLowerThan> mapPacketArrivalTimes; - }; } // namespace RTC From e7808c181181b89ae83f91c60984692f30e3f117 Mon Sep 17 00:00:00 2001 From: Junlong Zou Date: Sun, 25 Jun 2023 09:07:46 +0800 Subject: [PATCH 05/27] Update worker/src/RTC/TransportCongestionControlServer.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Iñaki Baz Castillo --- worker/src/RTC/TransportCongestionControlServer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/worker/src/RTC/TransportCongestionControlServer.cpp b/worker/src/RTC/TransportCongestionControlServer.cpp index f3b4ef6e4b..f60df34d2e 100644 --- a/worker/src/RTC/TransportCongestionControlServer.cpp +++ b/worker/src/RTC/TransportCongestionControlServer.cpp @@ -135,6 +135,7 @@ namespace RTC this->receivedTransportWideSeqNumber = true; auto result = this->mapPacketArrivalTimes.emplace(wideSeqNumber, nowMs); + if (result.second) { MayDropOldPacketArrivalTimes(wideSeqNumber, nowMs); From 186d72fed5c0f52f8820b91491d8fcd2e711d897 Mon Sep 17 00:00:00 2001 From: penguinol Date: Tue, 11 Jul 2023 17:24:52 +0800 Subject: [PATCH 06/27] add comment --- worker/src/RTC/TransportCongestionControlServer.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/worker/src/RTC/TransportCongestionControlServer.cpp b/worker/src/RTC/TransportCongestionControlServer.cpp index f3b4ef6e4b..eeec12af42 100644 --- a/worker/src/RTC/TransportCongestionControlServer.cpp +++ b/worker/src/RTC/TransportCongestionControlServer.cpp @@ -125,6 +125,10 @@ namespace RTC break; } + // We may receive packets with sequence number lower than the one in previous + // tcc feedback, these packets may has been reported as lost previously, + // therefore we need to reset the start sequence num for the next tcc feedback. + if ( !this->receivedTransportWideSeqNumber || RTC::SeqManager::IsSeqLowerThan( From e3c935e96f9a7413522ececace0f43795992afc2 Mon Sep 17 00:00:00 2001 From: Junlong Zou Date: Tue, 2 Jan 2024 19:53:01 +0800 Subject: [PATCH 07/27] Update worker/src/RTC/TransportCongestionControlServer.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Luis Millán --- worker/src/RTC/TransportCongestionControlServer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/worker/src/RTC/TransportCongestionControlServer.cpp b/worker/src/RTC/TransportCongestionControlServer.cpp index 1221bc3916..11a8179430 100644 --- a/worker/src/RTC/TransportCongestionControlServer.cpp +++ b/worker/src/RTC/TransportCongestionControlServer.cpp @@ -126,7 +126,7 @@ namespace RTC } // We may receive packets with sequence number lower than the one in previous - // tcc feedback, these packets may has been reported as lost previously, + // 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 ( From d0c4c024bb5a8237fcc19a2a1a08eac6f04656e8 Mon Sep 17 00:00:00 2001 From: penguinol Date: Mon, 8 Jan 2024 15:12:35 +0800 Subject: [PATCH 08/27] prepare for test Modify nowMs as a parameter for testing purposes. --- .../include/RTC/TransportCongestionControlServer.hpp | 2 +- worker/src/RTC/TransportCongestionControlServer.cpp | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/worker/include/RTC/TransportCongestionControlServer.hpp b/worker/include/RTC/TransportCongestionControlServer.hpp index 02af54c637..8dc821843e 100644 --- a/worker/include/RTC/TransportCongestionControlServer.hpp +++ b/worker/include/RTC/TransportCongestionControlServer.hpp @@ -60,7 +60,7 @@ namespace RTC void SendTransportCcFeedback(); void MayDropOldPacketArrivalTimes(uint16_t seqNum, uint64_t nowMs); void FillAndSendTransportCcFeedback(); - void MaySendLimitationRembFeedback(); + void MaySendLimitationRembFeedback(uint64_t nowMs); void UpdatePacketLoss(double packetLoss); /* Pure virtual methods inherited from webrtc::RemoteBitrateEstimator::Listener. */ diff --git a/worker/src/RTC/TransportCongestionControlServer.cpp b/worker/src/RTC/TransportCongestionControlServer.cpp index ed515dceca..e7a61095f1 100644 --- a/worker/src/RTC/TransportCongestionControlServer.cpp +++ b/worker/src/RTC/TransportCongestionControlServer.cpp @@ -149,7 +149,7 @@ namespace RTC this->transportCcFeedbackSenderSsrc = 0u; this->transportCcFeedbackMediaSsrc = packet->GetSsrc(); - MaySendLimitationRembFeedback(); + MaySendLimitationRembFeedback(nowMs); break; } @@ -243,7 +243,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); } } @@ -295,6 +297,8 @@ namespace RTC { MS_TRACE(); + // Ignore nowMs value if it's smaller than 500 in order to avoid negative values + // (Should never happen) and return early if the condition is met. if (nowMs >= PacketArrivalTimestampWindow) { auto expiryTimestamp = nowMs - PacketArrivalTimestampWindow; @@ -310,12 +314,10 @@ namespace RTC } } - 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) { From 9e69ee03ad1c48c8da25f411e74d507110057025 Mon Sep 17 00:00:00 2001 From: penguinol Date: Thu, 18 Jan 2024 15:57:58 +0800 Subject: [PATCH 09/27] fix tcc feedback fix situation of [1 2 4 5] [3 6 7] --- .../include/RTC/RTCP/FeedbackRtpTransport.hpp | 1 + .../RTC/TransportCongestionControlServer.hpp | 2 +- worker/src/RTC/RTCP/FeedbackRtpTransport.cpp | 20 +++-- .../RTC/TransportCongestionControlServer.cpp | 78 ++++++++++--------- .../src/RTC/RTCP/TestFeedbackRtpTransport.cpp | 55 +++++++++++-- 5 files changed, 102 insertions(+), 54 deletions(-) diff --git a/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp b/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp index 6c4398664d..170714efb7 100644 --- a/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp +++ b/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp @@ -213,6 +213,7 @@ namespace RTC public: AddPacketResult AddPacket(uint16_t sequenceNumber, uint64_t timestamp, size_t maxRtcpPacketLen); + void SetBase(uint16_t sequenceNumber, uint64_t timestamp); // Just for locally generated packets. void Finish(); bool IsFull() diff --git a/worker/include/RTC/TransportCongestionControlServer.hpp b/worker/include/RTC/TransportCongestionControlServer.hpp index 1937afd824..96e25aaba0 100644 --- a/worker/include/RTC/TransportCongestionControlServer.hpp +++ b/worker/include/RTC/TransportCongestionControlServer.hpp @@ -55,11 +55,11 @@ 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 MayDropOldPacketArrivalTimes(uint16_t seqNum, uint64_t nowMs); - void FillAndSendTransportCcFeedback(); void MaySendLimitationRembFeedback(uint64_t nowMs); void UpdatePacketLoss(double packetLoss); diff --git a/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp b/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp index 0e41548ada..0d6f384736 100644 --- a/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp +++ b/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp @@ -286,17 +286,6 @@ namespace RTC 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((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. @@ -373,6 +362,15 @@ namespace RTC return AddPacketResult::SUCCESS; } + void FeedbackRtpTransportPacket::SetBase(uint16_t sequenceNumber, uint64_t timestamp) + { + // Let's see if we must set our base. + this->baseSequenceNumber = sequenceNumber; + this->referenceTime = static_cast((timestamp & 0x1FFFFFC0) / 64); + this->latestSequenceNumber = sequenceNumber - 1; + this->latestTimestamp = (timestamp >> 6) * 64; // IMPORTANT: Loose precision. + } + void FeedbackRtpTransportPacket::Finish() { MS_TRACE(); diff --git a/worker/src/RTC/TransportCongestionControlServer.cpp b/worker/src/RTC/TransportCongestionControlServer.cpp index 3c5cc54b1e..202565d3a3 100644 --- a/worker/src/RTC/TransportCongestionControlServer.cpp +++ b/worker/src/RTC/TransportCongestionControlServer.cpp @@ -178,56 +178,62 @@ namespace RTC inline void TransportCongestionControlServer::FillAndSendTransportCcFeedback() { MS_TRACE(); + + auto it = this->mapPacketArrivalTimes.lower_bound(this->transportCcFeedbackStartSeqNum); - for (auto it = this->mapPacketArrivalTimes.find(this->transportCcFeedbackStartSeqNum); - it != this->mapPacketArrivalTimes.end(); - ++it) + if (it != this->mapPacketArrivalTimes.end()) { - auto result = - this->transportCcFeedbackPacket->AddPacket(it->first, it->second, this->maxRtcpPacketLen); + // set base sequence num and reference time + this->transportCcFeedbackPacket->SetBase(this->transportCcFeedbackStartSeqNum, it->second); - switch (result) + for (; it != this->mapPacketArrivalTimes.end(); ++it) { - case RTC::RTCP::FeedbackRtpTransportPacket::AddPacketResult::SUCCESS: + auto result = + this->transportCcFeedbackPacket->AddPacket(it->first, it->second, this->maxRtcpPacketLen); + + switch (result) { - // If the feedback packet is full, send it now. - if (this->transportCcFeedbackPacket->IsFull()) + case RTC::RTCP::FeedbackRtpTransportPacket::AddPacketResult::SUCCESS: { - MS_DEBUG_DEV("transport-cc feedback packet is full, sending feedback now"); + // 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(); - } + SendTransportCcFeedback(); + } - break; - } + break; + } - case RTC::RTCP::FeedbackRtpTransportPacket::AddPacketResult::MAX_SIZE_EXCEEDED: - { - // This should not happen - MS_DEBUG_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::MAX_SIZE_EXCEEDED: + { + // This should not happen + MS_DEBUG_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)); + 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); + // Use current packet count. + // NOTE: Do not increment it since the previous ongoing feedback + // packet was not sent. + this->transportCcFeedbackPacket->SetFeedbackPacketCount( + this->transportCcFeedbackPacketCount); - break; + break; + } } } - } - SendTransportCcFeedback(); + SendTransportCcFeedback(); + } } void TransportCongestionControlServer::SetMaxIncomingBitrate(uint32_t bitrate) @@ -289,7 +295,7 @@ namespace RTC // Increment packet count. this->transportCcFeedbackPacket->SetFeedbackPacketCount(++this->transportCcFeedbackPacketCount); - this->transportCcFeedbackStartSeqNum = latestWideSeqNumber; + this->transportCcFeedbackStartSeqNum = latestWideSeqNumber + 1; } inline void TransportCongestionControlServer::MayDropOldPacketArrivalTimes( diff --git a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp index e34c99ae8e..51acedaf56 100644 --- a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp +++ b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp @@ -98,7 +98,14 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" for (auto& input : inputs) { - packet->AddPacket(input.sequenceNumber, input.timestamp, input.maxPacketSize); + if (&input == &inputs.front()) + { + packet->SetBase(input.sequenceNumber + 1, input.timestamp); + } + else + { + packet->AddPacket(input.sequenceNumber, input.timestamp, input.maxPacketSize); + } } REQUIRE(packet->GetLatestSequenceNumber() == 1013); @@ -173,7 +180,14 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" for (auto& input : inputs) { - packet->AddPacket(input.sequenceNumber, input.timestamp, input.maxPacketSize); + if (&input == &inputs.front()) + { + packet->SetBase(input.sequenceNumber + 1, input.timestamp); + } + else + { + packet->AddPacket(input.sequenceNumber, input.timestamp, input.maxPacketSize); + } } packet->Finish(); @@ -236,9 +250,17 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" packet->SetFeedbackPacketCount(1); + for (auto& input : inputs) { - packet->AddPacket(input.sequenceNumber, input.timestamp, input.maxPacketSize); + if (&input == &inputs.front()) + { + packet->SetBase(input.sequenceNumber + 1, input.timestamp); + } + else + { + packet->AddPacket(input.sequenceNumber, input.timestamp, input.maxPacketSize); + } } packet->Finish(); @@ -296,7 +318,14 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" for (auto& input : inputs) { - packet->AddPacket(input.sequenceNumber, input.timestamp, input.maxPacketSize); + if (&input == &inputs.front()) + { + packet->SetBase(input.sequenceNumber + 1, input.timestamp); + } + else + { + packet->AddPacket(input.sequenceNumber, input.timestamp, input.maxPacketSize); + } } packet->Finish(); @@ -363,7 +392,14 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" for (auto& input : inputs) { - packet->AddPacket(input.sequenceNumber, input.timestamp, input.maxPacketSize); + if (&input == &inputs.front()) + { + packet->SetBase(input.sequenceNumber + 1, input.timestamp); + } + else + { + packet->AddPacket(input.sequenceNumber, input.timestamp, input.maxPacketSize); + } } packet->Finish(); @@ -424,7 +460,14 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" for (auto& input : inputs2) { - packet2->AddPacket(input.sequenceNumber, input.timestamp, input.maxPacketSize); + if (&input == &inputs2.front()) + { + packet2->SetBase(input.sequenceNumber + 1, input.timestamp); + } + else + { + packet2->AddPacket(input.sequenceNumber, input.timestamp, input.maxPacketSize); + } } packet2->Finish(); From ba0b2b8c65a7f1ce996e12bef0aebc9a852dcab1 Mon Sep 17 00:00:00 2001 From: penguinol Date: Thu, 18 Jan 2024 16:15:10 +0800 Subject: [PATCH 10/27] cosmetic --- worker/src/RTC/TransportCongestionControlServer.cpp | 2 +- worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/worker/src/RTC/TransportCongestionControlServer.cpp b/worker/src/RTC/TransportCongestionControlServer.cpp index 202565d3a3..d7d0ee8019 100644 --- a/worker/src/RTC/TransportCongestionControlServer.cpp +++ b/worker/src/RTC/TransportCongestionControlServer.cpp @@ -178,7 +178,7 @@ namespace RTC inline void TransportCongestionControlServer::FillAndSendTransportCcFeedback() { MS_TRACE(); - + auto it = this->mapPacketArrivalTimes.lower_bound(this->transportCcFeedbackStartSeqNum); if (it != this->mapPacketArrivalTimes.end()) diff --git a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp index 51acedaf56..2d1cd3e2e9 100644 --- a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp +++ b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp @@ -247,9 +247,8 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" /* clang-format on */ auto* packet = new FeedbackRtpTransportPacket(senderSsrc, mediaSsrc); - + packet->SetFeedbackPacketCount(1); - for (auto& input : inputs) { From 66d435a1535a2e24e9288b8f33af2f8d0ff07955 Mon Sep 17 00:00:00 2001 From: penguinol Date: Thu, 18 Jan 2024 17:35:33 +0800 Subject: [PATCH 11/27] Update TestFeedbackRtpTransport.cpp --- worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp index 2d1cd3e2e9..70b59b3d11 100644 --- a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp +++ b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp @@ -247,9 +247,9 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" /* clang-format on */ auto* packet = new FeedbackRtpTransportPacket(senderSsrc, mediaSsrc); - + packet->SetFeedbackPacketCount(1); - + for (auto& input : inputs) { if (&input == &inputs.front()) From 3ac5a059b2a91107fb2b1461e0491a1eec9e4280 Mon Sep 17 00:00:00 2001 From: penguinol Date: Tue, 23 Jan 2024 19:35:18 +0800 Subject: [PATCH 12/27] optimize --- .../src/RTC/TransportCongestionControlServer.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/worker/src/RTC/TransportCongestionControlServer.cpp b/worker/src/RTC/TransportCongestionControlServer.cpp index d7d0ee8019..68632158e1 100644 --- a/worker/src/RTC/TransportCongestionControlServer.cpp +++ b/worker/src/RTC/TransportCongestionControlServer.cpp @@ -125,10 +125,15 @@ namespace RTC break; } + // Only insert the packet when receiving it for the first time. + if (!this->mapPacketArrivalTimes.try_emplace(wideSeqNumber, nowMs).second) + { + break; + } + // 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->receivedTransportWideSeqNumber || RTC::SeqManager::IsSeqLowerThan( @@ -138,12 +143,7 @@ namespace RTC } this->receivedTransportWideSeqNumber = true; - auto result = this->mapPacketArrivalTimes.emplace(wideSeqNumber, nowMs); - - if (result.second) - { - MayDropOldPacketArrivalTimes(wideSeqNumber, nowMs); - } + MayDropOldPacketArrivalTimes(wideSeqNumber, nowMs); // Update the RTCP media SSRC of the ongoing Transport-CC Feedback packet. this->transportCcFeedbackSenderSsrc = 0u; From 8bf5758a6189b7f26b8cd5ed65a6085e1b1265f2 Mon Sep 17 00:00:00 2001 From: penguinol Date: Wed, 24 Jan 2024 15:51:49 +0800 Subject: [PATCH 13/27] add test case --- worker/meson.build | 1 + .../TestTransportCongestionControlServer.cpp | 250 ++++++++++++++++++ 2 files changed, 251 insertions(+) create mode 100644 worker/test/src/RTC/TestTransportCongestionControlServer.cpp diff --git a/worker/meson.build b/worker/meson.build index e2c294ce9f..77125be6e0 100644 --- a/worker/meson.build +++ b/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', diff --git a/worker/test/src/RTC/TestTransportCongestionControlServer.cpp b/worker/test/src/RTC/TestTransportCongestionControlServer.cpp new file mode 100644 index 0000000000..0f08646443 --- /dev/null +++ b/worker/test/src/RTC/TestTransportCongestionControlServer.cpp @@ -0,0 +1,250 @@ +#include "common.hpp" +#include "DepLibUV.hpp" +#include "RTC/TransportCongestionControlServer.hpp" +#include + +using namespace RTC; + +struct TestTransportCongestionControlServerInput +{ + uint16_t wideSeqNumber; + uint64_t nowMs; +}; + +struct TestTransportCongestionControlServerResult +{ + uint16_t wideSeqNumber; + bool received; + uint64_t timestamp; +}; + +using TestResults = std::deque>; + +class TestTransportCongestionControlServerListener : public TransportCongestionControlServer::Listener +{ +public: + virtual void OnTransportCongestionControlServerSendRtcpPacket( + RTC::TransportCongestionControlServer* tccServer, RTC::RTCP::Packet* packet) override + { + auto* tccPacket = dynamic_cast(packet); + if (tccPacket == nullptr) + { + return; + } + + auto packetResults = tccPacket->GetPacketResults(); + + REQUIRE(!this->results.empty()); + + auto testResults = this->results.front(); + this->results.pop_front(); + + REQUIRE(testResults.size() == packetResults.size()); + + auto packetResultIt = packetResults.begin(); + auto testResultIt = testResults.begin(); + for (; packetResultIt != packetResults.end() && testResultIt != testResults.end(); + ++packetResultIt, ++testResultIt) + { + REQUIRE(packetResultIt->sequenceNumber == testResultIt->wideSeqNumber); + REQUIRE(packetResultIt->received == testResultIt->received); + if (packetResultIt->received) + { + REQUIRE(packetResultIt->receivedAtMs == testResultIt->timestamp); + } + } + } + +public: + void SetResults(TestResults& results) + { + this->results = results; + } + + void Check() + { + REQUIRE(this->results.empty()); + } + +private: + TestResults results; +}; + +// clang-format off +uint8_t buffer[] = +{ + 0x90, 0x01, 0x00, 0x01, + 0x00, 0x00, 0x00, 0x04, + 0x00, 0x00, 0x00, 0x05, + 0xbe, 0xde, 0x00, 0x01, // Header Extensions + 0x51, 0x60, 0xee, 0x00 // TCC Feedback +}; +// clang-format on + +void validate(std::vector& inputs, TestResults& results) +{ + TestTransportCongestionControlServerListener listener; + auto tccServer = + TransportCongestionControlServer(&listener, RTC::BweType::TRANSPORT_CC, RTC::MtuSize); + tccServer.SetMaxIncomingBitrate(150000); + tccServer.TransportConnected(); + + RtpPacket* packet = RtpPacket::Parse(buffer, sizeof(buffer)); + packet->SetTransportWideCc01ExtensionId(5); + packet->SetSequenceNumber(1); + + // save results + listener.SetResults(results); + + uint64_t startTs = inputs[0].nowMs; + uint64_t TransportCcFeedbackSendInterval{ 100u }; // In ms. + + for (auto input : inputs) + { + // Periodic sending TCC packets + uint64_t diffTs = input.nowMs - startTs; + if (diffTs >= TransportCcFeedbackSendInterval) + { + tccServer.FillAndSendTransportCcFeedback(); + startTs = input.nowMs; + } + + packet->UpdateTransportWideCc01(input.wideSeqNumber); + tccServer.IncomingPacket(input.nowMs, packet); + } + + tccServer.FillAndSendTransportCcFeedback(); + + listener.Check(); +}; + +SCENARIO("TransportCongestionControlServer", "[rtc]") +{ + SECTION("Normal time and sequence") + { + // clang-format off + std::vector inputs + { + { 1u, 1000u }, + { 2u, 1050u }, + { 3u, 1100u }, + { 4u, 1150u }, + { 5u, 1200u }, + }; + + TestResults results + { + { + { 1u, true, 1000u }, + { 2u, true, 1050u }, + }, + { + { 3u, true, 1100u }, + { 4u, true, 1150u }, + }, + { + { 5u, true, 1200u }, + }, + }; + // clang-format on + + validate(inputs, results); + } + + SECTION("Lost packets") + { + // clang-format off + std::vector inputs + { + { 1u, 1000u }, + { 3u, 1050u }, + { 5u, 1100u }, + { 6u, 1150u }, + }; + + TestResults results + { + { + { 1u, true, 1000u }, + { 2u, false, 0u }, + { 3u, true, 1050u }, + }, + { + { 4u, false, 0u }, + { 5u, true, 1100u }, + { 6u, true, 1150u }, + }, + }; + // clang-format on + + validate(inputs, results); + } + + SECTION("Duplicate packets") + { + // clang-format off + std::vector inputs + { + { 1u, 1000u }, + { 1u, 1050u }, + { 2u, 1100u }, + { 3u, 1150u }, + { 3u, 1200u }, + { 4u, 1250u }, + }; + + TestResults results + { + { + { 1u, true, 1000u }, + }, + { + { 2u, true, 1100u }, + { 3u, true, 1150u }, + }, + { + { 3u, true, 1150u }, + { 4u, true, 1250u }, + }, + }; + // clang-format on + + validate(inputs, results); + } + + SECTION("Packets arrive out of order") + { + // clang-format off + std::vector inputs + { + { 1u, 1000u }, + { 2u, 1050u }, + { 4u, 1100u }, + { 5u, 1150u }, + { 3u, 1200u }, // Out of order + { 6u, 1250u }, + }; + + TestResults results + { + { + { 1u, true, 1000u }, + { 2u, true, 1050u }, + }, + { + { 3u, false, 0u }, + { 4u, true, 1100u }, + { 5u, true, 1150u }, + }, + { + { 3u, true, 1200u }, + { 4u, true, 1100u }, + { 5u, true, 1150u }, + { 6u, true, 1250u }, + }, + }; + // clang-format on + + validate(inputs, results); + } +} From 3d3a0549f3f4754f1e9dc3d08d678914a794d24d Mon Sep 17 00:00:00 2001 From: penguinol Date: Wed, 24 Jan 2024 16:15:31 +0800 Subject: [PATCH 14/27] try fix compile error --- worker/src/RTC/TransportCongestionControlServer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/worker/src/RTC/TransportCongestionControlServer.cpp b/worker/src/RTC/TransportCongestionControlServer.cpp index 68632158e1..b5f3a96615 100644 --- a/worker/src/RTC/TransportCongestionControlServer.cpp +++ b/worker/src/RTC/TransportCongestionControlServer.cpp @@ -175,7 +175,7 @@ namespace RTC } } - inline void TransportCongestionControlServer::FillAndSendTransportCcFeedback() + void TransportCongestionControlServer::FillAndSendTransportCcFeedback() { MS_TRACE(); From 9ef5cf0e749148663d3a50026d705c43b1dd5ab6 Mon Sep 17 00:00:00 2001 From: penguinol Date: Wed, 24 Jan 2024 16:47:33 +0800 Subject: [PATCH 15/27] fix test case --- worker/test/src/RTC/TestTransportCongestionControlServer.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/worker/test/src/RTC/TestTransportCongestionControlServer.cpp b/worker/test/src/RTC/TestTransportCongestionControlServer.cpp index 0f08646443..720525a28a 100644 --- a/worker/test/src/RTC/TestTransportCongestionControlServer.cpp +++ b/worker/test/src/RTC/TestTransportCongestionControlServer.cpp @@ -203,7 +203,6 @@ SCENARIO("TransportCongestionControlServer", "[rtc]") { 3u, true, 1150u }, }, { - { 3u, true, 1150u }, { 4u, true, 1250u }, }, }; From fd441650a97288e2dd8c1db63ebe7d0501a0f17c Mon Sep 17 00:00:00 2001 From: Junlong Zou Date: Tue, 30 Jan 2024 19:32:45 +0800 Subject: [PATCH 16/27] Update worker/src/RTC/RTCP/FeedbackRtpTransport.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Iñaki Baz Castillo --- worker/src/RTC/RTCP/FeedbackRtpTransport.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp b/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp index 0d6f384736..f8224cfb6b 100644 --- a/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp +++ b/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp @@ -364,7 +364,6 @@ namespace RTC void FeedbackRtpTransportPacket::SetBase(uint16_t sequenceNumber, uint64_t timestamp) { - // Let's see if we must set our base. this->baseSequenceNumber = sequenceNumber; this->referenceTime = static_cast((timestamp & 0x1FFFFFC0) / 64); this->latestSequenceNumber = sequenceNumber - 1; From 381b88982f1e0685af4fa521a4629038d0d07020 Mon Sep 17 00:00:00 2001 From: Junlong Zou Date: Tue, 30 Jan 2024 19:32:51 +0800 Subject: [PATCH 17/27] Update worker/src/RTC/TransportCongestionControlServer.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Iñaki Baz Castillo --- worker/src/RTC/TransportCongestionControlServer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/worker/src/RTC/TransportCongestionControlServer.cpp b/worker/src/RTC/TransportCongestionControlServer.cpp index b5f3a96615..b27be9c934 100644 --- a/worker/src/RTC/TransportCongestionControlServer.cpp +++ b/worker/src/RTC/TransportCongestionControlServer.cpp @@ -141,6 +141,7 @@ namespace RTC { this->transportCcFeedbackStartSeqNum = wideSeqNumber; } + this->receivedTransportWideSeqNumber = true; MayDropOldPacketArrivalTimes(wideSeqNumber, nowMs); From 882e6d9fb8c278a44cb12986240f2f92a1a74488 Mon Sep 17 00:00:00 2001 From: Junlong Zou Date: Tue, 30 Jan 2024 19:32:59 +0800 Subject: [PATCH 18/27] Update worker/src/RTC/TransportCongestionControlServer.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Iñaki Baz Castillo --- worker/src/RTC/TransportCongestionControlServer.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/worker/src/RTC/TransportCongestionControlServer.cpp b/worker/src/RTC/TransportCongestionControlServer.cpp index b27be9c934..12325a60bb 100644 --- a/worker/src/RTC/TransportCongestionControlServer.cpp +++ b/worker/src/RTC/TransportCongestionControlServer.cpp @@ -209,8 +209,9 @@ namespace RTC case RTC::RTCP::FeedbackRtpTransportPacket::AddPacketResult::MAX_SIZE_EXCEEDED: { - // This should not happen - MS_DEBUG_DEV("transport-cc feedback packet is 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)); From b6a9729aa0b522322b31bebe9362156f34f3e449 Mon Sep 17 00:00:00 2001 From: Junlong Zou Date: Tue, 30 Jan 2024 20:02:01 +0800 Subject: [PATCH 19/27] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Iñaki Baz Castillo --- worker/src/RTC/TransportCongestionControlServer.cpp | 7 ++++--- .../RTC/TestTransportCongestionControlServer.cpp | 13 +++++++++---- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/worker/src/RTC/TransportCongestionControlServer.cpp b/worker/src/RTC/TransportCongestionControlServer.cpp index 12325a60bb..cb6af0f9d1 100644 --- a/worker/src/RTC/TransportCongestionControlServer.cpp +++ b/worker/src/RTC/TransportCongestionControlServer.cpp @@ -184,7 +184,7 @@ namespace RTC if (it != this->mapPacketArrivalTimes.end()) { - // set base sequence num and reference time + // Set base sequence num and reference time. this->transportCcFeedbackPacket->SetBase(this->transportCcFeedbackStartSeqNum, it->second); for (; it != this->mapPacketArrivalTimes.end(); ++it) @@ -305,8 +305,9 @@ namespace RTC { MS_TRACE(); - // Ignore nowMs value if it's smaller than 500 in order to avoid negative values - // (Should never happen) and return early if the condition is met. + // 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) { auto expiryTimestamp = nowMs - PacketArrivalTimestampWindow; diff --git a/worker/test/src/RTC/TestTransportCongestionControlServer.cpp b/worker/test/src/RTC/TestTransportCongestionControlServer.cpp index 720525a28a..e5cbc42513 100644 --- a/worker/test/src/RTC/TestTransportCongestionControlServer.cpp +++ b/worker/test/src/RTC/TestTransportCongestionControlServer.cpp @@ -27,7 +27,8 @@ class TestTransportCongestionControlServerListener : public TransportCongestionC RTC::TransportCongestionControlServer* tccServer, RTC::RTCP::Packet* packet) override { auto* tccPacket = dynamic_cast(packet); - if (tccPacket == nullptr) + + if (!tccPacket) { return; } @@ -43,11 +44,13 @@ class TestTransportCongestionControlServerListener : public TransportCongestionC auto packetResultIt = packetResults.begin(); auto testResultIt = testResults.begin(); + for (; packetResultIt != packetResults.end() && testResultIt != testResults.end(); ++packetResultIt, ++testResultIt) { REQUIRE(packetResultIt->sequenceNumber == testResultIt->wideSeqNumber); REQUIRE(packetResultIt->received == testResultIt->received); + if (packetResultIt->received) { REQUIRE(packetResultIt->receivedAtMs == testResultIt->timestamp); @@ -86,14 +89,16 @@ void validate(std::vector& inputs, Te TestTransportCongestionControlServerListener listener; auto tccServer = TransportCongestionControlServer(&listener, RTC::BweType::TRANSPORT_CC, RTC::MtuSize); + tccServer.SetMaxIncomingBitrate(150000); tccServer.TransportConnected(); RtpPacket* packet = RtpPacket::Parse(buffer, sizeof(buffer)); + packet->SetTransportWideCc01ExtensionId(5); packet->SetSequenceNumber(1); - // save results + // Save results. listener.SetResults(results); uint64_t startTs = inputs[0].nowMs; @@ -101,8 +106,9 @@ void validate(std::vector& inputs, Te for (auto input : inputs) { - // Periodic sending TCC packets + // Periodic sending TCC packets. uint64_t diffTs = input.nowMs - startTs; + if (diffTs >= TransportCcFeedbackSendInterval) { tccServer.FillAndSendTransportCcFeedback(); @@ -114,7 +120,6 @@ void validate(std::vector& inputs, Te } tccServer.FillAndSendTransportCcFeedback(); - listener.Check(); }; From 096e48f32bd7c4e8bef5bf06e2057c205ef6d3a9 Mon Sep 17 00:00:00 2001 From: penguinol Date: Tue, 30 Jan 2024 20:08:29 +0800 Subject: [PATCH 20/27] Update TestFeedbackRtpTransport.cpp use std::addressof --- .../test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp index 70b59b3d11..0204f81480 100644 --- a/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp +++ b/worker/test/src/RTC/RTCP/TestFeedbackRtpTransport.cpp @@ -98,7 +98,7 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" for (auto& input : inputs) { - if (&input == &inputs.front()) + if (std::addressof(input) == std::addressof(inputs.front())) { packet->SetBase(input.sequenceNumber + 1, input.timestamp); } @@ -180,7 +180,7 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" for (auto& input : inputs) { - if (&input == &inputs.front()) + if (std::addressof(input) == std::addressof(inputs.front())) { packet->SetBase(input.sequenceNumber + 1, input.timestamp); } @@ -252,7 +252,7 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" for (auto& input : inputs) { - if (&input == &inputs.front()) + if (std::addressof(input) == std::addressof(inputs.front())) { packet->SetBase(input.sequenceNumber + 1, input.timestamp); } @@ -317,7 +317,7 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" for (auto& input : inputs) { - if (&input == &inputs.front()) + if (std::addressof(input) == std::addressof(inputs.front())) { packet->SetBase(input.sequenceNumber + 1, input.timestamp); } @@ -391,7 +391,7 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" for (auto& input : inputs) { - if (&input == &inputs.front()) + if (std::addressof(input) == std::addressof(inputs.front())) { packet->SetBase(input.sequenceNumber + 1, input.timestamp); } @@ -459,7 +459,7 @@ SCENARIO("RTCP Feeback RTP transport", "[parser][rtcp][feedback-rtp][transport]" for (auto& input : inputs2) { - if (&input == &inputs2.front()) + if (std::addressof(input) == std::addressof(inputs2.front())) { packet2->SetBase(input.sequenceNumber + 1, input.timestamp); } From 2bc46be4d850fbb4136b435f1ee36f15e083957f Mon Sep 17 00:00:00 2001 From: penguinol Date: Wed, 31 Jan 2024 14:43:25 +0800 Subject: [PATCH 21/27] add baseSet and other optimize --- .../include/RTC/RTCP/FeedbackRtpTransport.hpp | 4 +++- .../RTC/TransportCongestionControlServer.hpp | 5 ++-- worker/src/RTC/RTCP/FeedbackRtpTransport.cpp | 18 ++++++++------- .../RTC/TransportCongestionControlServer.cpp | 23 +++++++++++-------- 4 files changed, 30 insertions(+), 20 deletions(-) diff --git a/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp b/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp index 170714efb7..10ff223018 100644 --- a/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp +++ b/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp @@ -212,8 +212,8 @@ namespace RTC ~FeedbackRtpTransportPacket(); public: - AddPacketResult AddPacket(uint16_t sequenceNumber, uint64_t timestamp, size_t maxRtcpPacketLen); 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(); bool IsFull() @@ -318,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 }; diff --git a/worker/include/RTC/TransportCongestionControlServer.hpp b/worker/include/RTC/TransportCongestionControlServer.hpp index 96e25aaba0..ceca227694 100644 --- a/worker/include/RTC/TransportCongestionControlServer.hpp +++ b/worker/include/RTC/TransportCongestionControlServer.hpp @@ -92,8 +92,9 @@ namespace RTC uint8_t unlimitedRembCounter{ 0u }; std::deque packetLossHistory; double packetLoss{ 0 }; - bool receivedTransportWideSeqNumber{ false }; - uint16_t transportCcFeedbackStartSeqNum{ 0u }; + // Whether received any packet with transport wide sequence number. + bool transportWideSeqNumberReceived{ false }; + uint16_t transportCcFeedbackWideSeqNumStart{ 0u }; std::map::SeqLowerThan> mapPacketArrivalTimes; }; } // namespace RTC diff --git a/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp b/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp index f8224cfb6b..1482622441 100644 --- a/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp +++ b/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp @@ -279,11 +279,21 @@ namespace RTC return offset; } + void FeedbackRtpTransportPacket::SetBase(uint16_t sequenceNumber, uint64_t timestamp) + { + this->baseSet = true; + this->baseSequenceNumber = sequenceNumber; + this->referenceTime = static_cast((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"); // If the wide sequence number of the new packet is lower than the latest seen, @@ -362,14 +372,6 @@ namespace RTC return AddPacketResult::SUCCESS; } - void FeedbackRtpTransportPacket::SetBase(uint16_t sequenceNumber, uint64_t timestamp) - { - this->baseSequenceNumber = sequenceNumber; - this->referenceTime = static_cast((timestamp & 0x1FFFFFC0) / 64); - this->latestSequenceNumber = sequenceNumber - 1; - this->latestTimestamp = (timestamp >> 6) * 64; // IMPORTANT: Loose precision. - } - void FeedbackRtpTransportPacket::Finish() { MS_TRACE(); diff --git a/worker/src/RTC/TransportCongestionControlServer.cpp b/worker/src/RTC/TransportCongestionControlServer.cpp index cb6af0f9d1..6de6b7571f 100644 --- a/worker/src/RTC/TransportCongestionControlServer.cpp +++ b/worker/src/RTC/TransportCongestionControlServer.cpp @@ -135,14 +135,14 @@ namespace RTC // 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->receivedTransportWideSeqNumber || + !this->transportWideSeqNumberReceived || RTC::SeqManager::IsSeqLowerThan( - wideSeqNumber, this->transportCcFeedbackStartSeqNum)) + wideSeqNumber, this->transportCcFeedbackWideSeqNumStart)) { - this->transportCcFeedbackStartSeqNum = wideSeqNumber; + this->transportCcFeedbackWideSeqNumStart = wideSeqNumber; } - this->receivedTransportWideSeqNumber = true; + this->transportWideSeqNumberReceived = true; MayDropOldPacketArrivalTimes(wideSeqNumber, nowMs); @@ -180,12 +180,17 @@ namespace RTC { MS_TRACE(); - auto it = this->mapPacketArrivalTimes.lower_bound(this->transportCcFeedbackStartSeqNum); + if (!this->transportWideSeqNumberReceived) + { + return; + } + + auto it = this->mapPacketArrivalTimes.lower_bound(this->transportCcFeedbackWideSeqNumStart); if (it != this->mapPacketArrivalTimes.end()) { - // Set base sequence num and reference time. - this->transportCcFeedbackPacket->SetBase(this->transportCcFeedbackStartSeqNum, it->second); + // Set base sequence num and reference time + this->transportCcFeedbackPacket->SetBase(this->transportCcFeedbackWideSeqNumStart, it->second); for (; it != this->mapPacketArrivalTimes.end(); ++it) { @@ -297,7 +302,7 @@ namespace RTC // Increment packet count. this->transportCcFeedbackPacket->SetFeedbackPacketCount(++this->transportCcFeedbackPacketCount); - this->transportCcFeedbackStartSeqNum = latestWideSeqNumber + 1; + this->transportCcFeedbackWideSeqNumStart = latestWideSeqNumber + 1; } inline void TransportCongestionControlServer::MayDropOldPacketArrivalTimes( @@ -314,7 +319,7 @@ namespace RTC auto it = this->mapPacketArrivalTimes.begin(); while (it != this->mapPacketArrivalTimes.end() && - it->first != this->transportCcFeedbackStartSeqNum && + it->first != this->transportCcFeedbackWideSeqNumStart && RTC::SeqManager::IsSeqLowerThan(it->first, seqNum) && it->second <= expiryTimestamp) { From e9aeb1689c718621d1b81562ce0d6362394a3120 Mon Sep 17 00:00:00 2001 From: penguinol Date: Wed, 31 Jan 2024 14:55:30 +0800 Subject: [PATCH 22/27] Update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cf8fffe86..2558432312 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +### NEXT + +- Make transport-cc feedback works similarly to webrtc ([PR #1318](https://github.com/versatica/mediasoup/pull/1088) by @penguinol). + ### 3.13.17 - Fix prebuilt worker download ([PR #1319](https://github.com/versatica/mediasoup/pull/1319) by @brynrichards). From ea3b19c372b0fcd7bb29f4ed5db441575ce7540d Mon Sep 17 00:00:00 2001 From: Junlong Zou Date: Wed, 31 Jan 2024 17:23:35 +0800 Subject: [PATCH 23/27] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Iñaki Baz Castillo --- CHANGELOG.md | 2 +- worker/include/RTC/RTCP/FeedbackRtpTransport.hpp | 2 +- worker/src/RTC/RTCP/FeedbackRtpTransport.cpp | 2 ++ .../src/RTC/TestTransportCongestionControlServer.cpp | 10 +++++----- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2558432312..cf5127e101 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ### NEXT -- Make transport-cc feedback works similarly to webrtc ([PR #1318](https://github.com/versatica/mediasoup/pull/1088) by @penguinol). +- Make transport-cc feedbac works similarly to webrtc ([PR #1088](https://github.com/versatica/mediasoup/pull/1088) by @penguinol). ### 3.13.17 diff --git a/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp b/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp index 5be615094e..901df0c46c 100644 --- a/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp +++ b/worker/include/RTC/RTCP/FeedbackRtpTransport.hpp @@ -318,7 +318,7 @@ namespace RTC void AddPendingChunks(); private: - // Whether baseSequenceNumber has been set + // Whether baseSequenceNumber has been set. bool baseSet{ false }; uint16_t baseSequenceNumber{ 0u }; // 24 bits signed integer. diff --git a/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp b/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp index 1b180eb33e..21f09c9e8f 100644 --- a/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp +++ b/worker/src/RTC/RTCP/FeedbackRtpTransport.cpp @@ -280,6 +280,8 @@ namespace RTC void FeedbackRtpTransportPacket::SetBase(uint16_t sequenceNumber, uint64_t timestamp) { + MS_TRACE(); + this->baseSet = true; this->baseSequenceNumber = sequenceNumber; this->referenceTime = static_cast((timestamp & 0x1FFFFFC0) / 64); diff --git a/worker/test/src/RTC/TestTransportCongestionControlServer.cpp b/worker/test/src/RTC/TestTransportCongestionControlServer.cpp index e5cbc42513..27512b2b37 100644 --- a/worker/test/src/RTC/TestTransportCongestionControlServer.cpp +++ b/worker/test/src/RTC/TestTransportCongestionControlServer.cpp @@ -123,9 +123,9 @@ void validate(std::vector& inputs, Te listener.Check(); }; -SCENARIO("TransportCongestionControlServer", "[rtc]") +SCENARIO("TransportCongestionControlServer", "[rtp]") { - SECTION("Normal time and sequence") + SECTION("normal time and sequence") { // clang-format off std::vector inputs @@ -156,7 +156,7 @@ SCENARIO("TransportCongestionControlServer", "[rtc]") validate(inputs, results); } - SECTION("Lost packets") + SECTION("lost packets") { // clang-format off std::vector inputs @@ -185,7 +185,7 @@ SCENARIO("TransportCongestionControlServer", "[rtc]") validate(inputs, results); } - SECTION("Duplicate packets") + SECTION("duplicate packets") { // clang-format off std::vector inputs @@ -216,7 +216,7 @@ SCENARIO("TransportCongestionControlServer", "[rtc]") validate(inputs, results); } - SECTION("Packets arrive out of order") + SECTION("packets arrive out of order") { // clang-format off std::vector inputs From 4bd33f3e370a590bf66368b71bff8a0e2d950450 Mon Sep 17 00:00:00 2001 From: Junlong Zou Date: Wed, 31 Jan 2024 17:43:51 +0800 Subject: [PATCH 24/27] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Luis Millán Co-authored-by: Iñaki Baz Castillo --- CHANGELOG.md | 2 +- worker/include/RTC/TransportCongestionControlServer.hpp | 2 +- worker/src/RTC/TransportCongestionControlServer.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf5127e101..929af4dccd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ### NEXT -- Make transport-cc feedbac works similarly to webrtc ([PR #1088](https://github.com/versatica/mediasoup/pull/1088) by @penguinol). +- Make transport-cc feedback works similarly to webrtc ([PR #1088](https://github.com/versatica/mediasoup/pull/1088) by @penguinol). ### 3.13.17 diff --git a/worker/include/RTC/TransportCongestionControlServer.hpp b/worker/include/RTC/TransportCongestionControlServer.hpp index 01fc553c18..5cfd676796 100644 --- a/worker/include/RTC/TransportCongestionControlServer.hpp +++ b/worker/include/RTC/TransportCongestionControlServer.hpp @@ -92,7 +92,7 @@ namespace RTC uint8_t unlimitedRembCounter{ 0u }; std::deque packetLossHistory; double packetLoss{ 0 }; - // Whether received any packet with transport wide sequence number. + // Whether any packet with transport wide sequence number was received. bool transportWideSeqNumberReceived{ false }; uint16_t transportCcFeedbackWideSeqNumStart{ 0u }; std::map::SeqLowerThan> mapPacketArrivalTimes; diff --git a/worker/src/RTC/TransportCongestionControlServer.cpp b/worker/src/RTC/TransportCongestionControlServer.cpp index 6de6b7571f..5eb36e97fd 100644 --- a/worker/src/RTC/TransportCongestionControlServer.cpp +++ b/worker/src/RTC/TransportCongestionControlServer.cpp @@ -189,7 +189,7 @@ namespace RTC if (it != this->mapPacketArrivalTimes.end()) { - // Set base sequence num and reference time + // Set base sequence num and reference time. this->transportCcFeedbackPacket->SetBase(this->transportCcFeedbackWideSeqNumStart, it->second); for (; it != this->mapPacketArrivalTimes.end(); ++it) From a885f4872740d3b7ec784decfab3624329edcb6c Mon Sep 17 00:00:00 2001 From: Junlong Zou Date: Thu, 1 Feb 2024 10:08:14 +0800 Subject: [PATCH 25/27] Update CHANGELOG.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Iñaki Baz Castillo --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 929af4dccd..54da1c430b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ### NEXT -- Make transport-cc feedback works similarly to webrtc ([PR #1088](https://github.com/versatica/mediasoup/pull/1088) by @penguinol). +- Make transport-cc feedback work similarly to libwebrtc ([PR #1088](https://github.com/versatica/mediasoup/pull/1088) by @penguinol). ### 3.13.17 From 4856fa7f9f1c1d118ece68af262707ad033b6d7f Mon Sep 17 00:00:00 2001 From: penguinol Date: Thu, 1 Feb 2024 10:18:27 +0800 Subject: [PATCH 26/27] Update TransportCongestionControlServer.cpp --- .../RTC/TransportCongestionControlServer.cpp | 80 ++++++++++--------- 1 file changed, 41 insertions(+), 39 deletions(-) diff --git a/worker/src/RTC/TransportCongestionControlServer.cpp b/worker/src/RTC/TransportCongestionControlServer.cpp index 5eb36e97fd..2dff6c6413 100644 --- a/worker/src/RTC/TransportCongestionControlServer.cpp +++ b/worker/src/RTC/TransportCongestionControlServer.cpp @@ -187,60 +187,62 @@ namespace RTC auto it = this->mapPacketArrivalTimes.lower_bound(this->transportCcFeedbackWideSeqNumStart); - if (it != this->mapPacketArrivalTimes.end()) + if (it == this->mapPacketArrivalTimes.end()) { - // Set base sequence num and reference time. - this->transportCcFeedbackPacket->SetBase(this->transportCcFeedbackWideSeqNumStart, it->second); + return; + } - for (; it != this->mapPacketArrivalTimes.end(); ++it) - { - auto result = - this->transportCcFeedbackPacket->AddPacket(it->first, it->second, this->maxRtcpPacketLen); + // 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) + switch (result) + { + case RTC::RTCP::FeedbackRtpTransportPacket::AddPacketResult::SUCCESS: { - case RTC::RTCP::FeedbackRtpTransportPacket::AddPacketResult::SUCCESS: + // If the feedback packet is full, send it now. + if (this->transportCcFeedbackPacket->IsFull()) { - // 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(); - } + MS_DEBUG_DEV("transport-cc feedback packet is full, sending feedback now"); - break; + SendTransportCcFeedback(); } - case RTC::RTCP::FeedbackRtpTransportPacket::AddPacketResult::MAX_SIZE_EXCEEDED: - { - // This should not happen. - MS_WARN_DEV("transport-cc feedback packet is exceeded"); + break; + } - // Create a new feedback packet. - this->transportCcFeedbackPacket.reset(new RTC::RTCP::FeedbackRtpTransportPacket( - this->transportCcFeedbackSenderSsrc, this->transportCcFeedbackMediaSsrc)); - } + case RTC::RTCP::FeedbackRtpTransportPacket::AddPacketResult::MAX_SIZE_EXCEEDED: + { + // This should not happen. + MS_WARN_DEV("transport-cc feedback packet is exceeded"); - case RTC::RTCP::FeedbackRtpTransportPacket::AddPacketResult::FATAL: - { - // Create a new feedback packet. - this->transportCcFeedbackPacket.reset(new RTC::RTCP::FeedbackRtpTransportPacket( - this->transportCcFeedbackSenderSsrc, this->transportCcFeedbackMediaSsrc)); + // 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); + // Use current packet count. + // NOTE: Do not increment it since the previous ongoing feedback + // packet was not sent. + this->transportCcFeedbackPacket->SetFeedbackPacketCount( + this->transportCcFeedbackPacketCount); - break; - } + break; } } - - SendTransportCcFeedback(); } + + SendTransportCcFeedback(); } void TransportCongestionControlServer::SetMaxIncomingBitrate(uint32_t bitrate) From 11378523ac8bc6d3df846e7944c674ef7626f34d Mon Sep 17 00:00:00 2001 From: penguinol Date: Thu, 1 Feb 2024 11:36:25 +0800 Subject: [PATCH 27/27] Update TransportCongestionControlServer.cpp fix format --- worker/src/RTC/TransportCongestionControlServer.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/worker/src/RTC/TransportCongestionControlServer.cpp b/worker/src/RTC/TransportCongestionControlServer.cpp index 2dff6c6413..6fe639e175 100644 --- a/worker/src/RTC/TransportCongestionControlServer.cpp +++ b/worker/src/RTC/TransportCongestionControlServer.cpp @@ -198,7 +198,7 @@ namespace RTC for (; it != this->mapPacketArrivalTimes.end(); ++it) { auto result = - this->transportCcFeedbackPacket->AddPacket(it->first, it->second, this->maxRtcpPacketLen); + this->transportCcFeedbackPacket->AddPacket(it->first, it->second, this->maxRtcpPacketLen); switch (result) { @@ -222,20 +222,20 @@ namespace RTC // Create a new feedback packet. this->transportCcFeedbackPacket.reset(new RTC::RTCP::FeedbackRtpTransportPacket( - this->transportCcFeedbackSenderSsrc, this->transportCcFeedbackMediaSsrc)); + 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)); + 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); + this->transportCcFeedbackPacketCount); break; }