-
Notifications
You must be signed in to change notification settings - Fork 241
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
Replace sample generics with fixed f32 #678
Comments
✋ I can help. Devil's advocate: today, users can have a |
I expect the number of users that use non float Source's to be small since almost any effect would use f32. Of course effects are not the only use for So yes there will be a few users hit by this, thats why its breaking. We have an upgrade guide to help them out. Specifically I would point them to dasp::Sample. Would you, as advocate of the devil 😛, agree thats enough? |
My personal view: I like the flexible, hassle-free approach that Rodio offers. 16-bit PCM is at the basis of digital audio and I'd expect out-of-the-box support. I can see it depending on crate positioning. If I don't shy away from generics, particularly in Rust whose type system makes they robust and usually without any performance hit. But, if you were to take it out to settle on only a Done well, I can imagine it could be the same struct as you would need at the output to convert into whatever |
More like: cpal is the abstraction layer over OS-interfaces, so the opposite of bare metal. awedio is rodio without generics settled on i16 and with a different approach to controlling the source layers its impressive but not light-weight. Regarding rodio, I do not yet know what we are (still being discussed https://github.com/RustAudio/rodio/pull/654/files).
Thats quite a generic statement 😛. For me it depends where they are used, its fine to use
I was thinking about pointing users to dasp's conversion traits, maybe we should provide some examples with that? |
If we settle on |
I was trying to put Rodio into perspective. I'll add to #654.
You have my view - it has not yet changed.
fn main() -> Result<(), anyhow::Error> {
// ...
match config.sample_format() {
cpal::SampleFormat::F32 => run::<f32>(&device, &config.into())?,
cpal::SampleFormat::I16 => run::<i16>(&device, &config.into())?,
cpal::SampleFormat::U16 => run::<u16>(&device, &config.into())?,
}
// ...
}
fn run<T>(device: &cpal::Device, config: &cpal::StreamConfig) -> Result<(), anyhow::Error>
where
T: cpal::Sample,
{
// ...
} With or without |
I do not understand what you mean exactly, thats probably on me, I am kinda tired today 😅 |
I've quickly tried a few options, I can think of two options:
trait AnySource: Iterator<Item = Into<dasp::Sample>>;
struct SampleAdaptor<S> {
inner_source: S,
}
impl<S> SampleAdaptor<S> {
fn new(source: S) -> Self {
}
}
impl<S: AnySource> Iterator for SampleAdaptor<S> {
type Item = f32;
fn next(&mut self) -> Option<f32> {
f32::from(self.inner_source.next()?.into())
}
}
impl<S> Source for SampleAdaptor<S> {
..
}
fn to_rodio_sample(sample: Into<Sample>) -> f32 {
sample.into().to_f32()
} My preference goes to the latter, I think it will be easier to explain in the upgrade guide. |
Regarding |
and you can not use dasp without setting up cpal right? Dasp is cool, and if users want it we could add something in the future to make integration easier? Though right now the biggest wants from users seem to be around cross-fade and easier 'playlist' management (queue stuff). Once we have those the difference between rodio and dasp will be clearer.. |
For most use cases that I can entertain I would think the first is easier, because generally users work with a collection of samples and not just one. Something like this you will anyway need (?) at the end of the pipeline to cpal. If so, why not expose it to users also? Thinking out loud: instead of an iterator it could also return a converted collection. Scrap that: won't work for streaming sources. Brainfart 2: it does not need to be one or the other. The utility method in your latter example could be used in the iterator in the former. |
Idea by @roderickvd see #669 (comment).
The single argument for this change is how much simpler the rodio code base will be with it applied. That leads to better maintainability and that means more time for adding fun stuff :)
We will lose the ability to losslessly support audio using more then 24 bits (f32 supports integers up to 2^24 without loss of precision). This is a non argument since there is no purpose to such precision in audio playback, 16bit already being transparent to human hearing. The 24 bits provided by f32 give ample room for additional effects that could affect the noise floor.
As an example of the extra complexity induced by having a generic sample argument see: #670 (comment).
We would still need a sample convertor in the interface to cpal, though f32 outputs should be preferred.
This will be a big though simple effort, maybe we can take turns on working on this on a separate branch?
The text was updated successfully, but these errors were encountered: