Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include XR.SenderSSRC in its DestinationSSRC #169

Merged
merged 1 commit into from
Mar 1, 2024
Merged

Conversation

cnderrauber
Copy link
Member

Include XR.SenderSSRC in its DestinationSSRC

Description

Reference issue

Fixes #...

Copy link

codecov bot commented Mar 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.08%. Comparing base (f260c33) to head (679d82e).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #169      +/-   ##
==========================================
+ Coverage   76.08%   77.08%   +1.00%     
==========================================
  Files          21       21              
  Lines        2421     2422       +1     
==========================================
+ Hits         1842     1867      +25     
+ Misses        484      460      -24     
  Partials       95       95              
Flag Coverage Δ
go 77.08% <100.00%> (+1.00%) ⬆️
wasm 77.08% <100.00%> (+1.00%) ⬆️

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

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

@@ -646,7 +646,8 @@ func (x *ExtendedReport) Unmarshal(b []byte) error {

// DestinationSSRC returns an array of SSRC values that this packet refers to.
func (x *ExtendedReport) DestinationSSRC() []uint32 {
ssrc := make([]uint32, 0)
ssrc := make([]uint32, 0, len(x.Reports)+1)
ssrc = append(ssrc, x.SenderSSRC)
Copy link

Choose a reason for hiding this comment

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

That function name/meaning is specifically for destination. Guess, for XR case, it is okay to include sender also, but wondering if there is a cleaner option.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is not a naming issue, the DestinationSSRC used by pion to dispatch rtcp packets, the packet will be dropped if it doesn't include the SenderSSRC and report block doesn't contain a ssrc field.

Copy link

Choose a reason for hiding this comment

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

Got it. Thank you @cnderrauber

Copy link

@boks1971 boks1971 left a comment

Choose a reason for hiding this comment

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

lg!

Include XR.SenderSSRC in its DestinationSSRC
@cnderrauber cnderrauber merged commit b26a618 into master Mar 1, 2024
13 checks passed
@cnderrauber cnderrauber deleted the xr_ssrc branch March 1, 2024 08:22
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.

2 participants