Repository: pion/webrtc
Pull request #2664 (2024-02-05 08:10:15)

Refine rtx support
Always handle header extensions from packet read
from interceptor, let interceptor has consistent
chance to process headers

Fix rtx is not negotiated when there is multiple
codecs has same mime but different profile (H264)

Fix rtx stream info missed when SSRC group attr
shows after base track's ssrc attr.




codecov[bot]: ## [Codecov](https://app.codecov.io/gh/pion/webrtc/pull/2664?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) Report
Attention: `7 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`0f7dd0f`)](https://app.codecov.io/gh/pion/webrtc/commit/0f7dd0fc5ea75bb5794be9ce4f303e14c7907c9e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) 76.43% compared to head [(`457a2b1`)](https://app.codecov.io/gh/pion/webrtc/pull/2664?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) 76.34%.

| [Files](https://app.codecov.io/gh/pion/webrtc/pull/2664?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Patch % | Lines |
|---|---|---|
| [sdp.go](https://app.codecov.io/gh/pion/webrtc/pull/2664?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-c2RwLmdv) | 0.00% | [4 Missing and 1 partial :warning: ](https://app.codecov.io/gh/pion/webrtc/pull/2664?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) |
| [peerconnection.go](https://app.codecov.io/gh/pion/webrtc/pull/2664?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-cGVlcmNvbm5lY3Rpb24uZ28=) | 60.00% | [1 Missing and 1 partial :warning: ](https://app.codecov.io/gh/pion/webrtc/pull/2664?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #2664      +/-   ##
==========================================
- Coverage   76.43%   76.34%   -0.10%     
==========================================
  Files          87       87              
  Lines        9908     9921      +13     
==========================================
+ Hits         7573     7574       +1     
- Misses       1865     1874       +9     
- Partials      470      473       +3     
```

