karlstav / cava

Cross-platform Audio Visualizer
MIT License
3.92k stars 225 forks source link

Waves don't work properly in raw mode. #573

Closed rs-pro0 closed 2 weeks ago

rs-pro0 commented 2 weeks ago

A clear and concise description of what the bug is.

To Reproduce Steps to reproduce the behavior:

  1. Enable raw mode.
  2. Set binary data_format.
  3. Enable monstercat smoothing with waves.
  4. Waves do not look as they should(difference between bars that are in the same wave is too small that they are almost identical)

Expected behavior No matter the max height waves should look properly.

Screenshot Expected behavior: image Actual behavior: image

Desktop:

Terminal emulator Not using one.

Additional context To test this I'm using my Rust app that draws cava to the screen, here is code that reads the input if necessary.

 let mut cmd = Command::new("cava");
    cmd.arg("-p").arg("/dev/stdin");
    let cava_process = cmd
        .stdout(Stdio::piped())
        .stdin(Stdio::piped())
        .spawn()
        .expect("failed to spawn cava process");
    let mut cava_stdin = cava_process.stdin.unwrap();
    cava_stdin.write_all(string_cava_config.as_bytes()).unwrap();
    drop(cava_stdin);
    let cava_stdout = cava_process.stdout.unwrap();
    let cava_reader = BufReader::new(cava_stdout);
    // Next part is in a loop
        let mut cava_buffer: Vec<u8> = vec![0; bar_count as usize * 2];
        let mut unpacked_data: Vec<f32> = vec![0.0; bar_count as usize];
        cava_reader.read_exact(&mut cava_buffer).unwrap();
        for (unpacked_data_index, i) in (0..cava_buffer.len()).step_by(2).enumerate() {
            let num = u16::from_le_bytes([cava_buffer[i], cava_buffer[i + 1]]);
            unpacked_data[unpacked_data_index] = (num as f32) / 65530.0;
        }

Also the same can be achieved if you use ascii data_format and set ascii_max_range to 65530(probably any large number).

karlstav commented 2 weeks ago

I can't see how those filters should relate to the output mode. Are you sure it looks fine with the waves filter turned off?

rs-pro0 commented 2 weeks ago

Yes, it is because the max value is much bigger when using raw mode(65530), this also explains why it works as expected when using ascii data format with ascii_max_range set to 1000.

karlstav commented 2 weeks ago

try disabling waves, does it look ok then?

rs-pro0 commented 2 weeks ago

Yes, just using monstercat filter works the same way in raw mode and regular one.

karlstav commented 2 weeks ago

Doesn't make any sense, I tested it with ascii output (65530 max) and the numbers looked the same with and without waves enabled

rs-pro0 commented 2 weeks ago

You need to have monstercat enabled for waves to work afaik.

karlstav commented 2 weeks ago

I have that

rs-pro0 commented 2 weeks ago

But if they look the same then waves do nothing, right?

rs-pro0 commented 2 weeks ago

If you look at lines 184 and 188 in cava.c you can see that it subtracts a value that isn't affected by height. So it will subtract the same value for max 1000 and max 65530.

karlstav commented 2 weeks ago

oh right now I'm following, sorry. Was able to reproduce sort of similar failure using the sdl_glsl output as it is only 0-1 at the time of applying this filters.

hmm i did not write the monstercat or the waves filter, but I can look into it.

karlstav commented 2 weeks ago

it looks to be based around the assumption that the height and the number of bars are in the same ballpark type numbers. I tried normalizing the values, does it look better now?

rs-pro0 commented 2 weeks ago

With bigger height it just looks the same way if waves were disabled.

karlstav commented 2 weeks ago

i am not following, so now it doesn't look as if waves are enabled at all?

rs-pro0 commented 2 weeks ago

Yes, because value is too big and max function selects another value. For me this works: https://github.com/rs-pro0/cava/commit/23e7e0aee48195f5c16b9764d36df9b227146d11

rs-pro0 commented 2 weeks ago

I think you don't need to put the height normalizer in pow because then it is also squared which is not needed.

rs-pro0 commented 2 weeks ago

Does that look good to you?

karlstav commented 2 weeks ago

yes, that makes sense, i will make another commit

rs-pro0 commented 2 weeks ago

Thank you.