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

Receive: Fix handling for large timestamp/sequence jumps #266

Merged
merged 2 commits into from
Nov 22, 2024

Conversation

FelixMcFelix
Copy link
Member

This PR addresses some issues which have cropped up on voice receive at scale:

  • In unknown circumstances, we can be left with adjacent packets queued which have very different timestamps. The playout buffer would withhold its held packets, leading to the loss of many subsequent packets if the timestamp jump is larger than 64 frames. This seems to occur for some specific clients which join before a bot, suggesting the DAVE -> legacy switchover is involved.
  • Some loss patterns can leave us unable to correctly track the next expected sequence number (i.e., large loss runs), leaving the playout buffer unable to accept any packets if the packet sequence differed by over 64 entries.

The fixes are fallbacks which treat sufficiently large desynchronisation, and allow the playout to get back into a consistent state in both cases. Large timestamp jumps on adjacent packets now update the next expected TS (noting that we only want to withhold a few playout delays at most ). Failure to insert 0.25s of packets (or attempting to add a new sequence number into an empty buffer) can now take precedence.

Closes #261.

@FelixMcFelix
Copy link
Member Author

The main bot encountering these issues has had no recurrence after applying these fixes, so they appear good to go.

@FelixMcFelix FelixMcFelix merged commit af95296 into serenity-rs:current Nov 22, 2024
11 checks passed
@FelixMcFelix FelixMcFelix deleted the fix-rx-jump branch November 26, 2024 00:52
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

Successfully merging this pull request may close these issues.

Possible current_timestamp desynchronization in PlayoutBuffer
1 participant