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.

Sign up for GitHub

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

Jump to bottom

The Jitter buffer iterceptor doesn't handle packet loss #256

Open
braams opened this issue Jun 5, 2024 · 1 comment · May be fixed by #258
Open

The Jitter buffer iterceptor doesn't handle packet loss #256

braams opened this issue Jun 5, 2024 · 1 comment · May be fixed by #258
Assignees
@thatsnotright

Comments

@braams
Copy link

braams commented Jun 5, 2024 •
edited

Your environment.

What did you do?

I'm trying to use jitter buffer interceptor.

What did you expect?

I expect to get an error on read if the current packet was lost. And I expect to get the next packet on the next read.

What happened?

If even one packet was lost, all subsequent reads will return an error.

The reason

The interceptor does not handle the situation with the absence of the next packet and does not shift the expected packet number

interceptor/pkg/jitterbuffer/receiver_interceptor.go

Line 73 in 142f17f

if err != nil {

It looks like we should add something like this:

if errors.Is(err, ErrNotFound) {
	i.buffer.SetPlayoutHead(i.buffer.PlayoutHead() + 1)
}
@braams braams linked a pull request Jun 17, 2024 that will close this issue
Open
@thatsnotright thatsnotright self-assigned this Aug 30, 2024
@thatsnotright
Copy link
Contributor

@braams My intent here was that the user of the jitter buffer is in the best position to decide the next step. If a packet is lost and it's appropriate to retransmit then all of that has to be done outside the jitter buffer context. If you wish to skip the dropped packet then call SetPlayoutHead to skip the unrecoverable packets. I'm definitely open to suggestions or improvements here, and if adding an option to skip missed packets is helpful we can discuss what that might look like!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@thatsnotright thatsnotright

Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Shift the expected sequence number if a packet was lost braams/interceptor
2 participants
@thatsnotright @braams