From 83353d821c7e78214597fdec1e5da83f48aaa591 Mon Sep 17 00:00:00 2001 From: Raja Subramanian Date: Fri, 8 Sep 2023 20:59:58 +0530 Subject: [PATCH] Make a config option to skip unmanaged tracks in bandwidth estimation (#2050) * WIP commit * Make a config option to skip unmanaged tracks in bandwidth estimation --- pkg/config/config.go | 21 +++++++-------- pkg/sfu/downtrack.go | 30 +++++++++++++++++++--- pkg/sfu/streamallocator/streamallocator.go | 20 ++++++++++++++- 3 files changed, 57 insertions(+), 14 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 1f3ab2aba..5350134de 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -152,16 +152,17 @@ type CongestionControlChannelObserverConfig struct { } type CongestionControlConfig struct { - Enabled bool `yaml:"enabled,omitempty"` - AllowPause bool `yaml:"allow_pause,omitempty"` - NackRatioAttenuator float64 `yaml:"nack_ratio_attenuator,omitempty"` - ExpectedUsageThreshold float64 `yaml:"expected_usage_threshold,omitempty"` - UseSendSideBWE bool `yaml:"send_side_bandwidth_estimation,omitempty"` - ProbeMode CongestionControlProbeMode `yaml:"padding_mode,omitempty"` - MinChannelCapacity int64 `yaml:"min_channel_capacity,omitempty"` - ProbeConfig CongestionControlProbeConfig `yaml:"probe_config,omitempty"` - ChannelObserverProbeConfig CongestionControlChannelObserverConfig `yaml:"channel_observer_probe_config,omitempty"` - ChannelObserverNonProbeConfig CongestionControlChannelObserverConfig `yaml:"channel_observer_non_probe_config,omitempty"` + Enabled bool `yaml:"enabled,omitempty"` + AllowPause bool `yaml:"allow_pause,omitempty"` + NackRatioAttenuator float64 `yaml:"nack_ratio_attenuator,omitempty"` + ExpectedUsageThreshold float64 `yaml:"expected_usage_threshold,omitempty"` + UseSendSideBWE bool `yaml:"send_side_bandwidth_estimation,omitempty"` + ProbeMode CongestionControlProbeMode `yaml:"padding_mode,omitempty"` + MinChannelCapacity int64 `yaml:"min_channel_capacity,omitempty"` + ProbeConfig CongestionControlProbeConfig `yaml:"probe_config,omitempty"` + ChannelObserverProbeConfig CongestionControlChannelObserverConfig `yaml:"channel_observer_probe_config,omitempty"` + ChannelObserverNonProbeConfig CongestionControlChannelObserverConfig `yaml:"channel_observer_non_probe_config,omitempty"` + DisableEstimationUnmanagedTracks bool `yaml:"disable_etimation_unmanaged_tracks,omitempty"` } type AudioConfig struct { diff --git a/pkg/sfu/downtrack.go b/pkg/sfu/downtrack.go index 6216fa0d8..42f32887a 100644 --- a/pkg/sfu/downtrack.go +++ b/pkg/sfu/downtrack.go @@ -180,6 +180,9 @@ type DownTrackStreamAllocatorListener interface { // RTCP Receiver Report received OnRTCPReceiverReport(dt *DownTrack, rr rtcp.ReceptionReport) + + // check if track should participate in BWE + IsBWEEnabled(dt *DownTrack) bool } type ReceiverReportListener func(dt *DownTrack, report *rtcp.ReceiverReport) @@ -443,8 +446,13 @@ func (d *DownTrack) SetStreamAllocatorListener(listener DownTrackStreamAllocator d.streamAllocatorListener = listener d.streamAllocatorLock.Unlock() - // kick of a gratuitous allocation if listener != nil { + if !listener.IsBWEEnabled(d) { + d.absSendTimeExtID = 0 + d.transportWideExtID = 0 + } + + // kick of a gratuitous allocation listener.OnSubscriptionChanged(d) } } @@ -515,16 +523,32 @@ func (d *DownTrack) SubscriberID() livekit.ParticipantID { return d.params.SubID // Sets RTP header extensions for this track func (d *DownTrack) SetRTPHeaderExtensions(rtpHeaderExtensions []webrtc.RTPHeaderExtensionParameter) { + d.streamAllocatorLock.RLock() + listener := d.streamAllocatorListener + d.streamAllocatorLock.RUnlock() + + isBWEEnabled := true + if listener != nil { + isBWEEnabled = listener.IsBWEEnabled(d) + } for _, ext := range rtpHeaderExtensions { switch ext.URI { case sdp.ABSSendTimeURI: - d.absSendTimeExtID = ext.ID + if isBWEEnabled { + d.absSendTimeExtID = ext.ID + } else { + d.absSendTimeExtID = 0 + } case dd.ExtensionURI: d.dependencyDescriptorExtID = ext.ID case rtpextension.PlayoutDelayURI: d.playoutDelayExtID = ext.ID case sdp.TransportCCURI: - d.transportWideExtID = ext.ID + if isBWEEnabled { + d.transportWideExtID = ext.ID + } else { + d.transportWideExtID = 0 + } } } } diff --git a/pkg/sfu/streamallocator/streamallocator.go b/pkg/sfu/streamallocator/streamallocator.go index 91ad018cc..e188d54c3 100644 --- a/pkg/sfu/streamallocator/streamallocator.go +++ b/pkg/sfu/streamallocator/streamallocator.go @@ -499,6 +499,22 @@ func (s *StreamAllocator) OnActiveChanged(isActive bool) { } } +// called to check if track should participate in BWE +func (s *StreamAllocator) IsBWEEnabled(downTrack *sfu.DownTrack) bool { + if !s.params.Config.DisableEstimationUnmanagedTracks { + return true + } + + s.videoTracksMu.Lock() + defer s.videoTracksMu.Unlock() + + if track := s.videoTracks[livekit.TrackID(downTrack.ID())]; track != nil { + return track.IsManaged() + } + + return true +} + func (s *StreamAllocator) maybePostEventAllocateTrack(downTrack *sfu.DownTrack) { shouldPost := false s.videoTracksMu.Lock() @@ -1097,7 +1113,9 @@ func (s *StreamAllocator) allocateAllTracks() { updateStreamStateChange(track, allocation, update) // STREAM-ALLOCATOR-TODO: optimistic allocation before bitrate is available will return 0. How to account for that? - availableChannelCapacity -= allocation.BandwidthRequested + if !s.params.Config.DisableEstimationUnmanagedTracks { + availableChannelCapacity -= allocation.BandwidthRequested + } } if availableChannelCapacity < 0 {