Possible bug in down sample function in reverb? about sndfilter HOT 5 CLOSED

I haven't looked at this in a while, but that does look wrong.

Here's the code in Freeverb3 that I was attempting to clean up (I think):

http://git.savannah.gnu.org/cgit/freeverb3.git/tree/freeverb/src.cpp#n159

void FV3_(src)::src_d_iir2(fv3_float_t *input, fv3_float_t *output, long factor, long numsamples, FV3_(biquad) * iir)
{
  for(long i = 0;i < numsamples*factor;i ++) input[i] = iir->process(input[i]);
  for(long i = 0;i < numsamples;i ++) output[i] = input[factor*i];
}

They're processing in blocks, whereas I'm processing one sample at a time. So you can think of it as hard-coding numsamples to 1.

Original code in sndfilter:

// input length must be oversample->factor
static inline float oversample_stepdown(sf_rv_oversample_st *oversample, float *input){
  if (oversample->factor == 1)
    return input[0];
  for (int i = 0; i < oversample->factor; i++)
    biquad_step(&oversample->lpfD, input[i]);
  return input[0];
}

I think what I'm missing is saving the return value of biquad_step:

  for (int i = 0; i < oversample->factor; i++)
    input[i] = biquad_step(&oversample->lpfD, input[i]);
  return input[0];

Now, I'd rather not stomp the input array, so instead I should do something like:

// input length must be oversample->factor
static inline float oversample_stepdown(sf_rv_oversample_st *oversample, float *input){
  if (oversample->factor == 1)
    return input[0];
  float out = biquad_step(&oversample->lpfD, input[0]);
  for (int i = 1; i < oversample->factor; i++)
    biquad_step(&oversample->lpfD, input[i]);
  return out;
}

That seems like it matches the spirit of Freeverb3. What do you think?

from sndfilter.

Well that certainly looks like what the code is doing.

from sndfilter.

Also the code does not flush denormal numbers to zero. Do you configure the FCU to do this?

from sndfilter.

no, I debated whether to do optimizations like that, and decided against it to aid in readability... I think there are a lot of optimizations that could be done on the code (ex: iirc, Apple provides hardware instructions for doing biquad math on a buffer, which Chromium uses but I don't), but wanted to try and remain as simple as possible, so it was more accessible

I'm about to push up the fix

from sndfilter.

I just added a note to the readme to make it clear I favored simple over fast.

Thanks for bringing up these issues!

from sndfilter.