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

Continued NACK Improvements #9

Open
Sean-Der opened this issue Dec 4, 2020 · 2 comments
Open

Continued NACK Improvements #9

Sean-Der opened this issue Dec 4, 2020 · 2 comments

Comments

@Sean-Der
Copy link
Member

Sean-Der commented Dec 4, 2020

  • Currently nack is sent in a fixed intervals.
  • There is no limit for how many times a nack will be sent for a single packet (current implementation sends nacks as long as either the packet arrives, or the packet is older than max sequence number - buffer size)
  • Nacked packets/nack requests are sent using the same PayloadType as normal rtp packets. It is possbile to send them in a dedicated feedback stream. This is much more complicated, and current implementation works, so it should definietly be an improvement in a separate issue.
@Sean-Der Sean-Der mentioned this issue Dec 12, 2020
3 tasks
@aler9
Copy link
Member

aler9 commented Dec 26, 2020

Is it normal that the NACK SSRC is random?

senderSSRC := rand.Uint32() // #nosec

I didn't saw the specification, but the receiver SSRC should be shared between reports and nacks, therefore it is random but at a higher level.

Edit: i realized that interceptors work with a variable number of streams, therefore they must be able to generate SSRCs - but they must be shared between NACKs and receiver reports. We have to find a way to make it so.

@Sean-Der
Copy link
Member Author

@aler9 I think the best way to do this is the Option pattern we use in Pion.

You can see it in GeneratorOption there is also a ResponderOption in the same directory. Instead of doing a 'Configuration Struct' or a bunch of function arguments we do this.

We should add one to set the SSRC. If one isn't set then we can do random!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants