Skip to content

sdp: order simulcast rids by the a=simulcast attribute, not a=rid line order - #3472

Open
StefanoD wants to merge 1 commit into
pion:mainfrom
StefanoD:simulcast-order-by-attribute
Open

sdp: order simulcast rids by the a=simulcast attribute, not a=rid line order#3472
StefanoD wants to merge 1 commit into
pion:mainfrom
StefanoD:simulcast-order-by-attribute

Conversation

@StefanoD

Copy link
Copy Markdown

Description

getRids returned the simulcast rids in a=rid line order, consulting a=simulcast only to mark ~ paused streams. Per RFC 8853 §5.2, the a=simulcast send list — not the a=rid line order — carries the stream preference:

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

When a peer lists its a=rid lines in a different order than its a=simulcast:send list, the resulting RTPReceiveParameters.Encodings / Tracks() order — and the a=simulcast:recv list echoed in the answer — followed the a=rid order instead of the offered preference.

This stable-sorts 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.

Latent in practice — browsers (Firefox 152, Chrome) emit both in the same order, and demux is by RID string — so this is a SHOULD-level alignment, not a spec violation: §5.2 "suggests" the order (non-normative), and §5.3.2 only mandates reversing send↔recv (SHALL) and forbids adding streams (MUST NOT); it does not itself require preserving order.

Closes #3471 (full reproduction and impact analysis there).

Test plan

  • Adds TestGetRIDsOrderFollowsSimulcast: an offer whose a=rid lines are in the opposite order (q, h, f) to a=simulcast:send f;h;q. getRids returns [q, h, f] before this change and [f, h, q] after.
  • go build ./..., go vet ., gofmt, and golangci-lint run (v2.10.1, --new-from-rev=main) are all clean.

@JoTurk JoTurk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, once you fix the parser issue we can merge this.

Comment thread sdp.go Outdated
@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.03%. Comparing base (d9b2a24) to head (145e1e8).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3472      +/-   ##
==========================================
- Coverage   86.18%   86.03%   -0.15%     
==========================================
  Files          81       81              
  Lines       10442    10456      +14     
==========================================
- Hits         8999     8996       -3     
- Misses       1007     1021      +14     
- Partials      436      439       +3     
Flag Coverage Δ
go 86.03% <100.00%> (-0.15%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

getRids returned the simulcast rids in a=rid line order, consulting
a=simulcast only to mark "~" paused streams. RFC 8853 §5.2 says the
a=simulcast send list "suggests a proposed order of preference, in
decreasing order"; the a=rid line order is not significant. When a
peer lists its a=rid lines in a different order than its
a=simulcast:send list, the resulting encodings — and the
a=simulcast:recv list echoed in the answer — followed the a=rid order
instead of the offered preference.

Sort the rids by their first position in the a=simulcast attribute
(stable, so any rid absent from the attribute keeps its a=rid
declaration order at the end). Browsers emit both in the same order,
so this is latent in practice, but an SFU or another client may not.

Also parse the send list per RFC 8853 §5.1: it is a ";"-separated
list of simulcast streams, each a ","-separated list of alternative
rids. The previous code split on ";" only, so a value like
"send f,h;q" treated "f,h" as one opaque token, breaking both the
reorder and the "~" paused detection.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Stefano Di Martino <stefano.d@posteo.de>
@StefanoD
StefanoD force-pushed the simulcast-order-by-attribute branch from 686a3cd to 145e1e8 Compare July 17, 2026 18:02
@StefanoD
StefanoD requested a review from JoTurk July 17, 2026 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants