Skip to content

Simulcast: getRids orders layers by a=rid line order, not the a=simulcast preference list (RFC 8853 §5.2) #3471

Description

@StefanoD

Summary

getRids (sdp.go) determines the order of simulcast streams from the order in which the a=rid attribute lines appear, and consults a=simulcast only to flag ~-prefixed paused streams. Per RFC 8853 §5.2, it's the a=simulcast send list — not the a=rid line order — that expresses the stream preference order:

The order of the listed simulcast streams in the "send" direction suggests a proposed order of preference, in decreasing order

So when a peer lists its a=rid lines in a different order than its a=simulcast:send list, pion follows the a=rid order.

Where

  • getRids (sdp.go) appends one simulcastRid per a=rid attribute in document order; the a=simulcast value is scanned only to set paused on ~-prefixed ids. It is never used to order the slice.
  • That order then flows to:
    • trackDetailsToRTPReceiveParameters (sdp.go)encodings[i].RID = trackDetails.rids[i], i.e. the receiver's encoding order == a=rid line order.
    • addTransceiverSDP answer emission — iterates mediaSection.rids emitting a=rid:<id> recv and a=simulcast:recv <joined> in that same order.

Reproduce

An offer whose a=rid lines are in the opposite order to a=simulcast:send:

a=rid:q send
a=rid:h send
a=rid:f send
a=simulcast:send f;h;q

getRids returns [q, h, f] (a=rid order); the a=simulcast preference is [f, h, q]. A table test (included in the patch below) fails today:

Expected: [f h q]
Actual:   [q h f]

Impact

Latent in practice. Browsers (Firefox 152, Chrome) emit a=rid and a=simulcast in the same order, and runtime demux is by RID string, so media isn't broken. It affects the order of RTPReceiveParameters.Encodings / Tracks() and the a=simulcast:recv preference list echoed back in the answer — i.e. pion doesn't convey the offerer's stated preference order back to it.

To be clear about normative strength: this is a SHOULD-level alignment, not a spec violation. §5.2 "suggests" the order (non-normative wording), and §5.3.2 only mandates reversing send↔recv (SHALL) and forbids adding streams (MUST NOT) — it does not itself require preserving order. Mirroring the offered preference back is simply the correct, interoperable behavior.

Proposed fix

Stable-sort the rids by their first position in the a=simulcast attribute; any rid absent from it keeps its a=rid declaration order at the end. The patch below builds, is go vet / gofmt clean, and the new test fails before / passes after:

Patch (sdp.go + sdp_test.go)
diff --git a/sdp.go b/sdp.go
index 1525f049..96d1d1b0 100644
--- a/sdp.go
+++ b/sdp.go
@@ -6,9 +6,11 @@
 package webrtc
 
 import (
+	"cmp"
 	"encoding/base64"
 	"errors"
 	"fmt"
+	"math"
 	"net/url"
 	"regexp"
 	"slices"
@@ -298,10 +300,17 @@ func getRids(media *sdp.MediaDescription) []*simulcastRid { // nolint:cyclop
 		if space := strings.Index(simulcastAttr, " "); space > 0 {
 			simulcastAttr = simulcastAttr[space+1:]
 		}
+		// RFC 8853 §5.2: the a=simulcast send list "suggests a proposed order of
+		// preference, in decreasing order"; the order of the a=rid lines is not
+		// significant. Record that order and sort the rids by it below so a
+		// generated answer mirrors the offered preference instead of the
+		// incidental a=rid line order.
+		simulcastOrder := map[string]int{}
 		ridStates := strings.SplitSeq(simulcastAttr, ";")
 		for ridState := range ridStates {
+			ridID := ridState
 			if len(ridState) > 0 && ridState[:1] == "~" {
-				ridID := ridState[1:]
+				ridID = ridState[1:]
 				for _, rid := range rids {
 					if rid.id == ridID {
 						rid.paused = true
@@ -310,7 +319,20 @@ func getRids(media *sdp.MediaDescription) []*simulcastRid { // nolint:cyclop
 					}
 				}
 			}
+			if _, ok := simulcastOrder[ridID]; !ok {
+				simulcastOrder[ridID] = len(simulcastOrder)
+			}
 		}
+		orderOf := func(id string) int {
+			if idx, ok := simulcastOrder[id]; ok {
+				return idx
+			}
+
+			return math.MaxInt
+		}
+		slices.SortStableFunc(rids, func(a, b *simulcastRid) int {
+			return cmp.Compare(orderOf(a.id), orderOf(b.id))
+		})
 	}
 
 	return rids
diff --git a/sdp_test.go b/sdp_test.go
index 540ecde7..4798bfc3 100644
--- a/sdp_test.go
+++ b/sdp_test.go
@@ -1306,6 +1306,33 @@ func TestGetRIDs(t *testing.T) {
 	}
 }
 
+// TestGetRIDsOrderFollowsSimulcast verifies that getRids orders the simulcast
+// streams by the a=simulcast attribute, not by the order of the a=rid lines.
+// RFC 8853 §5.2: the a=simulcast send list "suggests a proposed order of
+// preference, in decreasing order"; the order of the a=rid lines is not
+// significant. Here the a=rid lines are listed in the opposite order to
+// a=simulcast:send, so the result must follow the attribute (f, h, q).
+func TestGetRIDsOrderFollowsSimulcast(t *testing.T) {
+	media := &sdp.MediaDescription{
+		MediaName: sdp.MediaName{
+			Media: "video",
+		},
+		Attributes: []sdp.Attribute{
+			{Key: sdpAttributeRid, Value: "q send"},
+			{Key: sdpAttributeRid, Value: "h send"},
+			{Key: sdpAttributeRid, Value: "f send"},
+			{Key: sdpAttributeSimulcast, Value: "send f;h;q"},
+		},
+	}
+
+	got := make([]string, 0, 3)
+	for _, rid := range getRids(media) {
+		got = append(got, rid.id)
+	}
+
+	assert.Equal(t, []string{"f", "h", "q"}, got)
+}
+
 // TestGetRIDs_EmptySimulcastTokens verifies that getRids does not panic when the

I'm happy to open this as a PR if you'd like the change — just wanted to check whether you'd take it (and whether there was a deliberate reason for the current a=rid ordering) before sending one.

Context: found while auditing the sans-io Rust port (webrtc-rs/rtc), which had the identical behavior; I've prepared the equivalent fix there.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions