It appears that the JUCE Convolution class cannot actually be used for real-time audio processing. Looking at the code:
juce_Convolution.h: process calls processSamples on line 79
juce_Convolution.cpp: processSamples calls pimpl->processSamples on line 1219,
which calls processFifo() on line 724,
which calls loadImpulseResponse() on line 682,
which calls std::make_unique<FileInputStream> (currentInfo.fileImpulseResponse)); on line 810,
which first allocates heap memory and then performs file I/O, both on the process thread.
OK, thanks for letting us know @t0m ! While you’re at it, here are two more bugs in the Convolution engine:
if you give it an invalid impulse response file, and then call process, it multiplies the input with 0.125 for some reason (I would have expected that instead, it bypasses the input unchanged). This happens regardless of whether or not normalisation is enabled.
if you give it an impulse response that consist of all zeros, it output NaNs (I would have expected zeroes). Did you forget to check for zero before division in your normalisation?
And here is another annoying thing for which there is perhaps a quick fix?
copyAndLoadImpulseResponseFromBlock and copyAndLoadImpulseResponseFromBuffer take non-const blocks/buffers. But I don’t think they have to modify them? Can we make these const blocks/buffers? Otherwise my code doesn’t compile
Another request for a change, but I do not know how it will work with thread-safety…
IIRC, the current implementation allocates 60 seconds worth of space for loading impulse responses. This is OK for stereo use but when working with multichannel (Ambisonics, WFS, etc.) you might need 64 instances of the class. This becomes incredibly memory hungry! I hacked together a version where the max IR length is set in the constructor for cases when you know you won’t be loading a 10 second IR.
Is this something you could consider addressing? Thanks!
The issues mentioned in this thread should now be resolved on the juce6 branch. Note that copyAndLoadImpulseResponseFromBlock/Buffer functions have been removed and replaced with a single function taking an AudioBuffer<float>&&. This allows the implementation to take ownership of the passed IR, thereby avoiding the need to preallocate the large internal buffer.
Previously, the threading contract of the Convolution wasn’t formally specified, which made it very difficult to make the class threadsafe (obviously “any function may be called simultaneously with any other” is not particularly viable, especially in realtime contexts). In the new version, we’ve explicitly specified that the methods on the Convolution may not be interleaved. To load a new impulse response during processing, the call to loadImpulseResponse must by explicitly synchronised with the call to process.
