webrtc: Register RTX for incoming video tracks (#6125)

webrtc.ConfigureNack() only wires up the NACK RTCP interceptor. pion still needs an explicit RTX codec registered per media codec before it can answer a publisher's retransmission offer with one, so incomingVideoCodecs having none means NACK-triggered retransmission can never actually happen for incoming video, no matter what the publisher offers. Fixes #5678 --------- Co-authored-by: aler9 <46489434+aler9@users.noreply.github.com>
2026-08-24 14:40:20 +00:00
parent 7bf901ea9a
commit 55d319abfc
2 changed files with 231 additions and 0 deletions
@@ -106,6 +106,99 @@ var incomingVideoCodecs = []webrtc.RTPCodecParameters{
 		},
 		PayloadType: 106,
 	},
+	// RTX (RFC 4588) companions for every video codec above. ConfigureNack()
+	// only enables NACK handling; pion also needs an RTX codec registered for
+	// each video codec before it can negotiate retransmissions. Payload types
+	// must stay unique across both incomingVideoCodecs and incomingAudioCodecs,
+	// so these use the remaining free slots in order.
+	{
+		RTPCodecCapability: webrtc.RTPCodecCapability{
+			MimeType:    webrtc.MimeTypeRTX,
+			ClockRate:   90000,
+			SDPFmtpLine: "apt=96",
+		},
+		PayloadType: 107,
+	},
+	{
+		RTPCodecCapability: webrtc.RTPCodecCapability{
+			MimeType:    webrtc.MimeTypeRTX,
+			ClockRate:   90000,
+			SDPFmtpLine: "apt=97",
+		},
+		PayloadType: 108,
+	},
+	{
+		RTPCodecCapability: webrtc.RTPCodecCapability{
+			MimeType:    webrtc.MimeTypeRTX,
+			ClockRate:   90000,
+			SDPFmtpLine: "apt=98",
+		},
+		PayloadType: 109,
+	},
+	{
+		RTPCodecCapability: webrtc.RTPCodecCapability{
+			MimeType:    webrtc.MimeTypeRTX,
+			ClockRate:   90000,
+			SDPFmtpLine: "apt=99",
+		},
+		PayloadType: 110,
+	},
+	{
+		RTPCodecCapability: webrtc.RTPCodecCapability{
+			MimeType:    webrtc.MimeTypeRTX,
+			ClockRate:   90000,
+			SDPFmtpLine: "apt=100",
+		},
+		PayloadType: 123,
+	},
+	{
+		RTPCodecCapability: webrtc.RTPCodecCapability{
+			MimeType:    webrtc.MimeTypeRTX,
+			ClockRate:   90000,
+			SDPFmtpLine: "apt=101",
+		},
+		PayloadType: 124,
+	},
+	{
+		RTPCodecCapability: webrtc.RTPCodecCapability{
+			MimeType:    webrtc.MimeTypeRTX,
+			ClockRate:   90000,
+			SDPFmtpLine: "apt=102",
+		},
+		PayloadType: 125,
+	},
+	{
+		RTPCodecCapability: webrtc.RTPCodecCapability{
+			MimeType:    webrtc.MimeTypeRTX,
+			ClockRate:   90000,
+			SDPFmtpLine: "apt=103",
+		},
+		PayloadType: 126,
+	},
+	{
+		RTPCodecCapability: webrtc.RTPCodecCapability{
+			MimeType:    webrtc.MimeTypeRTX,
+			ClockRate:   90000,
+			SDPFmtpLine: "apt=104",
+		},
+		PayloadType: 127,
+	},
+	{
+		RTPCodecCapability: webrtc.RTPCodecCapability{
+			MimeType:    webrtc.MimeTypeRTX,
+			ClockRate:   90000,
+			SDPFmtpLine: "apt=105",
+		},
+		PayloadType: 35,
+	},
+	{
+		RTPCodecCapability: webrtc.RTPCodecCapability{
+			MimeType:    webrtc.MimeTypeRTX,
+			ClockRate:   90000,
+			SDPFmtpLine: "apt=106",
+		},
+		PayloadType: 36,
+	},
 }

 var incomingAudioCodecs = []webrtc.RTPCodecParameters{
@@ -0,0 +1,138 @@
+package webrtc
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/pion/interceptor"
+	"github.com/pion/sdp/v3"
+	"github.com/pion/webrtc/v4"
+	"github.com/stretchr/testify/require"
+
+	"github.com/bluenviron/mediamtx/internal/test"
+)
+
+// TestPeerConnectionReadRegistersRTXForVideoCodecs makes sure an incoming
+// video track's negotiated SDP actually carries an RTX payload type,
+// regardless of which of incomingVideoCodecs' codec families the publisher
+// is using. Registering the NACK RTCP interceptor alone (webrtc.ConfigureNack,
+// see Start()) isn't enough for pion to answer a publisher's retransmission
+// offer with one -- incomingVideoCodecs also needs an explicit RTX codec per
+// codec, or a real publisher's own do-retransmission support has nothing to
+// negotiate against.
+func TestPeerConnectionReadRegistersRTXForVideoCodecs(t *testing.T) {
+	for _, ca := range []struct {
+		name        string
+		mimeType    string
+		sdpFmtpLine string
+		payloadType uint8
+		rtxPT       uint8
+	}{
+		{"av1", webrtc.MimeTypeAV1, "profile=1", 96, 107},
+		{"vp9", webrtc.MimeTypeVP9, "profile-id=0", 101, 124},
+		{"vp8", webrtc.MimeTypeVP8, "", 102, 125},
+		{"h265", webrtc.MimeTypeH265, "level-id=93;profile-id=2;tier-flag=0;tx-mode=SRST", 103, 126},
+		{"h264", webrtc.MimeTypeH264, "level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42e01f", 106, 36},
+	} {
+		t.Run(ca.name, func(t *testing.T) {
+			// The publisher side needs to offer RTX itself for there to be anything
+			// for our answer to match -- exactly what a real GStreamer webrtcbin/
+			// whipclientsink publisher does automatically once do-nack is set (which
+			// it is, by default). A publisher that only offers the plain codec, with
+			// no RTX capability of its own, wouldn't exercise this fix at all: pion's
+			// answer only ever includes payload types present in the offer.
+			var pubMediaEngine webrtc.MediaEngine
+			err := pubMediaEngine.RegisterCodec(webrtc.RTPCodecParameters{
+				RTPCodecCapability: webrtc.RTPCodecCapability{
+					MimeType:    ca.mimeType,
+					ClockRate:   90000,
+					SDPFmtpLine: ca.sdpFmtpLine,
+				},
+				PayloadType: webrtc.PayloadType(ca.payloadType),
+			}, webrtc.RTPCodecTypeVideo)
+			require.NoError(t, err)
+			err = pubMediaEngine.RegisterCodec(webrtc.RTPCodecParameters{
+				RTPCodecCapability: webrtc.RTPCodecCapability{
+					MimeType:    webrtc.MimeTypeRTX,
+					ClockRate:   90000,
+					SDPFmtpLine: fmt.Sprintf("apt=%d", ca.payloadType),
+				},
+				PayloadType: webrtc.PayloadType(ca.rtxPT),
+			}, webrtc.RTPCodecTypeVideo)
+			require.NoError(t, err)
+
+			var pubInterceptorRegistry interceptor.Registry
+			err = webrtc.ConfigureNack(&pubMediaEngine, &pubInterceptorRegistry)
+			require.NoError(t, err)
+
+			api := webrtc.NewAPI(
+				webrtc.WithMediaEngine(&pubMediaEngine),
+				webrtc.WithInterceptorRegistry(&pubInterceptorRegistry),
+			)
+
+			pub, err := api.NewPeerConnection(webrtc.Configuration{})
+			require.NoError(t, err)
+			defer pub.Close() //nolint:errcheck
+
+			videoTrack, err := webrtc.NewTrackLocalStaticRTP(
+				webrtc.RTPCodecCapability{
+					MimeType:    ca.mimeType,
+					ClockRate:   90000,
+					SDPFmtpLine: ca.sdpFmtpLine,
+				},
+				"video", "publisher",
+			)
+			require.NoError(t, err)
+
+			_, err = pub.AddTrack(videoTrack)
+			require.NoError(t, err)
+
+			reader := &PeerConnection{
+				LocalRandomUDP:    true,
+				IPsFromInterfaces: true,
+				Publish:           false,
+				Log:               test.NilLogger,
+			}
+			err = reader.Start()
+			require.NoError(t, err)
+			defer reader.Close()
+
+			offer, err := pub.CreateOffer(nil)
+			require.NoError(t, err)
+
+			err = pub.SetLocalDescription(offer)
+			require.NoError(t, err)
+
+			answer, err := reader.CreateFullAnswer(&offer, false)
+			require.NoError(t, err)
+
+			var s sdp.SessionDescription
+			err = s.Unmarshal([]byte(answer.SDP))
+			require.NoError(t, err)
+
+			require.Len(t, s.MediaDescriptions, 1)
+			videoMedia := s.MediaDescriptions[0]
+			require.Equal(t, "video", videoMedia.MediaName.Media)
+
+			expectedFmtp := fmt.Sprintf("%d apt=%d", ca.rtxPT, ca.payloadType)
+			foundFmtp := false
+			for _, attr := range videoMedia.Attributes {
+				if attr.Key == "fmtp" && attr.Value == expectedFmtp {
+					foundFmtp = true
+				}
+			}
+			require.True(t, foundFmtp,
+				"answer should offer an RTX payload type (apt=%d) for the negotiated %s track, got SDP:\n%s",
+				ca.payloadType, ca.name, answer.SDP)
+
+			expectedRtpmap := fmt.Sprintf("%d rtx/90000", ca.rtxPT)
+			foundRTXRtpmap := false
+			for _, attr := range videoMedia.Attributes {
+				if attr.Key == "rtpmap" && attr.Value == expectedRtpmap {
+					foundRTXRtpmap = true
+				}
+			}
+			require.True(t, foundRTXRtpmap, "answer should declare rtpmap for the RTX payload type, got SDP:\n%s", answer.SDP)
+		})
+	}
+}