Avisynth 2.5.8 MT

[lych_necross]

...oh boy...
@leeperry
reread the first post. It explains the memory leaks. Another build won't change anything.

[leeperry]

ouh, rebuilding plugins...I'm using LSF/GrainFactory3 and rgb3dlut()

GrainF3 is based on AddGrainC, which is pretty buggy...especially in the memory leak department. I think I'll stick to 2.57 then..

[Fizick]

SEt,
The question from plugins writer:
is plugins compiled with your new modified 2.5.8MT header avisynth.h 100% compatible with older versions of Avisynth (2.5.6-2.5.7), including non-MT ?

(my previous versions of MVTools and other plugins used old avisynth.h from Avisynth v2.5.5 for wide compatibility)

[SEt]

It's as compatible as the header from official Avisynth 2.5.8.

The only additional problem you can get with my header is with compiling InterlockedIncrement -> _InterlockedIncrement mapping (depending on included headers and their order this can be redefinition). This is pure minor performance optimization for VS and that #if section can be safely removed without affecting the correctness.

If you want to use older header - just apply the difference of my header with official 2.5.8 without the end of IScriptEnvironment and "friend class CacheMT;" as these are MT-specific additions (won't harm even if present in header as long as you don't call them).

[Fizick]

OK. Thanks. I will do it in next release

Originally Posted by SEt

Well, i changed avisynth.h not out of nowhere:

void Release() { if (refcount==1) InterlockedDecrement(&vfb->refcount); InterlockedDecrement((long *)&refcount); }

is definitely not thread safe and

So, the current code is fully thread-safe? :

  void Release() { VideoFrameBuffer* vfb_local = vfb; if (!InterlockedDecrement(&refcount)) InterlockedDecrement(&vfb_local->refcount); }

Next question from dilettante: is any not-thread-safe code in new avisynth.h ?
For example, is IsWritable and GetWritePtr code thread-safe?

[SEt]

void Release() { VideoFrameBuffer* vfb_local = vfb; if (!InterlockedDecrement(&refcount)) InterlockedDecrement(&vfb_local->refcount); }

Is thread-safe for how it's used in Avisynth and plugins. Note that similar following code is not:

void Release() { if (!InterlockedDecrement(&refcount)) InterlockedDecrement(&vfb->refcount); }

Originally Posted by Fizick

Next question from dilettante: is any not-thread-safe code in new avisynth.h ?
For example, is IsWritable and GetWritePtr code thread-safe?

Well, it's not like i'm threading guru myself. Probably there is some not thread-safe code. And yes, IsWritable and GetWritePtr are very suspicious - i've used to crash a lot because of them earlier (NewVideoFrame, GetWritePtr with refcount 2 at that moment, NULL pointer dereference) but now it works far more stable (test run successfully run 10 hours while earlier it would crash after ~half an hour).

In general i don't like at all how this Read-Write state of frames work in current Avisynth - such things should be completely inside Avisynth itself, not in every plugin. Only than you would be able to say that they are thread-safe.

I have several suggestions for radical changes in Avisynth plugin API and now is good time for them - i think version 2.6 can drop backward compatibility for easier to use, safer for threading and internally flexible (like caching implementation) without every plugin recompilation API.

[Fizick]

