Filter unattached RTX when getting codecs

Recently added filtering out unattached RTX in SetCodecPrefrences. Turns out it is needed in `getCodecs()` also in the following scenario - AddTrack from answer side adds a tracks and has media engine codecs - An offer is received and negotiated codecs get updated in media engine. And this does not have one of the codecs added AddTrack above (default media engine codecs) - Generate answer will do `getCodecs` which will filter out the codec missing from the `offer`, but would have let the corresponding RTX pass through and get added to the answer.
2025-09-26 19:21:12 +08:00 · 2025-08-26 23:21:11 +05:30
parent 123f1388c4
commit 2af60a491d
3 changed files with 163 additions and 25 deletions
--- a/rtpcodec.go
+++ b/rtpcodec.go
@@ -188,6 +188,19 @@ func primaryPayloadTypeForRTXExists(needle RTPCodecParameters, haystack []RTPCod
 	return
 }

+// Filter out RTX codecs that do not have a primary codec.
+func filterUnattachedRTX(codecs []RTPCodecParameters) []RTPCodecParameters {
+	for i := len(codecs) - 1; i >= 0; i-- {
+		c := codecs[i]
+		if isRTX, primaryExists := primaryPayloadTypeForRTXExists(c, codecs); isRTX && !primaryExists {
+			// no primary for RTX, remove the RTX
+			codecs = append(codecs[:i], codecs[i+1:]...)
+		}
+	}
+
+	return codecs
+}
+
 // For now, only FlexFEC is supported.
 func findFECPayloadType(haystack []RTPCodecParameters) PayloadType {
 	for _, c := range haystack {
--- a/rtptransceiver.go
+++ b/rtptransceiver.go
@@ -8,7 +8,6 @@ package webrtc

 import (
 	"fmt"
-	"strings"
 	"sync"
 	"sync/atomic"

@@ -62,25 +61,12 @@ func (t *RTPTransceiver) SetCodecPreferences(codecs []RTPCodecParameters) error
 		}
 	}

-	// remove RTX codecs if there is no corresponding primary codec
-	for i := len(codecs) - 1; i >= 0; i-- {
-		c := codecs[i]
-		if !strings.EqualFold(c.MimeType, MimeTypeRTX) {
-			continue
-		}
-
-		if isRTX, primaryExists := primaryPayloadTypeForRTXExists(c, codecs); isRTX && !primaryExists {
-			// no primary for RTX, remove the RTX
-			codecs = append(codecs[:i], codecs[i+1:]...)
-		}
-	}
-
-	t.codecs = codecs
+	t.codecs = filterUnattachedRTX(codecs)

 	return nil
 }

-// Codecs returns list of supported codecs.
+// getCodecs returns list of supported codecs.
 func (t *RTPTransceiver) getCodecs() []RTPCodecParameters {
 	t.mu.RLock()
 	defer t.mu.RUnlock()
@@ -101,7 +87,7 @@ func (t *RTPTransceiver) getCodecs() []RTPCodecParameters {
 		}
 	}

-	return filteredCodecs
+	return filterUnattachedRTX(filteredCodecs)
 }

 // Sender returns the RTPTransceiver's RTPSender if it has one.
