Send NACKs when processing RTX packets. (#5000)

* Send NACKs when processing RTX packets.

Applying the fix proposed in https://github.com/livekit/livekit/issues/4999

* Add test for NACKs sent while processing RTX packets.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

pkg/sfu/buffer/buffer.go

@@ -327,15 +327,24 @@ func (b *Buffer) NotifyRTX(ssrc uint32, repairSSRC uint32, rsid string) {
 }
 
 func (b *Buffer) writeRTX(rtxPkt *rtp.Packet, arrivalTime int64) {
+	rtcpPackets := b.writeRTXLocked(rtxPkt, arrivalTime)
+	if len(rtcpPackets) != 0 {
+		if cb := b.getOnRtcpFeedback(); cb != nil {
+			cb(rtcpPackets)
+		}
+	}
+}
+
+func (b *Buffer) writeRTXLocked(rtxPkt *rtp.Packet, arrivalTime int64) []rtcp.Packet {
 	b.Lock()
 	defer b.Unlock()
 	if !b.isBound {
-		return
+		return nil
 	}
 
 	if rtxPkt.PayloadType != b.rtxPayloadType {
 		b.logger.Debugw("unexpected rtx payload type", "expected", b.rtxPayloadType, "actual", rtxPkt.PayloadType)
-		return
+		return nil
 	}
 
 	if b.rtxPktBuf == nil {
@@ -344,7 +353,7 @@ func (b *Buffer) writeRTX(rtxPkt *rtp.Packet, arrivalTime int64) {
 
 	if len(rtxPkt.Payload) < 2 {
 		b.logger.Warnw("rtx payload too short", nil, "size", len(rtxPkt.Payload))
-		return
+		return nil
 	}
 
 	repairedPkt := *rtxPkt
@@ -355,10 +364,10 @@ func (b *Buffer) writeRTX(rtxPkt *rtp.Packet, arrivalTime int64) {
 	n, err := repairedPkt.MarshalTo(b.rtxPktBuf)
 	if err != nil {
 		b.logger.Errorw("could not marshal repaired packet", err, "ssrc", b.BufferBase.SSRC(), "sn", repairedPkt.SequenceNumber)
-		return
+		return nil
 	}
 
-	b.calc(b.rtxPktBuf[:n], &repairedPkt, arrivalTime, false, true)
+	return b.calc(b.rtxPktBuf[:n], &repairedPkt, arrivalTime, false, true)
 }
 
 func (b *Buffer) Read(buff []byte) (n int, err error) {

pkg/sfu/buffer/buffer_test.go

@@ -182,6 +182,59 @@ func TestNack(t *testing.T) {
 	})
 }
 
+func TestNackOnRTX(t *testing.T) {
+	buff := NewBuffer(123, 1, 1)
+	buff.codecType = webrtc.RTPCodecTypeVideo
+	var mu sync.Mutex
+	var nacked []uint16
+	buff.OnRtcpFeedback(func(fb []rtcp.Packet) {
+		mu.Lock()
+		defer mu.Unlock()
+		for _, pkt := range fb {
+			if p, ok := pkt.(*rtcp.TransportLayerNack); ok && p.MediaSSRC == 123 {
+				for _, pair := range p.Nacks {
+					nacked = append(nacked, pair.PacketList()...)
+				}
+			}
+		}
+	})
+	require.NoError(t, buff.Bind(webrtc.RTPParameters{
+		Codecs: []webrtc.RTPCodecParameters{vp8Codec},
+	}, vp8Codec.RTPCodecCapability, 0))
+	buff.rtxPayloadType = 97
+	rtt := uint32(20)
+	buff.nacker.SetRTT(rtt)
+
+	payload := []byte{0xff, 0xff, 0xff, 0xfd, 0xb4, 0x9f, 0x94, 0x1}
+	// sequence numbers 1 and 2 are lost
+	for _, sn := range []uint16{0, 3} {
+		pkt := rtp.Packet{
+			Header:  rtp.Header{Version: 2, PayloadType: 96, SequenceNumber: sn, Timestamp: uint32(sn), SSRC: 123},
+			Payload: payload,
+		}
+		b, err := pkt.Marshal()
+		require.NoError(t, err)
+		_, err = buff.Write(b)
+		require.NoError(t, err)
+	}
+
+	time.Sleep(time.Duration(2*rtt) * time.Millisecond)
+	mu.Lock()
+	nacked = nil
+	mu.Unlock()
+
+	// RTX repairs sequence number 2, the NACK for 1 is due and must be sent
+	rtxPkt := rtp.Packet{
+		Header:  rtp.Header{Version: 2, PayloadType: 97, SequenceNumber: 500, Timestamp: 2, SSRC: 456},
+		Payload: append([]byte{0x00, 0x02}, payload...),
+	}
+	buff.writeRTX(&rtxPkt, time.Now().UnixNano())
+
+	mu.Lock()
+	defer mu.Unlock()
+	require.Equal(t, []uint16{1}, nacked)
+}
+
 func TestNewBuffer(t *testing.T) {
 	tests := []struct {
 		name string