Welcome to Doom9's Forum, THE in-place to be for everyone interested in DVD conversion.

Before you start posting please read the forum rules. By posting to this forum you agree to abide by the rules.

 

Go Back   Doom9's Forum > Capturing and Editing Video > Avisynth Development

Closed Thread
 
Thread Tools Search this Thread Display Modes
Old 25th October 2013, 09:04   #181  |  Link
ultim
AVS+ Dev
 
ultim's Avatar
 
Join Date: Aug 2013
Posts: 359
Avisynth Plugin Writing Tips #2: Parallel execution

Avisynth-MT and VapourSynth both support multithreading, and it is being implemented in Avisynth+ too. All of them require the same things from your plugin. Here is a list of what you as a plugin author can do to support execution on multiple threads. If you are the author of any Avisynth plugin, please update your filter according to these rules if needed. Doing so will make sure your plugin can execute seamlessly when multithreaded. Furthermore, following these rules will not only guarantee correct execution in multihtreaded environments, it will also provide optimal mulithreaded performance.

Short list for those on the run:
- Unless you have the slowest filter in the world, don't start threads in your plugin.
- Never use global or static variables. In addition, your filter class should only have read-only members which are initialized during construction.
- Don't reuse the IScriptEnvionment pointer between method executions.

And again, the same points with a bit of more explanation:
- In general, do not slice up your frame and start multiple threads on your own. Threading has its own performance overhead, and it is only worth doing it manually if your filter takes a lot of time to execute. And even if your filter is extremely slow (like fft3dfilter), you should try to optimize its single-threaded performance (by using SIMD instructions or choosing a more efficient algorithm) rather then manually threading it. Optimize for single-threaded performance, and as long as you follow the other rules below, you will get automatic and correct multithreading from Avisynth.
- Do not cache frames yourself. You might think it is efficient because you won't have to request/compute them in the next frame, but you are wrong, and there are several reasons why. First, if you write your own cache, you will have to introduce a global state to your filter, which means you will have to take care of synchronization between multiple threads too, which is not easy to do efficiently with caches. Second, keeping copies of past frames also means there will always be multiple references to them, thus Avisynth cannot pass them as write pointers to other filters, and will have to do an extra copy of it more often. And last but not least, Avisynth has a very extensive caching mechanism, and if you request the same frame multiple times (even when you need it for different frame requests), chances are you will get it for free anyway, so your own caching is just pure overhead.
- As a general extension to the previous rule, try not to keep any state between frames. In the optimal case your filter class should only have read-only members which are initialized during construction. Surely this is not always possible with every algorithm, but most times it is, and this is what you should strive for.
- As stated before, for best multithreading always try to implement algorithms which require no state between frames. Whenever this is violated, be sure to group reads and writes to the state (do not spread them), and guard them in critical sections (as few and as short as possible). For example, it is a good practice to copy all your writable class variables (in a critical section) at the start of each frame into local stack variables, compute the whole frame outside of the critical section (updating the local variables that captured the global state as needed), then write them back together at the end of your frame in another critical section. Do not request an automatic lock around your whole filter from Avisynth, because it will serialize your filter's execution.
- If you have class variables that must be writable in every frame, you will also have to keep in mind that Avisynth does not guarantee that frames will be processed in their natural order. Just a reminder.
- Only store variables in classes and in method stacks. Per-frame heap allocations should be avoided, because they can act as implicit synchronization points between threads. And most importantly, never store anything in static variables or in the (global) namespace scope. Read the previous sentence a few more times.
- Do not store the IScriptEnvironment pointer anywhere yourself (except locally on the stack), and never reuse those pointers outside of the methods where they were supplied to you. Not even between different executions of the same method! There is a reason why you get that pointer separately for each method, which is that it may be different every time, especially in multithreaded scenarios. If you reuse it, the consequences will be different between every implementation, but you can get anything from race conditions to program crashes.

Last edited by ultim; 25th October 2013 at 20:57.
ultim is offline  
Old 25th October 2013, 17:50   #182  |  Link
Robert Martens
Registered User
 
Join Date: Feb 2010
Location: New York
Posts: 116
Quote:
Originally Posted by ultim View Post
Do not store the IScriptEnvironment pointer anywhere yourself (except locally on the stack), and never reuse those pointers outside of the methods where they were supplied to you. Not even between different executions of the same method! There is a reason why you get that pointer separately for each method, which is that it may be different every time, especially in multithreaded scenarios. If you reuse it, the consequences will be different between every implementation, but you can get anything from race conditions to program crashes.
I have to own up to being a dummy here and having done that, thinking the rule only applied to storing it as a global variable. I had made the--admittedly careless--assumption that storing the environment pointer as a non-static class variable would be safe since each instance of the host would have its own instance of my filter class. I take it I'm supposed to understand that multiple instances of a host application might try to access a single instance of my filter?

