Repository: pion/webrtc
Pull request #1677 (2021-02-16 15:21:37)

Fix register the same codec

#### Description
Fix register the same codec, otherwise addTransceiverSDP will add duplicate codec sdp
Otherwise addTransceiverSDP will add duplicate codec sdp

#### Reference issue
Fix https://github.com/pion/ion-sfu/issues/425
```
v=0
o=- 5454648987554646034 1613488747 IN IP4 0.0.0.0
s=-
t=0 0
a=fingerprint:sha-256 56:62:E1:01:34:08:29:A2:72:E2:0E:28:4E:C6:C4:48:C9:C2:18:72:06:CF:A3:C9:2A:49:30:4F:6E:90:C9:89
a=group:BUNDLE 0 1 2 3 4
m=audio 9 UDP/TLS/RTP/SAVPF 111 111
c=IN IP4 0.0.0.0
a=setup:actpass
a=mid:0
a=ice-ufrag:KObOgVaHjOnnAtdX
a=ice-pwd:FMuiwaSPGdKPtXpjVLBaNoEMUjpKOson
a=rtcp-mux
a=rtcp-rsize
a=rtpmap:111 opus/48000/2
a=fmtp:111 minptime=10;useinbandfec=1
a=rtpmap:111 opus/48000/2
a=fmtp:111 minptime=10;useinbandfec=1
a=ssrc:2193652934 cname:webm_0xc000163b80
a=ssrc:2193652934 msid:webm_0xc000163b80 audio
a=ssrc:2193652934 mslabel:webm_0xc000163b80
a=ssrc:2193652934 label:audio
a=msid:webm_0xc000163b80 audio
a=sendonly
m=video 9 UDP/TLS/RTP/SAVPF 96 96
c=IN IP4 0.0.0.0
a=setup:actpass
a=mid:1
a=ice-ufrag:KObOgVaHjOnnAtdX
a=ice-pwd:FMuiwaSPGdKPtXpjVLBaNoEMUjpKOson
a=rtcp-mux
a=rtcp-rsize
a=rtpmap:96 vp8/90000
a=rtcp-fb:96 goog-remb
a=rtcp-fb:96 ccm fir
a=rtcp-fb:96 nack
a=rtcp-fb:96 nack pli
a=rtpmap:96 vp8/90000
a=rtcp-fb:96 goog-remb
a=rtcp-fb:96 ccm fir
a=rtcp-fb:96 nack
a=rtcp-fb:96 nack pli
a=ssrc:2498583866 cname:webm_0xc000163b80
a=ssrc:2498583866 msid:webm_0xc000163b80 video
a=ssrc:2498583866 mslabel:webm_0xc000163b80
a=ssrc:2498583866 label:video
a=msid:webm_0xc000163b80 video
a=sendonly
m=audio 9 UDP/TLS/RTP/SAVPF 111 111
c=IN IP4 0.0.0.0
a=setup:actpass
a=mid:2
a=ice-ufrag:KObOgVaHjOnnAtdX
a=ice-pwd:FMuiwaSPGdKPtXpjVLBaNoEMUjpKOson
a=rtcp-mux
a=rtcp-rsize
a=rtpmap:111 opus/48000/2
a=fmtp:111 minptime=10;useinbandfec=1
a=rtpmap:111 opus/48000/2
a=fmtp:111 minptime=10;useinbandfec=1
a=ssrc:1801454766 cname:webm_0xc000162000
a=ssrc:1801454766 msid:webm_0xc000162000 audio
a=ssrc:1801454766 mslabel:webm_0xc000162000
a=ssrc:1801454766 label:audio
a=msid:webm_0xc000162000 audio
a=sendonly
m=video 9 UDP/TLS/RTP/SAVPF 96 96
c=IN IP4 0.0.0.0
a=setup:actpass
a=mid:3
a=ice-ufrag:KObOgVaHjOnnAtdX
a=ice-pwd:FMuiwaSPGdKPtXpjVLBaNoEMUjpKOson
a=rtcp-mux
a=rtcp-rsize
a=rtpmap:96 vp8/90000
a=rtcp-fb:96 goog-remb
a=rtcp-fb:96 ccm fir
a=rtcp-fb:96 nack
a=rtcp-fb:96 nack pli
a=rtpmap:96 vp8/90000
a=rtcp-fb:96 goog-remb
a=rtcp-fb:96 ccm fir
a=rtcp-fb:96 nack
a=rtcp-fb:96 nack pli
a=ssrc:2963805140 cname:webm_0xc000162000
a=ssrc:2963805140 msid:webm_0xc000162000 video
a=ssrc:2963805140 mslabel:webm_0xc000162000
a=ssrc:2963805140 label:video
a=msid:webm_0xc000162000 video
a=sendonly
m=application 9 UDP/DTLS/SCTP webrtc-datachannel
c=IN IP4 0.0.0.0
a=setup:actpass
a=mid:4
a=sendrecv
a=sctp-port:5000
a=ice-ufrag:KObOgVaHjOnnAtdX
a=ice-pwd:FMuiwaSPGdKPtXpjVLBaNoEMUjpKOson
```


