Fix trackDetailsFromSDP not handling fec ssrc

Author: 3DRX

peerconnection.go

@@ -1700,7 +1700,10 @@ func (pc *PeerConnection) handleIncomingSSRC(rtpStream io.Reader, ssrc SSRC) err
 
 	// If a SSRC already exists in the RemoteDescription don't perform heuristics upon it
 	for _, track := range trackDetailsFromSDP(pc.log, remoteDescription.parsed) {
-		if track.repairSsrc != nil && ssrc == *track.repairSsrc {
+		if track.rtxSsrc != nil && ssrc == *track.rtxSsrc {
+			return nil
+		}
+		if track.fecSsrc != nil && ssrc == *track.fecSsrc {
 			return nil
 		}
 		for _, trackSsrc := range track.ssrcs {

sdp.go

@@ -23,13 +23,14 @@ import (
 // trackDetails represents any media source that can be represented in a SDP
 // This isn't keyed by SSRC because it also needs to support rid based sources.
 type trackDetails struct {
-	mid        string
-	kind       RTPCodecType
-	streamID   string
-	id         string
-	ssrcs      []SSRC
-	repairSsrc *SSRC
-	rids       []string
+	mid      string
+	kind     RTPCodecType
+	streamID string
+	id       string
+	ssrcs    []SSRC
+	rtxSsrc  *SSRC
+	fecSsrc  *SSRC
+	rids     []string
 }
 
 func trackDetailsForSSRC(trackDetails []trackDetails, ssrc SSRC) *trackDetails {
@@ -91,6 +92,7 @@ func trackDetailsFromSDP(
 	for _, media := range s.MediaDescriptions {
 		tracksInMediaSection := []trackDetails{}
 		rtxRepairFlows := map[uint64]uint64{}
+		fecRepairFlows := map[uint64]uint64{}
 
 		// Plan B can have multiple tracks in a single media section
 		streamID := ""
@@ -143,7 +145,35 @@ func trackDetailsFromSDP(
 						for i := range tracksInMediaSection {
 							if tracksInMediaSection[i].ssrcs[0] == SSRC(baseSsrc) {
 								repairSsrc := SSRC(rtxRepairFlow)
-								tracksInMediaSection[i].repairSsrc = &repairSsrc
+								tracksInMediaSection[i].rtxSsrc = &repairSsrc
+							}
+						}
+					}
+				} else if split[0] == sdp.SemanticTokenForwardErrorCorrectionFramework {
+					// Similar to above, lines like `a=ssrc-group:FEC-FR aaaaa bbbbb`
+					// means for video ssrc aaaaa, there's a FEC track bbbbb
+					if len(split) == 3 {
+						baseSsrc, err := strconv.ParseUint(split[1], 10, 32)
+						if err != nil {
+							log.Warnf("Failed to parse SSRC: %v", err)
+
+							continue
+						}
+						fecRepairFlow, err := strconv.ParseUint(split[2], 10, 32)
+						if err != nil {
+							log.Warnf("Failed to parse SSRC: %v", err)
+
+							continue
+						}
+						fecRepairFlows[fecRepairFlow] = baseSsrc
+						tracksInMediaSection = filterTrackWithSSRC(
+							tracksInMediaSection,
+							SSRC(fecRepairFlow),
+						) // Remove if fec was added as track before
+						for i := range tracksInMediaSection {
+							if tracksInMediaSection[i].ssrcs[0] == SSRC(baseSsrc) {
+								repairSsrc := SSRC(fecRepairFlow)
+								tracksInMediaSection[i].fecSsrc = &repairSsrc
 							}
 						}
 					}
@@ -171,6 +201,9 @@ func trackDetailsFromSDP(
 				if _, ok := rtxRepairFlows[ssrc]; ok {
 					continue // This ssrc is a RTX repair flow, ignore
 				}
+				if _, ok := fecRepairFlows[ssrc]; ok {
+					continue // This ssrc is a FEC repair flow, ignore
+				}
 
 				if len(split) == 3 && strings.HasPrefix(split[1], "msid:") {
 					streamID = split[1][len("msid:"):]
@@ -197,7 +230,13 @@ func trackDetailsFromSDP(
 				for r, baseSsrc := range rtxRepairFlows {
 					if baseSsrc == ssrc {
 						repairSsrc := SSRC(r) //nolint:gosec // G115
-						trackDetails.repairSsrc = &repairSsrc
+						trackDetails.rtxSsrc = &repairSsrc
+					}
+				}
+				for r, baseSsrc := range fecRepairFlows {
+					if baseSsrc == ssrc {
+						fecSsrc := SSRC(r) //nolint:gosec // G115
+						trackDetails.fecSsrc = &fecSsrc
 					}
 				}
 
@@ -243,8 +282,12 @@ func trackDetailsToRTPReceiveParameters(trackDetails *trackDetails) RTPReceivePa
 			encodings[i].SSRC = trackDetails.ssrcs[i]
 		}
 
-		if trackDetails.repairSsrc != nil {
-			encodings[i].RTX.SSRC = *trackDetails.repairSsrc
+		if trackDetails.rtxSsrc != nil {
+			encodings[i].RTX.SSRC = *trackDetails.rtxSsrc
+		}
+
+		if trackDetails.fecSsrc != nil {
+			encodings[i].FEC.SSRC = *trackDetails.fecSsrc
 		}
 	}
 
@@ -430,10 +473,26 @@ func addSenderSDP(
 		sendParameters := sender.GetParameters()
 		for _, encoding := range sendParameters.Encodings {
 			if encoding.RTX.SSRC != 0 {
-				media = media.WithValueAttribute("ssrc-group", fmt.Sprintf("FID %d %d", encoding.SSRC, encoding.RTX.SSRC))
+				media = media.WithValueAttribute(
+					"ssrc-group",
+					fmt.Sprintf(
+						"%s %d %d",
+						sdp.SemanticTokenFlowIdentification,
+						encoding.SSRC,
+						encoding.RTX.SSRC,
+					),
+				)
 			}
 			if encoding.FEC.SSRC != 0 {
-				media = media.WithValueAttribute("ssrc-group", fmt.Sprintf("FEC-FR %d %d", encoding.SSRC, encoding.FEC.SSRC))
+				media = media.WithValueAttribute(
+					"ssrc-group",
+					fmt.Sprintf(
+						"%s %d %d",
+						sdp.SemanticTokenForwardErrorCorrectionFramework,
+						encoding.SSRC,
+						encoding.FEC.SSRC,
+					),
+				)
 			}
 
 			media = media.WithMediaSource(

sdp_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/pion/sdp/v3"
 	"github.com/pion/transport/v3/test"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestExtractFingerprint(t *testing.T) {
@@ -454,6 +455,38 @@ func TestTrackDetailsFromSDP(t *testing.T) {
 		}
 	})
 
+	t.Run("Tracks unknown, video with RTX and FEC", func(t *testing.T) {
+		descr := &sdp.SessionDescription{
+			MediaDescriptions: []*sdp.MediaDescription{
+				{
+					MediaName: sdp.MediaName{
+						Media: "video",
+					},
+					Attributes: []sdp.Attribute{
+						{Key: "mid", Value: "0"},
+						{Key: "sendrecv"},
+						{Key: "ssrc-group", Value: "FID 3000 4000"},
+						{Key: "ssrc-group", Value: "FEC-FR 3000 5000"},
+						{Key: "ssrc", Value: "3000 msid:video_trk_label video_trk_guid"},
+						{Key: "ssrc", Value: "4000 msid:rtx_trk_label rtx_trk_guid"},
+						{Key: "ssrc", Value: "5000 msid:fec_trk_label fec_trk_guid"},
+					},
+				},
+			},
+		}
+
+		tracks := trackDetailsFromSDP(nil, descr)
+		assert.Equal(t, 1, len(tracks))
+		track := tracks[0]
+		assert.Equal(t, RTPCodecTypeVideo, track.kind)
+		assert.Equal(t, SSRC(3000), track.ssrcs[0])
+		assert.Equal(t, "video_trk_label", track.streamID)
+		require.NotNil(t, track.rtxSsrc, "missing RTX ssrc for video track")
+		assert.Equal(t, SSRC(4000), *track.rtxSsrc)
+		require.NotNil(t, track.fecSsrc, "missing FEC ssrc for video track")
+		assert.Equal(t, SSRC(5000), *track.fecSsrc)
+	})
+
 	t.Run("inactive and recvonly tracks ignored", func(t *testing.T) {
 		descr := &sdp.SessionDescription{
 			MediaDescriptions: []*sdp.MediaDescription{
@@ -512,8 +545,8 @@ func TestTrackDetailsFromSDP(t *testing.T) {
 
 		tracks := trackDetailsFromSDP(nil, descr)
 		assert.Equal(t, 2, len(tracks))
-		assert.Equal(t, SSRC(4000), *tracks[0].repairSsrc)
-		assert.Equal(t, SSRC(6000), *tracks[1].repairSsrc)
+		assert.Equal(t, SSRC(4000), *tracks[0].rtxSsrc)
+		assert.Equal(t, SSRC(6000), *tracks[1].rtxSsrc)
 	})
 }
 
@@ -1235,13 +1268,13 @@ a=sendrecv
 		t.Run(testCase.name, func(t *testing.T) {
 			checkRTXSupport := func(s *sdp.SessionDescription) {
 				// RTX is never enabled for audio
-				assert.Nil(t, trackDetailsFromSDP(nil, s)[0].repairSsrc)
+				assert.Nil(t, trackDetailsFromSDP(nil, s)[0].rtxSsrc)
 
 				// RTX is conditionally enabled for video
 				if testCase.rtxExpected {
-					assert.NotNil(t, trackDetailsFromSDP(nil, s)[1].repairSsrc)
+					assert.NotNil(t, trackDetailsFromSDP(nil, s)[1].rtxSsrc)
 				} else {
-					assert.Nil(t, trackDetailsFromSDP(nil, s)[1].repairSsrc)
+					assert.Nil(t, trackDetailsFromSDP(nil, s)[1].rtxSsrc)
 				}
 			}