ErikNatanael / knyst

Apache License 2.0
34 stars 4 forks source link

Using Mult node on stereo output shifts balance slightly to the right #25

Closed Tuurlijk closed 1 week ago

Tuurlijk commented 2 weeks ago

Hi,

Big thanks for writing knyst! I have just started exploring rust audio and came across this great lib.

I was getting to grips with connecting up graphs for a stereo file. I used a sound test file to sort out the correct parameters for the nodes. I want to be able to set the volume for the wav file. I think the way to do that with knyst is to use a Mult node.

When using one mult node per channel on a stereo wav, the balance is shifted to the right ear. You can hear this clearly in this sample application: https://github.com/Tuurlijk/knyst-croptesting

Am I not connecting the graph correctly? Or mising something else? Without Mult, the file plays correctly.

I also tried with (chatgpt-fixed-up) untable code for the inner workings of Mult, same result.

Tuurlijk commented 2 weeks ago

Seems like the right channel is delayed 64 samples or 1 block_size.

ErikNatanael commented 2 weeks ago

Thank you for your bug report, I can confirm the behaviour. At a glance, I can't tell why that doesn't work, but using the new Handles API instead of the old Connections API seems to work correctly, can you confirm?

    k.init_local_graph(settings);
    let playback_node_id = buffer_reader_multi(buffer, 1.0, false, StopAction::FreeSelf);

    if args.volume == 1.0 {
        println!("Outputting raw file");
        graph_output(0, playback_node_id);
    } else {
        println!(
            "Outputting through `Mult` with multiplier of {:?}",
            args.volume
        );
        graph_output(0, playback_node_id * args.volume);
    }

    let note_graph_id = k.upload_local_graph().unwrap();
    graph_output(0, note_graph_id);
Tuurlijk commented 2 weeks ago

Hi, unfortunately I can not confirm. This code drops the right channel of the sample file. So I'm only left with left and center.

ErikNatanael commented 2 weeks ago

You're right, there was a bug in setting the number of channels in buffer_reader_multi, I just pushed a fix to the main branch.

ErikNatanael commented 2 weeks ago

https://github.com/ErikNatanael/knyst-croptesting

Tuurlijk commented 2 weeks ago

Great, that fixes the issue in case one is using the Handles API. The issue is not fixed for the other graph method.

Is there some more documentation on using Handles? I saw examples on using Graph::new() that worked for me. I use a lot of envelopes for frequencies and volumes. I don't know how to implement that using the new Handles API.

I'll take a look around the code to see if I can spot a similar issue as you fixed in the buffer_reader_multi.

I wrote a wav file Gen that dumps a wav file for inspection. Then I overlay the channels in https://www.sonicvisualiser.org/ to see if generated tones look like they are supposed to. I'll also look into writing some tests to automate the checks.

ErikNatanael commented 2 weeks ago

Thanks! I don't have time to investigate the original issue right now, but let me know if you manage to narrow it down further. I suspect it is something entirely different.

Have a look at this example: https://github.com/ErikNatanael/knyst/blob/main/knyst/examples/more_advanced_example.rs

I should really update all of the examples to use the Handles API

For tests, KnystOffline allows you to get the output buffers.

Tuurlijk commented 1 week ago

The left channel is delayed by buffer_size. When using KnystOffline setting the buffer size to sample rate, the left channel is delayed by 1 second. In this example, both sample_rate and buffer_size are 48000. The right channel is OK.

image

Tuurlijk commented 1 week ago

Ok, I updated my croptesting repo with:

The sample rate and blocksize are both set to 48000, so the shifting is clearly observable. The effect occurs both on a regular Oscilator and a Buffer Reader. So the problem seems to be with the way I use Mult?

I don't get how adding two separate Mult instances for each channel should f* things up.

If I use the Multiplier gen the files come out OK. So that's what I use in my project for now.

At least now there i a very clear and reproducible effect to investigate further.

ErikNatanael commented 1 week ago

I've narrowed it down to a bug in calculating the order of node evaluation for multi output nodes, working on a fix.

ErikNatanael commented 1 week ago

Fixed by 40131cfd56cb653cbb8ac3f7616bfc9fc502e8db