From e9c2254b0d63ab06b2a16697a6f1486006bdfde8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?I=C3=B1aki=20Baz=20Castillo?= Date: Wed, 25 Jun 2025 19:05:14 +0200 Subject: [PATCH 1/3] Consumer: Only drop packets in RTP sequence manager when they belong to current spatial layer ## Details - Fixes #1547. --- CHANGELOG.md | 1 + worker/include/RTC/RtcLogger.hpp | 2 - worker/src/RTC/PipeConsumer.cpp | 38 ++++++------ worker/src/RTC/RtcLogger.cpp | 2 - worker/src/RTC/SimpleConsumer.cpp | 48 +++++++-------- worker/src/RTC/SimulcastConsumer.cpp | 89 +++++++++++++++++----------- worker/src/RTC/SvcConsumer.cpp | 22 ++++--- 7 files changed, 111 insertions(+), 91 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b3e32bce3..3c1d54b869 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - libuv: Update to v1.51.0 ([PR #1543](https://github.com/versatica/mediasoup/pull/1543)). - libsrtp: Update to v3.0.0-beta version in our fork ([PR #1544](https://github.com/versatica/mediasoup/pull/1544)). +- `XxxxConsumer.cpp`: Only drop packets in RTP sequence manager when they belong to current spatial layer ([PR #1549](https://github.com/versatica/mediasoup/pull/1549)). ### 3.16.0 diff --git a/worker/include/RTC/RtcLogger.hpp b/worker/include/RTC/RtcLogger.hpp index 334e9df552..6dbe07b1f9 100644 --- a/worker/include/RTC/RtcLogger.hpp +++ b/worker/include/RTC/RtcLogger.hpp @@ -23,10 +23,8 @@ namespace RTC NOT_A_KEYFRAME, EMPTY_PAYLOAD, SPATIAL_LAYER_MISMATCH, - TOO_HIGH_TIMESTAMP_EXTRA_NEEDED, PACKET_PREVIOUS_TO_SPATIAL_LAYER_SWITCH, DROPPED_BY_CODEC, - SEND_RTP_STREAM_DISCARDED, }; static absl::flat_hash_map dropReason2String; diff --git a/worker/src/RTC/PipeConsumer.cpp b/worker/src/RTC/PipeConsumer.cpp index 8baedfefa5..bc3e0d7f36 100644 --- a/worker/src/RTC/PipeConsumer.cpp +++ b/worker/src/RTC/PipeConsumer.cpp @@ -222,12 +222,30 @@ namespace RTC packet->logger.consumerId = this->id; #endif + auto ssrc = this->mapMappedSsrcSsrc.at(packet->GetSsrc()); + auto* rtpStream = this->mapSsrcRtpStream.at(ssrc); + auto& syncRequired = this->mapRtpStreamSyncRequired.at(rtpStream); + auto& rtpSeqManager = this->mapRtpStreamRtpSeqManager.at(rtpStream); + if (!IsActive()) { #ifdef MS_RTC_LOGGER_RTP packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::CONSUMER_INACTIVE); #endif + rtpSeqManager->Drop(packet->GetSequenceNumber()); + + return; + } + + // If we need to sync, support key frames and this is not a key frame, ignore + // the packet. + if (syncRequired && this->keyFrameSupported && !packet->IsKeyFrame()) + { + // NOTE: No need to drop the packet in the RTP sequence manager since here + // we are blocking all packets but the key frame that would trigger sync + // below. + return; } @@ -243,21 +261,7 @@ namespace RTC packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::UNSUPPORTED_PAYLOAD_TYPE); #endif - return; - } - - auto ssrc = this->mapMappedSsrcSsrc.at(packet->GetSsrc()); - auto* rtpStream = this->mapSsrcRtpStream.at(ssrc); - auto& syncRequired = this->mapRtpStreamSyncRequired.at(rtpStream); - auto& rtpSeqManager = this->mapRtpStreamRtpSeqManager.at(rtpStream); - - // If we need to sync, support key frames and this is not a key frame, ignore - // the packet. - if (syncRequired && this->keyFrameSupported && !packet->IsKeyFrame()) - { -#ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::NOT_A_KEYFRAME); -#endif + rtpSeqManager->Drop(packet->GetSequenceNumber()); return; } @@ -265,12 +269,12 @@ namespace RTC // Packets with only padding are not forwarded. if (packet->GetPayloadLength() == 0) { - rtpSeqManager->Drop(packet->GetSequenceNumber()); - #ifdef MS_RTC_LOGGER_RTP packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::EMPTY_PAYLOAD); #endif + rtpSeqManager->Drop(packet->GetSequenceNumber()); + return; } diff --git a/worker/src/RTC/RtcLogger.cpp b/worker/src/RTC/RtcLogger.cpp index 4237dcd90f..2cb6706e66 100644 --- a/worker/src/RTC/RtcLogger.cpp +++ b/worker/src/RTC/RtcLogger.cpp @@ -20,10 +20,8 @@ namespace RTC { RtpPacket::DropReason::NOT_A_KEYFRAME, "NotAKeyframe" }, { RtpPacket::DropReason::EMPTY_PAYLOAD, "EmptyPayload" }, { RtpPacket::DropReason::SPATIAL_LAYER_MISMATCH, "SpatialLayerMismatch" }, - { RtpPacket::DropReason::TOO_HIGH_TIMESTAMP_EXTRA_NEEDED, "TooHighTimestampExtraNeeded" }, { RtpPacket::DropReason::PACKET_PREVIOUS_TO_SPATIAL_LAYER_SWITCH, "PacketPreviousToSpatialLayerSwitch" }, { RtpPacket::DropReason::DROPPED_BY_CODEC, "DroppedByCodec" }, - { RtpPacket::DropReason::SEND_RTP_STREAM_DISCARDED, "SendRtpStreamDiscarded" }, }; // clang-format on diff --git a/worker/src/RTC/SimpleConsumer.cpp b/worker/src/RTC/SimpleConsumer.cpp index a6178796c3..2af59ef1d2 100644 --- a/worker/src/RTC/SimpleConsumer.cpp +++ b/worker/src/RTC/SimpleConsumer.cpp @@ -325,6 +325,17 @@ namespace RTC return; } + // If we need to sync, support key frames and this is not a key frame, ignore + // the packet. + if (this->syncRequired && this->keyFrameSupported && !packet->IsKeyFrame()) + { + // NOTE: No need to drop the packet in the RTP sequence manager since here + // we are blocking all packets but the key frame that would trigger sync + // below. + + return; + } + auto payloadType = packet->GetPayloadType(); // NOTE: This may happen if this Consumer supports just some codecs of those @@ -342,6 +353,18 @@ namespace RTC return; } + // Packets with only padding are not forwarded. + if (packet->GetPayloadLength() == 0) + { +#ifdef MS_RTC_LOGGER_RTP + packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::EMPTY_PAYLOAD); +#endif + + this->rtpSeqManager->Drop(packet->GetSequenceNumber()); + + return; + } + bool marker; // Process the payload if needed. Drop packet if necessary. @@ -362,31 +385,6 @@ namespace RTC return; } - // If we need to sync, support key frames and this is not a key frame, ignore - // the packet. - if (this->syncRequired && this->keyFrameSupported && !packet->IsKeyFrame()) - { -#ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::NOT_A_KEYFRAME); -#endif - - this->rtpSeqManager->Drop(packet->GetSequenceNumber()); - - return; - } - - // Packets with only padding are not forwarded. - if (packet->GetPayloadLength() == 0) - { -#ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::EMPTY_PAYLOAD); -#endif - - this->rtpSeqManager->Drop(packet->GetSequenceNumber()); - - return; - } - // Whether this is the first packet after re-sync. const bool isSyncPacket = this->syncRequired; diff --git a/worker/src/RTC/SimulcastConsumer.cpp b/worker/src/RTC/SimulcastConsumer.cpp index 49bd8965ca..e2ae2df53c 100644 --- a/worker/src/RTC/SimulcastConsumer.cpp +++ b/worker/src/RTC/SimulcastConsumer.cpp @@ -732,24 +732,36 @@ namespace RTC packet->logger.consumerId = this->id; #endif + auto spatialLayer = this->mapMappedSsrcSpatialLayer.at(packet->GetSsrc()); + if (!IsActive()) { + // Only drop the packet in the RTP sequence manager if it belongs to the + // current spatial layer. + if (spatialLayer == this->currentSpatialLayer) + { #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::CONSUMER_INACTIVE); + packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::CONSUMER_INACTIVE); #endif - this->rtpSeqManager->Drop(packet->GetSequenceNumber()); + this->rtpSeqManager->Drop(packet->GetSequenceNumber()); + } return; } if (this->targetTemporalLayer == -1) { + // Only drop the packet in the RTP sequence manager if it belongs to the + // current spatial layer. + if (spatialLayer == this->currentSpatialLayer) + { #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::INVALID_TARGET_LAYER); + packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::INVALID_TARGET_LAYER); #endif - this->rtpSeqManager->Drop(packet->GetSequenceNumber()); + this->rtpSeqManager->Drop(packet->GetSequenceNumber()); + } return; } @@ -760,16 +772,22 @@ namespace RTC // in the corresponding Producer. if (!this->supportedCodecPayloadTypes[payloadType]) { - MS_DEBUG_DEV("payload type not supported [payloadType:%" PRIu8 "]", payloadType); + // Only drop the packet in the RTP sequence manager if it belongs to the + // current spatial layer. + if (spatialLayer == this->currentSpatialLayer) + { + MS_DEBUG_DEV("payload type not supported [payloadType:%" PRIu8 "]", payloadType); #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::UNSUPPORTED_PAYLOAD_TYPE); + packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::UNSUPPORTED_PAYLOAD_TYPE); #endif + this->rtpSeqManager->Drop(packet->GetSequenceNumber()); + } + return; } - auto spatialLayer = this->mapMappedSsrcSpatialLayer.at(packet->GetSsrc()); bool shouldSwitchCurrentSpatialLayer{ false }; // Check whether this is the packet we are waiting for in order to update @@ -779,11 +797,8 @@ namespace RTC // Ignore if not a key frame. if (!packet->IsKeyFrame()) { -#ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::NOT_A_KEYFRAME); -#endif - - this->rtpSeqManager->Drop(packet->GetSequenceNumber()); + // NOTE: Don't drop the packet in the RTP sequence manager since this + // packet doesn't belong to the current spatial layer. return; } @@ -798,34 +813,44 @@ namespace RTC // drop it. else if (spatialLayer != this->currentSpatialLayer) { -#ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::SPATIAL_LAYER_MISMATCH); -#endif + // NOTE: Don't drop the packet in the RTP sequence manager since this + // packet doesn't belong to the current spatial layer. return; } // If we need to sync and this is not a key frame, ignore the packet. + // NOTE: syncRequired is true if packet is a key frame of the target spatial + // layer or if transport just connected or consumer resumed. if (this->syncRequired && !packet->IsKeyFrame()) { + // Only drop the packet in the RTP sequence manager if it belongs to the + // current spatial layer. + if (spatialLayer == this->currentSpatialLayer) + { #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::NOT_A_KEYFRAME); + packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::NOT_A_KEYFRAME); #endif - this->rtpSeqManager->Drop(packet->GetSequenceNumber()); + this->rtpSeqManager->Drop(packet->GetSequenceNumber()); + } return; } - // If the packet belongs to current spatial layer being sent and packet does - // not have payload other than padding, then drop it. - if (spatialLayer == this->currentSpatialLayer && packet->GetPayloadLength() == 0) + // Packets with only padding are not forwarded. + if (packet->GetPayloadLength() == 0) { + // Only drop the packet in the RTP sequence manager if it belongs to the + // current spatial layer. + if (spatialLayer == this->currentSpatialLayer) + { #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::EMPTY_PAYLOAD); + packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::EMPTY_PAYLOAD); #endif - this->rtpSeqManager->Drop(packet->GetSequenceNumber()); + this->rtpSeqManager->Drop(packet->GetSequenceNumber()); + } return; } @@ -834,7 +859,7 @@ namespace RTC const bool isSyncPacket = this->syncRequired; // Sync sequence number and timestamp if required. - if (isSyncPacket && (this->spatialLayerToSync == -1 || this->spatialLayerToSync == spatialLayer)) + if (isSyncPacket && (this->spatialLayerToSync == -1 || spatialLayer == this->spatialLayerToSync)) { if (packet->IsKeyFrame()) { @@ -941,11 +966,8 @@ namespace RTC this->syncRequired = false; this->spatialLayerToSync = -1; -#ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::TOO_HIGH_TIMESTAMP_EXTRA_NEEDED); -#endif - - this->rtpSeqManager->Drop(packet->GetSequenceNumber()); + // NOTE: Don't drop the packet in the RTP sequence manager since this + // packet doesn't belong to the current spatial layer. return; } @@ -983,7 +1005,10 @@ namespace RTC if (!shouldSwitchCurrentSpatialLayer && this->checkingForOldPacketsInSpatialLayer) { - // If this is a packet previous to the spatial layer switch, ignore the packet. + // If this is a packet previous to the spatial layer switch, ignore the + // packet. + // NOTE: We drop it in RTP sequence manager because this packet belongs + // to current spatial layer. if (SeqManager::IsSeqLowerThan( packet->GetSequenceNumber(), this->snReferenceSpatialLayer)) { @@ -1034,6 +1059,8 @@ namespace RTC auto previousTemporalLayer = this->encodingContext->GetCurrentTemporalLayer(); // Rewrite payload if needed. Drop packet if necessary. + // NOTE: We drop it in RTP sequence manager because this packet belongs + // to current spatial layer. if (!packet->ProcessPayload(this->encodingContext.get(), marker)) { #ifdef MS_RTC_LOGGER_RTP @@ -1112,10 +1139,6 @@ namespace RTC origSsrc, origSeq, origTimestamp); - -#ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::SEND_RTP_STREAM_DISCARDED); -#endif } // Restore packet fields. diff --git a/worker/src/RTC/SvcConsumer.cpp b/worker/src/RTC/SvcConsumer.cpp index 8b8ce217b9..58ec0f6171 100644 --- a/worker/src/RTC/SvcConsumer.cpp +++ b/worker/src/RTC/SvcConsumer.cpp @@ -667,6 +667,16 @@ namespace RTC return; } + // If we need to sync and this is not a key frame, ignore the packet. + if (this->syncRequired && !packet->IsKeyFrame()) + { + // NOTE: No need to drop the packet in the RTP sequence manager since here + // we are blocking all packets but the key frame that would trigger sync + // below. + + return; + } + auto payloadType = packet->GetPayloadType(); // NOTE: This may happen if this Consumer supports just some codecs of those @@ -684,18 +694,6 @@ namespace RTC return; } - // If we need to sync and this is not a key frame, ignore the packet. - if (this->syncRequired && !packet->IsKeyFrame()) - { -#ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::NOT_A_KEYFRAME); -#endif - - this->rtpSeqManager->Drop(packet->GetSequenceNumber()); - - return; - } - // Packets with only padding are not forwarded. if (packet->GetPayloadLength() == 0) { From 0aa4ac0b632f07cdee8d0f3a76cd125efc6366ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?I=C3=B1aki=20Baz=20Castillo?= Date: Thu, 26 Jun 2025 10:36:52 +0200 Subject: [PATCH 2/3] rename DropReason to DiscardReason and add some logs back --- worker/include/RTC/RtcLogger.hpp | 12 ++++---- worker/src/RTC/PipeConsumer.cpp | 23 +++++++++++---- worker/src/RTC/RtcLogger.cpp | 44 +++++++++++++++------------- worker/src/RTC/SimpleConsumer.cpp | 25 ++++++++++++---- worker/src/RTC/SimulcastConsumer.cpp | 40 +++++++++++++++++-------- worker/src/RTC/SvcConsumer.cpp | 27 ++++++++++++----- 6 files changed, 114 insertions(+), 57 deletions(-) diff --git a/worker/include/RTC/RtcLogger.hpp b/worker/include/RTC/RtcLogger.hpp index 6dbe07b1f9..76dbe1991e 100644 --- a/worker/include/RTC/RtcLogger.hpp +++ b/worker/include/RTC/RtcLogger.hpp @@ -11,7 +11,7 @@ namespace RTC class RtpPacket { public: - enum class DropReason : uint8_t + enum class DiscardReason : uint8_t { NONE = 0, PRODUCER_NOT_FOUND, @@ -25,14 +25,16 @@ namespace RTC SPATIAL_LAYER_MISMATCH, PACKET_PREVIOUS_TO_SPATIAL_LAYER_SWITCH, DROPPED_BY_CODEC, + TOO_HIGH_TIMESTAMP_EXTRA_NEEDED, + SEND_RTP_STREAM_DISCARDED }; - static absl::flat_hash_map dropReason2String; + static absl::flat_hash_map discardReason2String; RtpPacket() = default; ~RtpPacket() = default; void Sent(); - void Dropped(DropReason dropReason); + void Discarded(DiscardReason discardReason); private: void Log() const; @@ -49,8 +51,8 @@ namespace RTC uint32_t sendRtpTimestamp{}; uint16_t recvSeqNumber{}; uint16_t sendSeqNumber{}; - bool dropped{}; - DropReason dropReason{ DropReason::NONE }; + bool discarded{}; + DiscardReason discardReason{ DiscardReason::NONE }; }; }; // namespace RtcLogger } // namespace RTC diff --git a/worker/src/RTC/PipeConsumer.cpp b/worker/src/RTC/PipeConsumer.cpp index bc3e0d7f36..dd93586f2b 100644 --- a/worker/src/RTC/PipeConsumer.cpp +++ b/worker/src/RTC/PipeConsumer.cpp @@ -230,7 +230,7 @@ namespace RTC if (!IsActive()) { #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::CONSUMER_INACTIVE); + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::CONSUMER_INACTIVE); #endif rtpSeqManager->Drop(packet->GetSequenceNumber()); @@ -242,6 +242,10 @@ namespace RTC // the packet. if (syncRequired && this->keyFrameSupported && !packet->IsKeyFrame()) { +#ifdef MS_RTC_LOGGER_RTP + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::NOT_A_KEYFRAME); +#endif + // NOTE: No need to drop the packet in the RTP sequence manager since here // we are blocking all packets but the key frame that would trigger sync // below. @@ -255,10 +259,10 @@ namespace RTC // in the corresponding Producer. if (!this->supportedCodecPayloadTypes[payloadType]) { - MS_DEBUG_DEV("payload type not supported [payloadType:%" PRIu8 "]", payloadType); + MS_WARN_DEV("payload type not supported [payloadType:%" PRIu8 "]", payloadType); #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::UNSUPPORTED_PAYLOAD_TYPE); + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::UNSUPPORTED_PAYLOAD_TYPE); #endif rtpSeqManager->Drop(packet->GetSequenceNumber()); @@ -270,7 +274,7 @@ namespace RTC if (packet->GetPayloadLength() == 0) { #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::EMPTY_PAYLOAD); + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::EMPTY_PAYLOAD); #endif rtpSeqManager->Drop(packet->GetSequenceNumber()); @@ -286,7 +290,12 @@ namespace RTC { if (packet->IsKeyFrame()) { - MS_DEBUG_TAG(rtp, "sync key frame received"); + MS_DEBUG_TAG( + rtp, + "sync key frame received [ssrc:%" PRIu32 ", seq:%" PRIu16 ", ts:%" PRIu32 "]", + packet->GetSsrc(), + packet->GetSequenceNumber(), + packet->GetTimestamp()); } rtpSeqManager->Sync(packet->GetSequenceNumber() - 1); @@ -345,6 +354,10 @@ namespace RTC packet->GetTimestamp(), origSsrc, origSeq); + +#ifdef MS_RTC_LOGGER_RTP + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::SEND_RTP_STREAM_DISCARDED); +#endif } // Restore packet fields. diff --git a/worker/src/RTC/RtcLogger.cpp b/worker/src/RTC/RtcLogger.cpp index 2cb6706e66..f509517a22 100644 --- a/worker/src/RTC/RtcLogger.cpp +++ b/worker/src/RTC/RtcLogger.cpp @@ -9,19 +9,21 @@ namespace RTC namespace RtcLogger { // clang-format off - absl::flat_hash_map RtpPacket::dropReason2String = { - { RtpPacket::DropReason::NONE, "None" }, - { RtpPacket::DropReason::PRODUCER_NOT_FOUND, "ProducerNotFound" }, - { RtpPacket::DropReason::RECV_RTP_STREAM_NOT_FOUND, "RecvRtpStreamNotFound" }, - { RtpPacket::DropReason::RECV_RTP_STREAM_DISCARDED, "RecvRtpStreamDiscarded" }, - { RtpPacket::DropReason::CONSUMER_INACTIVE, "ConsumerInactive" }, - { RtpPacket::DropReason::INVALID_TARGET_LAYER, "InvalidTargetLayer" }, - { RtpPacket::DropReason::UNSUPPORTED_PAYLOAD_TYPE, "UnsupportedPayloadType" }, - { RtpPacket::DropReason::NOT_A_KEYFRAME, "NotAKeyframe" }, - { RtpPacket::DropReason::EMPTY_PAYLOAD, "EmptyPayload" }, - { RtpPacket::DropReason::SPATIAL_LAYER_MISMATCH, "SpatialLayerMismatch" }, - { RtpPacket::DropReason::PACKET_PREVIOUS_TO_SPATIAL_LAYER_SWITCH, "PacketPreviousToSpatialLayerSwitch" }, - { RtpPacket::DropReason::DROPPED_BY_CODEC, "DroppedByCodec" }, + absl::flat_hash_map RtpPacket::discardReason2String = { + { RtpPacket::DiscardReason::NONE, "None" }, + { RtpPacket::DiscardReason::PRODUCER_NOT_FOUND, "ProducerNotFound" }, + { RtpPacket::DiscardReason::RECV_RTP_STREAM_NOT_FOUND, "RecvRtpStreamNotFound" }, + { RtpPacket::DiscardReason::RECV_RTP_STREAM_DISCARDED, "RecvRtpStreamDiscarded" }, + { RtpPacket::DiscardReason::CONSUMER_INACTIVE, "ConsumerInactive" }, + { RtpPacket::DiscardReason::INVALID_TARGET_LAYER, "InvalidTargetLayer" }, + { RtpPacket::DiscardReason::UNSUPPORTED_PAYLOAD_TYPE, "UnsupportedPayloadType" }, + { RtpPacket::DiscardReason::NOT_A_KEYFRAME, "NotAKeyframe" }, + { RtpPacket::DiscardReason::EMPTY_PAYLOAD, "EmptyPayload" }, + { RtpPacket::DiscardReason::SPATIAL_LAYER_MISMATCH, "SpatialLayerMismatch" }, + { RtpPacket::DiscardReason::PACKET_PREVIOUS_TO_SPATIAL_LAYER_SWITCH, "PacketPreviousToSpatialLayerSwitch" }, + { RtpPacket::DiscardReason::DROPPED_BY_CODEC, "DroppedByCodec" }, + { RtpPacket::DiscardReason::TOO_HIGH_TIMESTAMP_EXTRA_NEEDED, "TooHighTimestampExtraNeeded"}, + { RtpPacket::DiscardReason::SEND_RTP_STREAM_DISCARDED, "SendRtpStreamDiscarded"} }; // clang-format on @@ -29,18 +31,18 @@ namespace RTC { MS_TRACE(); - this->dropped = false; + this->discarded = false; Log(); Clear(); } - void RtpPacket::Dropped(DropReason dropReason) + void RtpPacket::Discarded(DiscardReason discardReason) { MS_TRACE(); - this->dropped = true; - this->dropReason = dropReason; + this->discarded = true; + this->discardReason = discardReason; Log(); Clear(); @@ -78,8 +80,8 @@ namespace RTC std::cout << ", \"sendRtpTimestamp\": " << this->sendRtpTimestamp; std::cout << ", \"recvSeqNumber\": " << this->recvSeqNumber; std::cout << ", \"sendSeqNumber\": " << this->sendSeqNumber; - std::cout << ", \"dropped\": " << (this->dropped ? "true" : "false"); - std::cout << ", \"dropReason\": '" << dropReason2String[this->dropReason] << "'"; + std::cout << ", \"discarded\": " << (this->discarded ? "true" : "false"); + std::cout << ", \"discardReason\": '" << discardReason2String[this->discardReason] << "'"; std::cout << "}" << std::endl; } @@ -91,8 +93,8 @@ namespace RTC this->routerId = {}; this->producerId = {}; this->sendSeqNumber = { 0 }; - this->dropped = { false }; - this->dropReason = { DropReason::NONE }; + this->discarded = { false }; + this->discardReason = { DiscardReason::NONE }; } } // namespace RtcLogger } // namespace RTC diff --git a/worker/src/RTC/SimpleConsumer.cpp b/worker/src/RTC/SimpleConsumer.cpp index 2af59ef1d2..63b76ce70c 100644 --- a/worker/src/RTC/SimpleConsumer.cpp +++ b/worker/src/RTC/SimpleConsumer.cpp @@ -317,7 +317,7 @@ namespace RTC if (!IsActive()) { #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::CONSUMER_INACTIVE); + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::CONSUMER_INACTIVE); #endif this->rtpSeqManager->Drop(packet->GetSequenceNumber()); @@ -329,6 +329,10 @@ namespace RTC // the packet. if (this->syncRequired && this->keyFrameSupported && !packet->IsKeyFrame()) { +#ifdef MS_RTC_LOGGER_RTP + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::NOT_A_KEYFRAME); +#endif + // NOTE: No need to drop the packet in the RTP sequence manager since here // we are blocking all packets but the key frame that would trigger sync // below. @@ -342,10 +346,10 @@ namespace RTC // in the corresponding Producer. if (!this->supportedCodecPayloadTypes[payloadType]) { - MS_DEBUG_DEV("payload type not supported [payloadType:%" PRIu8 "]", payloadType); + MS_WARN_DEV("payload type not supported [payloadType:%" PRIu8 "]", payloadType); #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::UNSUPPORTED_PAYLOAD_TYPE); + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::UNSUPPORTED_PAYLOAD_TYPE); #endif this->rtpSeqManager->Drop(packet->GetSequenceNumber()); @@ -357,7 +361,7 @@ namespace RTC if (packet->GetPayloadLength() == 0) { #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::EMPTY_PAYLOAD); + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::EMPTY_PAYLOAD); #endif this->rtpSeqManager->Drop(packet->GetSequenceNumber()); @@ -377,7 +381,7 @@ namespace RTC packet->GetTimestamp()); #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::DROPPED_BY_CODEC); + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::DROPPED_BY_CODEC); #endif this->rtpSeqManager->Drop(packet->GetSequenceNumber()); @@ -393,7 +397,12 @@ namespace RTC { if (packet->IsKeyFrame()) { - MS_DEBUG_TAG(rtp, "sync key frame received"); + MS_DEBUG_TAG( + rtp, + "sync key frame received [ssrc:%" PRIu32 ", seq:%" PRIu16 ", ts:%" PRIu32 "]", + packet->GetSsrc(), + packet->GetSequenceNumber(), + packet->GetTimestamp()); } this->rtpSeqManager->Sync(packet->GetSequenceNumber() - 1); @@ -450,6 +459,10 @@ namespace RTC packet->GetSequenceNumber(), packet->GetTimestamp(), origSeq); + +#ifdef MS_RTC_LOGGER_RTP + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::SEND_RTP_STREAM_DISCARDED); +#endif } // Restore packet fields. diff --git a/worker/src/RTC/SimulcastConsumer.cpp b/worker/src/RTC/SimulcastConsumer.cpp index e2ae2df53c..798f043151 100644 --- a/worker/src/RTC/SimulcastConsumer.cpp +++ b/worker/src/RTC/SimulcastConsumer.cpp @@ -741,7 +741,7 @@ namespace RTC if (spatialLayer == this->currentSpatialLayer) { #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::CONSUMER_INACTIVE); + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::CONSUMER_INACTIVE); #endif this->rtpSeqManager->Drop(packet->GetSequenceNumber()); @@ -757,7 +757,7 @@ namespace RTC if (spatialLayer == this->currentSpatialLayer) { #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::INVALID_TARGET_LAYER); + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::INVALID_TARGET_LAYER); #endif this->rtpSeqManager->Drop(packet->GetSequenceNumber()); @@ -776,10 +776,10 @@ namespace RTC // current spatial layer. if (spatialLayer == this->currentSpatialLayer) { - MS_DEBUG_DEV("payload type not supported [payloadType:%" PRIu8 "]", payloadType); + MS_WARN_DEV("payload type not supported [payloadType:%" PRIu8 "]", payloadType); #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::UNSUPPORTED_PAYLOAD_TYPE); + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::UNSUPPORTED_PAYLOAD_TYPE); #endif this->rtpSeqManager->Drop(packet->GetSequenceNumber()); @@ -824,14 +824,14 @@ namespace RTC // layer or if transport just connected or consumer resumed. if (this->syncRequired && !packet->IsKeyFrame()) { +#ifdef MS_RTC_LOGGER_RTP + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::NOT_A_KEYFRAME); +#endif + // Only drop the packet in the RTP sequence manager if it belongs to the // current spatial layer. if (spatialLayer == this->currentSpatialLayer) { -#ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::NOT_A_KEYFRAME); -#endif - this->rtpSeqManager->Drop(packet->GetSequenceNumber()); } @@ -846,7 +846,7 @@ namespace RTC if (spatialLayer == this->currentSpatialLayer) { #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::EMPTY_PAYLOAD); + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::EMPTY_PAYLOAD); #endif this->rtpSeqManager->Drop(packet->GetSequenceNumber()); @@ -863,7 +863,12 @@ namespace RTC { if (packet->IsKeyFrame()) { - MS_DEBUG_TAG(rtp, "sync key frame received"); + MS_DEBUG_TAG( + rtp, + "sync key frame received [ssrc:%" PRIu32 ", seq:%" PRIu16 ", ts:%" PRIu32 "]", + packet->GetSsrc(), + packet->GetSequenceNumber(), + packet->GetTimestamp()); } uint32_t tsOffset{ 0u }; @@ -966,6 +971,11 @@ namespace RTC this->syncRequired = false; this->spatialLayerToSync = -1; +#ifdef MS_RTC_LOGGER_RTP + packet->logger.Discarded( + RtcLogger::RtpPacket::DiscardReason::TOO_HIGH_TIMESTAMP_EXTRA_NEEDED); +#endif + // NOTE: Don't drop the packet in the RTP sequence manager since this // packet doesn't belong to the current spatial layer. @@ -1013,8 +1023,8 @@ namespace RTC packet->GetSequenceNumber(), this->snReferenceSpatialLayer)) { #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped( - RtcLogger::RtpPacket::DropReason::PACKET_PREVIOUS_TO_SPATIAL_LAYER_SWITCH); + packet->logger.Discarded( + RtcLogger::RtpPacket::DiscardReason::PACKET_PREVIOUS_TO_SPATIAL_LAYER_SWITCH); #endif this->rtpSeqManager->Drop(packet->GetSequenceNumber()); @@ -1064,7 +1074,7 @@ namespace RTC if (!packet->ProcessPayload(this->encodingContext.get(), marker)) { #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::DROPPED_BY_CODEC); + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::DROPPED_BY_CODEC); #endif this->rtpSeqManager->Drop(packet->GetSequenceNumber()); @@ -1139,6 +1149,10 @@ namespace RTC origSsrc, origSeq, origTimestamp); + +#ifdef MS_RTC_LOGGER_RTP + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::SEND_RTP_STREAM_DISCARDED); +#endif } // Restore packet fields. diff --git a/worker/src/RTC/SvcConsumer.cpp b/worker/src/RTC/SvcConsumer.cpp index 58ec0f6171..2095b6338e 100644 --- a/worker/src/RTC/SvcConsumer.cpp +++ b/worker/src/RTC/SvcConsumer.cpp @@ -643,7 +643,7 @@ namespace RTC if (!IsActive()) { #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::CONSUMER_INACTIVE); + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::CONSUMER_INACTIVE); #endif this->rtpSeqManager->Drop(packet->GetSequenceNumber()); @@ -659,7 +659,7 @@ namespace RTC // clang-format on { #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::INVALID_TARGET_LAYER); + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::INVALID_TARGET_LAYER); #endif this->rtpSeqManager->Drop(packet->GetSequenceNumber()); @@ -670,6 +670,10 @@ namespace RTC // If we need to sync and this is not a key frame, ignore the packet. if (this->syncRequired && !packet->IsKeyFrame()) { +#ifdef MS_RTC_LOGGER_RTP + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::NOT_A_KEYFRAME); +#endif + // NOTE: No need to drop the packet in the RTP sequence manager since here // we are blocking all packets but the key frame that would trigger sync // below. @@ -683,10 +687,10 @@ namespace RTC // in the corresponding Producer. if (!this->supportedCodecPayloadTypes[payloadType]) { - MS_DEBUG_DEV("payload type not supported [payloadType:%" PRIu8 "]", payloadType); + MS_WARN_DEV("payload type not supported [payloadType:%" PRIu8 "]", payloadType); #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::UNSUPPORTED_PAYLOAD_TYPE); + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::UNSUPPORTED_PAYLOAD_TYPE); #endif this->rtpSeqManager->Drop(packet->GetSequenceNumber()); @@ -698,7 +702,7 @@ namespace RTC if (packet->GetPayloadLength() == 0) { #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::EMPTY_PAYLOAD); + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::EMPTY_PAYLOAD); #endif this->rtpSeqManager->Drop(packet->GetSequenceNumber()); @@ -714,7 +718,12 @@ namespace RTC { if (packet->IsKeyFrame()) { - MS_DEBUG_TAG(rtp, "sync key frame received"); + MS_DEBUG_TAG( + rtp, + "sync key frame received [ssrc:%" PRIu32 ", seq:%" PRIu16 ", ts:%" PRIu32 "]", + packet->GetSsrc(), + packet->GetSequenceNumber(), + packet->GetTimestamp()); } this->rtpSeqManager->Sync(packet->GetSequenceNumber() - 1); @@ -732,7 +741,7 @@ namespace RTC if (!packet->ProcessPayload(this->encodingContext.get(), marker)) { #ifdef MS_RTC_LOGGER_RTP - packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::DROPPED_BY_CODEC); + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::DROPPED_BY_CODEC); #endif this->rtpSeqManager->Drop(packet->GetSequenceNumber()); @@ -806,6 +815,10 @@ namespace RTC packet->GetTimestamp(), origSsrc, origSeq); + +#ifdef MS_RTC_LOGGER_RTP + packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::SEND_RTP_STREAM_DISCARDED); +#endif } // Restore packet fields. From 6cd427994b9cf21ab14f5d815e756311eb7f47cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?I=C3=B1aki=20Baz=20Castillo?= Date: Thu, 26 Jun 2025 10:48:07 +0200 Subject: [PATCH 3/3] XxxxConsumer.cpp files: remove inline keyworks in class method definitions --- worker/src/RTC/PipeConsumer.cpp | 5 ++--- worker/src/RTC/SimpleConsumer.cpp | 6 +++--- worker/src/RTC/SimulcastConsumer.cpp | 16 ++++++++-------- worker/src/RTC/SvcConsumer.cpp | 8 ++++---- 4 files changed, 17 insertions(+), 18 deletions(-) diff --git a/worker/src/RTC/PipeConsumer.cpp b/worker/src/RTC/PipeConsumer.cpp index dd93586f2b..28f56f8883 100644 --- a/worker/src/RTC/PipeConsumer.cpp +++ b/worker/src/RTC/PipeConsumer.cpp @@ -727,7 +727,7 @@ namespace RTC } } - inline void PipeConsumer::OnRtpStreamScore( + void PipeConsumer::OnRtpStreamScore( RTC::RtpStream* /*rtpStream*/, uint8_t /*score*/, uint8_t /*previousScore*/) { MS_TRACE(); @@ -735,8 +735,7 @@ namespace RTC // Do nothing. } - inline void PipeConsumer::OnRtpStreamRetransmitRtpPacket( - RTC::RtpStreamSend* rtpStream, RTC::RtpPacket* packet) + void PipeConsumer::OnRtpStreamRetransmitRtpPacket(RTC::RtpStreamSend* rtpStream, RTC::RtpPacket* packet) { MS_TRACE(); diff --git a/worker/src/RTC/SimpleConsumer.cpp b/worker/src/RTC/SimpleConsumer.cpp index 63b76ce70c..46e982634e 100644 --- a/worker/src/RTC/SimpleConsumer.cpp +++ b/worker/src/RTC/SimpleConsumer.cpp @@ -740,7 +740,7 @@ namespace RTC this->listener->OnConsumerKeyFrameRequested(this, mappedSsrc); } - inline void SimpleConsumer::EmitScore() const + void SimpleConsumer::EmitScore() const { MS_TRACE(); @@ -756,7 +756,7 @@ namespace RTC notificationOffset); } - inline void SimpleConsumer::OnRtpStreamScore( + void SimpleConsumer::OnRtpStreamScore( RTC::RtpStream* /*rtpStream*/, uint8_t /*score*/, uint8_t /*previousScore*/) { MS_TRACE(); @@ -765,7 +765,7 @@ namespace RTC EmitScore(); } - inline void SimpleConsumer::OnRtpStreamRetransmitRtpPacket( + void SimpleConsumer::OnRtpStreamRetransmitRtpPacket( RTC::RtpStreamSend* /*rtpStream*/, RTC::RtpPacket* packet) { MS_TRACE(); diff --git a/worker/src/RTC/SimulcastConsumer.cpp b/worker/src/RTC/SimulcastConsumer.cpp index 798f043151..e34a974147 100644 --- a/worker/src/RTC/SimulcastConsumer.cpp +++ b/worker/src/RTC/SimulcastConsumer.cpp @@ -1678,7 +1678,7 @@ namespace RTC } } - inline bool SimulcastConsumer::CanSwitchToSpatialLayer(int16_t spatialLayer) const + bool SimulcastConsumer::CanSwitchToSpatialLayer(int16_t spatialLayer) const { MS_TRACE(); @@ -1704,7 +1704,7 @@ namespace RTC // clang-format on } - inline void SimulcastConsumer::EmitScore() const + void SimulcastConsumer::EmitScore() const { MS_TRACE(); @@ -1720,7 +1720,7 @@ namespace RTC notificationOffset); } - inline void SimulcastConsumer::EmitLayersChange() const + void SimulcastConsumer::EmitLayersChange() const { MS_TRACE(); @@ -1750,7 +1750,7 @@ namespace RTC notificationOffset); } - inline RTC::RtpStreamRecv* SimulcastConsumer::GetProducerCurrentRtpStream() const + RTC::RtpStreamRecv* SimulcastConsumer::GetProducerCurrentRtpStream() const { MS_TRACE(); @@ -1763,7 +1763,7 @@ namespace RTC return this->producerRtpStreams.at(this->currentSpatialLayer); } - inline RTC::RtpStreamRecv* SimulcastConsumer::GetProducerTargetRtpStream() const + RTC::RtpStreamRecv* SimulcastConsumer::GetProducerTargetRtpStream() const { MS_TRACE(); @@ -1776,7 +1776,7 @@ namespace RTC return this->producerRtpStreams.at(this->targetSpatialLayer); } - inline RTC::RtpStreamRecv* SimulcastConsumer::GetProducerTsReferenceRtpStream() const + RTC::RtpStreamRecv* SimulcastConsumer::GetProducerTsReferenceRtpStream() const { MS_TRACE(); @@ -1789,7 +1789,7 @@ namespace RTC return this->producerRtpStreams.at(this->tsReferenceSpatialLayer); } - inline void SimulcastConsumer::OnRtpStreamScore( + void SimulcastConsumer::OnRtpStreamScore( RTC::RtpStream* /*rtpStream*/, uint8_t /*score*/, uint8_t /*previousScore*/) { MS_TRACE(); @@ -1809,7 +1809,7 @@ namespace RTC } } - inline void SimulcastConsumer::OnRtpStreamRetransmitRtpPacket( + void SimulcastConsumer::OnRtpStreamRetransmitRtpPacket( RTC::RtpStreamSend* /*rtpStream*/, RTC::RtpPacket* packet) { MS_TRACE(); diff --git a/worker/src/RTC/SvcConsumer.cpp b/worker/src/RTC/SvcConsumer.cpp index 2095b6338e..6a318b61b7 100644 --- a/worker/src/RTC/SvcConsumer.cpp +++ b/worker/src/RTC/SvcConsumer.cpp @@ -1263,7 +1263,7 @@ namespace RTC } } - inline void SvcConsumer::EmitScore() const + void SvcConsumer::EmitScore() const { MS_TRACE(); @@ -1279,7 +1279,7 @@ namespace RTC notificationOffset); } - inline void SvcConsumer::EmitLayersChange() const + void SvcConsumer::EmitLayersChange() const { MS_TRACE(); @@ -1309,7 +1309,7 @@ namespace RTC notificationOffset); } - inline void SvcConsumer::OnRtpStreamScore( + void SvcConsumer::OnRtpStreamScore( RTC::RtpStream* /*rtpStream*/, uint8_t /*score*/, uint8_t /*previousScore*/) { MS_TRACE(); @@ -1329,7 +1329,7 @@ namespace RTC } } - inline void SvcConsumer::OnRtpStreamRetransmitRtpPacket( + void SvcConsumer::OnRtpStreamRetransmitRtpPacket( RTC::RtpStreamSend* /*rtpStream*/, RTC::RtpPacket* packet) { MS_TRACE();