Parse pt= parameter in RID attributes#3453
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3453 +/- ##
==========================================
+ Coverage 85.47% 85.57% +0.10%
==========================================
Files 81 81
Lines 9857 9887 +30
==========================================
+ Hits 8425 8461 +36
+ Misses 1011 1007 -4
+ Partials 421 419 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR extends SDP RID (RFC 8851) handling by parsing the optional pt= parameter from a=rid lines into simulcastRid, and emitting pt= back into generated SDP while filtering to payload types supported by the selected codecs.
Changes:
- Parse
pt=payload type lists froma=ridattributes ingetRids(). - Add
payloadTypes []uint8tosimulcastRidand include filteredpt=in generated RID attributes inaddTransceiverSDP(). - Add tests for RID
pt=parsing and SDP round-trip/filtering behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| sdp.go | Adds RID pt= parsing/storage and re-emits filtered pt= in generated SDP RID attributes. |
| sdp_test.go | Adds unit tests covering parsing and SDP generation/filtering for RID payload types. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if attr.Key == sdpAttributeRid { | ||
| split := strings.Split(attr.Value, " ") | ||
| rids = append(rids, &simulcastRid{id: split[0], attrValue: attr.Value}) | ||
|
|
||
| // Parse additional parameters including pt= per RFC 8851 Section 4. | ||
| rid := &simulcastRid{id: split[0], attrValue: attr.Value} |
asnyatkov
force-pushed
the
feat/rid-payload-type-parsing
branch
3 times, most recently
from
cb16e05 to
3191f9d
Compare
| for _, pt := range payloadTypes { | ||
| if availablePTs[uint64(pt)] { | ||
| validPTs = append(validPTs, strconv.FormatUint(uint64(pt), 10)) | ||
| } | ||
| } |
There was a problem hiding this comment.
availablePTs comes from the answer and payloadTypes here comes from the offer, right? if yes we should match PT values and return the values from the answer. and not compare PT values between the answer and the offer. We have some helpers for this, because we do it in multiple places.
Note: In the case that the answerer uses different PT values to
represent a codec than the offerer did, the "a=rid" values in the
answer use the PT values that are present in its answer.
https://datatracker.ietf.org/doc/rfc8851/
There was a problem hiding this comment.
The axiom of WebRTC development: no matter how many RFCs you read, there's always another one, essentially creating infinite set of RFCs.
I'll fix the logic, but there's very confusing item 6.4 in that RFC ("intersections") which is not covered by any existing tests. Can we address that later?
Parse the optional pt= parameter from a=rid lines as defined in RFC 8851 Section 4, storing the allowed payload types in the simulcastRid struct. RFC 8851 defines the RID attribute syntax as: a=rid:<id> <direction> [pt=<fmt list>;<restrictions>] The pt= parameter lists payload types allowed in the RTP stream. For example: a=rid:1 send pt=96,97;max-width=1280;max-height=720 a=rid:2 send pt=96,97;max-width=640;max-height=360 Changes: - Parse pt= from RID attributes in getRids() - Add payloadTypes field to simulcastRid struct - Write pt= back in SDP answer in addTransceiverSDP(), filtering to only include payload types that match available codecs - Comprehensive test coverage for parsing and round-trip
asnyatkov
force-pushed
the
feat/rid-payload-type-parsing
branch
from
3191f9d to
7e66916
Compare
Parse the optional pt= parameter from a=rid lines as defined in RFC 8851 Section 4, storing the allowed payload types in the simulcastRid struct.
RFC 8851 defines the RID attribute syntax as:
a=rid: [pt=;]
The pt= parameter lists payload types allowed in the RTP stream. For example:
a=rid:1 send pt=96,97;max-width=1280;max-height=720
a=rid:2 send pt=96,97;max-width=640;max-height=360
Changes: