Repository: pion/webrtc
Pull request #1773 (2021-04-20 14:50:42)

Fixes MediaEngine: codec apt negotiation
codec has apt fmtp can't negotiation because it's
apt payloadtype can't be found in negotiation codecs.

Description
codec has apt fmtp can't negotiation because it's apt payloadtype can't be found in mediaengine's negotiation codecs.

Fix
transfer exactMatch and partialMatch codec to matchRemoteCodec method, so apt payloadtype can be found in codecs.


codecov[bot]: # [Codecov](https://codecov.io/gh/pion/webrtc/pull/1773?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) Report
> Merging [#1773](https://codecov.io/gh/pion/webrtc/pull/1773?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (cfba47e) into [master](https://codecov.io/gh/pion/webrtc/commit/91a12e8cfbe1e8b4290ade1a6a029b12b72dde8c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (91a12e8) will **increase** coverage by `0.08%`.
> The diff coverage is `70.00%`.

[![Impacted file tree graph](https://codecov.io/gh/pion/webrtc/pull/1773/graphs/tree.svg?width=650&height=150&src=pr&token=XOOuQo07I2&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)](https://codecov.io/gh/pion/webrtc/pull/1773?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)

```diff
@@            Coverage Diff             @@
##           master    #1773      +/-   ##
==========================================
+ Coverage   74.89%   74.97%   +0.08%     
==========================================
  Files          81       81              
  Lines        5879     5887       +8     
==========================================
+ Hits         4403     4414      +11     
+ Misses       1087     1083       -4     
- Partials      389      390       +1     
```

| Flag | Coverage Δ | |
|---|---|---|
| go | `76.66% <70.00%> (+0.09%)` | :arrow_up: |
| wasm | `70.00% <ø> (ø)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/pion/webrtc/pull/1773?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | |
|---|---|---|
| [mediaengine.go](https://codecov.io/gh/pion/webrtc/pull/1773/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-bWVkaWFlbmdpbmUuZ28=) | `81.29% <70.00%> (-0.60%)` | :arrow_down: |
| [icetransport.go](https://codecov.io/gh/pion/webrtc/pull/1773/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-aWNldHJhbnNwb3J0Lmdv) | `67.45% <0.00%> (-1.19%)` | :arrow_down: |
| [peerconnection.go](https://codecov.io/gh/pion/webrtc/pull/1773/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cGVlcmNvbm5lY3Rpb24uZ28=) | `74.72% <0.00%> (-0.17%)` | :arrow_down: |
| [dtlstransport.go](https://codecov.io/gh/pion/webrtc/pull/1773/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-ZHRsc3RyYW5zcG9ydC5nbw==) | `66.82% <0.00%> (+1.84%)` | :arrow_up: |
| [sctptransport.go](https://codecov.io/gh/pion/webrtc/pull/1773/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-c2N0cHRyYW5zcG9ydC5nbw==) | `79.44% <0.00%> (+3.33%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/webrtc/pull/1773?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pion/webrtc/pull/1773?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Last update [91a12e8...cfba47e](https://codecov.io/gh/pion/webrtc/pull/1773?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).


Patch:
diff --git a/mediaengine.go b/mediaengine.go
index 540f20a8..2b9b5895 100644
--- a/mediaengine.go
+++ b/mediaengine.go
@@ -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
@@ -347,7 +347,24 @@ func (m *MediaEngine) matchRemoteCodec(remoteCodec RTPCodecParameters, typ RTPCo
 			return codecMatchNone, err
 		}
 
-		if _, _, err = m.getCodecByPayload(PayloadType(payloadType)); err != nil {
+		var aptFound bool
+		for _, codec := range exactMatches {
+			if codec.PayloadType == PayloadType(payloadType) {
+				aptFound = true
+				break
+			}
+		}
+
+		if !aptFound {
+			for _, codec := range partialMatches {
+				if codec.PayloadType == PayloadType(payloadType) {
+					aptFound = true
+					break
+				}
+			}
+		}
+
+		if !aptFound {
 			return codecMatchNone, nil // not an error, we just ignore this codec we don't support
 		}
 	}
@@ -416,7 +433,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
 			}


Test patch:
diff --git a/mediaengine_test.go b/mediaengine_test.go
index a34386c9..42ec3bef 100644
--- a/mediaengine_test.go
+++ b/mediaengine_test.go
@@ -280,6 +280,33 @@ a=rtpmap:96 VP8/90000
 		_, _, err := m.getCodecByPayload(96)
 		assert.NoError(t, err)
 	})
+
+	t.Run("Matches when rtx apt exists", 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 96 97
+a=rtpmap:96 VP8/90000
+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:        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)
+	})
 }
 
 func TestMediaEngineHeaderExtensionDirection(t *testing.T) {