Skip to content
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

ChannelVolume misbehaving with non power of 2 number of channels #484

Open
DivineGod opened this issue Mar 5, 2023 · 14 comments
Open

ChannelVolume misbehaving with non power of 2 number of channels #484

DivineGod opened this issue Mar 5, 2023 · 14 comments
Labels

Comments

@DivineGod
Copy link
Contributor

DivineGod commented Mar 5, 2023

I seem to have stumbled into a strange issue where adding more output channels to a ChannelVolume will make it mess up the output.

My setup is that I have a custom source which is basically an extended version of Spatial with a self.input which is a ChannelVolume if I create an audio interface with 2 or 4 channels and configure the ChannelVolume to have 2 or 4 channels the output seems fine. But if I add more than 4 the output which was meant to go to channel 1 goes to channel 5 and channel 2 goes to channel 6.

Shown here is the loopback interface where the volume is shown for the 6 channel version:
Screenshot 2023-03-06 at 10 50 43

Here is the version with 4 channels which shows the correct behaviour:
Screenshot 2023-03-06 at 10 51 39

The only thing changed is the number of channels in the output configuration which impacts ChannelVolume getting either 4 or 6 channels. I have confirmed that the set_volume call does get called with the correct index and volume levels as per my spatial algorithm in both cases.

Edited to add:

Upon further inspection it seems that any number of channels that are not powers of two say 6 or 10 will exhibit this behaviour while power of two number of channels look like they behave perfectly okay

@DivineGod DivineGod changed the title ChannelVolume misbehaving with more than 4 channels ChannelVolume misbehaving with non power of 2 number of channels Mar 6, 2023
@DivineGod
Copy link
Contributor Author

Still trying to make a reduce repro case. Currently my code seem to have the wrong data buffer after dynamic mixer_rx iterator is run.

I tried copying my setup in a test case in the dynamic mixer with a ChannelVolume with 6 channels and a SampleBuffer source with a few samples but it did not convert into the expected error. So I’ll keep digging into this.

@DivineGod

This comment was marked as outdated.

@DivineGod
Copy link
Contributor Author

The above didn't seem to give me anything on an input source that takes from a device, but from a file there might be something even worse going on. I've got crackling and weirdness going on with an ogg file using rodio::Decoder into my dbap sink (spatial sink with more than 2 channels basically) which has a channel volume inside

@DivineGod
Copy link
Contributor Author

The above didn't seem to give me anything on an input source that takes from a device, but from a file there might be something even worse going on. I've got crackling and weirdness going on with an ogg file using rodio::Decoder into my dbap sink (spatial sink with more than 2 channels basically) which has a channel volume inside

Crackling due to codec being in 44100Hz and output in 48000Hz. Really not useful. Hopefully it makes sense to someone and there's a way to fix this?

@DivineGod
Copy link
Contributor Author

I managed to recreate the issue by modifying one of the examples:

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/music.ogg").unwrap();
    let input = rodio::Decoder::new(BufReader::new(file)).unwrap();
    let chan_vol = ChannelVolume::new(input, vec![0.5, 0.5, 0.0, 0.0, 0.0, 0.0]);
    sink.append(chan_vol);

    sink.sleep_until_end();
}

I've used Loopback to create a 6 channel output device and macOS Audio MIDI Setup utility to make sure the sample rate is correct and that the device is selected as the default output device. This is done so I didn't have to mess with selecting the right config in the code.

Attached screenshot of the Loopback interface showing output on channels 5 and 6 even though as seen by the code it's meant to be channels 1 and 2

Screenshot 2023-04-13 at 16 31 16

@DivineGod
Copy link
Contributor Author

I tried using a SineWave source in my example and had the output device set to 44.1kHz to match the music.ogg source file, as it seemed that resampling didn't work.

Using the SineWave I can confirm that mapping to different sampling rates breaks things in more ways when there's a ChannelVolume in the mix, but if there isn't then it seems to be fine. This is still with 6 channels as opposed to say 2 or 4 which seem to work fine.

@DivineGod
Copy link
Contributor Author

Instead of using a sink to control play I tried with handle.play_raw() and it seems to behave as I would expect, no issues with channels getting mixed up.

@DivineGod
Copy link
Contributor Author

Looking at what is being output from the mixer_rx in new_output_stream_with_format in src/stream.rs the data buffer is initially being filled with data where the first 12 samples are the same value followed by 24 samples of 0.0 then repeated until the end of the buffer at 3072 samples (512 * 6)

The next time through, the data ends up with being 4 samples of 0.0 then 2 samples of data, which is then repeated until the end of the buffer.

First off it seems weird that the data is 12 + 24 samples the first time through the mixer and secondly that it then sort of corrects itself but not quite after that. I do notice the first bit of weird data as an odd click and can see the channels activate in loopback.

@DivineGod
Copy link
Contributor Author

ChannelCountConverter is wrong for the first iteration is most likely what's happening with that.

@DivineGod
Copy link
Contributor Author

I think I've got a fix. It's mostly to do with Empty having 1 channel it messes up everything. It should have 0 channels, because it doesn't actually do anything and then there should be special cases handling such a case, because in that case you want to have the output channel number used.

@DivineGod
Copy link
Contributor Author

I spoke too soon. The fixes worked for the case of SineWave but an ogg file is still messed up and now the channels are 3+4 instead of 5+6. An only if the sample_rate is correct for the output and input.

@DivineGod
Copy link
Contributor Author

I've identified a number of issue stemming from the use of the Emtpy source when initially populating the queue in Sink it would cause the various conversions to fail so I've implemented some fixes that seem to fix that.

@DivineGod
Copy link
Contributor Author

I've marked #493 as ready for review. It deals with the immediate issue of channels mapping wrongly. The Sample Rate conversion issue I'll investigate in a separate PR

@dvdsk dvdsk added the bug label Mar 25, 2024
@PetrGlad
Copy link
Collaborator

PetrGlad commented Dec 1, 2024

@DivineGod Regarding cracking there is a bug filed regarding SampleRateConverter #208. The output stream by default tries to open output device at 44.1kHz. Is your input and output in 48kHz? Or is it an arbitrary choice? The rodio would need some revamping to avoid conversions internally, otherwise a better resampler may help avoiding cracking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants