Skip to content

Commit c82d96c

Browse files
boks1971boks1971
committed
Remove RTX codec if no primary
While adding transceivers from SetRemoteDescription, the filtered codecs could filter out the primart codec and leave the RTX codec in. Generating an answer with that fails `SetRemoteDescription` on remote peer due to an unrecognisable codec. Fix it by filtering out RTX is primary is not there.
1 parent 8efd17e commit c82d96c

File tree

3 files changed

+240
-0
lines changed

3 files changed

+240
-0
lines changed

‎rtpcodec.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package webrtc
55

66
import (
77
"fmt"
8+
"strconv"
89
"strings"
910

1011
"github.com/pion/webrtc/v4/internal/fmtp"
@@ -155,6 +156,38 @@ func findRTXPayloadType(needle PayloadType, haystack []RTPCodecParameters) Paylo
155156
return PayloadType(0)
156157
}
157158

159+
// Given needle CodecParameters, returns if needle is RTX and
160+
// if primary codec corresponding to that needle is in the haystack of codecs.
161+
func primaryPayloadTypeForRTXExists(needle RTPCodecParameters, haystack []RTPCodecParameters) (
162+
isRTX bool, primaryExists bool,
163+
) {
164+
if !strings.EqualFold(needle.MimeType, MimeTypeRTX) {
165+
return
166+
}
167+
168+
isRTX = true
169+
parsed := fmtp.Parse(needle.MimeType, needle.ClockRate, needle.Channels, needle.SDPFmtpLine)
170+
aptPayload, ok := parsed.Parameter("apt")
171+
if !ok {
172+
return
173+
}
174+
175+
primaryPayloadType, err := strconv.Atoi(aptPayload)
176+
if err != nil || primaryPayloadType < 0 || primaryPayloadType > 255 {
177+
return
178+
}
179+
180+
for _, c := range haystack {
181+
if c.PayloadType == PayloadType(primaryPayloadType) {
182+
primaryExists = true
183+
184+
return
185+
}
186+
}
187+
188+
return
189+
}
190+
158191
// For now, only FlexFEC is supported.
159192
func findFECPayloadType(haystack []RTPCodecParameters) PayloadType {
160193
for _, c := range haystack {

‎rtpcodec_test.go

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,199 @@ import (
99
"github.com/stretchr/testify/assert"
1010
)
1111

12+
func TestFindPrimaryPayloadTypeForRTX(t *testing.T) {
13+
for _, test := range []struct {
14+
Name string
15+
Needle RTPCodecParameters
16+
Haystack []RTPCodecParameters
17+
ResultIsRTX bool
18+
ResultPrimaryExists bool
19+
}{
20+
{
21+
Name: "not RTX",
22+
Needle: RTPCodecParameters{
23+
PayloadType: 2,
24+
RTPCodecCapability: RTPCodecCapability{
25+
MimeType: MimeTypeH264,
26+
ClockRate: 90000,
27+
SDPFmtpLine: "apt=2",
28+
},
29+
},
30+
Haystack: []RTPCodecParameters{
31+
{
32+
PayloadType: 1,
33+
RTPCodecCapability: RTPCodecCapability{
34+
MimeType: MimeTypeH264,
35+
ClockRate: 90000,
36+
},
37+
},
38+
},
39+
ResultIsRTX: false,
40+
ResultPrimaryExists: false,
41+
},
42+
{
43+
Name: "incorrect fmtp",
44+
Needle: RTPCodecParameters{
45+
PayloadType: 2,
46+
RTPCodecCapability: RTPCodecCapability{
47+
MimeType: MimeTypeRTX,
48+
ClockRate: 90000,
49+
SDPFmtpLine: "incorrect-fmtp",
50+
},
51+
},
52+
Haystack: []RTPCodecParameters{
53+
{
54+
PayloadType: 1,
55+
RTPCodecCapability: RTPCodecCapability{
56+
MimeType: MimeTypeH264,
57+
ClockRate: 90000,
58+
},
59+
},
60+
},
61+
ResultIsRTX: true,
62+
ResultPrimaryExists: false,
63+
},
64+
{
65+
Name: "incomplete fmtp",
66+
Needle: RTPCodecParameters{
67+
PayloadType: 2,
68+
RTPCodecCapability: RTPCodecCapability{
69+
MimeType: MimeTypeRTX,
70+
ClockRate: 90000,
71+
SDPFmtpLine: "apt=",
72+
},
73+
},
74+
Haystack: []RTPCodecParameters{
75+
{
76+
PayloadType: 1,
77+
RTPCodecCapability: RTPCodecCapability{
78+
MimeType: MimeTypeH264,
79+
ClockRate: 90000,
80+
},
81+
},
82+
},
83+
ResultIsRTX: true,
84+
ResultPrimaryExists: false,
85+
},
86+
{
87+
Name: "primary payload type outside range (negative)",
88+
Needle: RTPCodecParameters{
89+
PayloadType: 2,
90+
RTPCodecCapability: RTPCodecCapability{
91+
MimeType: MimeTypeRTX,
92+
ClockRate: 90000,
93+
SDPFmtpLine: "apt=-10",
94+
},
95+
},
96+
Haystack: []RTPCodecParameters{
97+
{
98+
PayloadType: 1,
99+
RTPCodecCapability: RTPCodecCapability{
100+
MimeType: MimeTypeH264,
101+
ClockRate: 90000,
102+
},
103+
},
104+
},
105+
ResultIsRTX: true,
106+
ResultPrimaryExists: false,
107+
},
108+
{
109+
Name: "primary payload type outside range (high positive)",
110+
Needle: RTPCodecParameters{
111+
PayloadType: 2,
112+
RTPCodecCapability: RTPCodecCapability{
113+
MimeType: MimeTypeRTX,
114+
ClockRate: 90000,
115+
SDPFmtpLine: "apt=1000",
116+
},
117+
},
118+
Haystack: []RTPCodecParameters{
119+
{
120+
PayloadType: 1,
121+
RTPCodecCapability: RTPCodecCapability{
122+
MimeType: MimeTypeH264,
123+
ClockRate: 90000,
124+
},
125+
},
126+
},
127+
ResultIsRTX: true,
128+
ResultPrimaryExists: false,
129+
},
130+
{
131+
Name: "non-matching needle",
132+
Needle: RTPCodecParameters{
133+
PayloadType: 2,
134+
RTPCodecCapability: RTPCodecCapability{
135+
MimeType: MimeTypeRTX,
136+
ClockRate: 90000,
137+
SDPFmtpLine: "apt=23",
138+
},
139+
},
140+
Haystack: []RTPCodecParameters{
141+
{
142+
PayloadType: 1,
143+
RTPCodecCapability: RTPCodecCapability{
144+
MimeType: MimeTypeH264,
145+
ClockRate: 90000,
146+
},
147+
},
148+
},
149+
ResultIsRTX: true,
150+
ResultPrimaryExists: false,
151+
},
152+
{
153+
Name: "matching needle",
154+
Needle: RTPCodecParameters{
155+
PayloadType: 2,
156+
RTPCodecCapability: RTPCodecCapability{
157+
MimeType: MimeTypeRTX,
158+
ClockRate: 90000,
159+
SDPFmtpLine: "apt=1",
160+
},
161+
},
162+
Haystack: []RTPCodecParameters{
163+
{
164+
PayloadType: 1,
165+
RTPCodecCapability: RTPCodecCapability{
166+
MimeType: MimeTypeH264,
167+
ClockRate: 90000,
168+
},
169+
},
170+
},
171+
ResultIsRTX: true,
172+
ResultPrimaryExists: true,
173+
},
174+
{
175+
Name: "matching fmtp is a substring",
176+
Needle: RTPCodecParameters{
177+
PayloadType: 2,
178+
RTPCodecCapability: RTPCodecCapability{
179+
MimeType: MimeTypeRTX,
180+
ClockRate: 90000,
181+
SDPFmtpLine: "apt=1;rtx-time:2000",
182+
},
183+
},
184+
Haystack: []RTPCodecParameters{
185+
{
186+
PayloadType: 1,
187+
RTPCodecCapability: RTPCodecCapability{
188+
MimeType: MimeTypeH264,
189+
ClockRate: 90000,
190+
},
191+
},
192+
},
193+
ResultIsRTX: true,
194+
ResultPrimaryExists: true,
195+
},
196+
} {
197+
t.Run(test.Name, func(t *testing.T) {
198+
isRTX, primaryExists := primaryPayloadTypeForRTXExists(test.Needle, test.Haystack)
199+
assert.Equal(t, test.ResultIsRTX, isRTX)
200+
assert.Equal(t, test.ResultPrimaryExists, primaryExists)
201+
})
202+
}
203+
}
204+
12205
func TestFindFECPayloadType(t *testing.T) {
13206
for _, test := range []struct {
14207
Haystack []RTPCodecParameters

‎rtptransceiver.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package webrtc
88

99
import (
1010
"fmt"
11+
"strings"
1112
"sync"
1213
"sync/atomic"
1314

@@ -60,6 +61,19 @@ func (t *RTPTransceiver) SetCodecPreferences(codecs []RTPCodecParameters) error
6061
}
6162
}
6263

64+
// remove RTX codecs if there is no corresponding primary codec
65+
for i := len(codecs) - 1; i >= 0; i-- {
66+
c := codecs[i]
67+
if !strings.EqualFold(c.MimeType, MimeTypeRTX) {
68+
continue
69+
}
70+
71+
if isRTX, primaryExists := primaryPayloadTypeForRTXExists(c, codecs); isRTX && !primaryExists {
72+
// no primary for RTX, remove the RTX
73+
codecs = append(codecs[:i], codecs[i+1:]...)
74+
}
75+
}
76+
6377
t.codecs = codecs
6478

6579
return nil

0 commit comments

Comments
 (0)