From 9d22225e92956eaa16ccee7e26fe8e5ce4615ab4 Mon Sep 17 00:00:00 2001 From: Raja Subramanian Date: Mon, 15 Aug 2022 15:57:19 +0530 Subject: [PATCH] A few misc changes (#915) - Do not update jitter on padding only packet. Padding only packet may not have proper timestamp. If it does, it probably has the time stamp of the last packet with payload. That will also affect jitter calculation, i. e. wall clock time is moving, but RTP time is the same. - Do not send `onMaxLayer` changed on bind. It was probably racing with update when max layer is updated when adaptive stream is off. There is no need to send that update as the default would be OFF. It will be enabled when adaptive stream subscription turns it on or when max layer is set when down track bind happens and adaptive stream is off. --- pkg/rtc/dynacastmanager.go | 6 ++++++ pkg/rtc/mediatracksubscriptions.go | 28 ++++++++++++++-------------- pkg/sfu/buffer/rtpstats.go | 3 ++- pkg/sfu/downtrack.go | 3 --- 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/pkg/rtc/dynacastmanager.go b/pkg/rtc/dynacastmanager.go index e671ea4f2..9f10b0678 100644 --- a/pkg/rtc/dynacastmanager.go +++ b/pkg/rtc/dynacastmanager.go @@ -174,6 +174,12 @@ func (d *DynacastManager) updateMaxQualityForMime(mime string, maxQuality liveki func (d *DynacastManager) update(force bool) { d.lock.Lock() + if len(d.maxSubscribedQuality) == 0 { + // no mime has been added, nothing to update + d.lock.Unlock() + return + } + // add or remove of a mime triggers an update changed := len(d.maxSubscribedQuality) != len(d.committedMaxSubscribedQuality) downgradesOnly := !changed diff --git a/pkg/rtc/mediatracksubscriptions.go b/pkg/rtc/mediatracksubscriptions.go index c00bedaf9..84b29ea15 100644 --- a/pkg/rtc/mediatracksubscriptions.go +++ b/pkg/rtc/mediatracksubscriptions.go @@ -160,6 +160,20 @@ func (t *MediaTrackSubscriptions) AddSubscriber(sub types.LocalParticipant, wr * subTrack.SetPublisherMuted(t.params.MediaTrack.IsMuted()) }) + downTrack.OnStatsUpdate(func(_ *sfu.DownTrack, stat *livekit.AnalyticsStat) { + t.params.Telemetry.TrackStats(livekit.StreamType_DOWNSTREAM, subscriberID, trackID, stat) + }) + + downTrack.OnMaxLayerChanged(func(dt *sfu.DownTrack, layer int32) { + if t.onSubscriberMaxQualityChange != nil { + t.onSubscriberMaxQualityChange(subscriberID, dt.Codec(), layer) + } + }) + + downTrack.OnRttUpdate(func(_ *sfu.DownTrack, rtt uint32) { + go sub.UpdateRTT(rtt) + }) + var transceiver *webrtc.RTPTransceiver var sender *webrtc.RTPSender @@ -240,20 +254,6 @@ func (t *MediaTrackSubscriptions) AddSubscriber(sub types.LocalParticipant, wr * downTrack.SetTransceiver(transceiver) - downTrack.OnStatsUpdate(func(_ *sfu.DownTrack, stat *livekit.AnalyticsStat) { - t.params.Telemetry.TrackStats(livekit.StreamType_DOWNSTREAM, subscriberID, trackID, stat) - }) - - downTrack.OnMaxLayerChanged(func(dt *sfu.DownTrack, layer int32) { - if t.onSubscriberMaxQualityChange != nil { - t.onSubscriberMaxQualityChange(subscriberID, dt.Codec(), layer) - } - }) - - downTrack.OnRttUpdate(func(_ *sfu.DownTrack, rtt uint32) { - go sub.UpdateRTT(rtt) - }) - downTrack.OnCloseHandler(func(willBeResumed bool) { go t.downTrackClosed(sub, subTrack, willBeResumed, sender) }) diff --git a/pkg/sfu/buffer/rtpstats.go b/pkg/sfu/buffer/rtpstats.go index ec8eae0cd..5d6e18001 100644 --- a/pkg/sfu/buffer/rtpstats.go +++ b/pkg/sfu/buffer/rtpstats.go @@ -294,9 +294,10 @@ func (r *RTPStats) Update(rtph *rtp.Header, payloadSize int, paddingSize int, pa if rtph.Marker { r.frames++ } + + r.updateJitter(rtph, packetTime) } - r.updateJitter(rtph, packetTime) } return diff --git a/pkg/sfu/downtrack.go b/pkg/sfu/downtrack.go index 03584962b..4cc5573cb 100644 --- a/pkg/sfu/downtrack.go +++ b/pkg/sfu/downtrack.go @@ -298,9 +298,6 @@ func (d *DownTrack) Bind(t webrtc.TrackLocalContext) (webrtc.RTPCodecParameters, d.bound.Store(true) d.bindLock.Unlock() - if d.onMaxLayerChanged != nil { - d.onMaxLayerChanged(d, d.MaxLayers().Spatial) - } d.connectionStats.Start(d.receiver.TrackInfo()) d.logger.Debugw("downtrack bound")