Only the slightest bit more consideration put into my problem and I've eliminated what I thought was the need to store the pointer, so it's no big deal for me to bring my code into compliance (luckily I work so slowly I haven't released a version with this problem included), but for the sake of curiosity I thought I'd ask.
Robert Martens is offline  
Old 25th October 2013, 19:40   #183  |  Link
Myrsloik
Professional Code Monkey
 
Myrsloik's Avatar
 
Join Date: Jun 2003
Location: Kinnarps Chair
Posts: 2,547
Quote:
Originally Posted by Robert Martens View Post
I have to own up to being a dummy here and having done that, thinking the rule only applied to storing it as a global variable. I had made the--admittedly careless--assumption that storing the environment pointer as a non-static class variable would be safe since each instance of the host would have its own instance of my filter class. I take it I'm supposed to understand that multiple instances of a host application might try to access a single instance of my filter?

Only the slightest bit more consideration put into my problem and I've eliminated what I thought was the need to store the pointer, so it's no big deal for me to bring my code into compliance (luckily I work so slowly I haven't released a version with this problem included), but for the sake of curiosity I thought I'd ask.
Always pass the environment pointer as an argument to your functions. It's a simple rule. Because Avisynth isn't multithreaded you'll never have two threads accessing your filter at once. However it's still a very bad practice. (not sure exactly what the MT side does to get around it)

Note that this issue affects everyone trying to use Avisynth plugins. For example RemoveGrain commits the biggest sin of all. It stores the environment pointer as a global variable when AvisynthPluginInit2 is called. This makes VapourSynth, which creates fake Avisynth environments only when needed, crash. A lot. Since the environment pointer will be different between plugin instances and registering functions.

I'd like to add another filter writing tip myself.

ONLY call AddFunction in AvisynthPluginInit2. Do not throw errors. DON'T DO ANYTHING ELSE THERE. Do initialization when one of your functions is called the first time.


It's the biggest reason for autoloading "breaking" in avisynth and a pile of other issues.
__________________
VapourSynth - proving that scripting languages and video processing isn't dead yet
Myrsloik is offline  
Old 25th October 2013, 20:34   #184  |  Link
ultim
AVS+ Dev
 
ultim's Avatar
 
Join Date: Aug 2013
Posts: 359
Quote:
Originally Posted by Robert Martens View Post
I take it I'm supposed to understand that multiple instances of a host application might try to access a single instance of my filter?
Almost. Only one host application will use your filter instance, but that single host application (the Avisynth library to be precise) can use different IScriptEnvironment objects depending on which thread your filter instance is running on. Avisynth-MT, for example, uses special IScriptEnvironments instances to implement thread-local storage (TLS), and I'm implementing something similar in Avs+ too. Apparently, VapourSynth makes use of fake IScriptEnvironments objects too. The thing about TLS objects are, well, that they are local to the thread, so if the given multithreading implementation (for example Avs-MT mode 1) creates just one filter instance for all threads, your filter will get different IScriptEnvironment pointers based on the thread it is running on. Because you cannot know in your filter what threading implementation of Avisynth is in use, it is best not to store the environment pointer between calls at all, and only then will you be safe in every case.

Quote:
Originally Posted by Myrsloik View Post
Because Avisynth isn't multithreaded you'll never have two threads accessing your filter at once.
Nonononono, forget what Myrsloik said here. This statement can be very misleading, because it is only true for some versions of Avisynth (like classic non-threaded version). The point of this discussion is to let plugins prepare for multithreaded versions (and/or multithreading modes). An important point of my tips is that if you can, you SHOULD assume that multiple threads will be accessing your filter at once, as it really can happen, even with existing implementations.

Quote:
Originally Posted by Myrsloik View Post
... in AvisynthPluginInit2. Do not throw errors.
A little bit off-topic (as far as multithreading issues go), but for best practices about exceptions, look at this tip.

Last edited by ultim; 25th October 2013 at 22:02.
ultim is offline  
Old 25th October 2013, 20:53   #185  |  Link
Robert Martens
Registered User
 
Join Date: Feb 2010
Location: New York
Posts: 116
Thanks for the detail, that clears everything up! I've already eliminated the broken code and pushed the fix, now I just need to sort out one spot in my test executable where I'm catching an AvisynthError to grab its error message and I should be done with your list of Don'ts for the time being.
Robert Martens is offline  
Old 26th October 2013, 05:40   #186  |  Link
Stereodude
Registered User
 
Join Date: Dec 2002
Location: Region 0
Posts: 1,436
While I don't have anything technical to add, I would like to thank ultim and others for their efforts to modernize AVIsynth by dragging it kicking and screaming into this decade.
Stereodude is offline  
Old 29th October 2013, 17:45   #187  |  Link
ajp_anton
Registered User
 
ajp_anton's Avatar
 
Join Date: Aug 2006
Location: Stockholm/Helsinki
Posts: 805
If there's room for feature requests, can we skip some arguments when calling functions like this:
some_function(a,b,,d,e)
if I don't want to change the default value for the third argument, and don't want to explicitly use the names of the 4th and 5th arguments.
ajp_anton is offline  
Old 29th October 2013, 20:21   #188  |  Link
Gavino
Avisynth language lover
 
Join Date: Dec 2007
Location: Spain
Posts: 3,431
Quote:
Originally Posted by ajp_anton View Post
If there's room for feature requests, can we skip some arguments when calling functions like this:
some_function(a,b,,d,e)
You can get that effect now (more or less) by using (from v2.60):
Code:
some_function(a,b,Undefined(),d,e)
or if you prefer
Code:
global _ = Undefined()
...
some_function(a,b,_,d,e)
I don't like the idea of allowing some_function(a,b,,d,e) because a user could easily put an extra comma by mistake and not realise he wasn't getting what he expected.
__________________
GScript and GRunT - complex Avisynth scripting made easier
Gavino is offline  
Old 30th October 2013, 14:56   #189  |  Link
l33tmeatwad
Registered User
 
l33tmeatwad's Avatar
 
Join Date: Jun 2007
Posts: 414
Came across something interesting when testing things....if you have fft3dfilter (version 2.1.1) in a script twice and the bw & bh settings in one is 3x or more than the other then there is a green and red bar at the top. This odd glitch only happens in AviSynth+ and does not show up when using AviSynth 2.6 a5. Figured this was worth mentioning, I haven't tested this on another machine yet.
__________________
Github | AviSynth 101 | VapourSynth 101
l33tmeatwad is offline  
Old 30th October 2013, 17:10   #190  |  Link
cwk
Registered User
 
Join Date: Jan 2004
Location: earth, barely
Posts: 96
I also get the green bar when using the fft3dfilter, although I see it with just a single placement of the tool within a script. I am working around it for the time being by merging chroma back in at the end. I'll admit this means you can't use fft to filter your chroma.
cwk is offline  
Old 30th October 2013, 21:15   #191  |  Link
TurboPascal7
Registered User
 
TurboPascal7's Avatar
 
Join Date: Jan 2010
Posts: 270
Please post the minimal script you can reproduce the issue with (I couldn't). Also, what colorspace and what resolution you're working with?
__________________
Me on GitHub | AviSynth+ - the (dead) future of AviSynth
TurboPascal7 is offline  
Old 30th October 2013, 22:43   #192  |  Link
cwk
Registered User
 
Join Date: Jan 2004
Location: earth, barely
Posts: 96
I can see it with the following script

Code:
avisource("1_fields.avi")
converttoyv12()
fft3dfilter()
The source file is huffyuv, 688 x 476. I also see the lines if I resize to 704x480 or 720x480. fps is 59.940, if that matters.
cwk is offline  
Old 30th October 2013, 22:57   #193  |  Link
TurboPascal7
Registered User
 
TurboPascal7's Avatar
 
Join Date: Jan 2010
Posts: 270
Reproduced, thank you.
----
Okay, it's a bug in env->BitBlt.

fft3d calls it in CoverbufToPlanarPlane
Code:
for (h=0; h<dst_height; h++)
{
    env->BitBlt(dstp, dst_pitch, coverbuf1, coverpitch, dst_width, 1); // copy pure frame size only
    dstp += dst_pitch;
    coverbuf1 += coverpitch;
}
That in BitBlt, passing the height == 1 condition, goes to
Code:
#ifdef X86_32
if (GetCPUFlags() & CPUF_INTEGER_SSE)
    memcpy_amd(dstp, srcp, src_pitch*height);  // SSE
else
#endif
    memcpy(dstp, srcp, src_pitch*height);      // fallback
}
which in any case ends up broken because you actually have to copy row_size bytes instead of src_pitch bytes in this case.

So either we could remove the height == 1 check, replace src_pitch with row_size (they're equal when height != 1) or fix fft3d to call BitBlt once for the whole frame instead of every line.

Original Avisynth actually does it right:
Code:
if (height == 1 || (src_pitch == dst_pitch && dst_pitch == row_size)) {
    memcpy_amd(dstp, srcp, row_size*height);
} else {
    asm_BitBlt_ISSE(dstp,dst_pitch,srcp,src_pitch,row_size,height);
}
__________________
Me on GitHub | AviSynth+ - the (dead) future of AviSynth

Last edited by TurboPascal7; 31st October 2013 at 00:08. Reason: Merged the bug explanation from the (removed) post below
TurboPascal7 is offline  
Old 31st October 2013, 23:47   #194  |  Link
ultim
AVS+ Dev
 
ultim's Avatar
 
Join Date: Aug 2013
Posts: 359
Quote:
Originally Posted by TurboPascal7 View Post
Okay, it's a bug in env->BitBlt.
You might be right. Actually I'm greatly annoyed by Avisynth's BitBlt, IMHO it is a messed up implementation, but when I "fixed" it last time, it broke some plugins doing interleaved copy of non-interleaved frames. Even today I still don't know if the ability to do such copies is intentional, or is it an accident that plugins started to rely upon. Anyway, I changed "row_size*height" to "src_pitch*height" because I thought it is much more logical, thinking they are the same anyway because of the condition row_size==dst_pitch==src_pitch that guards it. Here is where I made the error: I accidently overlooked that this last condition need not be true if height==1. Fix in next release. And thx for looking into this for me.

edit: added clarification of what plugins got broken.

Last edited by ultim; 1st November 2013 at 12:05. Reason: rule 4: no profanity
ultim is offline  
Old 1st November 2013, 00:16   #195  |  Link
Guest
Guest
 
Join Date: Jan 2002
Posts: 21,901
"row_size==dst_pitch==src_pitch"

Are you sure about that? I was under the impression that a Crop can change that. It's always been a common error with Avisynth plugins to assume that they are the same.
Guest is offline  
Old 1st November 2013, 00:28   #196  |  Link
TurboPascal7
Registered User
 
TurboPascal7's Avatar
 
Join Date: Jan 2010
Posts: 270
Quote:
Originally Posted by neuron2 View Post
"row_size==dst_pitch==src_pitch"

Are you sure about that? I was under the impression that a Crop can change that. It's always been a common error with Avisynth plugins to assume that they are the same.
This is irrelevant. It's simply a condition. If it isn't true - the code will go the other way, that is all.
__________________
Me on GitHub | AviSynth+ - the (dead) future of AviSynth
TurboPascal7 is offline  
Old 1st November 2013, 00:33   #197  |  Link
Guest
Guest
 
Join Date: Jan 2002
Posts: 21,901
Thanks for the lesson on coding but as a condition "row_size==dst_pitch==src_pitch" is nonsense.
Guest is offline  
Old 1st November 2013, 00:37   #198  |  Link
ultim
AVS+ Dev
 
ultim's Avatar
 
Join Date: Aug 2013
Posts: 359
Quote:
Originally Posted by neuron2 View Post
"row_size==dst_pitch==src_pitch"

Are you sure about that? I was under the impression that a Crop can change that. It's always been a common error with Avisynth plugins to assume that they are the same.
Of course that equality needs not be true, not just because of crop, also because of alignment. But the piece of code in question here is guarded by an "if" to be able to take a more optimal path. It does not actually require that row_size==dst_pitch==src_pitch, if that is unmet, it will take a slower path.

Edit: I do find it a nonsense that the code is therefor optimized for the minority of the cases, which is (only) part of the reason I hate the BitBlt implementation. Fixing that though is not trivial due to existing plugins relying on the current implementation.

Last edited by ultim; 1st November 2013 at 00:43.
ultim is offline  
Old 1st November 2013, 00:40   #199  |  Link
TurboPascal7
Registered User
 
TurboPascal7's Avatar
 
Join Date: Jan 2010
Posts: 270
neuron2, no problems, don't hesitate to ask. By the way, this condition is also present in the original avs core. So you're a bit late with the nonsense part.
__________________
Me on GitHub | AviSynth+ - the (dead) future of AviSynth
TurboPascal7 is offline  
Old 1st November 2013, 00:42   #200  |  Link
Guest
Guest
 
Join Date: Jan 2002
Posts: 21,901
LOL. I don't actually need any help or guidance from you in SW design or coding. The stated condition was nonsense in the post that I responded to. Let it go, friend, or take it to PM.
Guest is offline  
Closed Thread

Thread Tools Search this Thread
Search this Thread:

Advanced Search
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 +1. The time now is 10:48.


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