| [Flag](https://app.codecov.io/gh/pion/webrtc/pull/2664/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | |
|---|---|---|
| [go](https://app.codecov.io/gh/pion/webrtc/pull/2664/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | `77.87% <61.11%> (-0.11%)` | :arrow_down: |
| [wasm](https://app.codecov.io/gh/pion/webrtc/pull/2664/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | `64.54% <ø> (ø)` | |

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.


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/pion/webrtc/pull/2664?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?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 5e5a2bfd..04d989cc 100644
--- a/mediaengine.go
+++ b/mediaengine.go
@@ -414,9 +414,11 @@ func (m *MediaEngine) matchRemoteCodec(remoteCodec RTPCodecParameters, typ RTPCo
 		}
 
 		aptMatch := codecMatchNone
+		var aptCodec RTPCodecParameters
 		for _, codec := range exactMatches {
 			if codec.PayloadType == PayloadType(payloadType) {
 				aptMatch = codecMatchExact
+				aptCodec = codec
 				break
 			}
 		}
@@ -425,6 +427,7 @@ func (m *MediaEngine) matchRemoteCodec(remoteCodec RTPCodecParameters, typ RTPCo
 			for _, codec := range partialMatches {
 				if codec.PayloadType == PayloadType(payloadType) {
 					aptMatch = codecMatchPartial
+					aptCodec = codec
 					break
 				}
 			}
@@ -434,8 +437,14 @@ func (m *MediaEngine) matchRemoteCodec(remoteCodec RTPCodecParameters, typ RTPCo
 			return codecMatchNone, nil // not an error, we just ignore this codec we don't support
 		}
 
+		// replace the apt value with the original codec's payload type
+		toMatchCodec := remoteCodec
+		if aptMatched, mt := codecParametersFuzzySearch(aptCodec, codecs); mt == aptMatch {
+			toMatchCodec.SDPFmtpLine = strings.Replace(toMatchCodec.SDPFmtpLine, fmt.Sprintf("apt=%d", payloadType), fmt.Sprintf("apt=%d", aptMatched.PayloadType), 1)
+		}
+
 		// if apt's media codec is partial match, then apt codec must be partial match too
-		_, matchType := codecParametersFuzzySearch(remoteCodec, codecs)
+		_, matchType := codecParametersFuzzySearch(toMatchCodec, codecs)
 		if matchType == codecMatchExact && aptMatch == codecMatchPartial {
 			matchType = codecMatchPartial
 		}
diff --git a/peerconnection.go b/peerconnection.go
index c7c92f99..aef7b7b3 100644
--- a/peerconnection.go
+++ b/peerconnection.go
@@ -1587,12 +1587,11 @@ func (pc *PeerConnection) handleIncomingSSRC(rtpStream io.Reader, ssrc SSRC) err
 		return err
 	}
 
-	var mid, rid, rsid string
-	payloadType, paddingOnly, err := handleUnknownRTPPacket(b[:i], uint8(midExtensionID), uint8(streamIDExtensionID), uint8(repairStreamIDExtensionID), &mid, &rid, &rsid)
-	if err != nil {
-		return err
+	if i < 4 {
+		return errRTPTooShort
 	}
 
+	payloadType := PayloadType(b[1] & 0x7f)
 	params, err := pc.api.mediaEngine.getRTPParametersByPayloadType(payloadType)
 	if err != nil {
 		return err
@@ -1604,6 +1603,8 @@ func (pc *PeerConnection) handleIncomingSSRC(rtpStream io.Reader, ssrc SSRC) err
 		return err
 	}
 
+	var mid, rid, rsid string
+	var paddingOnly bool
 	for readCount := 0; readCount <= simulcastProbeCount; readCount++ {
 		if mid == "" || (rid == "" && rsid == "") {
 			// skip padding only packets for probing
diff --git a/sdp.go b/sdp.go
index ce076415..49783d91 100644
--- a/sdp.go
+++ b/sdp.go
@@ -128,6 +128,12 @@ func trackDetailsFromSDP(log logging.LeveledLogger, s *sdp.SessionDescription) (
 						}
 						rtxRepairFlows[rtxRepairFlow] = baseSsrc
 						tracksInMediaSection = filterTrackWithSSRC(tracksInMediaSection, SSRC(rtxRepairFlow)) // Remove if rtx was added as track before
+						for i := range tracksInMediaSection {
+							if tracksInMediaSection[i].ssrcs[0] == SSRC(baseSsrc) {
+								repairSsrc := SSRC(rtxRepairFlow)
+								tracksInMediaSection[i].repairSsrc = &repairSsrc
+							}
+						}
 					}
 				}
 


Test patch:
diff --git a/mediaengine_test.go b/mediaengine_test.go
index 00022da1..d9a9e7b1 100644
--- a/mediaengine_test.go
+++ b/mediaengine_test.go
@@ -308,8 +308,18 @@ a=rtpmap:96 VP8/90000
 o=- 4596489990601351948 2 IN IP4 127.0.0.1
 s=-
 t=0 0
-m=video 60323 UDP/TLS/RTP/SAVPF 94 96 97
+m=video 60323 UDP/TLS/RTP/SAVPF 94 95 106 107 108 109 96 97 
 a=rtpmap:94 VP8/90000
+a=rtpmap:95 rtx/90000
+a=fmtp:95 apt=94
+a=rtpmap:106 H264/90000
+a=fmtp:106 level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42001f
+a=rtpmap:107 rtx/90000
+a=fmtp:107 apt=106
+a=rtpmap:108 H264/90000
+a=fmtp:108 level-asymmetry-allowed=1;packetization-mode=0;profile-level-id=42001f
+a=rtpmap:109 rtx/90000
+a=fmtp:109 apt=108
 a=rtpmap:96 VP9/90000
 a=fmtp:96 profile-id=2
 a=rtpmap:97 rtx/90000
@@ -318,22 +328,64 @@ 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.RegisterCodec(RTPCodecParameters{
+			RTPCodecCapability: RTPCodecCapability{MimeTypeH264, 90000, 0, "level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42001f", nil},
+			PayloadType:        102,
+		}, RTPCodecTypeVideo))
+		assert.NoError(t, m.RegisterCodec(RTPCodecParameters{
+			RTPCodecCapability: RTPCodecCapability{"video/rtx", 90000, 0, "apt=102", nil},
+			PayloadType:        103,
+		}, RTPCodecTypeVideo))
+		assert.NoError(t, m.RegisterCodec(RTPCodecParameters{
+			RTPCodecCapability: RTPCodecCapability{MimeTypeH264, 90000, 0, "level-asymmetry-allowed=1;packetization-mode=0;profile-level-id=42001f", nil},
+			PayloadType:        104,
+		}, RTPCodecTypeVideo))
+		assert.NoError(t, m.RegisterCodec(RTPCodecParameters{
+			RTPCodecCapability: RTPCodecCapability{"video/rtx", 90000, 0, "apt=104", nil},
+			PayloadType:        105,
+		}, RTPCodecTypeVideo))
+		assert.NoError(t, m.RegisterCodec(RTPCodecParameters{
+			RTPCodecCapability: RTPCodecCapability{MimeTypeVP9, 90000, 0, "profile-id=2", nil},
+			PayloadType:        98,
+		}, RTPCodecTypeVideo))
+		assert.NoError(t, m.RegisterCodec(RTPCodecParameters{
+			RTPCodecCapability: RTPCodecCapability{"video/rtx", 90000, 0, "apt=98", nil},
+			PayloadType:        99,
+		}, RTPCodecTypeVideo))
 		assert.NoError(t, m.updateFromRemoteDescription(mustParse(profileLevels)))
 
 		assert.True(t, m.negotiatedVideo)
 
-		_, _, err := m.getCodecByPayload(97)
+		vp9Codec, _, err := m.getCodecByPayload(96)
+		assert.NoError(t, err)
+		assert.Equal(t, vp9Codec.MimeType, MimeTypeVP9)
+		vp9RTX, _, err := m.getCodecByPayload(97)
+		assert.NoError(t, err)
+		assert.Equal(t, vp9RTX.MimeType, "video/rtx")
+
+		h264P1Codec, _, err := m.getCodecByPayload(106)
+		assert.NoError(t, err)
+		assert.Equal(t, h264P1Codec.MimeType, MimeTypeH264)
+		assert.Equal(t, h264P1Codec.SDPFmtpLine, "level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42001f")
+		h264P1RTX, _, err := m.getCodecByPayload(107)
+		assert.NoError(t, err)
+		assert.Equal(t, h264P1RTX.MimeType, "video/rtx")
+		assert.Equal(t, h264P1RTX.SDPFmtpLine, "apt=106")
+
+		h264P0Codec, _, err := m.getCodecByPayload(108)
+		assert.NoError(t, err)
+		assert.Equal(t, h264P0Codec.MimeType, MimeTypeH264)
+		assert.Equal(t, h264P0Codec.SDPFmtpLine, "level-asymmetry-allowed=1;packetization-mode=0;profile-level-id=42001f")
+		h264P0RTX, _, err := m.getCodecByPayload(109)
 		assert.NoError(t, err)
+		assert.Equal(t, h264P0RTX.MimeType, "video/rtx")
+		assert.Equal(t, h264P0RTX.SDPFmtpLine, "apt=108")
 	})
 
 	t.Run("Matches when rtx apt for partial match codec", func(t *testing.T) {