I wrote this template class for creating lock-free FIFOs of arbitrary type, based on AbstractFifo
:
template <typename T, size_t capacity>
struct GenericFifo
{
bool push (T element)
{
auto write = fifo.write (1);
if (write.blockSize1 > 0)
{
buffer[write.startIndex1] = element;
return true;
}
else if (write.blockSize2 > 0)
{
buffer[write.startIndex2] = element;
return true;
}
else
return false;
}
bool pull (T& element)
{
auto read = fifo.read (1);
if (read.blockSize1 > 0)
{
element = buffer[read.startIndex1];
return true;
}
else if (read.blockSize2 > 0)
{
element = buffer[read.startIndex2];
return true;
}
else
return false;
}
juce::AbstractFifo fifo { capacity };
T buffer[capacity];
};
I’m using it in a plugin project to push MidiMessage
s to the MidiBuffer
in AudioProcessor::processBlock
when a Button
is clicked in the GUI.
So in my AudioProcessor
, I have:
GenericFifo<juce::MidiMessage, 64> midiInputQueue;
And in AudioProcessor::processBlock
:
juce::MidiMessage m;
while (midiInputQueue.pull (m))
midiMessages.addEvent (m, 0);
And in the Button
's onClick
lambda in my AudioProcessorEditor
, I have:
while (! processor->midiInputQueue.push (midiMessage));
Now, when I click the Button
, sometimes it works. Other times, I get an EXC_BAD_ACCESS
in AbstractFifo::finishedWrite
. The fact that it only fails sometimes makes me think there is some kind of data race going on?
When I debug at the point of failure, it says oddly that midiInputQueue.fifo.bufferSize
is 0
. But placing a breakpoint in the AudioProcessor
constructor shows that it was indeed initialized to 64
.
Does anyone have an idea what’s going on here? I’m not sure how else to debug this…
Not sure the point of using the abstract fifo here you may be better off with just a vector push & pop or something?
in your push method you’re returning before you ever call finishedWrite so your circular buffer will never move forward. likewise in your pull method.
Not sure on the crash though…
Right, my bad! I updated the initial post to use the RAII classes in AbstractFifo
, which should automatically call finishedRead
/finishedWrite
when they go out of scope. Still getting the same error though…
And I decided to use AbstractFifo
because I needed to update the queue indices atomically + not lock the audio thread (using an std::vector
would cause allocation/deallocation with each push/pop, no?). Am I missing something?
Ahh true yeah that’s a good point!
& nice I hadn’t seen those new methods yet!
That is really funky. I’m not super experienced on templated classes but sounds like things are not constructing how they seem 0.o maybe those initializers are not guaranteed to construct the class members like that properly?
Shouldn’t you use MidiMessageCollector
for that?
https://docs.juce.com/master/classMidiMessageCollector.html
Also, you should use std::array<T, capacity> buffer
;
Shouldn’t you use MidiMessageCollector
for that?
Ah, yeah… that sounds like the reasonable thing to do I guess I was hoping for something lock-free, and a little more lightweight, since I don’t need the MidiKeyboardState
or MidiInput
functionality. But I’m just being picky
Also, you should use std::array<T, capacity> buffer;
I’m curious, why?
it’s a cost-free container that is safer to use than a raw array.
Shouldn’t you use MidiMessageCollector
for that?
So I tried this — in AudioProcessor
I added a public member:
juce::MidiMessageCollector midiMessageCollector
Then in AudioProcessor::prepareToPlay
I call:
midiMessageCollector.reset(sampleRate);
And in AudioProcessor::processBlock
I call:
midiMessageCollector.removeNextBlockOfMessages(midiMessages, buffer.getNumSamples());
And lastly in my Button
’s onClick
lambda I call:
processor->midiMessageCollector.addMessageToQueue (midiMessage);
BUT… I get an assertion failure whenever I click the Button
, at juce_MidiMessageCollector.cpp:54
jassert (hasCalledReset); // you need to call reset() to set the correct sample rate before using this object
… even though reset
was already called in prepareToPlay
… this is very strange…
Got it fixed! Credit goes to @adamski for identifying the problem:
I was declaring my Button::onClick
lambdas using capture by reference:
button.onClick = [&] { ... };
When I called on processor
(which shadows this->processor
) in the lambda, it captured a reference to this
, which only stays alive for the scope of the function (this
has the same value but a different memory address in different member functions within an object).
I needed to capture this
by value instead:
button.onClick = [this] { ... };
2 Likes
You should capture this
with a Component::SafePointer<>
, in case your lambda is called in the odd instance where your editor window has closed. it’s a common issue with asynchronous programming. you must be certain your captured variables are still valid when the lambda is invoked before using them.
4 Likes