Custom Listener and thread safety

General JUCE discussion
1

Hi, I’m a new entry in the JUCE world and audio programming in general and I encountered a problem in one of my first projects.
I’m porting a rather simple onset detector, from another framework and I’m developing a demo application to understand interaction with JUCE.
The AudioProcessor of my demo app owns a OnsetDetector object, initializes it an provides audio blocks that will be buffered and analysed.
When the object detects the beginning of a note (onset) it has to communicate this event in an async fashion. For this I implemented a OnsetDetector::Listener class similarly to Button::Listener which contains a pure virtual callback function:

virtual void onsetDetected (OnsetDetector *) = 0;

OnsetDetector keeps a ListenerList (same as Button) and, when the onset is detected, it uses the ListenerList::call method:

listeners.call([this] (Listener& l) { l.onsetDetected (this); });

(Button uses callChecked instead, however I still have to understand what a BailoutChecker is, maybe I’m spinning too many plates at this point)

Now, for my demo the OnsetDetector Object is owned by the AudioProcessor and I would like to change color of a Component in the Editor when the onset is detected. I’ve read about AbstractFifo and updating regularly the Editor to pull parameter values from the Processor but it seems overkill for my purpose right now. It also seems inappropriate for an async event like this one.
I tried using only the listener in 2 progressive steps:

  • Implementing the onsetDetected() method and registering the listener in the Processor works fine but I obviously cannot access editor elements. Logging shows that it works.
  • Leaving the OnsetDetector public in the Processor, implementing onsetDetected() in the Editor and registering the Editor as listener to the detector only works partially:

The callback method in the editor receives the call, it lights up a virual light bulb (change color of a Rectangle) for 100ms and than it switches it off (I call repaint twice: on turn on phase and turnoff). It works but it keeps violating an assertion in juce_Component.cpp(r:1839):

JUCE_ASSERT_MESSAGE_MANAGER_IS_LOCKED

Explained 2 lines before as: If component methods are being called from threads other than the message thread, you’ll need to use a MessageManagerLock object to make sure it’s thread-safe.

  1. Does this mean that the callback is called by the audio thread? (if so, this is unacceptable right?)
  2. Is there a chance of making it work correctly (maybe with a MessageManagerLock) or am I going for a bad paradigm by using listeners like this?
  3. Why does the “lightbulb” still work, even if the app is failing what seems like a crucial assertion?

I hope I haven’t bored you to death, this is as concise as I can get since the introduction to the environment feels like relevant context.
I’m hoping that you can answer so this bit can be less of vodoo programming for me, thank you.

2
  1. Yes, the callback is being called on the audio thread currently, making the callback a bad place to update anything GUI related.
  2. Updating the GUI directly from the audio thread won’t work. At a minimum, you’d have to take some kind of lock in the callback (possibly the MessageManagerLock) which would lead to priority inversion, possibly causing audio dropouts.
  3. If you invoke undefined behaviour via data races in GUI components, stuff might appear to work perfectly, or it might immediately segfault or otherwise fail spectacularly. Whatever the observed behaviour of the program, it is still intrinsically broken (maybe it ran fine this time, but might not next time).

You could take a look at the AudioProcessorValueTreeState attachment classes to see how they deal with getting parameter updates onto the GUI thread.

In this very simple case, you could also consider just sticking a std::atomic<bool> thingIsActive; member on your processor, updating it in your DSP callback, and then polling it using a juce::Timer to check whether you should redraw the ‘bulb’ component.

2 Likes
3

The goal is to visualise a short 100ms light on the GUI, right?
I’d take this approach:

Add one atomic boolean, like @reuk suggests in the audioprocessor that is set to true in the audio thread when a note on arrives, the audio thread never sets it to false.

Then, a timer in the GUI thread reads the atomic bool at e.g. 30hz using atomic::exchange (false). If exchange return true, it means a note on arrived in the last 33 ms, the UI turns the light on, then after 100ms it should turn it off.

1 Like
4

Wow thanks to both of you for the quick and clear replies @reuk @jellebakker !
The std::atomic fix looks perfect for this simple case, then as soon as I can I’ll delve more into the inner workings of the ValueTreeState attachments.
Thanks for the clarifications.

1 Like