Consumer: Only drop packets in RTP sequence manager when they belong to current spatial layer

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
 

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,
@@ -23,18 +23,18 @@ 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,
+				TOO_HIGH_TIMESTAMP_EXTRA_NEEDED,
+				SEND_RTP_STREAM_DISCARDED
 			};
 
-			static absl::flat_hash_map<DropReason, std::string> dropReason2String;
+			static absl::flat_hash_map<DiscardReason, std::string> discardReason2String;
 
 			RtpPacket()  = default;
 			~RtpPacket() = default;
 			void Sent();
-			void Dropped(DropReason dropReason);
+			void Discarded(DiscardReason discardReason);
 
 		private:
 			void Log() const;
@@ -51,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

worker/src/RTC/PipeConsumer.cpp

@@ -222,55 +222,63 @@ 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);
+			packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::CONSUMER_INACTIVE);
 #endif
 
+			rtpSeqManager->Drop(packet->GetSequenceNumber());
+
 			return;
 		}
 
-		auto payloadType = packet->GetPayloadType();
-
-		// NOTE: This may happen if this Consumer supports just some codecs of those
-		// in the corresponding Producer.
-		if (!this->supportedCodecPayloadTypes[payloadType])
+		// If we need to sync, support key frames and this is not a key frame, ignore
+		// the packet.
+		if (syncRequired && this->keyFrameSupported && !packet->IsKeyFrame())
 		{
-			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.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.
+
 			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);
+		auto payloadType = packet->GetPayloadType();
 
-		// 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: This may happen if this Consumer supports just some codecs of those
+		// in the corresponding Producer.
+		if (!this->supportedCodecPayloadTypes[payloadType])
 		{
+			MS_WARN_DEV("payload type not supported [payloadType:%" PRIu8 "]", payloadType);
+
 #ifdef MS_RTC_LOGGER_RTP
-			packet->logger.Dropped(RtcLogger::RtpPacket::DropReason::NOT_A_KEYFRAME);
+			packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::UNSUPPORTED_PAYLOAD_TYPE);
 #endif
 
+			rtpSeqManager->Drop(packet->GetSequenceNumber());
+
 			return;
 		}
 
 		// 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);
+			packet->logger.Discarded(RtcLogger::RtpPacket::DiscardReason::EMPTY_PAYLOAD);
 #endif
 
+			rtpSeqManager->Drop(packet->GetSequenceNumber());
+
 			return;
 		}
 
@@ -282,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);
@@ -341,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.
@@ -710,16 +727,15 @@ namespace RTC
 		}
 	}
 
-	inline void PipeConsumer::OnRtpStreamScore(
+	void PipeConsumer::OnRtpStreamScore(
 	  RTC::RtpStream* /*rtpStream*/, uint8_t /*score*/, uint8_t /*previousScore*/)
 	{
 		MS_TRACE();
 
 		// Do nothing.
 	}
 
-	inline void PipeConsumer::OnRtpStreamRetransmitRtpPacket(
-	  RTC::RtpStreamSend* rtpStream, RTC::RtpPacket* packet)
+	void PipeConsumer::OnRtpStreamRetransmitRtpPacket(RTC::RtpStreamSend* rtpStream, RTC::RtpPacket* packet)
 	{
 		MS_TRACE();
 

worker/src/RTC/RtcLogger.cpp

@@ -9,40 +9,40 @@ namespace RTC
 	namespace RtcLogger
 	{
 		// clang-format off
-		absl::flat_hash_map<RtpPacket::DropReason, std::string> 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::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"             },
+		absl::flat_hash_map<RtpPacket::DiscardReason, std::string> 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
 
 		void RtpPacket::Sent()
 		{
 			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();
@@ -80,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;
 		}
 
@@ -93,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