From ae8c8bc9417c6cb26fe904d0a1e1be57311518f8 Mon Sep 17 00:00:00 2001 From: Raja Subramanian Date: Wed, 15 Jan 2025 10:32:59 +0530 Subject: [PATCH] Turn off TWCC for Firefox (#3333) * Debug FF TWCC * - TURN off TWCC for Firefox. Seems to fail with VP9 send, i.e. there are no TWCC feedback packets when sending VP9. - Relax thresholds for congestion as staging data is showing oscillations. - Clean up some logging. * debug log a few more signal messages * revert config * revert config * clean up --- pkg/rtc/participant_signal.go | 13 ++++++++---- pkg/rtc/transport.go | 4 ++-- pkg/service/rtcservice.go | 10 +++++++-- .../bwe/sendsidebwe/congestion_detector.go | 9 ++++---- pkg/sfu/bwe/sendsidebwe/packet_group.go | 21 +------------------ 5 files changed, 24 insertions(+), 33 deletions(-) diff --git a/pkg/rtc/participant_signal.go b/pkg/rtc/participant_signal.go index 9923cab9c..2e0a1081c 100644 --- a/pkg/rtc/participant_signal.go +++ b/pkg/rtc/participant_signal.go @@ -332,12 +332,17 @@ func (p *ParticipantImpl) writeMessage(msg *livekit.SignalResponse) error { err := sink.WriteMessage(msg) if errors.Is(err, psrpc.Canceled) { - p.params.Logger.Debugw("could not send message to participant", - "error", err, "messageType", fmt.Sprintf("%T", msg.Message)) + p.params.Logger.Debugw( + "could not send message to participant", + "error", err, + "messageType", fmt.Sprintf("%T", msg.Message), + ) return nil } else if err != nil { - p.params.Logger.Warnw("could not send message to participant", err, - "messageType", fmt.Sprintf("%T", msg.Message)) + p.params.Logger.Warnw( + "could not send message to participant", err, + "messageType", fmt.Sprintf("%T", msg.Message), + ) return err } return nil diff --git a/pkg/rtc/transport.go b/pkg/rtc/transport.go index a274548ec..4147acb31 100644 --- a/pkg/rtc/transport.go +++ b/pkg/rtc/transport.go @@ -379,7 +379,7 @@ func newPeerConnection(params TransportParams, onBandwidthEstimator func(estimat ir := &interceptor.Registry{} if params.IsSendSide { - if params.CongestionControlConfig.UseSendSideBWEInterceptor && !params.CongestionControlConfig.UseSendSideBWE { + if params.CongestionControlConfig.UseSendSideBWEInterceptor && !params.CongestionControlConfig.UseSendSideBWE && !params.ClientInfo.isFirefox() { params.Logger.Infow("using send side BWE - interceptor") gf, err := cc.NewInterceptor(func() (cc.BandwidthEstimator, error) { return gcc.NewSendSideBWE( @@ -482,7 +482,7 @@ func NewPCTransport(params TransportParams) (*PCTransport, error) { } if params.IsSendSide { - if params.CongestionControlConfig.UseSendSideBWE { + if params.CongestionControlConfig.UseSendSideBWE && !t.params.ClientInfo.isFirefox() { params.Logger.Infow("using send side BWE") t.bwe = sendsidebwe.NewSendSideBWE(sendsidebwe.SendSideBWEParams{ Config: params.CongestionControlConfig.SendSideBWE, diff --git a/pkg/service/rtcservice.go b/pkg/service/rtcservice.go index d9d52eda3..ca3af76ae 100644 --- a/pkg/service/rtcservice.go +++ b/pkg/service/rtcservice.go @@ -344,9 +344,11 @@ func (s *RTCService) ServeHTTP(w http.ResponseWriter, r *http.Request) { } res, ok := msg.(*livekit.SignalResponse) if !ok { - pLogger.Errorw("unexpected message type", nil, + pLogger.Errorw( + "unexpected message type", nil, "type", fmt.Sprintf("%T", msg), - "connID", cr.ConnectionID) + "connID", cr.ConnectionID, + ) continue } @@ -356,10 +358,14 @@ func (s *RTCService) ServeHTTP(w http.ResponseWriter, r *http.Request) { case *livekit.SignalResponse_Answer: pLogger.Debugw("sending answer", "answer", m) case *livekit.SignalResponse_Join: + pLogger.Debugw("sending join", "join", m) signalStats.ResolveRoom(m.Join.GetRoom()) signalStats.ResolveParticipant(m.Join.GetParticipant()) case *livekit.SignalResponse_RoomUpdate: + pLogger.Debugw("sending room update", "roomUpdate", m) signalStats.ResolveRoom(m.RoomUpdate.GetRoom()) + case *livekit.SignalResponse_Update: + pLogger.Debugw("sending participant update", "participantUpdate", m) } if count, err := sigConn.WriteResponse(res); err != nil { diff --git a/pkg/sfu/bwe/sendsidebwe/congestion_detector.go b/pkg/sfu/bwe/sendsidebwe/congestion_detector.go index 66e8e495d..c7f4092a6 100644 --- a/pkg/sfu/bwe/sendsidebwe/congestion_detector.go +++ b/pkg/sfu/bwe/sendsidebwe/congestion_detector.go @@ -121,8 +121,8 @@ var ( MinBytesRatio: 0.5, MinDurationRatio: 0.5, - JQRMinDelay: 15 * time.Millisecond, - DQRMaxDelay: 5 * time.Millisecond, + JQRMinDelay: 40 * time.Millisecond, + DQRMaxDelay: 15 * time.Millisecond, WeightedLoss: defaultWeightedLossConfig, JQRMinWeightedLoss: 0.25, @@ -450,8 +450,8 @@ var ( ProbeRegulator: ccutils.DefaultProbeRegulatorConfig, ProbeSignal: defaultProbeSignalConfig, - JQRMinDelay: 15 * time.Millisecond, - DQRMaxDelay: 5 * time.Millisecond, + JQRMinDelay: 40 * time.Millisecond, + DQRMaxDelay: 15 * time.Millisecond, WeightedLoss: defaultWeightedLossConfig, JQRMinWeightedLoss: 0.25, @@ -627,7 +627,6 @@ func (c *congestionDetector) HandleTWCCFeedback(report *rtcp.TransportLayerCC) { // try an older group for idx := len(c.packetGroups) - 2; idx >= 0; idx-- { opg := c.packetGroups[idx] - c.params.Logger.Debugw("send side bwe: trying older group", "packetInfo", pi, "packetGroup", opg) if err := opg.Add(pi, sendDelta, recvDelta, isLost); err == nil { return } else if err == errGroupFinalized { diff --git a/pkg/sfu/bwe/sendsidebwe/packet_group.go b/pkg/sfu/bwe/sendsidebwe/packet_group.go index 6a3fed27b..b64727fdf 100644 --- a/pkg/sfu/bwe/sendsidebwe/packet_group.go +++ b/pkg/sfu/bwe/sendsidebwe/packet_group.go @@ -167,13 +167,6 @@ func (p *packetGroup) Add(pi *packetInfo, sendDelta, recvDelta int64, isLost boo } if p.minSequenceNumber == 0 || pi.sequenceNumber < p.minSequenceNumber { - if p.minSequenceNumber != 0 { - p.params.Logger.Debugw( - "send side bwe, out-of-order min sequence number", - "packetInfo", pi, - "packetGroup", p, - ) - } p.minSequenceNumber = pi.sequenceNumber } p.maxSequenceNumber = max(p.maxSequenceNumber, pi.sequenceNumber) @@ -189,13 +182,8 @@ func (p *packetGroup) Add(pi *packetInfo, sendDelta, recvDelta int64, isLost boo p.maxRecvTime = max(p.maxRecvTime, pi.recvTime) p.acked.add(int(pi.size), pi.isRTX, pi.isProbe) - if p.snBitmap.IsSet(pi.sequenceNumber - p.minSequenceNumber) { + if int(pi.sequenceNumber-p.minSequenceNumber) < p.snBitmap.Len() && p.snBitmap.IsSet(pi.sequenceNumber-p.minSequenceNumber) { // an earlier packet reported as lost has been received - p.params.Logger.Debugw( - "send side bwe, received previously lost packet", - "packetInfo", pi, - "packetGroup", p, - ) p.snBitmap.Clear(pi.sequenceNumber - p.minSequenceNumber) p.lost.remove(int(pi.size), pi.isRTX, pi.isProbe) } @@ -225,13 +213,6 @@ func (p *packetGroup) lostPacket(pi *packetInfo) error { } if p.minSequenceNumber == 0 || pi.sequenceNumber < p.minSequenceNumber { - if p.minSequenceNumber != 0 { - p.params.Logger.Debugw( - "send side bwe, out-of-order min sequence number lost", - "packetInfo", pi, - "packetGroup", p, - ) - } p.minSequenceNumber = pi.sequenceNumber } p.maxSequenceNumber = max(p.maxSequenceNumber, pi.sequenceNumber)