-
Notifications
You must be signed in to change notification settings - Fork 54
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -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) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this 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
Include XR.SenderSSRC in its DestinationSSRC
Description
Reference issue
Fixes #...