codecov[bot]: # [Codecov](https://codecov.io/gh/pion/webrtc/pull/1677?src=pr&el=h1) Report
> Merging [#1677](https://codecov.io/gh/pion/webrtc/pull/1677?src=pr&el=desc) (1d05927) into [master](https://codecov.io/gh/pion/webrtc/commit/24f350c926909e1d41fe79565a226dd934d3a3bf?el=desc) (24f350c) will **decrease** coverage by `4.88%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/pion/webrtc/pull/1677/graphs/tree.svg?width=650&height=150&src=pr&token=XOOuQo07I2)](https://codecov.io/gh/pion/webrtc/pull/1677?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1677      +/-   ##
==========================================
- Coverage   74.81%   69.93%   -4.89%     
==========================================
  Files          79       57      -22     
  Lines        5702     2145    -3557     
==========================================
- Hits         4266     1500    -2766     
+ Misses       1062      542     -520     
+ Partials      374      103     -271     
```

| Flag | Coverage Δ | |
|---|---|---|
| go | `?` | |
| wasm | `69.93% <ø> (ø)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/pion/webrtc/pull/1677?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [icetcp.go](https://codecov.io/gh/pion/webrtc/pull/1677/diff?src=pr&el=tree#diff-aWNldGNwLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [rtpcodec.go](https://codecov.io/gh/pion/webrtc/pull/1677/diff?src=pr&el=tree#diff-cnRwY29kZWMuZ28=) | `0.00% <0.00%> (-89.48%)` | :arrow_down: |
| [icecandidatepair.go](https://codecov.io/gh/pion/webrtc/pull/1677/diff?src=pr&el=tree#diff-aWNlY2FuZGlkYXRlcGFpci5nbw==) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [track\_local.go](https://codecov.io/gh/pion/webrtc/pull/1677/diff?src=pr&el=tree#diff-dHJhY2tfbG9jYWwuZ28=) | `0.00% <0.00%> (-80.00%)` | :arrow_down: |
| [stats.go](https://codecov.io/gh/pion/webrtc/pull/1677/diff?src=pr&el=tree#diff-c3RhdHMuZ28=) | `0.00% <0.00%> (-67.86%)` | :arrow_down: |
| [internal/mux/muxfunc.go](https://codecov.io/gh/pion/webrtc/pull/1677/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbXV4L211eGZ1bmMuZ28=) | `0.00% <0.00%> (-47.83%)` | :arrow_down: |
| [gathering\_complete\_promise.go](https://codecov.io/gh/pion/webrtc/pull/1677/diff?src=pr&el=tree#diff-Z2F0aGVyaW5nX2NvbXBsZXRlX3Byb21pc2UuZ28=) | `60.00% <0.00%> (-40.00%)` | :arrow_down: |
| [internal/mux/endpoint.go](https://codecov.io/gh/pion/webrtc/pull/1677/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbXV4L2VuZHBvaW50Lmdv) | `22.22% <0.00%> (-38.89%)` | :arrow_down: |
| [sdpsemantics.go](https://codecov.io/gh/pion/webrtc/pull/1677/diff?src=pr&el=tree#diff-c2Rwc2VtYW50aWNzLmdv) | `0.00% <0.00%> (-37.50%)` | :arrow_down: |
| [dtlsrole.go](https://codecov.io/gh/pion/webrtc/pull/1677/diff?src=pr&el=tree#diff-ZHRsc3JvbGUuZ28=) | `68.96% <0.00%> (-31.04%)` | :arrow_down: |
| ... and [30 more](https://codecov.io/gh/pion/webrtc/pull/1677/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/webrtc/pull/1677?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pion/webrtc/pull/1677?src=pr&el=footer). Last update [24f350c...0b1460d](https://codecov.io/gh/pion/webrtc/pull/1677?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Sean-Der: @adwpc this LGTM! Mind adding a test, then we can merge.

If you don’t have time I can do tonight.

Patch:
diff --git a/mediaengine.go b/mediaengine.go
index 96f4de12..95527eec 100644
--- a/mediaengine.go
+++ b/mediaengine.go
@@ -182,6 +182,16 @@ func (m *MediaEngine) RegisterDefaultCodecs() error {
 	return nil
 }
 
+// addCodec will append codec if it not exists
+func (m *MediaEngine) addCodec(codecs []RTPCodecParameters, codec RTPCodecParameters) []RTPCodecParameters {
+	for _, c := range codecs {
+		if c.MimeType == codec.MimeType && c.PayloadType == codec.PayloadType {
+			return codecs
+		}
+	}
+	return append(codecs, codec)
+}
+
 // RegisterCodec adds codec to the MediaEngine
 // These are the list of codecs supported by this PeerConnection.
 // RegisterCodec is not safe for concurrent use.
@@ -189,9 +199,9 @@ func (m *MediaEngine) RegisterCodec(codec RTPCodecParameters, typ RTPCodecType)
 	codec.statsID = fmt.Sprintf("RTPCodec-%d", time.Now().UnixNano())
 	switch typ {
 	case RTPCodecTypeAudio:
-		m.audioCodecs = append(m.audioCodecs, codec)
+		m.audioCodecs = m.addCodec(m.audioCodecs, codec)
 	case RTPCodecTypeVideo:
-		m.videoCodecs = append(m.videoCodecs, codec)
+		m.videoCodecs = m.addCodec(m.videoCodecs, codec)
 	default:
 		return ErrUnknownType
 	}
@@ -328,9 +338,9 @@ func (m *MediaEngine) updateCodecParameters(remoteCodec RTPCodecParameters, typ
 
 	pushCodec := func(codec RTPCodecParameters) error {
 		if typ == RTPCodecTypeAudio {
-			m.negotiatedAudioCodecs = append(m.negotiatedAudioCodecs, codec)
+			m.negotiatedAudioCodecs = m.addCodec(m.negotiatedAudioCodecs, codec)
 		} else if typ == RTPCodecTypeVideo {
-			m.negotiatedVideoCodecs = append(m.negotiatedVideoCodecs, codec)
+			m.negotiatedVideoCodecs = m.addCodec(m.negotiatedVideoCodecs, codec)
 		}
 		return nil
 	}


Test patch:
diff --git a/mediaengine_test.go b/mediaengine_test.go
index 5c3077e7..6abb044a 100644
--- a/mediaengine_test.go
+++ b/mediaengine_test.go
@@ -242,3 +242,22 @@ func TestMediaEngineHeaderExtensionDirection(t *testing.T) {
 		assert.Error(t, m.RegisterHeaderExtension(RTPHeaderExtensionCapability{"pion-header-test"}, RTPCodecTypeAudio, RTPTransceiverDirection(0)), ErrRegisterHeaderExtensionInvalidDirection)
 	})
 }
+
+// If a user attempts to register a codec twice we should just discard duplicate calls
+func TestMediaEngineDoubleRegister(t *testing.T) {
+	m := MediaEngine{}
+
+	assert.NoError(t, m.RegisterCodec(
+		RTPCodecParameters{
+			RTPCodecCapability: RTPCodecCapability{MimeTypeOpus, 48000, 0, "", nil},
+			PayloadType:        111,
+		}, RTPCodecTypeAudio))
+
+	assert.NoError(t, m.RegisterCodec(
+		RTPCodecParameters{
+			RTPCodecCapability: RTPCodecCapability{MimeTypeOpus, 48000, 0, "", nil},
+			PayloadType:        111,
+		}, RTPCodecTypeAudio))
+
+	assert.Equal(t, len(m.audioCodecs), 1)
+}