From 3ba0bf9027ce26901ead331abd7c1e58c94ca44d Mon Sep 17 00:00:00 2001 From: Juan Navarro Date: Thu, 1 Aug 2024 12:04:23 +0200 Subject: [PATCH] RID order guaranteed by avoiding iteration over a map 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. Default code format applied. Unit tests passed. Fixes #2838 --- peerconnection.go | 2 +- sdp.go | 37 +++++++++++++++++++++---------------- sdp_test.go | 33 ++++++++++++++++++++++----------- 3 files changed, 44 insertions(+), 28 deletions(-) diff --git a/peerconnection.go b/peerconnection.go index 2b8a9038539..a764f98993e 100644 --- a/peerconnection.go +++ b/peerconnection.go @@ -2520,7 +2520,7 @@ func (pc *PeerConnection) generateMatchedSDP(transceivers []*RTPTransceiver, use mediaTransceivers := []*RTPTransceiver{t} 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)}) } } diff --git a/sdp.go b/sdp.go index f1b59940b10..28e416e9c61 100644 --- a/sdp.go +++ b/sdp.go @@ -202,8 +202,8 @@ func trackDetailsFromSDP(log logging.LeveledLogger, s *sdp.SessionDescription) ( id: trackID, rids: []string{}, } - for rid := range rids { - simulcastTrack.rids = append(simulcastTrack.rids, rid) + for _, rid := range rids { + simulcastTrack.rids = append(simulcastTrack.rids, rid.id) } tracksInMediaSection = []trackDetails{simulcastTrack} @@ -238,13 +238,13 @@ func trackDetailsToRTPReceiveParameters(t *trackDetails) RTPReceiveParameters { return RTPReceiveParameters{Encodings: encodings} } -func getRids(media *sdp.MediaDescription) map[string]*simulcastRid { - rids := map[string]*simulcastRid{} +func getRids(media *sdp.MediaDescription) []*simulcastRid { + rids := []*simulcastRid{} var simulcastAttr string for _, attr := range media.Attributes { if attr.Key == sdpAttributeRid { 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 { simulcastAttr = attr.Value } @@ -257,9 +257,12 @@ func getRids(media *sdp.MediaDescription) map[string]*simulcastRid { ridStates := strings.Split(simulcastAttr, ";") for _, ridState := range ridStates { if ridState[:1] == "~" { - rid := ridState[1:] - if r, ok := rids[rid]; ok { - r.paused = true + ridId := ridState[1:] + for _, rid := range rids { + if rid.id == ridId { + rid.paused = true + break + } } } } @@ -499,15 +502,16 @@ func addTransceiverSDP( media.WithExtMap(sdp.ExtMap{Value: rtpExtension.ID, URI: extURL}) } - if len(mediaSection.ridMap) > 0 { - recvRids := make([]string, 0, len(mediaSection.ridMap)) + if len(mediaSection.rids) > 0 { + recvRids := make([]string, 0, len(mediaSection.rids)) - for rid := range mediaSection.ridMap { - media.WithValueAttribute(sdpAttributeRid, rid+" recv") - if mediaSection.ridMap[rid].paused { - rid = "~" + rid + for _, rid := range mediaSection.rids { + ridId := rid.id + media.WithValueAttribute(sdpAttributeRid, ridId+" recv") + if rid.paused { + ridId = "~" + ridId } - recvRids = append(recvRids, rid) + recvRids = append(recvRids, ridId) } // Simulcast media.WithValueAttribute(sdpAttributeSimulcast, "recv "+strings.Join(recvRids, ";")) @@ -533,6 +537,7 @@ func addTransceiverSDP( } type simulcastRid struct { + id string attrValue string paused bool } @@ -542,7 +547,7 @@ type mediaSection struct { transceivers []*RTPTransceiver data bool matchExtensions map[string]int - ridMap map[string]*simulcastRid + rids []*simulcastRid } func bundleMatchFromRemote(matchBundleGroup *string) func(mid string) bool { diff --git a/sdp_test.go b/sdp_test.go index bda54fa6058..c448ac6e0d2 100644 --- a/sdp_test.go +++ b/sdp_test.go @@ -417,16 +417,18 @@ func TestPopulateSDP(t *testing.T) { tr := &RTPTransceiver{kind: RTPCodecTypeVideo, api: api, codecs: me.videoCodecs} tr.setDirection(RTPTransceiverDirectionRecvonly) - ridMap := map[string]*simulcastRid{ - "ridkey": { + rids := []*simulcastRid{ + { + id: "ridkey", attrValue: "some", }, - "ridPaused": { + { + id: "ridPaused", attrValue: "some2", paused: true, }, } - mediaSections := []mediaSection{{id: "video", transceivers: []*RTPTransceiver{tr}, ridMap: ridMap}} + mediaSections := []mediaSection{{id: "video", transceivers: []*RTPTransceiver{tr}, rids: rids}} d := &sdp.SessionDescription{} @@ -439,12 +441,14 @@ func TestPopulateSDP(t *testing.T) { if desc.MediaName.Media != "video" { continue } - ridInSDP := getRids(desc) - if ridKey, ok := ridInSDP["ridkey"]; ok && !ridKey.paused { - ridFound++ - } - if ridPaused, ok := ridInSDP["ridPaused"]; ok && ridPaused.paused { - ridFound++ + ridsInSDP := getRids(desc) + for _, rid := range ridsInSDP { + if rid.id == "ridkey" && !rid.paused { + ridFound++ + } + if rid.id == "ridPaused" && rid.paused { + ridFound++ + } } } assert.Equal(t, 2, ridFound, "All rid keys should be present") @@ -667,7 +671,14 @@ func TestGetRIDs(t *testing.T) { rids := getRids(m[0]) 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'") } }