From d05d26cc04912de00f3d98e55375c53c4656cf8e Mon Sep 17 00:00:00 2001
From: Raja Subramanian <raja.gobi@tutanota.com>
Date: Wed, 21 Dec 2022 11:12:40 +0530
Subject: [PATCH] Do not patch subscription setting when processing
 UpdateSubscription (#1247)

message.

There are is a sequence where a dupe could be detected due to patching
which could lead to issues.

The sequence is
- UpdataTrackSettings with some values
- UpdateSubscription with Subcribe: false - this will patch from above
  track settings
- UpdateSubscription with Subscribe: true - this will continue patching
- UpdateTrackSettings with the same settings as in the first step - this
  will be declared a dupe because the track is enabled and the patched
  settings will declare no change in settings.

This is okay in the current code as subscription settings are cached at
participant level and applied when somebody re-subscribes. But, that
down stream processing can change any time.

So, when processing `UpdateSubscription` message, just do not patch.
If a later `UpdateTrackSettings` comes along, let it pass even if it
is not changing anything.
---
 pkg/rtc/signaldeduper/subscriptiondeduper.go | 38 ++++++++------------
 1 file changed, 14 insertions(+), 24 deletions(-)

diff --git a/pkg/rtc/signaldeduper/subscriptiondeduper.go b/pkg/rtc/signaldeduper/subscriptiondeduper.go
index 79e146c95..6c85a10bc 100644
--- a/pkg/rtc/signaldeduper/subscriptiondeduper.go
+++ b/pkg/rtc/signaldeduper/subscriptiondeduper.go
@@ -25,16 +25,6 @@ func subscriptionSettingFromUpdateSubscription(us *livekit.UpdateSubscription) *
 	}
 }
 
-func subscriptionSettingPatchFromUpdateSubscription(us *livekit.UpdateSubscription, from *subscriptionSetting) *subscriptionSetting {
-	return &subscriptionSetting{
-		isEnabled: us.Subscribe,
-		quality:   from.quality,
-		width:     from.width,
-		height:    from.height,
-		fps:       from.fps,
-	}
-}
-
 func subscriptionSettingFromUpdateTrackSettings(uts *livekit.UpdateTrackSettings) *subscriptionSetting {
 	return &subscriptionSetting{
 		isEnabled: !uts.Disabled,
@@ -114,17 +104,17 @@ func (s *SubscriptionDeduper) updateSubscriptionsFromUpdateSubscription(
 	}
 
 	for trackID := range trackIDs {
-		subscriptionSetting := participantSubscriptions[trackID]
-		if subscriptionSetting == nil {
+		currentSetting := participantSubscriptions[trackID]
+		if currentSetting == nil {
 			// new track seen
-			subscriptionSetting := subscriptionSettingFromUpdateSubscription(us)
-			participantSubscriptions[trackID] = subscriptionSetting
+			currentSetting = subscriptionSettingFromUpdateSubscription(us)
+			participantSubscriptions[trackID] = currentSetting
 			isDupe = false
 		} else {
-			newSubscriptionSetting := subscriptionSettingPatchFromUpdateSubscription(us, subscriptionSetting)
-			if !subscriptionSetting.Equal(newSubscriptionSetting) {
+			newSetting := subscriptionSettingFromUpdateSubscription(us)
+			if !currentSetting.Equal(newSetting) {
 				// subscription setting change
-				participantSubscriptions[trackID] = newSubscriptionSetting
+				participantSubscriptions[trackID] = newSetting
 				isDupe = false
 			}
 		}
@@ -145,17 +135,17 @@ func (s *SubscriptionDeduper) updateSubscriptionsFromUpdateTrackSettings(
 	participantSubscriptions := s.getOrCreateParticipantSubscriptions(participantKey)
 
 	for _, trackSid := range uts.TrackSids {
-		subscriptionSetting := participantSubscriptions[livekit.TrackID(trackSid)]
-		if subscriptionSetting == nil {
+		currentSetting := participantSubscriptions[livekit.TrackID(trackSid)]
+		if currentSetting == nil {
 			// new track seen
-			subscriptionSetting := subscriptionSettingFromUpdateTrackSettings(uts)
-			participantSubscriptions[livekit.TrackID(trackSid)] = subscriptionSetting
+			currentSetting = subscriptionSettingFromUpdateTrackSettings(uts)
+			participantSubscriptions[livekit.TrackID(trackSid)] = currentSetting
 			isDupe = false
 		} else {
-			newSubscriptionSetting := subscriptionSettingFromUpdateTrackSettings(uts)
-			if !subscriptionSetting.Equal(newSubscriptionSetting) {
+			newSetting := subscriptionSettingFromUpdateTrackSettings(uts)
+			if !currentSetting.Equal(newSetting) {
 				// subscription setting change
-				participantSubscriptions[livekit.TrackID(trackSid)] = newSubscriptionSetting
+				participantSubscriptions[livekit.TrackID(trackSid)] = newSetting
 				isDupe = false
 			}
 		}