Replace sample generics with fixed f32

[dvdsk]

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?

[roderickvd]

✋ I can help.

Devil's advocate: today, users can have a Source with i16 samples and simply append them to a Sink. That may even be a common case. How sure are we about removing those ergonomics?

[dvdsk]

users can have a Source with i16 samples and simply append them to a Sink. That may even be a common case. How sure are we about removing those ergonomics?

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 Source you can also use it to generate audio or load from disk. For those use-cases rodio offers ready made sources.

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?

[roderickvd]

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 cpal is bare metal, awedio light-weight, then I regard Rodio as the extended yet no-frills toolkit.

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 f32 pipeline internally, then I would advocate having a pluggable "adapter" for taking some input and converting it.

Done well, I can imagine it could be the same struct as you would need at the output to convert into whatever cpal expects.

[dvdsk]

If cpal is bare metal, awedio light-weight, then I regard Rodio as the extended yet no-frills toolkit.

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).

I don't shy away from generics

Thats quite a generic statement 😛. For me it depends where they are used, its fine to use impl AsRef<Path> in a function. Its also not a problem if you expect the generics to be qualified by the direct user of your struct or if they will use dynamic dispatch. I do shy away from statically dispatched code in chains of structs. Statically dispatched generics then generally make the code hard to change. That makes it a chore to refactor or add new stuff. These generics distract while writing filters/effects. While easily writing those is one of rodio's (proposed feedback always appreciated) goals.

I would advocate having a pluggable "adapter" for taking some input and converting it.

I was thinking about pointing users to dasp's conversion traits, maybe we should provide some examples with that?

[PetrGlad]

If we settle on f32 rodio Sample is not really necessary anymore. Having custom sample trait in rodio limits where dasp_sample conversions can be used since we are requiring rorio::Sample in the API.

[roderickvd]

If cpal is bare metal, awedio light-weight, then I regard Rodio as the extended yet no-frills toolkit.

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).

I was trying to put Rodio into perspective.

I'll add to #654.

I would advocate having a pluggable "adapter" for taking some input and converting it.

I was thinking about pointing users to dasp's conversion traits, maybe we should provide some examples with that?

You have my view - it has not yet changed.

Well, when this is implemented then this whole thing will look a lot like dasp... https://github.com/RustAudio/dasp/blob/master/examples/synth.rs

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 <T: Sample> from some namespace?
Asking to understand - not be a wise guy ☮️

[dvdsk]

Well, when this is implemented then this whole thing will look a lot like dasp...
https://github.com/RustAudio/dasp/blob/master/examples/synth.rs

I do not understand what you mean exactly, thats probably on me, I am kinda tired today 😅

[dvdsk]

I would advocate having a pluggable "adapter" for taking some input and converting it.

I've quickly tried a few options, I can think of two options:

• introducting some kind of Source trait like the current with an adapter layer between:

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> {
    ..
}

• requiring users to change their existing sources to the new f32 non generic Source type but providing functions in rodio to the sample conversion

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.

[PetrGlad]

Regarding dasp, if sample format is limited, and we also do not have to convert sample rates then the API would look a lot like dasp. However, I have looked at the dasp API closer and it does not seem to have any channel manipulation logic (like mixing/routing).

[dvdsk]

Regarding dasp, if sample format is limited, and we also do not have to convert sample rates then the API would look a lot like dasp. However, I have looked at the dasp API closer and it does not seem to have any channel manipulation logic (like mixing/routing).

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..

[roderickvd]

I've quickly tried a few options, I can think of two options:

• introducting some kind of Source trait like the current with an adapter layer between:

• requiring users to change their existing sources to the new f32 non generic Source type but providing functions in rodio to the sample conversion

My preference goes to the latter, I think it will be easier to explain in the upgrade guide.

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.

[dvdsk]

Okay, I think we will hammer out the conversion adapters during implementation. We will write examples and see what fits best.

[dvdsk]

@tomaka @est31, I am considering removing the generic sample from Source replacing it with f32, concretely source will become an Iterator over f32 instead of an Iterator over Sample. I would like your stance on this.

• Performance: normally no impact since lots of rodio's sources already require a conversion to f32. Even if the user has a fully i16 pipeline the impact on performance is low (< 2% for the worst case).
• Audio Quality: no change, f32 can represent up to and including 24bit's audio. More then 24 bit precision does not make sense for audio.
• API: users writing sources themselves will have to adapt those to return f32 or we may provide an adaptor Source.
• Rodio Code: becomes simpler to maintain since all the Sample::to_f32 calls disappear and we can drop the sample-convertor step.

[tomaka]

Nothing against doing this 👍

[dvdsk]

@PetrGlad are you in favor of this? If yes I want to make a PR soon.

Sidenote: I just found another reason: a partial audio chain needs type annotations since the type of Sample can not be figured out. This is confusing users.

[PetrGlad]

