NiklasEi / bevy_kira_audio

A Bevy plugin to use Kira for game audio
Apache License 2.0
315 stars 54 forks source link

Make playbacktime accessible to users #23

Closed NiklasEi closed 2 years ago

NiklasEi commented 2 years ago

Pretty important for rhythm games...

fluffysquirrels commented 2 years ago

I am writing a rhythm game and am currently using this plugin to get audio on the web platform.

Can I contribute to this?

What's the strategy, given that there's a queue of commands going to another thread / system it looks like, how can we get the result back from a GetPlaybackTime command to the caller?

fluffysquirrels commented 2 years ago

You could have a command that turns on publishing playback time in another or cell to the caller, and then the caller can ask what the latest time is, which will be frame accurate. Does that sound like it would work?

fluffysquirrels commented 2 years ago

I made a hacky proof of concept where the caller can get the current playback time. The public Audio::play_* methods now return an InstanceHandle, through which the caller can ask for the playback time for the current frame. I thought an InstanceHandle like this would also be useful for stopping just one playing sound rather than all of them as currently is done.

This is implemented by an Arc<Atomic<f64>> shared by both the InstanceHandle and an internal InstanceHandlePriv (owned by AudioOutput on the audio thread). There is an on_update system that runs every frame and copies the current playback position from kira's InstanceHandle to our InstanceHandle, which the caller can read with a simple atomic load. Depending on your thoughts maybe this copying should be opt-in, as I'm not sure what the performance overhead will be per frame, especially if very many sounds are being played.

Check it out on my branch: https://github.com/fluffysquirrels/bevy_kira_audio/tree/playback_time

Let me know if you want me to clean it up and submit it as a PR.

NiklasEi commented 2 years ago

I took a look, thank you for the initiative! It looks good and I would appreciate a RP :+1:

I have very little experience with how many instances a bigger game might have playing at the same time, but we are removing stopped instances, so I would hope that this never introduces significant overhead. For smaller games this should not be an issue for sure, so we could keep it as default and consider other options when it is needed and we have some metrics.

fluffysquirrels commented 2 years ago

Playback position in kira is returned as seconds in an f64, I think we should just pass that through, even though something like a std::time::Duration might be preferable. Your thoughts?

NiklasEi commented 2 years ago

Closed in #32 See the new status example