COCKOS
CONFEDERATED FORUMS
Cockos : REAPER : NINJAM : Forums
Forum Home : Register : FAQ : Members List : Search :

Go Back   Cockos Incorporated Forums > Other Software Discussion > WDL users forum

Reply
 
Thread Tools Display Modes
Old 10-06-2017, 12:14 AM   #1
Nowhk
Human being with feelings
 
Join Date: Mar 2016
Posts: 234
Default Can you give to me a reason why to lock on OnParamChange?

Hi all,

I'm making some "re-design" stuff within my architecture on IPlug.
There are some important decisional aspects that I need to cover before switch.

One of the most important is:

Code:
// Implementations should set a mutex lock like in the no-op!
virtual void OnParamChange(int paramIdx) { IMutexLock lock(this); }
Why? Really, I miss why I should do this.

Analyzing the code, I catch which functions call that OnParamChange:

SetParameterFromGUI (which already lock)
OnParamReset (who call this function, is already locked)
VSTDispatcher (which already lock)
VSTSetParameter (which already lock)

So I don't see any reason why one should lock OnParamChange.
What am I missing? Why did you write that when developing it?

This has already be discussed by me and earlevel.

Can you give to us a valid motive? Oliver?
So I can get rid of it within OnParamChange

Thanks
Nowhk is offline   Reply With Quote
Old 10-06-2017, 02:41 AM   #2
olilarkin
Human being with feelings
 
Join Date: Apr 2009
Location: Berlin, Germany
Posts: 1,248
Default

Hello,

The truth is all this mutex locking was in IPlug from the beginning. I wasn't brave or knowledgeable enough to attempt to remove it, although I have wanted to for a long time (see todo.txt!)

About three years ago Justin had a look and I know that some products have been out in the wild that use his version with no reported problems.

https://github.com/justinfrankel/wdl-ol

So these changes are making their way into a new release that I am preparing. I hope that I can provide a preprocessor macro to explicitly disable the overly severe mutex locking in the original IPlug code but leave it there for beginners who don't necessarily want to think about thread safety themselves.
__________________
VirtualCZ | Endless Series | iPlug2 | Linkedin | Facebook
olilarkin is offline   Reply With Quote
Old 10-06-2017, 03:23 AM   #3
Xenakios
Human being with feelings
 
Xenakios's Avatar
 
Join Date: Feb 2007
Location: Oulu, Finland
Posts: 8,062
Default

Quote:
Originally Posted by olilarkin View Post
leave it there for beginners who don't necessarily want to think about thread safety themselves.
Although as it is now, it might be just as easy for them to forget about doing the mutex locking anyway. If they are not reading a tutorial that mentions the thread safety issues, the only place where they could see the comment about the mutex locking with OnParamChange is in the IPlug header file. (Missing seeing the comment could happen if they are looking at/copy-pasting some code that doesn't do the mutex locking in the overridden OnParamChange.)
__________________
I am no longer part of the REAPER community. Please don't contact me with any REAPER-related issues.
Xenakios is offline   Reply With Quote
Old 10-06-2017, 06:36 AM   #4
Nowhk
Human being with feelings
 
Join Date: Mar 2016
Posts: 234
Default

Quote:
Originally Posted by olilarkin View Post
but leave it there for beginners who don't necessarily want to think about thread safety themselves.
I'm not sure if I can consider myself a non-beginner

So, can I simply don't lock inside OnParamChange? Or this could introduce problems?
Nowhk is offline   Reply With Quote
Old 10-06-2017, 09:19 AM   #5
Xenakios
Human being with feelings
 
Xenakios's Avatar
 
Join Date: Feb 2007
Location: Oulu, Finland
Posts: 8,062
Default

Quote:
Originally Posted by Nowhk View Post
I'm not sure if I can consider myself a non-beginner

So, can I simply don't lock inside OnParamChange? Or this could introduce problems?
Of course it can introduce problems if the OnParamChange method manipulates variables that the audio processing thread is potentially using at the same time. You might get away when manipulating something like simple floating point variables, but even then it might not be guaranteed it all works OK. Stuff like resizing buffers etc most certainly will not work correctly. (Unless the buffers or such themselves have thread safety built-in. The C++ standard library or the WDL container classes do not have such thread safety.)
__________________
I am no longer part of the REAPER community. Please don't contact me with any REAPER-related issues.
Xenakios is offline   Reply With Quote
Old 10-06-2017, 11:13 AM   #6
Nowhk
Human being with feelings
 
Join Date: Mar 2016
Posts: 234
Default

Quote:
Originally Posted by Xenakios View Post
Of course it can introduce problems if the OnParamChange method manipulates variables that the audio processing thread is potentially using at the same time. You might get away when manipulating something like simple floating point variables, but even then it might not be guaranteed it all works OK. Stuff like resizing buffers etc most certainly will not work correctly. (Unless the buffers or such themselves have thread safety built-in. The C++ standard library or the WDL container classes do not have such thread safety.)
? No of course I'll keep mutexes

My concern is where I need to keep them.
Since OnParamChange is called from functions which already lock the section, I can't see any situation where troubles can come pop out if I don't lock within OnParamChange (because I'm already safe when IPlug call It). You know? Any example?
Nowhk is offline   Reply With Quote
Old 10-06-2017, 11:22 AM   #7
Xenakios
Human being with feelings
 
Xenakios's Avatar
 
Join Date: Feb 2007
Location: Oulu, Finland
Posts: 8,062
Default

Quote:
Originally Posted by Nowhk View Post
Since OnParamChange is called from functions which already lock the section
Ah, I didn't remember the code inside IPlug was like that! Maybe it's plugin format specific? So at least the VST2 wrapper does already lock the mutex, but maybe some other formats do not? (In any case, it's just a recursive mutex lock which assumably works if you lock again in OnParamChange. Recursive mutex locking tends to be a bit dubious, though...)
__________________
I am no longer part of the REAPER community. Please don't contact me with any REAPER-related issues.
Xenakios is offline   Reply With Quote
Old 10-07-2017, 01:51 AM   #8
Nowhk
Human being with feelings
 
Join Date: Mar 2016
Posts: 234
Default

Quote:
Originally Posted by Xenakios View Post
Ah, I didn't remember the code inside IPlug was like that! Maybe it's plugin format specific? So at least the VST2 wrapper does already lock the mutex, but maybe some other formats do not? (In any case, it's just a recursive mutex lock which assumably works if you lock again in OnParamChange. Recursive mutex locking tends to be a bit dubious, though...)
No, for what I see its the same for all format. And no, it could introduce some problems... https://forum.cockos.com/showthread.php?t=186686

Thats why I need to understand where to place/remove them
Nowhk is offline   Reply With Quote
Reply

Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump


All times are GMT -7. The time now is 02:07 AM.


Powered by vBulletin® Version 3.8.11
Copyright ©2000 - 2024, vBulletin Solutions Inc.