Fixed minor race condition and priority inversion

That was indeed the ideal approach that’s missing now from the editor.

That’s a good approach. only reason that @dave96’s inc/dec atomic is preferable is that having this within juce::AudioProcessor seems to be more DRY.
Implementing the shouldCalculateMagnitude approach means this will now be implemented by every developer who needs to decide about processing based on editor.

1 Like

I was suggesting this was something you add in your own code to be honest.
The reason being, that which was described by Jules.

Yes - it’s about 3 lines of code, and every app will be different, so they’ll name it appropriately for their particular use-case.

It’s partly my fault that a lot of people think about the audio thread incorrectly, because many years ago I didn’t really take seriously the separation of the processor and editor.

What I should have done was to make it impossible for the processor to get any kind of pointer to the editor, or to even know anything about it at all. The direction of data flow should always be that the audio thread gets sent any information it needs via atomic variables and FIFOs, it should never, ever interact with pointers that don’t belong to it.

1 Like

There’s a similarity between all of these apps. We’re talking about audio plugin processors, which may do additional audio analysis on the audio in case an editor is open (as an example, the frequency analyser and output level meter in Surfer EQ).

Indeed each plugin may do a different analysis, but in all of them it will be analysis for the GUI if there is one. Hence it’s a common need and Tal’s mention of DRY.

1 Like

Nobody likes DRY more than me, but that just isn’t the right place to do it.

The thing that’s wrong is that the processor should not have any notion of an editor even existing.

Yes, lots of people may need a flag to tell it to enable some extra analysis, but it’s completely wrong for the message to the processor to involve the word “editor”. The meaning being conveyed is “enable [some type of] analysis”.

Since not everyone needs this flag, and those who do will all need different names for it, and many people will need multiple flags for different types of work, and other will need to enable/disable it based on something other than the visibility of the whole editor window (e.g. they may have different pages in their editor for different types of display) then there’s no good generic solution for it. And given that it’s trivial to write, that’s what people should do :slight_smile:

But the processor owns a lot of message-thread stuff. If the only way to notify the editor of a (message-thread) event were to set a flag in the processor, the editor would have to check for all of them in a timer callback, no matter how sporadic they are. Isn’t that a waste?

That’s what I don’t get. If the method should only be called from the message thread, and the editor can only be deleted from the message thread, why is the lock in getActiveEditor needed? (I don’t mean it’s not, I just don’t understand it.)

Having another thread poll atomic variables IS the only safe way to communicate out of a real-time audio thread. If that’s news to you, I can recommend Dave’s excellent talk about this at the audio meet-up a couple of weeks ago: https://youtu.be/JCNyd1KGjMk

1 Like

Somewhere along the way some timer (or a looping thread/callback) would need to ‘waste’ cycles…

btw,
What Jules suggested was ‘wasting’ only on our processBlock that checks that bool.

Eventually I ended up adding a std::atomic<int> member to the processor that’s being inc/dec with constructor/destructor of the editor. (asserting it ends up to 0 upon destruction of processor).

Still those 3 lines would’ve been nice within JUCE itself imho, but that’s opinionated just as line indentation.

1 Like

I get that, yap. I’ve seen some older talks by Dave and Fabian about it, great stuff. I didn’t mean communicating from the audio thread but from the message thread itself. This already happens, through attachments, change listeners, etc. These indirectly keep a pointer to the editor. For example, I have a label displaying the latency in samples. In my case, the actual latency changes in the audio thread, but old versions of Cubase don’t like to be reported about it on anything but the message thread, so I call setLatencySamples through callAsync, and also notify the editor. I actually keep a separate pointer to the editor, set and cleared by it, so I don’t really use getActiveEditor.

Keep in mind, that it is legit to have more than one editors, hence the counter approach.

1 Like

Hm, I didn’t know that. This can be done by the host in any case, or it’s something you control from the plugin itself?

The host can call your factory method more than once without getting rid of it in the meantime (I don’t mean leaking). Sometimes they do to do some tests, or they display a miniature of it somewhere, all valid use cases.
I don’t know, how often that happens in practice though…
I think getActiveEditor() will then return the one, that has the focus, since that can only be one, but that is speculation.

Still, I think it is better to have an approach of how many editors were created, and how many were destroyed, similar to a reference counting.

2 Likes

Good. It has to be a broadcasting mechanism then, with an array of pointers. Thanks for the info!

No! The processor shouldn’t have any back-links to GUI stuff at all! That’s the wrong direction. The GUI needs a reference to the processor, but if you’re relying on anything that goes in the other direction, it’s just a bad design!

Clearly I failed to explain myself very well, I’ve tried a few times now…

The right mindset is to imagine that your DSP code is running in a separate process to the GUI. Because whether you like it or not, one day that’s going to be the case.

3 Likes

I don’t think you’ve failed to explain yourself, but also don’t see how I’m saying something different. From the docs of createEditor:

The correct way to handle the connection between an editor component and its processor is to use something like a ChangeBroadcaster so that the editor can register itself as a listener, and be told when a change occurs. This lets them safely unregister themselves when they are deleted.

That’s alright, but the processor owns more than the DSP code. The undo manager, for example -so the editor registers to it.

1 Like