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
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
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
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’ve sent a big update to the JUCE guys today, I might create a topic about that upcoming LogSmoothedValue class at some point
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?
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 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
The LogSmoothedValue class uses multiplications instead of additions
@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 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.
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.
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 !
Hi there,
I just wanted to ask, if there still is no viable solution for this convolution IR update artifacts problem yet?
My code uses a lot of IR updates and I recognized this “zipper noise” as well.
I also could not find a LogSmoothedValue class in JUCE 7 or an inclusion of the Pull Request from @davidpq.
I hope to hear from you!
Best regard
Marco