Calling setValueNotifyingHost() from processBlock()


#9

Nope, that definitely isn’t safe, I’m afraid.

Just because you haven’t heard of any problems doesn’t mean it’s safe. You’ve no idea what the host will do when you invoke that call, but it’s very likely that some of them will allocate memory or post a message, which will occasionally cause problems depending on what else the host is doing at a particular moment.

The only safe way to send events from your process method is to have your own lock-free fifo to post events which you later pick up in another thread (or on a timer) and do the work there.


#10

I don’t know about any particular quirks in Logic, but you need to make sure that anything you do in AudioProcessor::setValue() or AudioProcessorListener::audioProcessorXxxxx is thread-safe, fast and non-blocking as these are called as a result of the methods you mention.

It may just be that Logic is less forgiving of something going on in there…


#11

Yes, thank you. I was afraid that I have just been “lucky” until now. I always strive to make things better, however, so this is a great opportunity to refactor the code into something more robust!

The lock-free fifo idea sounds like it may very well be the way to go. I’ll be looking at that as my new approach.

In the mean time, I’ve been analyzing my code, out of curiosity, to try to determine why it works as well as it does? I mean, as I’ve mentioned three or four times, it works great in Pro Tools. You’d think it would fail there, if it was going to fail anywhere. And, it also works great in Tacktion/Waveform (you may have some insight here).

I make a point of making all my code in the processBlock() as tight as possible. And, for example, I bypass code that is not changing, which includes the setValueNotifyingHost() calls. In fact, in a steady signal, dozens, if not hundreds, of blocks can go by without setValueNotifyingHost() being called. So maybe this has something to do with my “getting away with it”.

So, I’ve got work to do. And, the lock-free fifo idea is not too different from the way I feed data to my meters (although, they are on a much more relaxed schedule!). We are always learning. I think that is one of the reasons we code!

Thank you for your great help!


#12

My approach is the problem for sure. But others may want to take note that Logic is doing something different, because my plugins are OK in most DAWs, and, yet, they have definite issues in Logic.

So, I’ll be changing what I am doing. And, Jules lock-free fifo sounds like a good approach (maybe with a timer).

Thank you so much for your help!


#13

Maybe it is even only a problem in a combination, like having a certain amount of plugins, you can’t know… still a good idea to improve it.
It doesn’t sound, like it’s ever going to be sample accurate, so why not simply use callAsync with the setValueNotifyingHost()? Sounds easier to implement…


#14

Then this would make a good addition to the documentation of those methods?

I remember that I recently had to suggest the same also for the thread-safe methods for obtaining/loading a ValueTree.

What do you think about tagging in some standard way the methods that are not realtime-safe (or, conversely, those that are safe for being called from the realtime thread)?


#15

Great suggestion! Maybe as an addition to all methods a hint of “Audio Thread - Message Thread - Any Thread”? That woulkd be very helpful


#16

Yes, definitely should be added to the docs.

@t0m maybe while you’re working on the audioprocessor stuff, we should throw in some assertions to make sure people don’t call unsafe stuff from the audio thread? We could have a thread-local flag set during the process method, and have assertions that check for it, kind of like we do in some places for the message thread.


#17

I don’t think this will work reliably, because there are hosts that call initialization methods from realtime threads before calling any process callback with them.

And therefore, all the methods called in those initialization phases will have a wrong indication because said hypotetical isRealtimeThread() method will return false at that point, but it will start returning true later on after the first processing callback has been called.

To be fair, on the other hand this may be a tolerated behavior, if we accept that “blocking” behavior can happen on a soon-to-be realtime thread, as long as it has not yet actually entered the processing loop


#18

Yes, I think that’s pretty much what you want. If a host calls initialisation routines, it must expect them to block, so it doesn’t matter what thread it uses for them.


#19

Uhm ok.

Playing the “devil’s advocate” now: what happens if the DAW decides to stop using a certain thread of its pool for realtime jobs, and assigns it to background stuff that still call into the plug-in?
That thread would still be marked as realtime while not really being anymore.

Maybe while designing this, allow for another method to query whether the current DAW is to be trusted or not. I suppose most DAWs will be implemented in a sensible way and the answer will be “yes”, but you may never know.


#20

I think you misunderstood what I meant: I was suggesting having a thread-local variable “isAudioThread”, which is set to true at the start of process() and set to false at the end. That way it doesn’t matter if the thread changes.


#21

I had a hard time with these kind of issues, some I could just settle but not resolve, when targeting FinalCut. e.g. here: Final Cut and the MessageThread
and some more…

I am super reluctant about too many asserts, I’d prefer educating developers using the documentation. In the linked case I was lucky, that removing the MessageManagerLock was an option.


#22

Ah yes, I misunderstood your proposal, I figured you wanted to set the flag to “true” upon first processBlock to “tag” the thread from then on, which is something that I considered myself for exactly this purpose.

Regarding your idea, it is equivalent to having a method that informs whether the AudioProcessor::getCallbackLock() is taken by the current thread.

On one hand, that’s a facility that is already available for the special case of the MessageManagerLock, so I don’t see why not adding it also for the callback lock.

But on the other hand, I remember that when I suggested implementing something like that several years ago for CriticalSection, you dismissed the idea with valid arguments (can’t remember now), probably linked to the fact that one could write code that waited for the lock without actually locking, which seemed a bad idea.

EDIT: found it, 8 years have passed, maybe something has changed in the mean time:


#23

FinalCut and WaveLab would probably be the two DAWs for which the isRealtimeFlagReliable() should return false, and hence all the assertions regarding isRealtimeThread() should take that into account, probably passing without being triggered.


#24

What seems to be working as a solution to the original issue is the following;

In the processor class, I have a few doubles at class scope.

I derive from HighResolutionTimer, and I literally cut and pasted the automation code from the processBlock to the hiresTimerCallback().

I then added an if statement in processBlock() to control who sets the values of the doubles (either the process block, or the hiresTimerCallback()).

Now things work as before, but the automation operations are offloaded to another thread.

On Intel processors, this should be workable. On other processors, it will depend on how they deal with doubles.

I am still waiting for feedback from my user, but my testing is going well, so far.

Thanks to all for their help!


#25

@jules, are there news about this?
I’m in a situation where it would be really nice to have the possibility to scatter around some assertions, to find where non-realtime-safe methods are being called from the realtime thread.


#26

No, hadn’t got around to this yet. If you need a quick solution for adding your own asserts, it’s actually something you could easily do yourself in a couple of lines of code with a thread-local variable.


#27

There are some implementation complications though. I was trying to write something like that myself:

I added to AudioProcessor one such member:

 static ThreadLocalValue <bool> inProcessBlock;

But then I need to access it in the wrappers that call processBlock(), and therefore said member should be public, which doesn’t seem very safe to me. What do you think?


#28

As a follow up on my original issue—my user who had an issue with Logic is now resolved. The fix I used (outlined above) did the trick. The work-around involved a HighResolutionTimer and its callback. The key being that the automation communication is handled on the timer thread.

In hindsight, it is a better way to do it. As usual, I learned something from this forum!

And, going forward, it would have been nice to have the asserts to warn of issues on the audio thread. That is a win for everybody.