--- a/rtptransceiver_test.go
+++ b/rtptransceiver_test.go
@@ -88,18 +88,28 @@ func Test_RTPTransceiver_SetCodecPreferences(t *testing.T) {

 // Assert that SetCodecPreferences properly filters codecs and PayloadTypes are respected.
 func Test_RTPTransceiver_SetCodecPreferences_PayloadType(t *testing.T) {
-	testCodec := RTPCodecParameters{
-		RTPCodecCapability: RTPCodecCapability{"video/testCodec", 90000, 0, "", nil},
+	notOfferedCodec := RTPCodecParameters{
+		RTPCodecCapability: RTPCodecCapability{"video/notOfferedCodec", 90000, 0, "", nil},
 		PayloadType:        50,
 	}
+	offeredCodec := RTPCodecParameters{
+		RTPCodecCapability: RTPCodecCapability{"video/offeredCodec", 90000, 0, "", nil},
+		PayloadType:        52,
+	}
+	offeredCodecRTX := RTPCodecParameters{
+		RTPCodecCapability: RTPCodecCapability{"video/rtx", 90000, 0, "apt=52", nil},
+		PayloadType:        53,
+	}

 	mediaEngine := &MediaEngine{}
 	assert.NoError(t, mediaEngine.RegisterDefaultCodecs())
+	assert.NoError(t, mediaEngine.RegisterCodec(offeredCodec, RTPCodecTypeVideo))
+	assert.NoError(t, mediaEngine.RegisterCodec(offeredCodecRTX, RTPCodecTypeVideo))

 	offerPC, err := NewAPI(WithMediaEngine(mediaEngine)).NewPeerConnection(Configuration{})
 	assert.NoError(t, err)

-	assert.NoError(t, mediaEngine.RegisterCodec(testCodec, RTPCodecTypeVideo))
+	assert.NoError(t, mediaEngine.RegisterCodec(notOfferedCodec, RTPCodecTypeVideo))

 	answerPC, err := NewAPI(WithMediaEngine(mediaEngine)).NewPeerConnection(Configuration{})
 	assert.NoError(t, err)
@@ -107,14 +117,21 @@ func Test_RTPTransceiver_SetCodecPreferences_PayloadType(t *testing.T) {
 	_, err = offerPC.AddTransceiverFromKind(RTPCodecTypeVideo)
 	assert.NoError(t, err)

-	answerTransceiver, err := answerPC.AddTransceiverFromKind(RTPCodecTypeVideo)
+	track, err := NewTrackLocalStaticRTP(RTPCodecCapability{MimeType: MimeTypeVP8}, "video", "pion")
+	assert.NoError(t, err)
+	answerTransceiver, err := answerPC.AddTransceiverFromTrack(
+		track,
+		RTPTransceiverInit{Direction: RTPTransceiverDirectionSendonly},
+	)
 	assert.NoError(t, err)

 	assert.NoError(t, answerTransceiver.SetCodecPreferences([]RTPCodecParameters{
-		testCodec,
+		notOfferedCodec,
+		offeredCodec,
+		offeredCodecRTX,
 		{
 			RTPCodecCapability: RTPCodecCapability{MimeTypeVP8, 90000, 0, "", nil},
-			PayloadType:        51,
+			PayloadType:        54,
 		},
 	}))

@@ -128,10 +145,132 @@ func Test_RTPTransceiver_SetCodecPreferences_PayloadType(t *testing.T) {
 	assert.NoError(t, err)

 	// VP8 with proper PayloadType
-	assert.NotEqual(t, -1, strings.Index(answer.SDP, "a=rtpmap:51 VP8/90000"))
+	assert.NotEqual(t, -1, strings.Index(answer.SDP, "a=rtpmap:54 VP8/90000"))
+
+	// testCodec1 and testCodec1RTX should be included as they are in the offer
+	assert.NotEqual(t, -1, strings.Index(answer.SDP, "a=rtpmap:52 offeredCodec/90000"))
+	assert.NotEqual(t, -1, strings.Index(answer.SDP, "a=rtpmap:53 rtx/90000"))
+	assert.NotEqual(t, -1, strings.Index(answer.SDP, "a=fmtp:53 apt=52"))

 	// testCodec is ignored since offerer doesn't support
-	assert.Equal(t, -1, strings.Index(answer.SDP, "testCodec"))
+	assert.Equal(t, -1, strings.Index(answer.SDP, "notOfferedCodec"))
+
+	closePairNow(t, offerPC, answerPC)
+}
+
+// Assert that SetCodecPreferences and getCodecs properly filters unattached RTX.
+func Test_RTPTransceiver_UnattachedRTX(t *testing.T) {
+	testCodec := RTPCodecParameters{
+		RTPCodecCapability: RTPCodecCapability{"video/testCodec", 90000, 0, "", nil},
+		PayloadType:        50,
+	}
+	testCodecRTX := RTPCodecParameters{
+		RTPCodecCapability: RTPCodecCapability{"video/rtx", 90000, 0, "apt=50", nil},
+		PayloadType:        51,
+	}
+
+	mediaEngine := &MediaEngine{}
+	assert.NoError(t, mediaEngine.RegisterDefaultCodecs())
+
+	offerPC, err := NewAPI(WithMediaEngine(mediaEngine)).NewPeerConnection(Configuration{})
+	assert.NoError(t, err)
+
+	assert.NoError(t, mediaEngine.RegisterCodec(testCodec, RTPCodecTypeVideo))
+	assert.NoError(t, mediaEngine.RegisterCodec(testCodecRTX, RTPCodecTypeVideo))
+
+	answerPC, err := NewAPI(WithMediaEngine(mediaEngine)).NewPeerConnection(Configuration{})
+	assert.NoError(t, err)
+
+	_, err = offerPC.AddTransceiverFromKind(RTPCodecTypeVideo)
+	assert.NoError(t, err)
+
+	answerTransceiver, err := answerPC.AddTransceiverFromKind(RTPCodecTypeVideo)
+	assert.NoError(t, err)
+
+	assert.NoError(t, answerTransceiver.SetCodecPreferences([]RTPCodecParameters{
+		testCodecRTX,
+		{
+			RTPCodecCapability: RTPCodecCapability{MimeTypeVP8, 90000, 0, "", nil},
+			PayloadType:        52,
+		},
+	}))
+
+	// rtx should not be in the list of transceiver codecs as testCodec (primary) is
+	// not given to SetCodecPreferences
+	answerTransceiver.mu.RLock()
+	foundRTX := false
+	for _, codec := range answerTransceiver.codecs {
+		if strings.EqualFold(codec.RTPCodecCapability.MimeType, MimeTypeRTX) {
+			foundRTX = true
+
+			break
+		}
+	}
+	assert.False(t, foundRTX)
+	answerTransceiver.mu.RUnlock()
+
+	assert.NoError(t, answerTransceiver.SetCodecPreferences([]RTPCodecParameters{
+		testCodec,
+		testCodecRTX,
+		{
+			RTPCodecCapability: RTPCodecCapability{MimeTypeVP8, 90000, 0, "", nil},
+			PayloadType:        52,
+		},
+	}))
+
+	// rtx should be in the list of transceiver codecs as testCodec (primary) is
+	// given to SetCodecPreferences
+	answerTransceiver.mu.RLock()
+	foundRTX = false
+	for _, codec := range answerTransceiver.codecs {
+		if strings.EqualFold(codec.RTPCodecCapability.MimeType, MimeTypeRTX) {
+			foundRTX = true
+
+			break
+		}
+	}
+	assert.True(t, foundRTX)
+	answerTransceiver.mu.RUnlock()
+
+	// getCodecs() should have RTX as remote offer has not been processed
+	codecs := answerTransceiver.getCodecs()
+	foundRTX = false
+	for _, codec := range codecs {
+		if strings.EqualFold(codec.RTPCodecCapability.MimeType, MimeTypeRTX) {
+			foundRTX = true
+
+			break
+		}
+	}
+	assert.True(t, foundRTX)
+
+	offer, err := offerPC.CreateOffer(nil)
+	assert.NoError(t, err)
+
+	assert.NoError(t, offerPC.SetLocalDescription(offer))
+	assert.NoError(t, answerPC.SetRemoteDescription(offer))
+
+	// getCodecs() should filter out RTX as remote does not offer testCodec (primary)
+	codecs = answerTransceiver.getCodecs()
+	foundRTX = false
+	for _, codec := range codecs {
+		if strings.EqualFold(codec.RTPCodecCapability.MimeType, MimeTypeRTX) {
+			foundRTX = true
+
+			break
+		}
+	}
+	assert.False(t, foundRTX)
+
+	answer, err := answerPC.CreateAnswer(nil)
+	assert.NoError(t, err)
+
+	// VP8 with proper PayloadType
+	assert.NotEqual(t, -1, strings.Index(answer.SDP, "a=rtpmap:52 VP8/90000"))
+
+	// testCodec is ignored since offerer doesn't support
+	assert.Equal(t, -1, strings.Index(answer.SDP, "testCodec"))
+	assert.Equal(t, -1, strings.Index(answer.SDP, "rtx"))

 	closePairNow(t, offerPC, answerPC)
 }