diff --git a/rtpcodec.go b/rtpcodec.go index d03dba0b..d5d03f52 100644 --- 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 { diff --git a/rtptransceiver.go b/rtptransceiver.go index 3c9de627..2afc20b0 100644 --- 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. diff --git a/rtptransceiver_test.go b/rtptransceiver_test.go index 298556a6..4be02e8a 100644 --- 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) }