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 #122 #123

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix #122 #123

wants to merge 3 commits into from

Conversation

voidentente
Copy link

This is how I'd solve this. It uses locking, necessarily, but I made sure the lock isn't held for longer than necessary. It uses a secondary buffer to minimize this time and reuse allocations.

I didn't use a channel because this is polling behavior, and we don't need blocking i.e. notifiers etc.

@tesselode
Copy link
Owner

I might be OK with the non-realtime-safe code if the error rate was always low, but considering you were getting "errors" from JACK every time the buffer size changed, I'm not confident about that. I think I'd be more comfortable with the fixed ringbuffer + an atomic counter of how many errors were discarded.

@voidentente
Copy link
Author

Got it :)

@tesselode
Copy link
Owner

Can you add a way to get the errors from the main thread?

@voidentente
Copy link
Author

voidentente commented Feb 18, 2025

Can you add a way to get the errors from the main thread?

All StreamErrors or only BackendSpecificError?

EDIT: I assume by "main thread" you mean the AudioManager via a method on CpalBackend accessible via backend_mut. Exposing the errors also might create uncertainty about which errors Kira has already handled.

@voidentente
Copy link
Author

There are two separate ringbuffers, one for internal use by kira (unhandled) and one for external use (handled). It wouldn't make sense to have the external use transport be unbounded, since it's likely that most applications won't poll kira for backend errors, thus inflating memory usage.

It might also make sense to invert the public-facing ringbuffer to only store the latest errors, not the oldest ones. I'm ambivalent to this policy, but I just wanted to point that out.

The Mutex is only there to make it Sync, it's not actually used for locking, similar to cpu_usage_consumer.

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.

2 participants