ChannelVolume usage causes crackling/distortion

[will3942]

We are using ChannelVolume to output Sources in mono on multichannel devices.

We have noticed that when using ChannelVolume the audio is distorted and crackly versus when not using ChannelVolume.

Code sample for good audio:

use std::io::BufReader;

use rodio::source::ChannelVolume;

fn main() {
    let (_stream, handle) = rodio::OutputStream::try_default().unwrap();
    let sink = rodio::Sink::try_new(&handle).unwrap();

    let file = std::fs::File::open("assets/sample.mp3").unwrap();
    let decoder = rodio::Decoder::new(BufReader::new(file)).unwrap();

    sink.append(decoder);

    sink.sleep_until_end();
}

Code sample for distortion:

use std::io::BufReader;

use rodio::source::ChannelVolume;

fn main() {
    let (_stream, handle) = rodio::OutputStream::try_default().unwrap();
    let sink = rodio::Sink::try_new(&handle).unwrap();

    let file = std::fs::File::open("assets/sample.mp3").unwrap();
    let decoder = rodio::Decoder::new(BufReader::new(file)).unwrap();

    let channel_volumes = ChannelVolume::new(decoder, [1.0, 1.0].to_vec());

    sink.append(channel_volumes);

    sink.sleep_until_end();
}

Sample mp3: https://github.com/will3942/test-rodio/raw/refs/heads/main/assets/sample.mp3

[will3942]

This occurs in both release / non-release mode (although more evident in release).

Particularly evident during 13 to 20s section.

[dvdsk]

can you try out PR #669? That might fix it.

[will3942]

I'm wondering is this simply a miss on the stereo->mono conversion algorithm? The way it works at the moment is the samples are all summed together (https://github.com/RustAudio/rodio/blob/master/src/source/channel_volume.rs#L36-L45)

A simple algorithm for stereo->mono conversion is to average all of the samples so perhaps (for stereo) we are just missing a division by two or for another number of channels a division by the number of channels.

[will3942]

can you try out PR #669? That might fix it.

As far as my ears can hear it that seems to fix it or at least come very close to fixing it.

[will3942]

@dvdsk Thanks for the input - what's your thinking on the timelines for the next release of rodio? We're planning on moving to production next week with our app and so will be running off a Rodio fork with a few of these changes (ChannelVolume temporary fix or ChannelRouter and this f32 change).

Happy to understand how we can support at all on the development - we're really interested in the changes to the resampler (e.g. Rubato) and ChannelRouter.

[dvdsk]

I'm wondering is this simply a miss on the stereo->mono conversion algorithm? The way it works at the moment is the samples are all summed together (https://github.com/RustAudio/rodio/blob/master/src/source/channel_volume.rs#L36-L45)

I agree this does not sound correct. I am still kinda surprised it gives crackling issues. A f32 (channel volume works with f32) can represent integers up tot 24bit. So an i16 (current decoder type) sample doubled takes up a maximum of 17 bits. So the volume would be louder then intended but not crackling. It could be that the sample is converted back to an i16 if the output does not support taking floats but then the conversion should scale it back down.

As far as my ears can hear it that seems to fix it or at least come very close to fixing it.

Nice, though I am not fully sure how it helps :) We are probably changing the whole audio pipelines to f32 soon, that might help further. There is also work being done on ChannelRouter which might deprecate ChannelVolume if channel volume gives no clear perf advantage.

what's your thinking on the timelines for the next release of rodio?

Ill be honest that will take a while. The next release will be the first breaking in a long time, we try to put all the breaking changes in one release that way users only have to upgrade their code once. Though given the sheer number of changes I am starting to doubt that plan. In any case we do keep the main (master) branch pretty stable, you should be able to use that especially if you pin to a specific commit. Running a fork with cherry picks is probably your best bet.

Happy to understand how we can support at all on the development - we're really interested in the changes to the resampler (e.g. Rubato) and ChannelRouter.

For now the rubato effort is paused till we decide whether to go with f32's or not, because that change would massively simplify the work. I'm not sure about the rest.

Thanks for the support it would help if you could figure out what is going wrong regarding this issue. It might not be easy, you can ask me for help if you run into a wall.

[will3942]

I agree this does not sound correct. I am still kinda surprised it gives crackling issues. A f32 (channel volume works with f32) can represent integers up tot 24bit. So an i16 (current decoder type) sample doubled takes up a maximum of 17 bits. So the volume would be louder then intended but not crackling. It could be that the sample is converted back to an i16 if the output does not support taking floats but then the conversion should scale it back down.

From what I can see from ChannelVolume is that it will return samples of the same type that they are provided in and not f32.

Given that the mp3 decoder provides samples in i16 could this be the issue? If we are summing two i16 samples then we will be reaching the maximum value the resulting i16 can hold.

I'm still working through this to be sure but this seems to be the case.

[will3942]

Adding in some debug logs and after Mono conversion there are quite a few samples that do reach the bounds of maximum i16 so I think that is what is happening.

[will3942]

I'm certain this is the issue here, in Rodio ChannelVolume uses .saturating_add() on a Sample's type which is u16, i16 or f32 which will cause two samples that sum to > MAX_VALUE to clip at the MAX_VALUE bounds.

In comparison awedio's ChannelCountConverter (https://github.com/10buttons/awedio/blob/56c7ac3810d6593a823bba4cd28ae67bcf9852bb/src/sounds/wrappers/channel_count_converter.rs#L128) which operates on i16s (and only Stereo->Mono) first casts both samples to i32 and then averages them (sums together and divides by 2) which will prevent a clip.

Moving to f32 will help but I believe can still overflow/clip as we are not averaging but instead summing.

I would suggest we also adjust this algorithm to divide by the channel count to ensure we are averaging and we don't overflow.

If you're happy with this I will make a PR.

[dvdsk]

From what I can see from ChannelVolume is that it will return samples of the same type that they are provided in and not f32.

correct, however by using the new decoders pr the ChannelVolume got f32 as input.

first casts both samples to i32 and then averages them (sums together and divides by 2) which will prevent a clip.

f32 should have plenty headroom for the sum right?

I would suggest we also adjust this algorithm to divide by the channel count to ensure we are averaging and we don't overflow.

even if we do not overflow it still throws off the volume

If you're happy with this I will make a PR.

Yes please! :)

[PetrGlad]

The I think channel mixer from #653 does proper mixing.

[will3942]

PR here @dvdsk / @PetrGlad #681