Fix disordered RIDs in SDP

Map iteration order is not guaranteed by Go, so it's an error to iterate
over a map in places where maintaining the same ordering is important.

This change replaces the map of simulcastRid{} with an array of the same
type. The simulcastRid{} type is extended to hold the rid-id which
previously was used as the key in the map.

Accesses to the map are replaced with range loops to find the desired
rid-id for each case.

Fixes #2838
This commit is contained in:
Juan Navarro
2024-08-01 12:04:23 +02:00
committed by Sean DuBois
parent 9836d58351
commit b8d3a7bba7
3 changed files with 44 additions and 28 deletions

View File

@@ -2500,7 +2500,7 @@ func (pc *PeerConnection) generateMatchedSDP(transceivers []*RTPTransceiver, use
mediaTransceivers := []*RTPTransceiver{t} mediaTransceivers := []*RTPTransceiver{t}
extensions, _ := rtpExtensionsFromMediaDescription(media) extensions, _ := rtpExtensionsFromMediaDescription(media)
mediaSections = append(mediaSections, mediaSection{id: midValue, transceivers: mediaTransceivers, matchExtensions: extensions, ridMap: getRids(media)}) mediaSections = append(mediaSections, mediaSection{id: midValue, transceivers: mediaTransceivers, matchExtensions: extensions, rids: getRids(media)})
} }
} }

37
sdp.go
View File

