tesselode / kira

Library for expressive game audio.
https://crates.io/crates/kira
Apache License 2.0
855 stars 43 forks source link

allow streaming sounds of unknown length #68

Closed fragsalat closed 10 months ago

fragsalat commented 10 months ago

Fixes #67

Hey there,

I tried to fix the issue I mentioned by making the num_frames an optional. It should be backward compatible and have no effect to existing solutions. In case there is no num_frames the playback end region is not known. This means the playing state can't be set to false as it is not known when the end is approached.

Indeed I yet don't know how to test the case when num_frames could not be determined but the sound indeed has an end. As the transport doesn't know about the data size it can't handle the true end. As well I have no idea what the impact of long running streams is. The data vector likely will just continuously be filled and never freed as long as the track is hold by the manager. Do you have insights here?

Feel free to give any feedback on that change :)

fragsalat commented 10 months ago

FYI: I recognized a position() method is missing for stream sounds. I added one returning optional Duration based on the presense of num_frames.

tesselode commented 10 months ago

Hi, thanks for the PR!

I have some concerns about how allowing infinite streaming sounds would affect Kira's API.

StreamingSoundData::new could just return an error if you use these features with infinite sounds, but I'd rather have an API that prevents you from doing the wrong thing at compile time.

StreamingSoundData is meant for finite streaming sounds, so maybe there should be a lower-level type for streaming sounds in general. Kira in general could be more modular so it's easier to set up bespoke Sound implementations - both static and streaming sounds have a lot of duplicated code that you can't reuse for your own types.

I'm open to API suggestions!

fragsalat commented 10 months ago

Hey there, thanks for the respone. I actually kind of expected something like that. What would you think of an additional RemoteStreamingSoundData which implements this. It could be provided by feature or additional package to keep this package more clean. It likely would use reqwest an offer a convenience api like ::from_url and in the settings the storage would be configured e.g. buffer or file, max buffer size etc.

fragsalat commented 10 months ago

This can be closed then. For my local project i will continue with my fork for now

tesselode commented 10 months ago

RemoteStreamingSoundData sounds like a great idea for a separate crate!

chayleaf commented 9 months ago

just wanted to say that this would have been useful for me too. However, in my experience this PR does the same as returning i64::MAX as the sample count.

fragsalat commented 9 months ago

Fine for me but be aware this fork is for a private project only and will not receive any further updates

tesselode commented 9 months ago

@chayleaf are you also using this for radio streaming?

chayleaf commented 7 months ago

I've been trying to use it for usfx. In practice, I simply couldn't get kira to work with it, so I switched to raw rodio.