-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Use git version of the rodio to fix the audio issue on macOS #310
Conversation
This is a welcome change, but we can't use git dependencies in bevy anymore because that would block crates.io releases. Lets leave this open until the next rodio crates.io release. |
Also what can we do with the OutputStream struct it is not Send + Sync and i am currently just std::mem::forget it. |
Hmm good question. Off the top of my head im not quite sure. It might be good to ask the Rodio folks directly, if the docs dont cover this case. |
The reason is that the platform-specific streams may not be thread-safe. This concerns at least Android (AAudio), maybe some other platforms, but CPAL made all streams non-Send and non-Sync so that the behavior is the same across all platforms. One solution would be for AudioOutput to create a separate thread that would own the stream and send the handle back. Drop implementation of AudioOutput would join the thread. |
Cool that seems reasonable. Thanks for the insight @endragor! Lazy half formed thought: @aclysma @lachlansneff maybe this could somehow play into our new task system? Create a new "audio thread pool" with one long lived thread? It probably wouldn't really benefit from that abstraction given that the work on that thread would be well defined. I'm more bringing this up in the interest of keeping our threading all under one abstraction. We could also just look into adding !Send !Sync resources/systems. |
Just a quite response to my ping. I haven't read this PR. The io task pool
is intended to cover things like audio.
…On Tue, Sep 1, 2020, 8:12 PM Carter Anderson ***@***.***> wrote:
Cool that seems reasonable. Thanks for the insight @endragor
<https://github.com/endragor>!
(I recognize you from your work on Godot's nim bindings 😄 )
Lazy half formed thought: @aclysma <https://github.com/aclysma>
@lachlansneff <https://github.com/lachlansneff> maybe this could somehow
play into our new task system? Create a new "audio thread pool" with one
long lived thread? It probably wouldn't really benefit from that
abstraction given that the work on that thread would be well defined. I'm
more bringing this up in the interest of keeping our threading all under
one abstraction.
We could also just look into adding !Send !Sync resources/systems.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#310 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHPIOK74T5C34DOTFAKT7FLSDWEVNANCNFSM4QI44AVA>
.
|
The new version of rodio has been published to crates.io. Its been a time since i created this draft so how we handle the main handler. I am not really familiar with the current state of bevy. |
Discussion continues on #638. I deleted my fork so i cannot add commits to the PR. |
Fixes #307 . I am creating this PR as draft since i am not sure if we should wait for rodio to update or just use the git version until it updates.