lrbalt / libsoxr-rs Goto Github PK

Hi! Thanks for your work.

I have the following example and I am obtaining the error on the title. Could you help me finding out what's going wrong? It works pretty well with Float32I but when I change to Float32S it crashes. I already tried changing to [[f32;48];2] and [[0.0; 96]; 2] for both source and target respectively, to no avail...

        let io_spec = IOSpec::new(Datatype::Float32S, Datatype::Float32S);
        // upscale factor 2, one channel with all the defaults
        let soxr = Soxr::create(1.0, 2.0, 2, Some(&io_spec), None, None).unwrap();

        // source data, taken from 1-single-block.c of libsoxr examples.
        let source: [f32; 96] = [
            0.0, 1.0, 0.0, -1.0, 0.0, 1.0, 0.0, -1.0, 0.0, 1.0, 0.0, -1.0, 0.0, 1.0, 0.0, -1.0,
            0.0, 1.0, 0.0, -1.0, 0.0, 1.0, 0.0, -1.0, 0.0, 1.0, 0.0, -1.0, 0.0, 1.0, 0.0, -1.0,
            0.0, 1.0, 0.0, -1.0, 0.0, 1.0, 0.0, -1.0, 0.0, 1.0, 0.0, -1.0, 0.0, 1.0, 0.0, -1.0,
            0.0, 1.0, 0.0, -1.0, 0.0, 1.0, 0.0, -1.0, 0.0, 1.0, 0.0, -1.0, 0.0, 1.0, 0.0, -1.0,
            0.0, 1.0, 0.0, -1.0, 0.0, 1.0, 0.0, -1.0, 0.0, 1.0, 0.0, -1.0, 0.0, 1.0, 0.0, -1.0,
            0.0, 1.0, 0.0, -1.0, 0.0, 1.0, 0.0, -1.0, 0.0, 1.0, 0.0, -1.0, 0.0, 1.0, 0.0, -1.0,
        ];

        // create room for 2*48 = 96 samples
        let mut target: [f32; 192] = [0.0; 192];

        // Two runs. First run will convert the source data into target.
        // Last run with None is to inform resampler of end-of-input so it can clean up
        soxr.process(Some(&source), &mut target).unwrap();
        soxr.process::<f32, _>(None, &mut target[0..]).unwrap();

        // just print the values in target
        println!("{:?}", target);
        println!("{:?}", target.len());

Soxr::process is a wrapper around soxr_process. It passes the lengths of the input and output buffers as a sample count, but what soxr_process actually expects (and returns) is sample-for-each-channel count. If the channel count is greater than one, soxr_process believes both buffers are longer than they actually are, quickly resulting in a crash.

To fix this, buf.len() and buf_out.len() must be divided by channel count before passing to soxr_process. In order to behave as the documentation for Soxr::process specifies, the results must also be multiplied by channel count.

I have confirmed in my own testing that, if I artificially divide the lengths of the passed slices by the channel count and multiply the returned values by the channel count, behavior is otherwise correct.

Note that I'm a beginner to Rust so I may be wrong.

The Soxr::process function doesn't require that arguments contain references to slices containing the same data type that was specified in IOSpec::new. It means that libsoxr may accidentally read or write beyond allocated memory if it was created with IOSpec specifying 64-bit float and a 32-bit float slice is given as an argument.

Also, it doesn't support split (planar) channels while original libsoxr does. The problem is that it gets input and output lengths for libsoxr from input & output slices lengths. It won't work if the arguments contain references to slices of references to slices (similarly to libsoxr API which excepts array of arrays) because the outer slice's length is in such case the number of channels, not number of samples.

Both issues could be solved (if I think correctly) by making the whole Soxr type generic over input and output types and removing IOSpec argument from the constructor.

I won't be able to propose a pull request myself because I don't know Rust well enough and implementing it requires fluency in generic programming.

I'm a beginner to Rust so I may be wrong.

According to The Rustonomicon,

Something can safely be Send unless it shares mutable state with something else without enforcing exclusive access to it.

so adding that line to soxr.rs should suffice:

unsafe impl Send for Soxr {}

When the Soxr::new constructor is given a None option for the quality_spec argument, the process code finishes as expected.

When specifying a custom QualitySpec like:

Some(QualitySpec::new(
  &QualityRecipe::VeryHigh,
  QualityFlags::HI_PREC_CLOCK,
))

The process method fails with "null pointer" error. As soon as the quality_spec is changed back to None, this error goes away.

I tried with other quality recipes and flags (including 0) with the same result.

Thank you for the awesome library!