It looks like iOS's core audio also supports only f32. As @roderickvd already suggested, we should settle on some minimal spec hardware that is supported to avoid agonizing over it again. It could be something like "Raspberry Pi Zero 2W" (but I do not have it at hand), which uses ARM1176JZF-S with floating point support. Or maybe the requirements may look like: "hardware support for f32 and at least 32bits wide atomics".

So yes. let us do it. I'd prefer to keep the sample format as type alias still.

[dvdsk]

Or maybe the requirements may look like: "hardware support for f32 and at least 32bits wide atomics".

I like that, lets specify the exact requirements rodio needs.

So yes. let us do it. I'd prefer to keep the sample format as type alias still.

Agreed, oh and btw I really like the ChannelCount and SampleRate aliases too thanks for adding those.

[PetrGlad]

@roderickvd @dvdsk Do you plan work on this change? I could pick this otherwise.

I think this may turn into "epic". For example with f32 linear resampler can be simplified (the interpolation part). Also then we may need a limiter for cases where the output amplitude exceeds -1.0...1.0.

Another major change would be to use immediate stream format checks (#694). That issue and this one may cause a lot of merge conflicts so probably should be done one after another. I'd prefer to change sample format first since it looks like a simpler one.

[dvdsk]

Do you plan work on this change? I could pick this otherwise.

I was planning too but something came up, if you want to take it that would be great. Its gonna be a ton of changes, maybe a smart sed command or vim macro can help you speed things up.

Also then we may need a limiter for cases where the output amplitude exceeds -1.0...1.0.

Isn't that a separate issue? The f32 pipeline already exist this change should just remove the others.

That issue and this one may cause a lot of merge conflicts so probably should be done one after another.

agreed

I'd prefer to change sample format first

yeah that's probably best

[dvdsk]

On a separate note, once this and #694 land we might want to add a old_rodio conversion source. I would prefer end users just migrate but maybe that is too much for some.

[roderickvd]

Do you plan work on this change? I could pick this otherwise.

I think this may turn into "epic". For example with f32 linear resampler can be simplified (the interpolation part). Also then we may need a limiter for cases where the output amplitude exceeds -1.0...1.0.

Open to help. Which initiatives or stories would you like me to take a look at?

I also would not mind working on the decoders a bit more first. My next stab would be at the Symphonia decoder, to make it more robust, expose more of its features, and solve an upcoming breakage of seeking when this is merged: pdeljanov/Symphonia#340.

Also then we may need a limiter for cases where the output amplitude exceeds -1.0...1.0.

Isn't that a separate issue? The f32 pipeline already exist this change should just remove the others.

• There's the automatic gain control;
• The typical let sample = (1.1 * i16::MAX as f32) as i16 is saturating (not panicking).

That issue and this one may cause a lot of merge conflicts so probably should be done one after another.

Would be great if we could have some sort of coordination in time (most of us probably have an outlook when we can work on things). So we can spend more time on fun new stuff and less on solving merge conflicts.

[dvdsk]

I also would not mind working on the decoders a bit more first. My next stab would be at the Symphonia decoder, to make it more robust, expose more of its features, and solve an upcoming breakage of seeking when this is merged: pdeljanov/Symphonia#340.

It should be pretty safe to work on seek. Regarding decoders, it seems you know your way around them now, would you mind helping out with #694 once I've got that started? You could fix/port the decoders while I take on the rest of rodio. We could work on a fork or we could add you as maintainer (the more the merrier right?) and work on a branch here. Let me know if that's something you would be open too.

So we can spend more time on fun new stuff and less on solving merge conflicts.

Completely agreed! I've paused my work on adding rubato/player and a few other bits till we get this and #694 merged. I might be able to get #694 done about a week after this lands. From then on we should be clear of most merge conflicts and the fun stuff begins again. Do take your time @PetrGlad there is no hurry, lets all remember its a hobby, its gotta be fun.

[roderickvd]

I also would not mind working on the decoders a bit more first. My next stab would be at the Symphonia decoder, to make it more robust, expose more of its features, and solve an upcoming breakage of seeking when this is merged: pdeljanov/Symphonia#340.

It should be pretty safe to work on seek. Regarding decoders, it seems you know your way around them now, would you mind helping out with #694 once I've got that started? You could fix/port the decoders while I take on the rest of rodio.

Sure thing.

We could work on a fork or we could add you as maintainer (the more the merrier right?) and work on a branch here. Let me know if that's something you would be open too.

Open for that. Working on Rust audio is my evening hobby after hard days work 😄

Completely agreed! I've paused my work on adding rubato/player and a few other bits till we get this and #694 merged. I might be able to get #694 done about a week after this lands. From then on we should be clear of most merge conflicts and the fun stuff begins again.

When you say you plan on getting #694 "done", which parts would you like me to contribute to specifically?

[PetrGlad]

OK, I'll pick this. I do not think we can avoid merge conflicts altogether, but yes, decoders should not interfere too much with this.

[PetrGlad]

@roderickvd I'd say solve whatever bothers you most, but bigger changes have to wait until this and #694.