Support simulcast from "SIM" ssrc-group - #3396
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3396 +/- ##
==========================================
- Coverage 85.66% 85.41% -0.25%
==========================================
Files 81 81
Lines 9819 9880 +61
==========================================
+ Hits 8411 8439 +28
- Misses 992 1024 +32
- Partials 416 417 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
281474f to
4a5e4c3
Compare
|
Updated the commit message to fix the metadata linter. Should I try to refactor |
4a5e4c3 to
3637ef3
Compare
|
Rebased and added method to read RTCP from simulcast tracks based on SSRC (instead of RID). Anything else I should do to help get this landed? Thanks! |
|
@fancycode I'll take a look. thank you. |
|
Friendly ping. Should I rebase to latest |
|
@fancycode I'm so sorry, I got busy with the DTLS 1.3 project, I'll try to get this reviewed after my work today. |
|
No worries, just wanted to check if there is something I could do to help. |
There was a problem hiding this comment.
Pull request overview
Adds support for SSRC-based simulcast SDP groups and per-encoding RTCP access.
Changes:
- Parses
SIMSSRC groups with RTX/FEC mappings. - Adds SSRC-specific simulcast RTCP read APIs.
- Updates receiver detection and SDP tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
sdp.go |
Parses SIM groups and encoding repair SSRCs. |
sdp_test.go |
Tests SIM, RTX, and FEC parsing. |
rtpreceiver.go |
Adds SSRC-specific RTCP reads. |
peerconnection.go |
Recognizes multiple repair SSRCs. |
errors.go |
Adds an SSRC lookup error. |
constants.go |
Adds SIM semantics and updated guidance. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } | ||
| } | ||
|
|
||
| tracksInMediaSection = []trackDetails{simulcastTrack} |
| // ReadSimulcastSSRC reads incoming RTCP for this RTPReceiver for given SSRC. | ||
| func (r *RTPReceiver) ReadSimulcastSSRC(b []byte, ssrc SSRC) (n int, a interceptor.Attributes, err error) { |
| continue | ||
| } | ||
|
|
||
| ssrcs = append(ssrcs, SSRC(ssrc)) |
| for rtx, base := range rtxRepairFlows { | ||
| baseSsrc := SSRC(base) //nolint:gosec // G115 | ||
| if pos := slices.Index(ssrcs, baseSsrc); pos != -1 { | ||
| repairSsrc := SSRC(rtx) //nolint:gosec // G115 | ||
| simulcastTrack.rtxSsrc[pos] = &repairSsrc |
| tracks := trackDetailsFromSDP(log, descr) | ||
| assert.Equal(t, 4, len(tracks)) | ||
| assert.Equal(t, []SSRC{3000, 4000, 5000}, tracks[0].ssrcs) | ||
| assert.Equal(t, []SSRC{3000, 5000}, tracks[1].ssrcs) | ||
| assert.Equal(t, []SSRC{3000, 4000, 5000}, tracks[2].ssrcs) |
|
@fancycode can you please look at copilot comments? just mark anything you think is made-up as resolved. and I'll try to review it tonight, thank you. |
3637ef3 to
f147538
Compare
|
Thanks, I rebased and addressed all issues found by Copilot but didn't add a end-to-end test as suggested in #3396 (comment). |
Required to correctly handle offer SDPs like this (taken from Nextcloud Talk with Chrome):
Without this change, only a single track with the first SSRC is signaled from
PeerConnection.OnTrack.