Skip to content
This repository has been archived by the owner on Mar 16, 2022. It is now read-only.

Commit

Permalink
Fixes MediaEngine: codec apt negotiation
Browse files Browse the repository at this point in the history
codec has apt fmtp can't negotiation because it's
apt payloadtype can't be found in negotiation codecs.
rtx codec must be partial match if apt codec is partial.

Was reverted in eeb67e1

Fixes pion#1785
  • Loading branch information
cnderrauber authored and Sean-Der committed Apr 24, 2021
1 parent 071d5f2 commit d2f672f
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 3 deletions.
30 changes: 27 additions & 3 deletions mediaengine.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ func (m *MediaEngine) collectStats(collector *statsReportCollector) {
}

// Look up a codec and enable if it exists
func (m *MediaEngine) matchRemoteCodec(remoteCodec RTPCodecParameters, typ RTPCodecType) (codecMatchType, error) {
func (m *MediaEngine) matchRemoteCodec(remoteCodec RTPCodecParameters, typ RTPCodecType, exactMatches, partialMatches []RTPCodecParameters) (codecMatchType, error) {
codecs := m.videoCodecs
if typ == RTPCodecTypeAudio {
codecs = m.audioCodecs
Expand All @@ -347,9 +347,33 @@ func (m *MediaEngine) matchRemoteCodec(remoteCodec RTPCodecParameters, typ RTPCo
return codecMatchNone, err
}

if _, _, err = m.getCodecByPayload(PayloadType(payloadType)); err != nil {
aptMatch := codecMatchNone
for _, codec := range exactMatches {
if codec.PayloadType == PayloadType(payloadType) {
aptMatch = codecMatchExact
break
}
}

if aptMatch == codecMatchNone {
for _, codec := range partialMatches {
if codec.PayloadType == PayloadType(payloadType) {
aptMatch = codecMatchPartial
break
}
}
}

if aptMatch == codecMatchNone {
return codecMatchNone, nil // not an error, we just ignore this codec we don't support
}

// if apt's media codec is partial match, then apt codec must be partial match too
_, matchType := codecParametersFuzzySearch(remoteCodec, codecs)
if matchType == codecMatchExact && aptMatch == codecMatchPartial {
matchType = codecMatchPartial
}
return matchType, nil
}

_, matchType := codecParametersFuzzySearch(remoteCodec, codecs)
Expand Down Expand Up @@ -416,7 +440,7 @@ func (m *MediaEngine) updateFromRemoteDescription(desc sdp.SessionDescription) e
partialMatches := make([]RTPCodecParameters, 0, len(codecs))

for _, codec := range codecs {
matchType, mErr := m.matchRemoteCodec(codec, typ)
matchType, mErr := m.matchRemoteCodec(codec, typ, exactMatches, partialMatches)
if mErr != nil {
return mErr
}
Expand Down
66 changes: 66 additions & 0 deletions mediaengine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,72 @@ a=rtpmap:96 VP8/90000
_, _, err := m.getCodecByPayload(96)
assert.NoError(t, err)
})

t.Run("Matches when rtx apt for exact match codec", func(t *testing.T) {
const profileLevels = `v=0
o=- 4596489990601351948 2 IN IP4 127.0.0.1
s=-
t=0 0
m=video 60323 UDP/TLS/RTP/SAVPF 94 96 97
a=rtpmap:94 VP8/90000
a=rtpmap:96 VP9/90000
a=fmtp:96 profile-id=2
a=rtpmap:97 rtx/90000
a=fmtp:97 apt=96
`
m := MediaEngine{}
assert.NoError(t, m.RegisterCodec(RTPCodecParameters{
RTPCodecCapability: RTPCodecCapability{MimeTypeVP8, 90000, 0, "", nil},
PayloadType: 94,
}, RTPCodecTypeVideo))
assert.NoError(t, m.RegisterCodec(RTPCodecParameters{
RTPCodecCapability: RTPCodecCapability{MimeTypeVP9, 90000, 0, "profile-id=2", nil},
PayloadType: 96,
}, RTPCodecTypeVideo))
assert.NoError(t, m.RegisterCodec(RTPCodecParameters{
RTPCodecCapability: RTPCodecCapability{"video/rtx", 90000, 0, "apt=96", nil},
PayloadType: 97,
}, RTPCodecTypeVideo))
assert.NoError(t, m.updateFromRemoteDescription(mustParse(profileLevels)))

assert.True(t, m.negotiatedVideo)

_, _, err := m.getCodecByPayload(97)
assert.NoError(t, err)
})

t.Run("Matches when rtx apt for partial match codec", func(t *testing.T) {
const profileLevels = `v=0
o=- 4596489990601351948 2 IN IP4 127.0.0.1
s=-
t=0 0
m=video 60323 UDP/TLS/RTP/SAVPF 94 96 97
a=rtpmap:94 VP8/90000
a=rtpmap:96 VP9/90000
a=fmtp:96 profile-id=2
a=rtpmap:97 rtx/90000
a=fmtp:97 apt=96
`
m := MediaEngine{}
assert.NoError(t, m.RegisterCodec(RTPCodecParameters{
RTPCodecCapability: RTPCodecCapability{MimeTypeVP8, 90000, 0, "", nil},
PayloadType: 94,
}, RTPCodecTypeVideo))
assert.NoError(t, m.RegisterCodec(RTPCodecParameters{
RTPCodecCapability: RTPCodecCapability{MimeTypeVP9, 90000, 0, "profile-id=1", nil},
PayloadType: 96,
}, RTPCodecTypeVideo))
assert.NoError(t, m.RegisterCodec(RTPCodecParameters{
RTPCodecCapability: RTPCodecCapability{"video/rtx", 90000, 0, "apt=96", nil},
PayloadType: 97,
}, RTPCodecTypeVideo))
assert.NoError(t, m.updateFromRemoteDescription(mustParse(profileLevels)))

assert.True(t, m.negotiatedVideo)

_, _, err := m.getCodecByPayload(97)
assert.ErrorIs(t, err, ErrCodecNotFound)
})
}

func TestMediaEngineHeaderExtensionDirection(t *testing.T) {
Expand Down

0 comments on commit d2f672f

Please sign in to comment.