From 4b5ef630b336e43e0ceac1dd83d5295ac18a3aef Mon Sep 17 00:00:00 2001
From: Lewis Wolfgang <lewis@daily.co>
Date: Mon, 12 Apr 2021 19:21:58 +0300
Subject: [PATCH] Fix "SimulcastConsumer cannot switch layers if initial 
 tsReferenceSpatialLayer disappears" (#492) * Set tsReferenceSpatialLayer 
 later, only after receiving an RTCP Sender Report. * Track and carry forward 
 offset from tsReferenceSpatialLayer.

---
 worker/include/RTC/SimulcastConsumer.hpp |  3 +-
 worker/src/RTC/SimulcastConsumer.cpp     | 65 ++++++++++++++++--------
 2 files changed, 45 insertions(+), 23 deletions(-)

diff --git a/worker/include/RTC/SimulcastConsumer.hpp b/worker/include/RTC/SimulcastConsumer.hpp
index 5374f3b8aa..198a960087 100644
--- a/worker/include/RTC/SimulcastConsumer.hpp
+++ b/worker/include/RTC/SimulcastConsumer.hpp
@@ -111,7 +111,8 @@ namespace RTC
 		int16_t currentSpatialLayer{ -1 };
 		int16_t tsReferenceSpatialLayer{ -1 }; // Used for RTP TS sync.
 		std::unique_ptr<RTC::Codecs::EncodingContext> encodingContext;
-		uint32_t tsOffset{ 0u }; // RTP Timestamp offset.
+		uint32_t tsOffset{ 0u };          // RTP Timestamp offset.
+		uint32_t tsReferenceOffset{ 0u }; // RTP Timestamp offset from the tsReferenceSpatialLayer.
 		bool keyFrameForTsOffsetRequested{ false };
 		uint64_t lastBweDowngradeAtMs{ 0u }; // Last time we moved to lower spatial layer due to BWE.
 	};
diff --git a/worker/src/RTC/SimulcastConsumer.cpp b/worker/src/RTC/SimulcastConsumer.cpp
index cfffe9b92a..07203e6b69 100644
--- a/worker/src/RTC/SimulcastConsumer.cpp
+++ b/worker/src/RTC/SimulcastConsumer.cpp
@@ -700,9 +700,27 @@ namespace RTC
 			uint32_t tsOffset{ 0u };
 
 			// Sync our RTP stream's RTP timestamp.
-			if (spatialLayer == this->tsReferenceSpatialLayer)
+			if (-1 == this->tsReferenceSpatialLayer)
+			{
+				if (shouldSwitchCurrentSpatialLayer)
+				{
+					// If we're actually switching streams, push the RTP timestamp up (we don't have a way to
+					// offset based on abs time yet) otherwise, if it's the first packet, we can just keep the
+					// timestamps
+					if (this->rtpStream->GetMaxPacketTs())
+						tsOffset = packet->GetTimestamp() - this->rtpStream->GetMaxPacketTs() -
+						           33 * this->rtpStream->GetClockRate() / 1000;
+					else
+						tsOffset = 0u;
+				}
+				else
+				{
+					tsOffset = this->tsOffset; // status quo
+				}
+			}
+			else if (spatialLayer == this->tsReferenceSpatialLayer)
 			{
-				tsOffset = 0u;
+				tsOffset = this->tsReferenceOffset;
 			}
 			// If this is not the RTP stream we use as TS reference, do NTP based RTP TS synchronization.
 			else
@@ -719,23 +737,18 @@ namespace RTC
 				  producerTargetRtpStream->GetSenderReportNtpMs(), "no Sender Report for current RTP stream");
 
 				// Calculate NTP and TS stuff.
-				auto ntpMs1 = producerTsReferenceRtpStream->GetSenderReportNtpMs();
-				auto ts1    = producerTsReferenceRtpStream->GetSenderReportTs();
-				auto ntpMs2 = producerTargetRtpStream->GetSenderReportNtpMs();
-				auto ts2    = producerTargetRtpStream->GetSenderReportTs();
-				int64_t diffMs;
-
-				if (ntpMs2 >= ntpMs1)
-					diffMs = ntpMs2 - ntpMs1;
-				else
-					diffMs = -1 * (ntpMs1 - ntpMs2);
+				auto ntpMs1    = producerTsReferenceRtpStream->GetSenderReportNtpMs();
+				auto ts1       = producerTsReferenceRtpStream->GetSenderReportTs();
+				auto ntpMs2    = producerTargetRtpStream->GetSenderReportNtpMs();
+				auto ts2       = producerTargetRtpStream->GetSenderReportTs();
+				int64_t diffMs = ntpMs2 - ntpMs1;
 
 				int64_t diffTs  = diffMs * this->rtpStream->GetClockRate() / 1000;
 				uint32_t newTs2 = ts2 - diffTs;
 
 				// Apply offset. This is the difference that later must be removed from the
 				// sending RTP packet.
-				tsOffset = newTs2 - ts1;
+				tsOffset = this->tsReferenceOffset + newTs2 - ts1;
 			}
 
 			// When switching to a new stream it may happen that the timestamp of this
@@ -812,6 +825,10 @@ namespace RTC
 
 			this->tsOffset = tsOffset;
 
+			// We need this update here and below, it picks up and carries forward any adjustments we made above
+			if (spatialLayer == this->tsReferenceSpatialLayer)
+				this->tsReferenceOffset = this->tsOffset;
+
 			// Sync our RTP stream's sequence number.
 			this->rtpSeqManager.Sync(packet->GetSequenceNumber() - 1);
 
@@ -826,6 +843,9 @@ namespace RTC
 			// Update current spatial layer.
 			this->currentSpatialLayer = this->targetSpatialLayer;
 
+			if (this->currentSpatialLayer == this->tsReferenceSpatialLayer)
+				this->tsReferenceOffset = this->tsOffset;
+
 			// Update target and current temporal layer.
 			this->encodingContext->SetTargetTemporalLayer(this->targetTemporalLayer);
 			this->encodingContext->SetCurrentTemporalLayer(packet->GetTemporalLayer());
@@ -858,6 +878,16 @@ namespace RTC
 				EmitLayersChange();
 		}
 
+		if (-1 == this->tsReferenceSpatialLayer && this->currentSpatialLayer == this->targetSpatialLayer)
+		{
+			auto* producerCurrentRtpStream = GetProducerCurrentRtpStream();
+			if (producerCurrentRtpStream && producerCurrentRtpStream->GetSenderReportNtpMs())
+			{
+				this->tsReferenceSpatialLayer = this->currentSpatialLayer;
+				this->tsReferenceOffset       = this->tsOffset;
+			}
+		}
+
 		// Update RTP seq number and timestamp based on NTP offset.
 		uint16_t seq;
 		uint32_t timestamp = packet->GetTimestamp() - this->tsOffset;
@@ -1328,15 +1358,6 @@ namespace RTC
 	{
 		MS_TRACE();
 
-		// If we don't have yet a RTP timestamp reference, set it now.
-		if (newTargetSpatialLayer != -1 && this->tsReferenceSpatialLayer == -1)
-		{
-			MS_DEBUG_TAG(
-			  simulcast, "using spatial layer %" PRIi16 " as RTP timestamp reference", newTargetSpatialLayer);
-
-			this->tsReferenceSpatialLayer = newTargetSpatialLayer;
-		}
-
 		if (newTargetSpatialLayer == -1)
 		{
 			// Unset current and target layers.