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

Fix busy-loop in opus.py DecodeManager #1395

Merged
merged 3 commits into from
Jun 10, 2022

Conversation

nikolabura
Copy link
Contributor

@nikolabura nikolabura commented May 31, 2022

Summary

When decoding Opus frames (receiving audio), the run() function can end up busy-looping and consuming all the CPU if the decode queue is empty. A millisecond-long time.sleep, added by this PR, seems to prevent this.

However, this may cause issues if more than 20 audio streams are being decoded, since Discord gives you an audio frame every 20 milliseconds - I have not looked into this. As per @plun1331's suggestion, if we only sleep when the queue is empty, this should be avoided.

Checklist

  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • If type: ignore comments were used, a comment is also left explaining why
  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, typehinting, examples, ...)

When decoding Opus frames (receiving audio), the run() function can end up busy-looping and consuming all the CPU if the decode queue is empty. A millisecond-long time.sleep prevents this.

However, this may cause issues if more than 20 audio streams are being decoded, since Discord gives you an audio frame every 20 milliseconds - I have not looked into this.
Lulalaby
Lulalaby previously approved these changes May 31, 2022
@plun1331
Copy link
Member

Maybe this could only sleep if the queue is empty?

@nikolabura
Copy link
Contributor Author

Maybe this could only sleep if the queue is empty?

Good point! I tested it with that, and it still works and doesn't use up CPU.

@Lulalaby Lulalaby enabled auto-merge (squash) May 31, 2022 21:17
@Lulalaby Lulalaby requested a review from Dorukyum May 31, 2022 21:41
@Lulalaby Lulalaby added bug Something isn't working priority: high High Priority status: awaiting review Awaiting review from a maintainer Merge with squash labels May 31, 2022
@Lulalaby Lulalaby requested a review from BobDotCom June 9, 2022 23:41
@Lulalaby Lulalaby merged commit cb5a21f into Pycord-Development:master Jun 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority: high High Priority status: awaiting review Awaiting review from a maintainer
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

4 participants