1. In my personal opinion (from user and plugin developer's point of view), we may broke plugins 2.5 compatibility in nearest Avisynth version only as a follows:
by providing new command LoadNewPlugin for new API plugins, and preserve LoadPlugin for old 2.5 API plugins. (So, do not break nothing, but do not provide much support of new features to old API)

2. probably we can return to discussion about roadmap of Avisynth, while IanB live in real life (may be hunting for crocodiles in Australian rivers
If you have not only suggestions, but also some time to implement your suggestions (with some discussion), you are welcome to contribute to some new version branch, for example 2.7. (Ask Wilbert for sourceforge access as a team member.)

3. May be I am complitely wrong both in 1 and in 2 points above (it was hot day).

[Fizick]

SEt, in meantime, can you provide (for me) some scenarios, showing thread-safety-notsafety of code fragments above?

[SEt]

void Release() { if (refcount==1) InterlockedDecrement(&vfb->refcount); InterlockedDecrement((long *)&refcount); }

Consider the following situation:
Two threads are holding this object so refcount=2.
They at the same time call Release().
Condition (refcount==1) is false for both, after that both decrement refcount and it becomes 0, but no one decrements vfb->refcount.

void Release() { if (!InterlockedDecrement(&refcount)) InterlockedDecrement(&vfb->refcount); }

Avisynth keeps a list of VideoFrame objects and reuses them instead of allocating/deallocating each time. After refcount is decremented to 0 object is considered free and can be reused immediately, so before we read vfb->refcount it can be overwritten by vfb of newly created frame (and i indeed seen this happen in practice). This results in decrementing refcount of wrong vfb.

void Release() { VideoFrameBuffer* vfb_local = vfb; if (!InterlockedDecrement(&refcount)) InterlockedDecrement(&vfb_local->refcount); }

Is free from both problems.

[buletti]

Originally Posted by Leak

Hmmm... it's been a while since I last worked on the timestamp calculation, but I'll take a look at it this weekend - but I'd rather fix it completely instead of taking out passFirstThrough...

Is there any news regarding the ffdshow patch? Do we need another solution or may Set's patch go into the ffdshow trunk? It would be good, if the MT-enabled Avisynth wasn't bound to ffdshow revision 2975 only.

[Fizick]

SEt, thanks.

Next question. One user reported a problem with your build of Avisynth2.5.8MT with the crazy script, including AnimeIVTC:
http://forum.ixbt.com/topic.cgi?id=29:9331-153#4577
Version 2.5.7MT and official 2.5.8 works without this problem.

[SEt]

AnimeIVTC is so monstrous that it's unusable i'd say. There are also several problems.

One of them is Cdeblend - it doesn't (always) work in official 2.5.8 too. Here is the patch:

--- Cdeblend_v1.1b.avsi	2007-05-18 16:40:16.000000000 +0400
+++ Cdeblend_v1.1b.avsi	2009-07-12 06:34:31.155704600 +0400
@@ -66,8 +66,6 @@
 btemp 	= default(dclip,input)
 global blendclip= (fnr==false ? btemp : clense(btemp)).bilinearresize(int(width(btemp)*xr)*8,int(height(btemp)*yr)*8)
 
-rate = framerate(input)
-
 ###### masking ######
 diff = mt_makediff(blendclip, blendclip.trim(1,0))
 global mask	= mt_lutxy(diff, diff.trim(1,0), yexpr="x 128 - abs y 128 - abs < x 128 - abs x y + 256 - abs < & x 128 - abs x 128 - abs 0.5 ^ - 2 ^ y 128 - abs x y + 256 - abs < y 128 - abs y 128 - abs 0.5 ^ - 2 ^ x y + 256 - abs x y + 256 - abs 0.5 ^ - 2 ^ ? ? x 128 - y 128 - * 0 > -1 1 ? * 128 +", uexpr="x", vexpr="x")
@@ -129,6 +127,6 @@
 mode > 1 ? scriptclip(output, "evaluate(LumaDifference(blendclip.trim(1,0),blendclip.trim(3,0)), AverageLuma(mask.trim(1,0)))") :
 	\	scriptclip(output, "evaluate(LumaDifference(blendclip,blendclip.trim(2,0)), AverageLuma(mask))")
 
-recl = last.changefps(rate*2).changefps(rate,linear=true)
-return(recl.addborders(0,0,8,0).crop(0,0,-8,-0)) 
+recl = changefps(FrameRateNumerator(), FrameRateDenominator(), linear=true)
+return recl
 }

The last two lines probably try to force frame requests in linear order but script still breaks with SetMTMode.

The other one i remember is EEDI2 distributed with it is built with very buggy OpenMP runtime. Just rebuilding solves the problem of random crashing.

[Fizick]

here is the simplest broken script with 2.5.8MT:

blankclip
return changefps(23.976)

[SEt]

Problem fixed, see the first post for updated version.

Notes:
lol, of course it will break if you change things like

union { float f; unsigned i; } value;
mantissa = (value.i & 0x7FFFFF) + 0x800000;  // add implicit bit on the left

into

union { double f; unsigned i; } value;
mantissa = (value.i & 0x7FFFFF) + 0x800000;  // add implicit bit on the left

Seems like Jeremy was removing warnings this way >_>. I've just rolled back affected files.

[Fizick]

OK, thanks! It works now. But I do not see updated source code.

EDIT - now I see it (fps.cpp) (firstly i missed your point about rolling back, not modifying)


(Oh, those warnings... )

[Jeremy Duncan]

Interesting thread. No real points to make but i would like to thank SEt for his fine efforts in this stuff.

I have done some tests with the tsp and set and my versions here: link. I just wanted you to see for something interesting to eyeball. I know I like to read such things. Anyway, keep up the interesting good work Mr. SEt.

[Gavino]

Originally Posted by SEt

Seems like Jeremy was removing warnings this way >_>. I've just rolled back affected files.

Have you reviewed all changes introduced by Jeremy? It would be reassuring for us all (him included) to know that there are no similar problems elsewhere in the code.

On a different note (prompted by seeing the Cdeblend code above): do ScriptClip and friends work reliably under MT Avisynth? Can different threads interfere with each other when setting 'current_frame'?

[Leak]

Originally Posted by buletti

Is there any news regarding the ffdshow patch? Do we need another solution or may Set's patch go into the ffdshow trunk? It would be good, if the MT-enabled Avisynth wasn't bound to ffdshow revision 2975 only.

Sorry, I was being swamped with stuff at work so I couldn't really get myself to do anything code-related at home...

I do have 2 weeks of vacation come next week, so I'll be taking a look at it then.

np: Tortoise - Charteroak Foundation (Beacons Of Ancestorship)

[Jeremy Duncan]

Originally Posted by Gavino

Have you reviewed all changes introduced by Jeremy? It would be reassuring for us all (him included) to know that there are no similar problems elsewhere in the code.

I compared the files in the src. The two src I compared were the original japan src and SEt july 12 src.

I found these files were not the same in the src.

stdafx.h
avisynth.cpp
avisynth.h
cache.cpp
cachemt.cpp
main.cpp
mt.cpp
mt.h
softwire_helpers.cpp
expression.cpp
color.cpp
convolution.cpp
edit.cpp
focus.cpp
greyscale.cpp
histogram.cpp
layer.cpp
layer.h
levels.cpp
merge.cpp
planeswap.cpp
resample.cpp
resize.cpp
transform.cpp
source.cpp
____________
pfc folder
string.cpp
utf8.cpp

Softwire folder
assembler.cpp
instructionset.cpp
operand.cpp

japan src
SEt july 12 src
ultraedit. sw I used to compare

click the ultraedit link, go to product tour and highlight the "Save time by comparing your files to track editing changes.
show/hide". IT will show you how to use the compare feature in this sw.
You open two files, then click the blue "uc" globe icon. When the compare screen opens then you press the arrow icon to go where the different code is.

I have to compare the distribute folder now and then go to my src and then the official 2.5.8 src so I will be updating this post later.
I did not compare the directshowsource or tcpdeliver files as I did not build these so didn't fiddle with their files.

Edit,

I have now finished comparing the japan and SEt src and distrib folder and the changes are in the list above. These are the files with different code.
________________________________________________________________
Edit,

I have redone the test with my src. My src is really SEt july 12 src but with a few files changed. Which files you ask? It's at the bottom of the edit below.

I tried to use ultraedit to compare the official avisynth 2.5.8 to both the japan and SEt src and mine too but it was all garbage and I couldn't get it done.
So I redid the work above but used my src instead of the one used before, cause I wanted too.
You probably see more files in the list below. I was more careful this time cause I knew how to do it right after the first try in the list above.

Anyway, below is the files changed in the SEt july 12 avisynth src from the original Japan avisynth mt src from way back when:

Audio folder
audio.cpp: lines 1400, 1450
avs-soundtouch.cpp: line 86
supereq.cpp: line 165

Convert folder

core folder

avisynth_c.cpp: lines 123
cache.cpp: lines 39, 514, 523, 533, 553, 824
cachemt.cpp: lines 295
main.cpp
mt.cpp: lines 102, 200, 284
mt.h: line 82
softwire_helpers.cpp: line 68

core parser folder
expression.cpp: lines 87, 93, 691

filters folder
color.cpp: lines 129, 169, 176, 209
convolution.cpp: line 216
edit.cpp: lines 404, 474, 596, 859, 878, 887, 896, 905, 914, 923, 933, 946, 959
focus.cpp: lines 339, 481, 486, 725, 753, 1088, 1104, 1132, 1148
greyscale.cpp: line 95
histogram.cpp: lines 495, 507, 548, 582, 604, 667, 674, 694, 698, 709, 713, 775, 782, 792, 810, 818, 824, 829, 837, 843, 847, 864, 870, 876, 883, 890
layer.cpp: lines 514, 1005, 2300
layer.h: line 160
levels.cpp: lines 136, 143, 277, 299, 952, 1057
merge.cpp: lines 94
planeswap.cpp: line 348
resample.cpp: lines 331, 379, 410, 437, 470, 1705
resize.cpp: lines 260, 272
transform.cpp: lines 149, 159, 382, 385, 390, 395, 398, 403, 435, 441, 452, 456, 511

filters overlay, conditional folders

sources folder
source.cpp: lines 90, 96

sources, avi folder
AVIReadHandler.cpp: line 1287


Note: Changes in the src tested from the SEt july 12 avisynth src.

change 1.) The main src folder, not the src subfolders, is from my src.
These are mainly settings for when I use visual studio to compile the dll, vcproj and other such files.

Change 2.) I also put the avisynth.cpp and avisynth.h files from my src into the SEt july 12 src.

Change 3.) Also I used the distrib, include: softwire, soundtouch, pfc files from my src.

other than that, the rest of the files were from the SEt july 12 avisynth src.

I'm sorry I changed so many files.

[SEt]

I've reviewed only changes between the version where this casting was added and previous one. Other files changes seems harmless.
The real problem is: why AVSValue stores floating point values as float, function named AsFloat, but returns double? I think it creates confusion for plugin writers too.

Updated ffdshow patch - commented out identifying as cache because in current Avisynth cache and MT gates are inseparable and you'll probably want to change MTMode right after it.

Updated Cdeblend patch: now it almost works with SetMTMode(5) - it can still process some first frames wrong due to inefficient caching after changefps(linear=true).
The general problem there: script has to use global variables to store previous LumaDifference results what breaks in case of out of order frame access typical for multithreaded modes. Solution would be introducing something like cached "clip" with numbers.