From 50c48ff29d0913cc940113d12905f8b70e41529f Mon Sep 17 00:00:00 2001
From: Raja Subramanian <raja.gobi@tutanota.com>
Date: Mon, 11 Mar 2024 11:30:01 +0530
Subject: [PATCH] Ignore out-of-order receiver side sender reports. (#2567)

---
 pkg/sfu/buffer/rtpstats_receiver.go | 14 ++++++--------
 pkg/sfu/buffer/rtpstats_sender.go   | 27 ++++++++++++++++-----------
 2 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/pkg/sfu/buffer/rtpstats_receiver.go b/pkg/sfu/buffer/rtpstats_receiver.go
index 5b5e502ca..2a789c443 100644
--- a/pkg/sfu/buffer/rtpstats_receiver.go
+++ b/pkg/sfu/buffer/rtpstats_receiver.go
@@ -276,16 +276,14 @@ func (r *RTPStatsReceiver) SetRtcpSenderReportData(srData *RTCPSenderReportData)
 	srDataCopy := *srData
 	srDataCopy.RTPTimestampExt = uint64(srDataCopy.RTPTimestamp) + tsCycles
 
-	r.maybeAdjustFirstPacketTime(srDataCopy.RTPTimestamp, r.timestamp.GetStart())
-
 	if r.srNewest != nil && srDataCopy.RTPTimestampExt < r.srNewest.RTPTimestampExt {
 		// This can happen when a track is replaced with a null and then restored -
 		// i. e. muting replacing with null and unmute restoring the original track.
-		// Under such a condition reset the sender reports to start from this point.
-		// Resetting will ensure sample rate calculations do not go haywire due to negative time.
+		// Or it could be due bad report generation.
+		// In any case, ignore out-of-order reports.
 		if r.outOfOrderSsenderReportCount%10 == 0 {
 			r.logger.Infow(
-				"received sender report, out-of-order, resetting",
+				"received sender report, out-of-order, skipping",
 				"first", r.srFirst,
 				"last", r.srNewest,
 				"current", &srDataCopy,
@@ -293,11 +291,11 @@ func (r *RTPStatsReceiver) SetRtcpSenderReportData(srData *RTCPSenderReportData)
 			)
 		}
 		r.outOfOrderSsenderReportCount++
-
-		r.srFirst = nil
-		r.srNewest = nil
+		return
 	}
 
+	r.maybeAdjustFirstPacketTime(srDataCopy.RTPTimestamp, r.timestamp.GetStart())
+
 	if r.srNewest != nil {
 		timeSinceLast := srData.NTPTimestamp.Time().Sub(r.srNewest.NTPTimestamp.Time()).Seconds()
 		rtpDiffSinceLast := srDataCopy.RTPTimestampExt - r.srNewest.RTPTimestampExt
diff --git a/pkg/sfu/buffer/rtpstats_sender.go b/pkg/sfu/buffer/rtpstats_sender.go
index e568bd6f7..585d9cb5c 100644
--- a/pkg/sfu/buffer/rtpstats_sender.go
+++ b/pkg/sfu/buffer/rtpstats_sender.go
@@ -665,16 +665,21 @@ func (r *RTPStatsSender) GetRtcpSenderReport(ssrc uint32, calculatedClockRate ui
 	nowRTPExtUsingTime := r.extStartTS + uint64(timeSinceFirst.Nanoseconds()*int64(r.params.ClockRate)/1e9)
 	nowRTPExt := nowRTPExtUsingTime
 
-	// It is possible that publisher is pacing at a slower rate.
-	// That would make `highestTS` to be lagging the RTP time stamp in the RTCP Sender Report from publisher.
-	// Check for that using calculated clock rate and use the later time stamp if applicable.
-	var nowRTPExtUsingRate uint64
-	if calculatedClockRate != 0 {
-		nowRTPExtUsingRate = r.extStartTS + uint64(float64(calculatedClockRate)*timeSinceFirst.Seconds())
-		if nowRTPExtUsingRate > nowRTPExt {
-			nowRTPExt = nowRTPExtUsingRate
+	/*
+		// TODO: Bad reports or unpaced publishing contorts the calculated clock rate a lot resulting in
+		// subsscriber sender reports jumping around. Needs more thinking.
+		//
+		// It is possible that publisher is pacing at a slower rate.
+		// That would make `highestTS` to be lagging the RTP time stamp in the RTCP Sender Report from publisher.
+		// Check for that using calculated clock rate and use the later time stamp if applicable.
+		var nowRTPExtUsingRate uint64
+		if calculatedClockRate != 0 {
+			nowRTPExtUsingRate = r.extStartTS + uint64(float64(calculatedClockRate)*timeSinceFirst.Seconds())
+			if nowRTPExtUsingRate > nowRTPExt {
+				nowRTPExt = nowRTPExtUsingRate
+			}
 		}
-	}
+	*/
 
 	srData := &RTCPSenderReportData{
 		NTPTimestamp:    nowNTP,
@@ -704,7 +709,7 @@ func (r *RTPStatsSender) GetRtcpSenderReport(ssrc uint32, calculatedClockRate ui
 					"timeSinceFirst", timeSinceFirst.String(),
 					"nowRTPExtUsingTime", nowRTPExtUsingTime,
 					"calculatedClockRate", calculatedClockRate,
-					"nowRTPExtUsingRate", nowRTPExtUsingRate,
+					// TODO "nowRTPExtUsingRate", nowRTPExtUsingRate,
 					"timeSinceLastReport", timeSinceLastReport.String(),
 					"rtpDiffSinceLastReport", rtpDiffSinceLastReport,
 					"windowClockRate", windowClockRate,
@@ -734,7 +739,7 @@ func (r *RTPStatsSender) GetRtcpSenderReport(ssrc uint32, calculatedClockRate ui
 			"timeSinceFirst", timeSinceFirst.String(),
 			"nowRTPExtUsingTime", nowRTPExtUsingTime,
 			"calculatedClockRate", calculatedClockRate,
-			"nowRTPExtUsingRate", nowRTPExtUsingRate,
+			// TODO "nowRTPExtUsingRate", nowRTPExtUsingRate,
 		)
 		return nil
 	}