From 55d319abfcabed9b58306a35e8322ce889b40335 Mon Sep 17 00:00:00 2001 From: Jonathan Rios <83503872+jonatrios@users.noreply.github.com> Date: Mon, 24 Aug 2026 16:40:20 +0200 Subject: [PATCH] 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> --- internal/protocols/webrtc/inbound_track.go | 93 ++++++++++++ .../protocols/webrtc/inbound_track_test.go | 138 ++++++++++++++++++ 2 files changed, 231 insertions(+) create mode 100644 internal/protocols/webrtc/inbound_track_test.go diff --git a/internal/protocols/webrtc/inbound_track.go b/internal/protocols/webrtc/inbound_track.go index 0cafae79..a8ca353a 100644 --- a/internal/protocols/webrtc/inbound_track.go +++ b/internal/protocols/webrtc/inbound_track.go @@ -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{ diff --git a/internal/protocols/webrtc/inbound_track_test.go b/internal/protocols/webrtc/inbound_track_test.go new file mode 100644 index 00000000..86dcec46 --- /dev/null +++ b/internal/protocols/webrtc/inbound_track_test.go @@ -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) + }) + } +}