Artifacts on IR update of dsp::Convolution in VST - JUCE 5.3.2 OSX 10.12

Hi,
Also, if you keep your sliders and values non logarithmic, for example keep the frequency slider in octaves rather than frequency (but displayed as frequency via the lambda) then you can still process the automation linearly in blocks (a fade out of one frequency value plus a fade in of the next, all times the audio) rather than using a one sample at a time logarithmic cross fade. After that, if you need, make all of the newly derived values logarithmic with block functions.
Believe it or not, this should be less CPU heavy, that is until you decide it needs a further anti-alias smoothing filter. The filtering can be a bonus offline bounce feature.
Ivan, I am not well versed on non-FIR type filters. Would you suggest optional offline bouncing upsampling with filterHalfBandPolyphaseIIR as your ultimate solution when sharp modulation is not desired?
What do you think of chkn’s solution?

chkn suggests-
May 2015
I usually also do also some low pass filtering to the smoothed parameter, to remove sharp edges
coeff = (float)(exp(log(0.01f)/( millisecondsToReach99percent * sampleRate * 0.001)));
and then per sample
float processLP(float inVal)
{
smoothed = coeff * (smoothed - inVal) + inVal;
return smoothed;
}

Hello there ! Check what I’m working on right now :

I’m going to create a topic and a blog post about this work soon :slight_smile:

3 Likes

fantastic

Hi Ivan,

Ran into another interesting issue on that front (pulled latest JUCE commits from the git a minute ago).

Increasing the smoothing time in juce_Convolution.cpp, or reducing the timerUpdateIntervalInMs in the PluginProcessor.h of the debug project (to e.g. 1 ms) will result in an overflow of the fifo stack (l. 489), which size is hardcoded in juce_Convolution.cpp to 256.

Thanks for the report. I’ll do a few updates + fixes on the DSP module after the ADC18, and I’ve just added this issue in my list :wink:

Will this also “fix” the use of unsigned types for channel and samples indexes?

Evening Ivan,

Would you perchance have an idea for a quick fix on that 256 fifo size limitation (other than increasing the hardcoded value)? It’ll sure spoil my happy digging, still, if ever you have a one-liner to drop the oldest fifo requests to make room for new ones and avoid the overflow, I’m taking it. Screw the fun :).

Oops sorry about the delay, I’ll have a look next week, and I’ll implement my LogSmoothedValue class there as well

Hello ! I’m currently working on the Convolution class fixes. Here is what I’m doing:

  • I have created a class called LogSmoothedValue. It works like LinearSmoothedValue, but with a different kind of ramp (spoiler: log instead of linear), and this is in practice very useful to handle IR changes in the Convolution class without changing the length of the ramp.
  • I don’t want to drop oldest FIFO requests to make room for new ones. That’s not how a FIFO and the whole class should work. If the FIFO is full, and you are still asking new changes, then that’s the thing which is wrong. So I have increased the FIFO size, and the code will fire an assertion when overflow is happening. Updating every 1 ms the IR in a class which can handle IR changes slowly by very definition is wrong whatever the viewpoint.
  • I won’t let the user modify the interpolation time, because in my opinion it won’t be useful with the new LogSmoothValue class handling the IR changes. And this way I prevent related FIFO size issues at the same time.
  • Some people suggested that the class shouldn’t allocate memory during run-time / processing, and I’ve seen issues with that behaviour specifically in Pro-Tools recently, so now the first time the FIFO will be processed won’t be during the first processSamples call, but during the prepare function first call.
1 Like

I’ve sent a big update to the JUCE guys today, I might create a topic about that upcoming LogSmoothedValue class at some point :slight_smile:

Thanks @IvanC, looking forward to that LogSmoothedValue class, with you programming it I expect it to be quite performant. Did you use look-up tables or any other efficient way to calculate the log()s?

You only need a multiplication *request* dsp::Oscillator should use a log frequency fade

The approach makes sense. The only issue I see is with the:

Where there’s no safety net for e.g. end users with slow machines: the plugin will crash, period. Devs will end up relying on a throttle mechanism. I would suggest (I love these “suggestions” when I’m not actually concerned with the implementation) to have this throttle mechanism implemented in the class. It’d be disabled by default with a comment on the “crashing line” that says why it crashed and that this mechanism can be enabled, though at the dev’s own risk (socket like packet drop warning).

Right, I even proposed that in another thread regarding ADSR :see_no_evil: Multiplication instead of addition.
Similar to logarithmic sweeps, one could argue to call it ExponentiallySmoothedValue (as exponential sweeps), but that would become a quite philosophical discussion :wink:

The LogSmoothedValue class uses multiplications instead of additions :wink:

@davidpq : if the FIFO is full, the plug-in won’t crash, it’s just that the Convolution class won’t add any new request. That’s the theory :slight_smile: Are you telling me that you had crashes during your tests ?

It does (again using the debug project with a small timer update value), although now I feel I missed something obvious everyone knows :). Here it is anyway:

When the processFifo() method in juce_Convolution.cpp calls requestsType.setUnchecked (numRequests, type) in its while loop with numRequests = 256, it fires the jassert (isPositiveAndBelow (indexToChange, values.size())); in juce_Array.h (l.524). Juce version 5.4.1.

1 Like

Ok I think I’ve found the issue. At line 490, it should be while (getNumRemainingEntries() > 0 && numRequests < fifoSize) instead of while (getNumRemainingEntries() > 0).

Thanks for spotted it !

Hi @IvanC,

I submitted a Pull Request to expose the IR switch ramp duration. I find it convenient for VST users to define a plugin “Impulse Response update rate” to fit their current needs (zipper noise free, highly responsive, etc.). Let me know if it doesn’t make sense with what you had in mind for the Convolution class implementation.

1 Like

This would be really useful for me as well. It’d be great if it could be integrated into the main JUCE repo. Thanks for the work on this, @davidpq.

I think that would be a great addition, and your code seems good to me !

1 Like