Repository: pion/webrtc
Pull request #2215 (2022-05-11 07:57:06)

Set extmap-allow-mixed by default
#### Description
For support with dependency descriptor, chrome will use
mixed extmap both one-byte and two-byte defined in RFC8285,
add the attribute to support that. Without this attribute, chrome will
discard dependency descriptor attached in key frame since there is
not enough space in one-byte extension for the dependency descriptor.
Chrome has set this by default https://chromestatus.com/feature/6269234631933952
and safari has this attribute too. Firefox don't have this but it has no problem work with this.

Actually in pion/rtp, it has no restrict for mixed extension, the only problem is add
the attribute to sdp then client knows that pion support this.

https://github.com/pion/sdp/pull/132

#### Reference issue
Fixes #...


codecov[bot]: # [Codecov](https://codecov.io/gh/pion/webrtc/pull/2215?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) Report
> Merging [#2215](https://codecov.io/gh/pion/webrtc/pull/2215?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (1452cb5) into [master](https://codecov.io/gh/pion/webrtc/commit/16f536785d4c8e5b1a7978aecc79f060899a2e33?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (16f5367) will **decrease** coverage by `6.87%`.
> The diff coverage is `n/a`.

```diff
@@            Coverage Diff             @@
##           master    #2215      +/-   ##
==========================================
- Coverage   77.08%   70.21%   -6.88%     
==========================================
  Files          87       64      -23     
  Lines        9153     3361    -5792     
==========================================
- Hits         7056     2360    -4696     
+ Misses       1667      873     -794     
+ Partials      430      128     -302     
```

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

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/2215?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | |
|---|---|---|
| [constants.go](https://codecov.io/gh/pion/webrtc/pull/2215/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-Y29uc3RhbnRzLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [icecandidatepair.go](https://codecov.io/gh/pion/webrtc/pull/2215/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-aWNlY2FuZGlkYXRlcGFpci5nbw==) | `0.00% <0.00%> (-81.82%)` | :arrow_down: |
| [rtpcodec.go](https://codecov.io/gh/pion/webrtc/pull/2215/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cnRwY29kZWMuZ28=) | `13.33% <0.00%> (-80.00%)` | :arrow_down: |
| [stats.go](https://codecov.io/gh/pion/webrtc/pull/2215/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-c3RhdHMuZ28=) | `0.00% <0.00%> (-75.00%)` | :arrow_down: |
| [track\_local.go](https://codecov.io/gh/pion/webrtc/pull/2215/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-dHJhY2tfbG9jYWwuZ28=) | `0.00% <0.00%> (-66.67%)` | :arrow_down: |
| [icemux.go](https://codecov.io/gh/pion/webrtc/pull/2215/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-aWNlbXV4Lmdv) | `0.00% <0.00%> (-54.55%)` | :arrow_down: |
| [internal/mux/endpoint.go](https://codecov.io/gh/pion/webrtc/pull/2215/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-aW50ZXJuYWwvbXV4L2VuZHBvaW50Lmdv) | `13.79% <0.00%> (-44.83%)` | :arrow_down: |
| [atomicbool.go](https://codecov.io/gh/pion/webrtc/pull/2215/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-YXRvbWljYm9vbC5nbw==) | `57.14% <0.00%> (-42.86%)` | :arrow_down: |
| [sdpsemantics.go](https://codecov.io/gh/pion/webrtc/pull/2215/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-c2Rwc2VtYW50aWNzLmdv) | `0.00% <0.00%> (-34.49%)` | :arrow_down: |
| [dtlsrole.go](https://codecov.io/gh/pion/webrtc/pull/2215/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-ZHRsc3JvbGUuZ28=) | `71.42% <0.00%> (-28.58%)` | :arrow_down: |
| ... and [37 more](https://codecov.io/gh/pion/webrtc/pull/2215/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/webrtc/pull/2215?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/2215?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Last update [16f5367...1452cb5](https://codecov.io/gh/pion/webrtc/pull/2215?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/peerconnection.go b/peerconnection.go
index 1bb34777..232be96b 100644
--- a/peerconnection.go
+++ b/peerconnection.go
@@ -2250,7 +2250,7 @@ func (pc *PeerConnection) generateUnmatchedSDP(transceivers []*RTPTransceiver, u
 		return nil, err
 	}
 
-	return populateSDP(d, isPlanB, dtlsFingerprints, pc.api.settingEngine.sdpMediaLevelFingerprints, pc.api.settingEngine.candidates.ICELite, pc.api.mediaEngine, connectionRoleFromDtlsRole(defaultDtlsRoleOffer), candidates, iceParams, mediaSections, pc.ICEGatheringState())
+	return populateSDP(d, isPlanB, dtlsFingerprints, pc.api.settingEngine.sdpMediaLevelFingerprints, pc.api.settingEngine.candidates.ICELite, true, pc.api.mediaEngine, connectionRoleFromDtlsRole(defaultDtlsRoleOffer), candidates, iceParams, mediaSections, pc.ICEGatheringState())
 }
 
 // generateMatchedSDP generates a SDP and takes the remote state into account
@@ -2277,6 +2277,7 @@ func (pc *PeerConnection) generateMatchedSDP(transceivers []*RTPTransceiver, use
 	if pc.pendingRemoteDescription != nil {
 		remoteDescription = pc.pendingRemoteDescription
 	}
+	isExtmapAllowMixed := isExtMapAllowMixedSet(remoteDescription.parsed)
 	localTransceivers := append([]*RTPTransceiver{}, transceivers...)
 	detectedPlanB := descriptionIsPlanB(remoteDescription)
 	mediaSections := []mediaSection{}
@@ -2371,7 +2372,7 @@ func (pc *PeerConnection) generateMatchedSDP(transceivers []*RTPTransceiver, use
 		return nil, err
 	}
 
-	return populateSDP(d, detectedPlanB, dtlsFingerprints, pc.api.settingEngine.sdpMediaLevelFingerprints, pc.api.settingEngine.candidates.ICELite, pc.api.mediaEngine, connectionRole, candidates, iceParams, mediaSections, pc.ICEGatheringState())
+	return populateSDP(d, detectedPlanB, dtlsFingerprints, pc.api.settingEngine.sdpMediaLevelFingerprints, pc.api.settingEngine.candidates.ICELite, isExtmapAllowMixed, pc.api.mediaEngine, connectionRole, candidates, iceParams, mediaSections, pc.ICEGatheringState())
 }
 
 func (pc *PeerConnection) setGatherCompleteHandler(handler func()) {
diff --git a/sdp.go b/sdp.go
index 185eca41..87e1ba1e 100644
--- a/sdp.go
+++ b/sdp.go
@@ -501,7 +501,7 @@ type mediaSection struct {
 }
 
 // populateSDP serializes a PeerConnections state into an SDP
-func populateSDP(d *sdp.SessionDescription, isPlanB bool, dtlsFingerprints []DTLSFingerprint, mediaDescriptionFingerprint bool, isICELite bool, mediaEngine *MediaEngine, connectionRole sdp.ConnectionRole, candidates []ICECandidate, iceParams ICEParameters, mediaSections []mediaSection, iceGatheringState ICEGatheringState) (*sdp.SessionDescription, error) {
+func populateSDP(d *sdp.SessionDescription, isPlanB bool, dtlsFingerprints []DTLSFingerprint, mediaDescriptionFingerprint bool, isICELite bool, isExtmapAllowMixed bool, mediaEngine *MediaEngine, connectionRole sdp.ConnectionRole, candidates []ICECandidate, iceParams ICEParameters, mediaSections []mediaSection, iceGatheringState ICEGatheringState) (*sdp.SessionDescription, error) {
 	var err error
 	mediaDtlsFingerprints := []DTLSFingerprint{}
 
@@ -552,6 +552,10 @@ func populateSDP(d *sdp.SessionDescription, isPlanB bool, dtlsFingerprints []DTL
 		d = d.WithValueAttribute(sdp.AttrKeyICELite, "")
 	}
 
+	if isExtmapAllowMixed {
+		d = d.WithPropertyAttribute(sdp.AttrKeyExtMapAllowMixed)
+	}
+
 	return d.WithValueAttribute(sdp.AttrKeyGroup, bundleValue), nil
 }
 
@@ -797,3 +801,13 @@ func isIceLiteSet(desc *sdp.SessionDescription) bool {
 
 	return false
 }
+
+func isExtMapAllowMixedSet(desc *sdp.SessionDescription) bool {
+	for _, a := range desc.Attributes {
+		if strings.TrimSpace(a.Key) == sdp.AttrKeyExtMapAllowMixed {
+			return true
+		}
+	}
+
+	return false
+}


Test patch:
diff --git a/sdp_test.go b/sdp_test.go
index b64124ee..200b68c5 100644
--- a/sdp_test.go
+++ b/sdp_test.go
@@ -354,7 +354,7 @@ func TestMediaDescriptionFingerprints(t *testing.T) {
 			s, err = populateSDP(s, false,
 				dtlsFingerprints,
 				SDPMediaDescriptionFingerprints,
-				false, engine, sdp.ConnectionRoleActive, []ICECandidate{}, ICEParameters{}, media, ICEGatheringStateNew)
+				false, true, engine, sdp.ConnectionRoleActive, []ICECandidate{}, ICEParameters{}, media, ICEGatheringStateNew)
 			assert.NoError(t, err)
 
 			sdparray, err := s.Marshal()
@@ -385,7 +385,7 @@ func TestPopulateSDP(t *testing.T) {
 
 		d := &sdp.SessionDescription{}
 
-		offerSdp, err := populateSDP(d, false, []DTLSFingerprint{}, se.sdpMediaLevelFingerprints, se.candidates.ICELite, me, connectionRoleFromDtlsRole(defaultDtlsRoleOffer), []ICECandidate{}, ICEParameters{}, mediaSections, ICEGatheringStateComplete)
+		offerSdp, err := populateSDP(d, false, []DTLSFingerprint{}, se.sdpMediaLevelFingerprints, se.candidates.ICELite, true, me, connectionRoleFromDtlsRole(defaultDtlsRoleOffer), []ICECandidate{}, ICEParameters{}, mediaSections, ICEGatheringStateComplete)
 		assert.Nil(t, err)
 
 		// Test contains rid map keys
@@ -428,7 +428,7 @@ func TestPopulateSDP(t *testing.T) {
 
 		d := &sdp.SessionDescription{}
 
-		offerSdp, err := populateSDP(d, false, []DTLSFingerprint{}, se.sdpMediaLevelFingerprints, se.candidates.ICELite, me, connectionRoleFromDtlsRole(defaultDtlsRoleOffer), []ICECandidate{}, ICEParameters{}, mediaSections, ICEGatheringStateComplete)
+		offerSdp, err := populateSDP(d, false, []DTLSFingerprint{}, se.sdpMediaLevelFingerprints, se.candidates.ICELite, true, me, connectionRoleFromDtlsRole(defaultDtlsRoleOffer), []ICECandidate{}, ICEParameters{}, mediaSections, ICEGatheringStateComplete)
 		assert.Nil(t, err)
 
 		// Test codecs
@@ -453,7 +453,7 @@ func TestPopulateSDP(t *testing.T) {
 		se := SettingEngine{}
 		se.SetLite(true)
 
-		offerSdp, err := populateSDP(&sdp.SessionDescription{}, false, []DTLSFingerprint{}, se.sdpMediaLevelFingerprints, se.candidates.ICELite, &MediaEngine{}, connectionRoleFromDtlsRole(defaultDtlsRoleOffer), []ICECandidate{}, ICEParameters{}, []mediaSection{}, ICEGatheringStateComplete)
+		offerSdp, err := populateSDP(&sdp.SessionDescription{}, false, []DTLSFingerprint{}, se.sdpMediaLevelFingerprints, se.candidates.ICELite, true, &MediaEngine{}, connectionRoleFromDtlsRole(defaultDtlsRoleOffer), []ICECandidate{}, ICEParameters{}, []mediaSection{}, ICEGatheringStateComplete)
 		assert.Nil(t, err)
 
 		var found bool
@@ -486,7 +486,7 @@ func TestPopulateSDP(t *testing.T) {
 
 		d := &sdp.SessionDescription{}
 
-		offerSdp, err := populateSDP(d, false, []DTLSFingerprint{}, se.sdpMediaLevelFingerprints, se.candidates.ICELite, me, connectionRoleFromDtlsRole(defaultDtlsRoleOffer), []ICECandidate{}, ICEParameters{}, mediaSections, ICEGatheringStateComplete)
+		offerSdp, err := populateSDP(d, false, []DTLSFingerprint{}, se.sdpMediaLevelFingerprints, se.candidates.ICELite, true, me, connectionRoleFromDtlsRole(defaultDtlsRoleOffer), []ICECandidate{}, ICEParameters{}, mediaSections, ICEGatheringStateComplete)
 		assert.NoError(t, err)
 
 		// Test codecs
@@ -502,6 +502,38 @@ func TestPopulateSDP(t *testing.T) {
 		}
 		assert.Equal(t, true, foundRejectedTrack, "rejected track wasn't present")
 	})
+	t.Run("allow mixed extmap", func(t *testing.T) {
+		se := SettingEngine{}
+		offerSdp, err := populateSDP(&sdp.SessionDescription{}, false, []DTLSFingerprint{}, se.sdpMediaLevelFingerprints, se.candidates.ICELite, true, &MediaEngine{}, connectionRoleFromDtlsRole(defaultDtlsRoleOffer), []ICECandidate{}, ICEParameters{}, []mediaSection{}, ICEGatheringStateComplete)
+		assert.Nil(t, err)
+
+		var found bool
+		// session-level attribute
+		for _, a := range offerSdp.Attributes {
+			if a.Key == sdp.AttrKeyExtMapAllowMixed {
+				if a.Value == "" {
+					found = true
+					break
+				}
+			}
+		}
+		assert.Equal(t, true, found, "AllowMixedExtMap key should be present")
+
+		offerSdp, err = populateSDP(&sdp.SessionDescription{}, false, []DTLSFingerprint{}, se.sdpMediaLevelFingerprints, se.candidates.ICELite, false, &MediaEngine{}, connectionRoleFromDtlsRole(defaultDtlsRoleOffer), []ICECandidate{}, ICEParameters{}, []mediaSection{}, ICEGatheringStateComplete)
+		assert.Nil(t, err)
+
+		found = false
+		// session-level attribute
+		for _, a := range offerSdp.Attributes {
+			if a.Key == sdp.AttrKeyExtMapAllowMixed {
+				if a.Value == "" {
+					found = true
+					break
+				}
+			}
+		}
+		assert.Equal(t, false, found, "AllowMixedExtMap key should not be present")
+	})
 }
 
 func TestGetRIDs(t *testing.T) {