@@ -202,8 +202,8 @@ func trackDetailsFromSDP(log logging.LeveledLogger, s *sdp.SessionDescription) (
id: trackID, id: trackID,
rids: []string{}, rids: []string{},
} }
for rid := range rids { for _, rid := range rids {
simulcastTrack.rids = append(simulcastTrack.rids, rid) simulcastTrack.rids = append(simulcastTrack.rids, rid.id)
} }
tracksInMediaSection = []trackDetails{simulcastTrack} tracksInMediaSection = []trackDetails{simulcastTrack}
@@ -238,13 +238,13 @@ func trackDetailsToRTPReceiveParameters(t *trackDetails) RTPReceiveParameters {
return RTPReceiveParameters{Encodings: encodings} return RTPReceiveParameters{Encodings: encodings}
} }
func getRids(media *sdp.MediaDescription) map[string]*simulcastRid { func getRids(media *sdp.MediaDescription) []*simulcastRid {
rids := map[string]*simulcastRid{} rids := []*simulcastRid{}
var simulcastAttr string var simulcastAttr string
for _, attr := range media.Attributes { for _, attr := range media.Attributes {
if attr.Key == sdpAttributeRid { if attr.Key == sdpAttributeRid {
split := strings.Split(attr.Value, " ") split := strings.Split(attr.Value, " ")
rids[split[0]] = &simulcastRid{attrValue: attr.Value} rids = append(rids, &simulcastRid{id: split[0], attrValue: attr.Value})
} else if attr.Key == sdpAttributeSimulcast { } else if attr.Key == sdpAttributeSimulcast {
simulcastAttr = attr.Value simulcastAttr = attr.Value
} }
@@ -257,9 +257,12 @@ func getRids(media *sdp.MediaDescription) map[string]*simulcastRid {
ridStates := strings.Split(simulcastAttr, ";") ridStates := strings.Split(simulcastAttr, ";")
for _, ridState := range ridStates { for _, ridState := range ridStates {
if ridState[:1] == "~" { if ridState[:1] == "~" {
rid := ridState[1:] ridID := ridState[1:]
if r, ok := rids[rid]; ok { for _, rid := range rids {
r.paused = true if rid.id == ridID {
rid.paused = true
break
}
} }
} }
} }
@@ -499,15 +502,16 @@ func addTransceiverSDP(
media.WithExtMap(sdp.ExtMap{Value: rtpExtension.ID, URI: extURL}) media.WithExtMap(sdp.ExtMap{Value: rtpExtension.ID, URI: extURL})
} }
if len(mediaSection.ridMap) > 0 { if len(mediaSection.rids) > 0 {
recvRids := make([]string, 0, len(mediaSection.ridMap)) recvRids := make([]string, 0, len(mediaSection.rids))
for rid := range mediaSection.ridMap { for _, rid := range mediaSection.rids {
media.WithValueAttribute(sdpAttributeRid, rid+" recv") ridID := rid.id
if mediaSection.ridMap[rid].paused { media.WithValueAttribute(sdpAttributeRid, ridID+" recv")
rid = "~" + rid if rid.paused {
ridID = "~" + ridID
} }
recvRids = append(recvRids, rid) recvRids = append(recvRids, ridID)
} }
// Simulcast // Simulcast
media.WithValueAttribute(sdpAttributeSimulcast, "recv "+strings.Join(recvRids, ";")) media.WithValueAttribute(sdpAttributeSimulcast, "recv "+strings.Join(recvRids, ";"))
@@ -533,6 +537,7 @@ func addTransceiverSDP(
} }
type simulcastRid struct { type simulcastRid struct {
id string
attrValue string attrValue string
paused bool paused bool
} }
@@ -542,7 +547,7 @@ type mediaSection struct {
transceivers []*RTPTransceiver transceivers []*RTPTransceiver
data bool data bool
matchExtensions map[string]int matchExtensions map[string]int
ridMap map[string]*simulcastRid rids []*simulcastRid
} }
func bundleMatchFromRemote(matchBundleGroup *string) func(mid string) bool { func bundleMatchFromRemote(matchBundleGroup *string) func(mid string) bool {

View File

@@ -381,16 +381,18 @@ func TestPopulateSDP(t *testing.T) {
tr := &RTPTransceiver{kind: RTPCodecTypeVideo, api: api, codecs: me.videoCodecs} tr := &RTPTransceiver{kind: RTPCodecTypeVideo, api: api, codecs: me.videoCodecs}
tr.setDirection(RTPTransceiverDirectionRecvonly) tr.setDirection(RTPTransceiverDirectionRecvonly)
ridMap := map[string]*simulcastRid{ rids := []*simulcastRid{
"ridkey": { {
id: "ridkey",
attrValue: "some", attrValue: "some",
}, },
"ridPaused": { {
id: "ridPaused",
attrValue: "some2", attrValue: "some2",
paused: true, paused: true,
}, },
} }
mediaSections := []mediaSection{{id: "video", transceivers: []*RTPTransceiver{tr}, ridMap: ridMap}} mediaSections := []mediaSection{{id: "video", transceivers: []*RTPTransceiver{tr}, rids: rids}}
d := &sdp.SessionDescription{} d := &sdp.SessionDescription{}
@@ -403,12 +405,14 @@ func TestPopulateSDP(t *testing.T) {
if desc.MediaName.Media != "video" { if desc.MediaName.Media != "video" {
continue continue
} }
ridInSDP := getRids(desc) ridsInSDP := getRids(desc)
if ridKey, ok := ridInSDP["ridkey"]; ok && !ridKey.paused { for _, rid := range ridsInSDP {
ridFound++ if rid.id == "ridkey" && !rid.paused {
} ridFound++
if ridPaused, ok := ridInSDP["ridPaused"]; ok && ridPaused.paused { }
ridFound++ if rid.id == "ridPaused" && rid.paused {
ridFound++
}
} }
} }
assert.Equal(t, 2, ridFound, "All rid keys should be present") assert.Equal(t, 2, ridFound, "All rid keys should be present")
@@ -631,7 +635,14 @@ func TestGetRIDs(t *testing.T) {
rids := getRids(m[0]) rids := getRids(m[0])
assert.NotEmpty(t, rids, "Rid mapping should be present") assert.NotEmpty(t, rids, "Rid mapping should be present")
if _, ok := rids["f"]; !ok { found := false
for _, rid := range rids {
if rid.id == "f" {
found = true
break
}
}
if !found {
assert.Fail(t, "rid values should contain 'f'") assert.Fail(t, "rid values should contain 'f'")
} }
} }