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.
This commit is contained in:
Raja Subramanian
2022-12-21 11:12:40 +05:30
committed by GitHub
parent f24c1b95c2
commit d05d26cc04
+14 -24
View File
@@ -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
}
}