Spatial_Audio_Framework icon indicating copy to clipboard operation
Spatial_Audio_Framework copied to clipboard

ISM RIR: Incorrect summing of bands?

Open mtmccrea opened this issue 2 years ago • 1 comments

Description

I may be mistaken, but it looks like the octave band splitting may not be writing the filtered signals out, but rather just copying the input to the output.

In particular, the summing stage of ims_shoebox_renderRIR() sums the input channels for a given band wrk->rir_bands[band][i] into the output buffer &(rir->data[i*(wrk->rir_len_samples)]

        /* Sum */
        for(i=0; i<echogram_abs->nChannels; i++)
            cblas_saxpy(wrk->rir_len_samples, 1.0f, wrk->rir_bands[band][i], 1, &(rir->data[i*(wrk->rir_len_samples)]), 1);

The previous block shows the filtered band channels being stored in the temp variable. I would assume it's this filtered signal, temp that should be summed back into the output.

This may not have been caught because it looks like the ambiRoomSim plugin only uses broadband absorption, and I couldn't find anywhere that ims_shoebox_renderRIRs() was actually called.

Here's the larger block for context https://github.com/leomccormack/Spatial_Audio_Framework/blob/82e3b75a9e4b9deb098e01416b1e91f6feeafb7f/framework/modules/saf_reverb/saf_reverb_internal.c#L685C3-L709

Thanks!

mtmccrea avatar Aug 07 '23 13:08 mtmccrea

Ah yes, this is indeed an incomplete implementation. However, I don't see myself getting round to implementing it in the near future. Therefore, if anyone reading this would like to take this on, please do so and submit a PR :)

leomccormack avatar Nov 21 '23 14:11 leomccormack