Proposal for AviSynth 2.6

[sh0dan]

I just saw that Avery Lee supports Y8 in his latest vdub, and this just spawned an idea for a meaningfull update to 2.5. Since the 2.5 is getting very stable and fast due to Ian's incredible refactoring job, I think we might look a bit further ahead and take a small leap forward in the core. I hope my inactivity doesn't disqualify me completely ;)

Design goal proposals:
*) Full compatibility with existing 2.5 plugins.
*) Full compatibility with API. Meaning 2.5 filters should recompile with new avisynth.h without modification.
*) New features must be implementable within reasonable timeframe.

Proposed additions:
*) Y8 planar format.
*) YUV 4:4:4 planar format.
*) All planes delivered to filters are 16 byte adress aligned and have mod 16 pitch.
*) Thread-safe avisynth.h.
*) Add MaskTools to the core.

Possible additions:
*) Official Win64 support.
*) Intelligent frame prefetching on multicpu/core.

Why:
*) Y8 planar format. We are doing a lot of masks and similar stuff. Not always having two chroma channels to worry about would yeild good speedups.
*) YUV 4:4:4. Not being able to do YUV without having subsampled chroma is just annoying.
*) MaskTools Simply a spectacular package, with really good generic tools used by many script fuctions. Would also be great for developers to be able to use the filters by using env->Invoke.


Implementation:

Y8 planar format.
Add new videotype specifier and make the adjustments for VideoInfo and VideoFrame struct methods.

VideoInfo->IsPlanar() should return true. VideoInfo->IsY8() should be added.

VideoFrame->GetHeight(), GetWidth, GetPitch should all return 0 if a chroma channel is needed. GetReadPtr/GetWritePtr could either return NULL, or an empty buffer of X bytes if there are stupid filters out there.

As for filter implementation, it is mostly trivial changes needed for Y8 support for 90% of all filters. In filters where it was obvious no changes should be needed, as they only check vi->IsPlanar() and request the proper sizes for each plane.

Converters to and from this format are trivial. For RGB conversions we could start out by using an intermediate Y8<->YUY2<->RGB, just as YV12 currently does.

YUV 4:4:4 planar format.
Similar as Y8. I haven't been able to find any official name for this format. Proposed name 'YV24'.

Since there is no official name for this we might automatically convert it to another format (YUY2) when exporting to an application, and put in a variable switch to force YUVP export, for the experts.

Converters are not 100% trivial, but as a starting point there are converters to/from all current formats in the "Overlay" filter.

16 byte alignment
Already prepared AFAICT. We can push this in a 2.6 upgrade.

Thread safety.
Already done AFAICT. Guess we owe TSP some thanks - now it just needs to be compiled into all filters. "2.6" would be the perfect excuse.

Add MaskTools to the core
If Manano allows, it would make a great addition to the core, since it has now proven itself stable in the field. Should still be a separate plugin with Manao in complete control.

Official Win64 support.
I would very much like this to happend, but it requires sooo much work. The current version is a great start, but I'm not sure it's in a realistic timeframe. Realisticly it might not be a 2.6.0 priority, but a slow transition towards this would be great.

Intelligent frame prefetching on multicpu/core.
**) Proposal 1 (simple)
Make async interface, where frame generation is handled by a second thread.
Right before returning frame X to the application the second thread can immediately begin producing frame X+1. I don't think this will help vdub much, but other apps are not as intelligent.

**) Proposal 2 (advanced)
When application requests frame X, generate frame X and X+1 in two separate threads. Avisynth could buffer a number of frames ahead this way. Much more risky, since it can only use threadsafe plugins.

Again not a 2.6.0 priority, but it should be in our plans.


What's left out
*) More than 8bit per component.
*) More managed audio.
*) Your favorite annoyance???!?

Step 1, however, is IMO getting 2.5.6 out the door. I don't know about the new DSS, but otherwise the core seems VERY stable in the latest beta!

Thoughts? And when do we get started? :)

Edit 1: Clarified Masktools.
Edit 2: YUVP -> YV24

[mg262]

*) Y8 planar format.
*) YUV 4:4:4 planar format.
Yes and yes.

MaskTools would be nice, assuming @manao didn't mind and that it didn't e.g. slow down his frequent and useful minor updates. The name clash for Invert would need to be resolved... I think that the one in the core is far less useful than the one in MaskTools.

If the pitch issue has been resolved in such a way as not to break anything it would be nice to have that in... at least I often find that I can't be bothered to put certain things into SSE2 because of the 8/16 issue (but probably just means I'm lazy ;) ).

Incidentally, thank you for organising that post to make it so clear and easy to read.

Edit: one other thing to think about is scripts which require older versions of MaskTools (particularly mfToon)... I can't remember why this is; it may just be because of the removal of a function (YV12layer?) that could just be added back? [I realise that you never said that all the MaskTools functions should necessarily be moved into the core.]

On the thread safety count, if it is added into AVISynth 2.6, a note specifying what new filters should/should not do would be really useful.

As you say, Win64 support seems to be on a different timescale to the other additions, and I think it may raise the difficulty that a relatively small proportion of the current user base is in a position to test it. To me, doesn't seem to fit in with the coherent set of core additions that you suggested... but then I don't have a 64-bit processor, so I'm probably biased!

[Wilbert]

Thoughts?
Sounds great!

I very much would like to see ImageMagick support: http://forum.doom9.org/showthread.php?s=&threadid=90131

[ARDA]

@sh0dan

First of all glad to see you back with many ideas.
More than a suggestion I have some questions that could open a small
change and could maybe drive to some improvement.To point.

Would it be possible to make a smaller core, that means, to compile
many of the filters that are nowadays included, in a separate project
as you have done with directshow.
What for?
In that way any change made over any filter will not need a whole avisynth recompile.
And if some change in the core not obliged to recompile all the filters.That could also
allow many authors to continue its development separately without problems.
I don't know if that could create problems with plugins directory; or any other I dont get
to forsee.
But thinking in the movements to future 64 bits version (now I have a laptop with ML37 turion);
I've thought working over a smaller core should be easier,and step by step recompile filters.
This is something that has been over my head many times, and the poor skills I have dont
allow me to go deeper in this idea.

Anyway any improvement and changes are always welcome, including bugs that makes
things more interesting.

Once more thanks
ARDA

[sh0dan]

@mg262: I clarified the Masktool bit. Of course Manao should still maintain complete control!

@ARDA: Some people are already hitting the ~70 plugin limit (imposed by windows DLL limitations AFAIK). So a complete splitup isn't the best. But I see your point - especially when porting is an issue.

[stickboy]

Add MaskTools to the core
If Manano allows, it would make a great addition to the core, since it has now proven itself stable in the field. Should still be a separate plugin with Manao in complete control. Yeah, I agree that in this case it'd be better to simply include it as a separate DLL like DirectShowSource.
@ARDA: Some people are already hitting the ~70 plugin limit (imposed by windows DLL limitations AFAIK). So a complete splitup isn't the best. But I see your point - especially when porting is an issue.Exactly what's the deal with this limit? Is it that 70 DLLs can't be loaded simultaneously? Can AviSynth use a lazy loading scheme?

[Manao]

Hi guys,

What you plan to do would be great. I'll ask Kurosu ( original author of the masktools ) but he should agree.

However, at the moment, I'm rewriting the set of filters, to ease its portability ( linux -> no inline asm ) / maintenability ( avs 2.5 / avs 3.0 -> need a core totally independant from avisynth ). I'm half through that process.

I'll use this opportunity to break some things too. Mainly some filters default behaviors which sometimes aren't especially smart, and perhaps some other things will change ( frame boundary's behavior of edgemasks to help mmxing those which aren't ), or won't be supported anymore ( float dedgemask, special modes that can be emulated with yv12lut, float convolution, ... ), because they clutter greatly the code.

Filters should be able to support 4:0:0, 4:2:0 and 4:4:4 easily.

one other thing to think about is scripts which require older versions of MaskTools (particularly mfToon)... I can't remember why this is; it may just be because of the removal of a function (YV12layer?)It's indeed YV12Layer, which I never supported. YV12Lutxy can do everything YV12Layer was doing, so I removed YV12Layer ( which was buggy anyway ). mf never cared to update its script, but I can bug him if it bothers some.

[Didée]

YV12Lutxy can do everything YV12Layer was doing, so I removed YV12Layer ( which was buggy anyway ).
If you would (find the time to) cut out the exact formulas that were used by YV12Layer (seeing is better than guessing), I could quickly put the according LUTs into a custom function that rebuilds YV12Layer through YV12Lutxy.

[Manao]

add : ((y - x) * level) / 256 + x
add_no_cr : ((128 - x) * level) / 256 + x
sub : ((255 - y - x) * level) / 256 + x
sub_no_cr : ((128 - x) * level) / 256 + x
fast : (x + y) / 2 + 1
mul_cr : ((y - x) * level) / 256 + x
mul_no_cr : ((128 - x) * level / 65536 + x
mul_y : (((y * x) / 256 - x) * level) / 256 + x

Explanations :
- x, y : as for yv12lutxy.
- no_cr : chroma processing uses no_cr if (!chroma)

Now, as you can see, some formulas seem plain wrong ( but taken from the code ), which is why I never bothered trying to understand them. But :
* 'fast' creates an offset of 0.5
* mul_no_cr divides two times by 256, instead of one. ( but that's obviously a bug in the C version, since the mmx one seems correct and divides only once by 256 )

[IanB]

Design goal proposals:
*) Full compatibility with existing 2.5 plugins.
*) Full compatibility with API. Meaning 2.5 filters should recompile with new avisynth.h without modification.
*) New features must be implementable within reasonable timeframe.I was thinking about doing a NULL api transformation by ripping all the baked in code out of avisynth.h

Interface version 2 is the current baked in code, interface version 3 would be 100% source compatible but with no baked in code. Later with no baked in code we get to change the way things work internally without anybody needing to know or care or recompile. Also this would open the gate for David's avisynth3 to support version 3 api pluggins without a recompile. Also we should loose all the public variables and implement get/set methods, unfortunatly this is hard to make source compatible i.e. vi.width=640 --> vi.SetWidth(640) but for the get methods we could define suitable textual replacments ie "#define width GetWidth()" perhaps some C++ namespace trickery could get a cleaner result.

Proposed additions:
*) Y8 planar format.
*) YUV 4:4:4 planar format.I was also thinking of a 4:2:2 planar format mapped straight from YUY2. This would make it easy to enlist the resizer core to do the hack work of resampling the chroma. i.e. just provide 444<->RGB and 422<->YUY2 and build everything else from the resizers.

Also we could just decouple the planesize relationships so we internally support any planar format one could dream up. We provide some standard templates like YV12, Y8, YUV24, YUV16 and YUV9 so filter authors can quickly check if they support the current shape. For output we simply Throw an exception if the shape is not well defined, like we do with non-mod2 now. This of course means no more hookey code that assume width/2 or other evil incantations.

Possible additions:
*) Official Win64 support.The killer is what to do with the 1000's of lines of __ASM we have now, this code is the reason avisynth is fast. Perhaps softwire will get their 64 bit support out real soon now. But on a positive note we can at least aim to stop doing all the evil unportable address <-> int assumptions. And it is a chance to try out the C code versions.

Implementation:
...
VideoInfo->IsPlanar() should return true. VideoInfo->IsY8() should be added.

VideoFrame->GetHeight(), GetWidth, GetPitch should all return 0 if a chroma channel is NOT needed. GetReadPtr/GetWritePtr could either return NULL, or an empty buffer of X bytes if there are stupid filters out there.Unfortunatly we don't have a "this is a new video format you do not support" state. So to maintain source compatibilty we might need a VideoInfo->IsPlanarEx() to get to the new formats, i.e. IsPlanar() only returns true for YV12 and maybe throws an exception for extended planar formats.

It is probably cleanest to just Throw exceptions when an old filter tries to process new formats, this forces the user to knowledgably "cast" the format appropriatly.

16 byte alignment
Already prepared AFAICT. We can push this in a 2.6 upgrade.If we get rid of the baked in code this becomes much easier. The upcoming implementation is really a bag on the side, chroma 16/luma 32 just so the baked in code is kept happy.

I was also thinking of including diagnostic filters that randomises and test the alignment relationship so we can QA suspected plugins for illegal assumptions.

Thread safety.
Already done AFAICT. Guess we owe TSP some thanks - now it just needs to be compiled into all filters. "2.6" would be the perfect excuse.I have parked this in a branch pending a 2.5.7/2.6 release. Again the baked in code makes this ugly to implement.

Step 1, however, is IMO getting 2.5.6 out the door. I don't know about the new DSSI had hoped to be doing release candidates by now, the main core is ready, however I have been sucked into the DirectShow quicksand, it is proving very chalanging making it come to heal. (Bad dog!) But it should be worth waiting for. I hope to have it house trained by next weekend.

Also the rumblings about YUY2 <-> YV12 conversions, I am currently leaving it alone. I will probably do an interim 2.5.7 maintenance release to put that out (along with any latent bugs).

IanB

[mg262]

unfortunatly this is hard to make source compatible i.e. vi.width=640 --> vi.SetWidth(640) but for the get methods we could define suitable textual replacments ie "#define width GetWidth()" perhaps some C++ namespace trickery could get a cleaner result.

I I think this probably isn't the right solution in this case, but it's an interesting idea so...:



There is the option of having a function vi.Width() that allows the following sort of code to be written:

i = vi.Width();
vi.Width() = 340;

Which works by returning an object that behaves like a reference ... and i.e.

Pseudowidthreference Width();
Const_Pseudowidthreference Width() const;

And Const_Pseudowidthreference and Pseudowidthreference are built in such a way as to perform the appropriate get/set behaviour.

[MfA]

Would be nice if each frame had a pointer for some custom data.

[IanB]

Also there is stuffing with operator = And there has to be a balance between source API compatibility and devious unmaintainable implementation.

Keep the ideas flowing :D

IanB

[MfA]

That's cryptic. I can see how the data needs a destructor, but the operator = doesn't do a deep copy right? Just add add a class definition for custom data, with a pointer to the data, an ident string and a destructor. Add a pointer for custom data to the videoframe. Null the reference in the videoframe smartpointer init. If custom data is present call the destructor in release if reference count drops to 0. Pretty straightforward.

(I want to make some HDR filters, and string them together ... could always just store a pointer in the actual buffer I guess, fugly but it works.)

[IanB]

@Mfa, Sorry the reference was to Mg262's post, but I obviously inspired some illumination. ;)

Many moons ago I had some thoughts on the per frame user data issue, I concluded a simple pointer wasn't adequate (pluggins would fight for it). I think I proposed some sort of simple keyed access i.e. value=frame->GetUserData(key); frame->SetUserDate(key, value);

IanB

[Bidoche]

*) All planes delivered to filters are 16 byte adress aligned and have mod 16 pitch.
On that point, I would like to suggest to go one step further and add guards on the memory block.
That is to say extra space before the 1st scanline and behind the last one.

The point is to allow to make simpler code that doesn't have to worry about edge conditions.

For example, the asm version of RGB32 to RGB24 process 4 pixels at once, ie it reads 16 bytes and writes 12 in an inner loop.
With a guard of 16 bytes it could be done without handling the 3 edge cases.
At worst it would output 9 bytes of garbage, but the guard would guarantee its safetiness.

It's a change I am currently considering for 3.0.

YUV 4:4:4 planar format.
Similar as Y8. I haven't been able to find any official name for this format. Proposed name 'YUVP'. Easier to distinguish from YUY2 than 'YUV'. 'P' is for planar.I am using 'YV24' in 3.0.
If a consensus is found for another name, I will change it.

[Bidoche]

Also we could just decouple the planesize relationships so we internally support any planar format one could dream up. We provide some standard templates like YV12, Y8, YUV24, YUV16 and YUV9 so filter authors can quickly check if they support the current shape. For output we simply Throw an exception if the shape is not well defined, like we do with non-mod2 now. This of course means no more hookey code that assume width/2 or other evil incantations.My solution in 3.0 is these methods :
//inside ColorSpace class (but it can be inside VideoInfo in 2.5)
char * GetPlaneList() const; //each char identifies a plane, '~' alone for interleaved
bool HasPlane(char plane) const;
void ToPlaneDim(int& x, int& y, char plane) const; //converts frame dimension to plane dimensionThen everything should rely on these and never make assumptions.

[mg262]

I am using 'YV24' in 3.0.This is not a major point, but it's not very clear whether YV24 means 4:4:4 or a 16-bit version of YV12... or at least, I wasn't sure until I read this post.

[sh0dan]

I had hoped to be doing release candidates by now, the main core is ready, however I have been sucked into the DirectShow quicksand, it is proving very chalanging making it come to heal. (Bad dog!) But it should be worth waiting for. I hope to have it house trained by next weekend.

Yeah - I remember implementing audio and seeking in DSS into the original code. Lot's of guessing and strange behaviour.

Could it be a solution to release 2.56 with the unmodified DSS (with the VFR fix) - after all 2.56 is miles better than 2.55, and DSS is easy upgradeable anyway.


Unfortunatly we don't have a "this is a new video format you do not support" state. So to maintain source compatibilty we might need a VideoInfo->IsPlanarEx() to get to the new formats, i.e. IsPlanar() only returns true for YV12 and maybe throws an exception for extended planar formats.

Not sure I get this. 2.5 compiled plugins will only work with YV12, but if they are provided with an updated avisynth.h it should be able to return true for all 2.6 supported plugins.
Old (recompiled) filters checking isYV12() will (naturally) only support YV12. Old (recompiled) filters checking isPlanar() should process new formats - which should be ok.

Generic planar support sounds like a good idea. Y8/YV16/YV24 sound like good ideas. Using the resizer for chroma upsampling seem like a good idea. One concern. How do we indicate chroma placement on arbitrary planar formats?

The reason I proposed sticking with the baked code, and fixed formats is that it is a change that I could do myself. It's quick - for us to implement - and for developers to adopt. I would work - even if it's suboptimal.
It would make a great update for 95% of all users and developers. Basicly it's a "0.x" update, and not a "x.0" update.

I'm justifying the conservative thinking by trying to do something that we can do in a forseeable future.

[Mug Funky]

while on the subject of masktools... is it safe to use on HT systems? the version i'm using (not sure if it's the latest - the docs say version 1.4.16) doesn't work on hyperthreaded P4's. i've disabled HT on my machine, but there's other machines here that are often needed, and turning it off on all machines isn't a good option (and it does give a legitimate speed gain when encoding and using Spruce at the same time, as the latter uses different instructions to the former).

i'm all for merging it into avisynth, and Y8 looks exciting too. i guess i just want to be sure that it'll continue to work on HT machines.

btw, the new resizers are awesome. really good idea.

[Manao]

The crash with HT systems was due to UPX ( which I use to compress the dll ). Now that I don't use UPX anymore, it doesn't crash.

Version 1.5.8 (http://manao4.free.fr/MaskTools-v1.5.8.zip) should be 'HT safe'.

[IanB]

Not sure I get this. 2.5 compiled plugins will only work with YV12, but if they are provided with an updated avisynth.h it should be able to return true for all 2.6 supported plugins.
Old (recompiled) filters checking isYV12() will (naturally) only support YV12. Old (recompiled) filters checking isPlanar() should process new formats - which should be ok.The problem is we only have 1 planar format, YV12, and unfortunatly there abounds a large use of isPlanar() when it should have been isYV12(). So as a concession to mild stupidity I propose a safe alternative, isPlanarEx(). Of course by doing so we penalise those who have used isPlanar() correctly. Safety or functionality? :confused:

Generic planar support sounds like a good idea. Y8/YV16/YV24 sound like good ideas. Using the resizer for chroma upsampling seem like a good idea. One concern. How do we indicate chroma placement on arbitrary planar formats?For the RGB<->4:4:4 and the YUY2<->4:2:2 we "declare" there is no relocation! As the resizer core also does a great job as a subpixel shifter, internally there is no problem supporting arbitary chroma placement, the tricky bit is forming an easy to use set of script verbs that expose the functionality in an understandable simple fashion.

The core reason for pushing generic planar is that it forces the lazy assumption issue with filter authors, basicly they won't be able to get away with it. :devil:

The reason I proposed sticking with the baked code, and fixed formats is that it is a change that I could do myself. It's quick - for us to implement - and for developers to adopt. I would work - even if it's suboptimal.
It would make a great update for 95% of all users and developers. Basicly it's a "0.x" update, and not a "x.0" update.Taking the baked in code out is trivial, it's just changing all the code to declarations and pasteing the code into another core source file. The not so easy bit is getting rid of the public variables.

IanB

[mg262]

Safety or functionality?In this particular case, I'm very much for IsPlanar rather than IsPlanarEx... but is there an easy way of checking e.g. the set of filters on WalkedEnterprises for IsPlanar calls?

[sh0dan]

The problem is we only have 1 planar format, YV12, and unfortunatly there abounds a large use of isPlanar() when it should have been isYV12(). So as a concession to mild stupidity I propose a safe alternative, isPlanarEx(). Of course by doing so we penalise those who have used isPlanar() correctly. Safety or functionality? :confused:

I don't think the safety issue is that big. I seem to remember being quite careful myself only using isPlanar, where it applied - a bug or two might show up, but nothing major. I also haven't seen anyone using isPlanar in plugins, where it did not apply. Furthermore, since plugins will need to be recompiled to have extended planar support, I think we are reasonably safe, since it will be tested anyway.

We actually have plenty of values to do with in "pixel_type". We could have chroma placement as extended information. Using the current framework:


enum {
CS_BGR = 1<<28,
CS_YUV = 1<<29,
CS_INTERLEAVED = 1<<30,
CS_PLANAR = 1<<31
};
// Chroma placement bits 24 -> 27
enum {
CS_UNKOWN_CHROMA_PLACEMENT = 0 <<24;
CS_MPEG2_CHROMA_PLACEMENT = 1 <<24;
CS_MPEG2_CHROMA_PLACEMENT = 2 <<24;
CS_YUY2_CHROMA_PLACEMENT = 3 <<24;
CS_TOPLEFT_CHROMA_PLACEMENT = 4 <<24;
};

// Specific colorformats
enum { CS_UNKNOWN = 0,
CS_BGR24 = 1<<0 | CS_BGR | CS_INTERLEAVED,
CS_BGR32 = 1<<1 | CS_BGR | CS_INTERLEAVED,
CS_YUY2 = 1<<2 | CS_YUV | CS_INTERLEAVED,
CS_YV12 = 1<<3 | CS_YUV | CS_PLANAR, // y-v-u 4:2:0 , planar
CS_I420 = 1<<4 | CS_YUV | CS_PLANAR, // y-u-v 4:2:0, planar
CS_IYUV = 1<<5 | CS_YUV | CS_PLANAR // same as above
CS_YV24 = 1<<6 | CS_YUV | CS_PLANAR // YUV 4:4:4 planar
CS_YV16 = 1<<7 | CS_YUV | CS_PLANAR // YUV 4:2:2 planar
CS_Y8 = 1<<8 | CS_YUV | CS_PLANAR // Y 4:0:0 planar
CS_Y411 = 1<<9 | CS_YUV | CS_PLANAR // YUV 4:1:1 planar
};
int pixel_type; // changed to int as of 2.5

Code is supposed to use vi.IsSameColorspace(VideoInfo vi2) anyway.

Taking the baked in code out is trivial, it's just changing all the code to declarations and pasteing the code into another core source file. The not so easy bit is getting rid of the public variables.
The first part sounds very reassuring. I wouldn't have any idea how to do either one. ;)

Edit: Y410 -> Y411

[Wilbert]

CS_Y410 = 1<<9 | CS_YUV | CS_PLANAR // YUV 4:1:0 planar
Y410 (YUV9) is YV12 evil squared, no? Since Indeo seems to be the only codec which uses it, i propose we add support for 4:1:1 instead :)

[sh0dan]

Y410 (YUV9) is YV12 evil squared, no? Since Indeo seems to be the only codec which uses it, i propose we add support for 4:1:1 instead :)
Yeah - meant 4:1:1. :) Thought it might be useful, as I saw an OS DV codec has been done - with support for NTSC.

[IanB]

Please excuse me for this isPlanarEx() rant, I was just having a blond moment.

For old pluggins we only need to make sure the old baked in isPlanar() code only returns true for YV12. The new 2.6 version of course can still be called isPlanar() and work as intended. The implementation is simple, we just need to reclassify the bits so that old use does not conflict with new concept.

Specifically the old baked in code is :-
bool IsPlanar() const { return !!(pixel_type & CS_PLANAR); } we cannot change this, it is already compiled into every plugin. This current definition CS_PLANAR = 1<<31 is is also compiled in. So in the 2.6 code bit 31 must only be set for YV12 thus CS_PLANAR must be assigned a new bit and bit 31 must become reserved for legacy YV12 planar.

This 1 simple example I hope shows why baked in code in API definitions is such a bad idea and why I am so keen to fix it. Fiddling with bit definitions and having legacy restriction on there values really bites down the track. If the definition for isPlanar() had been opaque and the code in avisynth.dll instead of in all the plugins this would not have been an issue.

================

I am no sure packing chroma placement information into pixel_type is particularly clever. If I put on a Java hat for a second the answer becomes obvious, make it a class, that is always the answer in Java. And again the implementation should be opaque. We need 8 methods <get,set>Chroma<U,V>Placement<X,Y>(). At a more abstract level we can have precanned definitions like Mpeg1, Mpeg2, DVntsc, DVpal, etc. Internally I would lean towards float or binary fraction as the representation. It could be as trivial as 5 fixed value set but if it's opaque it doesn't matter it can be changed, transparently!

Also latest VDub's seem to directly supports DV natively.

IanB

[squid_80]

Speaking of baked in code, could the assign operator for AVSValue be changed so it's a bit more architecture friendly? It's not really a big problem since plugins need to be recompiled anyway for win64 but it would help if I didn't need to change avisynth.h all the time.

[IanB]

@Squid,

Exactly! Hence the priority to stop using baked in code. If it were just a declaration in avisynth.h, then the code is whatever it needs to be today inside avisynth.dll. If the assign operator code was not bake into avisynth.h, hence all plugins, then implementing your win64 and also things like 64 bit integers and double floats in the script language would be an absolute no brainer.

IanB

[MfA]

BTW Im a bit fuzzy on the plan for 2.6 now ... is it to be binary compatible or source level compatible with existing plugins?

[IanB]

@Mfa, for my money,

2.6 will load and accept 2.5 plugins binaries. New features (eg colour spaces) may not be available. Possibly exceptions like "2.5 plugin does not support 2.6 feature".

2.5 plugin source will compile with 2.6 avisynth.h, may get increased functionality/bugs, might need minor code tweak (hopefully not). Will need new code to use new features.

Sorry 2.5 can't load 2.6 plugin binaries.

My opinion only! IanB

[sh0dan]

@IanB: Regarding compatebility, I completely agree.

As to changing the planar bit, we can just wait and see if it is needed. I don't think so, but the nice thing is that we can always do it, if code isn't baked anymore.

I going away for a few days from tomorrow, but I look forward to getting back, as I think this a a very productive and positive discussion!

[MfA]

Hmm, okay ... so after a little googling I see that means no new data members, that's a shame. Maybe a good idea to use the d-pointer/pimpl trick to sidestep this next time?

[mg262]

2.6 will load and accept 2.5 plugins binaries. New features (eg colour spaces) may not be available. Possibly exceptions like "2.5 plugin does not support 2.6 feature".I don't follow here... under what circumstances would a 2.5 plug-in possibly try to use a 2.6 feature?

Sorry 2.5 can't load 2.6 plugin binaries.I think it would be a little unreasonable of us to expect that!

Incidentally, I hope that it is generally apparent, but just to state it specifically: everyone does really appreciate the work you are putting into this.

[Leak]

I don't follow here... under what circumstances would a 2.5 plug-in possibly try to use a 2.6 feature?
I think he meant that an exception like that would have to be thrown if you'd try something like feeding an "old" filter one of the proposed new colorspaces...

np: Galoppierende Zuversicht - LIVE @ [KONTROL] San Francisco - June 18 2005

[mg262]

Ahhh... I see. On that note, and considering this:
What's left out
*) More than 8bit per component.
... perhaps it may be worth adding a (pure) virtual function like Is8BitPerComponent (currently implemented to always return true), to avoid similar problems recurring in the future?

In particular, it would be nice to be able to write filters that work on any planar colour space with eight bits per component (which essentially means filters that only process luma).

[MfA]

Many moons ago I had some thoughts on the per frame user data issue, I concluded a simple pointer wasn't adequate (pluggins would fight for it). I think I proposed some sort of simple keyed access i.e. value=frame->GetUserData(key); frame->SetUserDate(key, value);
Could add this useing a slightly fugly hack, by adding an extra pointer at the end of the data in vfb (luckily the constructors are implemented outside of the header). Even useing GUIDs to allow multiple user data pointers to be associated with a videoframe it would be a relatively minor change though.

Any chance such a thing would be accepted?

[IanB]

@Sh0dan,

> As to changing the planar bit, ... if code isn't baked anymore...
Only catch is the ENUM definition is still likely to be baked, if we remove it we loose source transparency, but users should NOT be using these bits they should be using the isXYZZY() functions. How mean do we feel?

@mg262,

That sort of idea is on the right wavelength. ;)

@MfA,

Oh no, must not add any public data; must add virtual methods that are implemented opaquely.

IanB

[MfA]

You misunderstand me, I know I cant add data members ... but I need to have a way to associate the videoframebuffer with the userdata area, enlarging vfb->data and storing a pointer at the end seems the easiest route and is completely transparant to existing code (I can think of one other way, but that is even fuglier, and would take more code).

BTW, I have no experience in the matter ... but when I googled for C++ binary compatibility I came upon a list of dos and donts from the KDE folks. They say adding new virtual methods is a big nono. On the face of it that seems reasonable to me, virtual function table is presumably baked in as you say ... new methods just add an extra signature which can be linked againsts by new code right? (Or does it work differently with DLLs?)

They also suggested always adding a d-pointer/pimpl to classes like this for future expansion.

[IanB]

@MfA,

No, I have not misunderstood. Class public data members are anything but opaque. It would be easy to add them but I am taking a very strong stand against any non-opaque addition to the Avisynth API. To get what you want we would likely expose public methods, the code for which is hidden in the avisynth.dll.

I don't think you would really want data associated with VFB's. More likely you want it associated with a PVideoFrame. Methods like Get/SetUserData() with some sort of identifier key and a void* pointer to the data and maybe a size would probably fit the bill. Incidently VFB's should really have been invisible at the API level.

And yes what we are planning is a bit naughty in that we have to know how the compiler implements classes in memory so that the bits we add are at the end of structures so the old baked code won't notice anything has changed.

IanB

[MfA]

It isnt that I was suggesting that filters directly access the userdata directly through the pointer, you just need some way to associate the user data with the videoframe object. The entirety of PVideoFrame is implemented in the header, can't do nothing with that. Usually you would store a pointer in the private data of VideoFrame, but since you are trying to retain binary compatibility and that is not possible just tacking it onto vfb->data seems the easiest route. You either need to store the data with the object, or have some identifier for the videoframe object which you can use with a lookup table to find the user data (and the only identifiers available are pointers, and that is far fuglier than adding it directly to vfb->data).

Sure in principle the vfb->data pointer is readily exposed to filters, it is private though, but the syntactic suger in C++ never really hides anything ... privacy is a matter of manners in C++, nothing more.

Why do you want to implement it with virtual functions? Normal functions can be added without breaking compatibility without needing any tricks, and the implementation can still be in avisynth.cpp so they are still opaque/non-inlined. Implementing it with virtual functions seems a little roundabout ... when would you ever want to inherit VideoFrame and override those functions?

[mg262]

I think we ought to leave sometime for thought and discussion before throwing in the associated-values. There are several issues to resolve. For example, what about namespace clashes? Are we going to give each DLL its own namespace or are they all going to share one? If the former, does a DLL have a mechanism to access information from the namespace of another DLL? Etc.

@MfA,

Could I ask what in particular you are intending to use the associated values for?

[Bidoche]

Dlls should each use their own namespace, but even if they don't, why would it clash ?

[mg262]

Sorry, I meant namespace for keys. So if two different plug-ins both try and attach a value with a a key called, say, "size" to a frame then are they accessing the same value or different values?

[MfA]

Could I ask what in particular you are intending to use the associated values for?HDR filters.

As I said, Id use a GUID for a key ... a little less user friendly, filters who want to cooperate need to get the appropriate GUID from some higher level, but no chance of interference.

[Bidoche]

No need for string or guids for keys.

just :struct Key { };

Key myPluginKey; //inside my plugin somwhere, use &mypluginKey as keyThat's the simplest approach

[IanB]

@MfA,

By implementing it with virtual functions and then deriving new classes we can support any differing requirements with different child classes. It's about providing the maximum scope for the future and it needs to be analysed on a case by case basis, for some cases normal methods may be preferable to virtual methods. If done correctly then 2.6 api plugins should be loadable in any future version. Don't let my forthright opinions stop you pushing the envelope, only by chalanging ideas can we beat bugs out of them before we code them.

@mg262,

There are many methods for both avoiding and allowing collisions, i.e the way HANDLES work in windows, note I'm not saying we need something this perverse, but it is an example. The 2 cases I see are A) 2 separate instance of the same filter each wanting their own unique data set and B) a set of filters that want to pass data tags up the GetFrame chain between themselves. And of course there is the amalgum of the 2. Things like the "this" pointer make good unique values. And the environment variables provide a method for sharing key information.

IanB

[Bidoche]

Here are what I use (simplified) in 3.0 :class Key
{

public: //structors

Key() { }
virtual ~Key() { }


public: //Key interface

virtual bool operator==(Key const& other) const { return &other == this; }

};

class Property
{

public: //structors

Property() { }
virtual ~Property() { }


public: //Property interface

virtual PKey GetKey() const = 0;

};both handled after with smart ptrs PKey and PProperty

This can handle your A) and B)

[mg262]

This all seems very sensible. But I was trying (not very well) to say this: sh0dan very sensibly put forward a set of additions for 2.6 which it was very clear how to implement, and this one, much as I would like it, seems to me to need a lot more thought and clarification than the others. Namespaces were meant to be an example.

Or is it that they are coming up now not because we want implement them immediately, but because we want to make sure we don't break anything in doing so (which to some extent is something that can be applied to any feature)?

[MfA]

IanB : Well VideoFrame is an everything and the kitchen sink object, every type of videoframe you want is enumerated and stored in there ... it's as anti-inheritance as it gets, seems futile to start adding virtual functions now. If in the future the needs change, add a new enum ;)

Anyway, an extra lookup doesnt matter much I guess (using the this pointer of the VideoFrame object as an index). all that pointer chasing which will be necessary to search the user data through a dynamically sized two dimensional lookup table isn't pretty, but compared to the actual work a filter does it isnt really relevant.

mg262, the changes to the API are relatively small for what I ask. Just two new public methods for VideoFrame, using something like Bidoches classes it would be something like void AddProperty(PProperty) and PProperty GetProperty(PKey). Now Im not big on OO design, but I dont think it should be too hard for those who are to agree on the interface (and implementation is relatively easy).

[mg262]

If it's just interface, then that seems very sensible to me!

[foxyshadis]

(Love the sideband data push, even if it might have to get shelved for 2.7.x it's still awesome.)

I'm not sure if this should be a 2.5.x or a 2.6 request, but I was wondering if changing the cropping options for resize filters was on the map. The right/bottom parameters aren't currently compatible with the negative offsets regular crop can use.

Edit: I just noticed that it was committed to 2.5.6 CVS shortly before my request. Sorry.

[Richard Berg]

I just noticed that Avery Lee made a very extensive post on a next-gen filter interface (http://www.virtualdub.org/blog/pivot/entry.php?id=64#body). I think you guys need to talk to him -- not only is he a really smart & helpful video guy, but if both platforms are going to be changing at the same time then it makes sense to build in better compatibility. After all, most AVS users still use VDub every day, and vice versa.

MS keeps me pretty busy, but let me know if there's anything I can do to help -- if you find bugs in DirectShow, for example, that helps us too. (I work on Visual Studio, so developers are literally my customers). I think you'll find the C++ features in Whidbey (http://msdn.microsoft.com/visualc/whidbey/default.aspx) are much improved, even if you don't use C++/CLI.

[sh0dan]

I created a branch for 2.6. Called "avisynth_2_6":

http://cvs.sourceforge.net/viewcvs.py/avisynth2/avisynth/src/core/?sortby=date&only_with_tag=avisynth_2_6
(New files not reflected there yet)

I committed my first changes:

Added preliminary general planar support:
- Formats added to avisynth.h.
- NewVideoFrame can produce general planar images.
- PVideoFrame should return correct values for rowsize, pitch, height.
- Cache behaves correctly on new format.
- All code assuming UVwidth = Ywidth/2 an similar should be gone.
- So far old plugins behave nicely!

- SubFrame is BROKEN! It should be replaced by a bitblit.

Added SSE3 to CPU detection. Not tested.


The changes are mainly to be able to get started on the filter work, which is my strongest side. Assuming there will be no changes to the avisynth.h interface from a filter POV we can always remove (new) the baked code before beta 1.

[tsp]

I made some changes (http://forum.doom9.org/showthread.php?p=682790#post682790) to make avisynth more threadsafe. You might want to add some of those.

[ARDA]

bool sse3sup()
{
bool sse3prt=false;
_asm{
pushad
sub eax,eax
cpuid

mov eax,1
cpuid
and ecx,00000000000000000000000000000001b //bit0
jz noexistesse3
mov [sse3prt],1
align 16
noexistesse3:
sub eax,eax
cpuid
popad
};
return sse3prt;
}

I didnt look your code but this I ve tested and I think it works ok
I hope this could be useful
Luck with all this new work. ARDA

[IanB]

- SubFrame is BROKEN! It should be replaced by a bitblit.I assume you mean SubFrame can cause non aligned data, this is not broken, just unfortunate. Doing bitblits to correct alignment is a really bad thing because they are slow, about 5 orders of magnitude slower than passing a pointer. Or is it about the existing baked code having YV12 only assumptions, hence my isPlanar() bit shuffle.

I had a thought about introducing virtual edges to the API where frames can have junk around the edges to keep the alignment and virtual width and height. Would also mean all the power of 2 width/height restriction could just go. Only filters that care to notice the the true width and height need make of it. From the core point of view, the output blitter will need to know and I guess the resizers could make efffective use.

IanB

[sh0dan]

I assume you mean SubFrame can cause non aligned data, this is not broken, just unfortunate. Doing bitblits to correct alignment is a really bad thing because they are slow, about 5 orders of magnitude slower than passing a pointer. Or is it about the existing baked code having YV12 only assumptions, hence my isPlanar() bit shuffle.

It'll get more and more messy with various formats, to adjust offsets for each plane. From the top of my mind I can only think of crop that is widely used, which uses SubFrame. I think it'll be cleaner for the future if we just drop this and use blits instead.

@ARDA: Looks like the code I put in. Code is here:
http://cvs.sourceforge.net/viewcvs.py/avisynth2/avisynth/src/sources/avi/cpuaccel.cpp?view=log&rev=1.3&sortby=date&only_with_tag=MAIN

@tsp: I thought the thread safety had been implemented already, but I see it hasn't. I'll apply it ASAP.

@all: Just saw I accidently committed the code to MAIN. I'll correct this back when I get home tonight. Sorry.

[bill_baroud]

sorry if this sound dumb, but while i'm reading your very interesting discussion about the design of avisynth, there is one thing i don't understand : what's the "baked code" ? I'm pretty sure that a litteral translation can't apply here :)

[tsp]

sh0dan: The source changes I made include both the neccesary changes to make avisynth threadsafe (mainly in the avisynth.cpp, cache.cpp and internal.h) and a filter to actually run avisynth multithreaded (although it's not completly done yet). I don't know if you want both thing in 2.60.

[IanB]

sorry if this sound dumb, but while i'm reading your very interesting discussion about the design of avisynth, there is one thing i don't understand : what's the "baked code" ? I'm pretty sure that a litteral translation can't apply here :)Code that is include inline. In this all the filthy code in avisynth.h. The result is all that code is "baked" into all and every plugin instead being called from avisynth.dll.

The problem with baked in code it cannot be changed without recompiling the plugin. If it were just a declaration only then the active code would be whatever is in the current avisynth.dll.

IanB

[mg262]

From the top of my mind I can only think of crop that is widely used, which uses SubFrame.
I thought SeparateFields did as well... yes it does. (Does that mean that a filter is not allowed to trash values all the way out to pitch? How far beyond the width is illegal to access? Only up to PLANAR_Y_ALIGNED rowsize?)

Perhaps one possibility would be to add an optional align parameter to GetFrame? (It would be a done by adding a separate function call with three arguments rather than actually as an optional argument.) So the default implementation would call the existing GetFrame function and then copy if necessary.

[IanB]

... SubFrame()... I think it'll be cleaner for the future if we just drop this and use blits instead.The current implementation of SubFrame() is what I call zero cost, in practical terms, it's more than 100,000 times faster than a blit. So I feel it would be a great disservice to abandon this model. Besides the arithmetic for the arguments to BitBlit() is as near as damnit the same as for the SubFrame() arguments.


Also as part of the virtual width/height idea, I was thinking of a filter pair to unfold and refold fields side by side like the filter in VirtualDub, only to make them zero cost by just rejigging the width, height and pitch. Minor drawback would be a junk bar down the middle, mostly 0-15 bytes, occassionally more. This could be a very fast way of processing field data semi-independantly.unfold()
width+=pitch;
pitch*=2;
height/=2;

refold()
pitch/=2;
height*=2;
width-=pitch;How far beyond the width is illegal to access? Only up to PLANAR_Y_ALIGNED rowsize?)You may safely access up to pitch (note: pitch may vary from frame to frame). The canvas is height*pitch+(a little bit), but don't ever count on the +(a little bit).

IanB

[mg262]

You may safely access up to pitch (note: pitch may vary from frame to frame). The canvas is height*pitch+(a little bit), but don't ever count on the +(a little bit).IanB
Thank you.

It has been said before, but it would be useful to be able to count on that little bit (and perhaps a little bit at the start) -- perhaps pitch bytes of leeway.

[sh0dan]

I fixed the CVS issue now. ViewCVS still isn't updated, but be should be back on a clean MAIN, and changes in the 2_6 branch!

IanB - It reassures me that you say that it's easy to maintain - Great! I can also see that it has been added to ScriptEnv - good idea! Let's keep it!

I hope to get some more work done tonight. Hopefully a converter or two. :)

[MfA]

It has been said before, but it would be useful to be able to count on that little bit (and perhaps a little bit at the start) -- perhaps pitch bytes of leeway.Id agree, but just go whole hog then and add an entire extra line before and after ... having to do prologue and epilogue code sucks.

Hmm, when working with blocks a single line wouldnt be enough though ... what I really want is variable padding I guess. Would be nice for a filter to be able to tell avisynth to provide a padded/border-extended version of a frame without having to DIY it.

[sh0dan]

Id agree, but just go whole hog then and add an entire extra line before and after ... having to do prologue and epilogue code sucks.

Hmm, when working with blocks a single line wouldnt be enough though ... what I really want is variable padding I guess. Would be nice for a filter to be able to tell avisynth to provide a padded/border-extended version of a frame without having to DIY it.
Border issues will always exist. You'd also have to duplicate the outer pixels - otherwise you are just reading junk. The overhead for such a solution is far too high.

[Manao]

Border issues will always exist. You'd also have to duplicate the outer pixels - otherwise you are just reading junk. The overhead for such a solution is far too high.It depends. If every frame are surrounded by a 16-pixels-wide margin, then a filter which needs padding could do it itself, without having to copy data.

Several advantages :
* the filter can use the padding it needs ( zeroes, mirror, classic padding... )
* no overhead if you don't use it
* no more border issues, at a very small speed penalty, since you don't copy. That would _greatly_ simply code for convolutions & co, where, quite frankly, borders are a pain in the a...
Two drawbacks I can think of :
* more memory used
* copy won't be as fast ( since the bitblt will never reach the condition width == pitch ). However, that condition can be changed into pitch = pitch, if 32 bytes / lines added don't cost too much.

For SubFrame : it must be kept : you lose 16 byte alignment, but doing a copy cost more. It's up to the filter's devs to write too versions of their SSE2 code : one aligned and one not aligned ( it'll still be faster than C code, though it might be slower than the mmx one ).

[fisix]

I was also thinking of including diagnostic filters that.... IanB very useful. helps plugin development if they are general enough, and could be used as a standard for testing core changes from version to version. would be nice to have a master script that could be used to test everything... validation is important but hardly ever done; an easy, built in tool would be nirvana.

[sh0dan]

I've implemented and tested basic support. I use Overlay for conversions right now.

New commit:

Fixed bugs in general planar support:
- Overlay can convert to/from YV24 and Y8 (used as simple converter)
- Resize handles arbitrary resizes.
- Fixed SubFrame to new formats. Since VFB now hass pixel_type it is able to calculate missing values.
- ColorYUV works on all formats
- Blur/Sharpen works on all formats
- Crop works on all formats.
- Levels, temporalsoften, turn confirmed working.

Note that Vdub 1.6.10 supports Y8 out of the box - and it works nicely.

To convert something using overlay, use: overlay(last,last,output="Y8"). As noted Y24 and Y8 is currently supported. Changes to filters are mostly trivial.

[Wilbert]

I'm trying to read avisynth.h a bit :)

CS_YV411 = 1<<9 | CS_YUV | CS_PLANAR, // YUV 4:1:1 planar
For DV, it's an interleaved format, not planar :) The sampling is as follows:

In 4:1:1 systems (NTSC DV & DVCAM, DVCPRO) the color data are sampled half as frequently as in 4:2:2, resulting in 180 color samples per scanline. The Cr and Cb samples are considered to be co-sited with every fourth luma sample.
So, it's actually the fourth, not the first luma sample. I guess Avery can confirm this.

case CS_I420:
case CS_YV411:
return 12;

http://www.adamwilt.com/DV-FAQ-tech.html#colorSampling

[ARDA]

Originally Posted by sh0dan
@ARDA: Looks like the code I put in. Code is here:
http://cvs.sourceforge.net/viewcvs....y_with_tag=MAIN


Thanks I'll look


Originally Posted by mg262
I thought SeparateFields did as well... yes it does. (Does that mean that a filter is not allowed
to trash values all the way out to pitch? How far beyond the width is illegal to access?
Only up to PLANAR_Y_ALIGNED rowsize?)


Without understanding I've pointed this question 23rd February 2004, 13:06
http://forum.doom9.org/showthread.php?t=71452

Here I've pointed the problem with separatefield when modifying LumaFilter and change it by LumaYV12
http://forum.doom9.org/showthread.php?p=673168#post673168


Originally Posted by IanB
Doing bitblits to correct alignment is a really bad thing because they are slow, about 5 orders of magnitude slower
than passing a pointer.



Originally Posted by sh0dan
It'll get more and more messy with various formats, to adjust offsets for each plane.
From the top of my mind I can only think of crop that is widely used, which uses SubFrame.
I think it'll be cleaner for the future if we just drop this and use blits instead.


I will just refer to separatefields cause I've been fighting with it when I tried to improve LumaFilter,
and trying to do version of different filters that could work with area frame(pitch*height)
Blits are slower than passing a pointer and modifying pitch size; the magnitude depends on which machine
you're running, blit is optimized for amd machines, and doesnt have a good perfomance in some cases
for P4 architecture,and I dont say Intel chips, just P4.I have tryied some time ago to develope a version
for such architecture but I was unable cause my poor skills and some other problems.

My benchmark tests tells me that a blit in separatefields is usefull if you have a big filter chain
before weave, using two real smaller memory areas improves perfomance in any filter you have in that chain.

Finally my sugestion is to include two arguments in separatefields,aligment(bool) and value(int), why two?
First one is obvious to move to different areas, second to define the boundary aligment, if nowadays we write 16
we'll have chromas 8 alignment, so we could have the option to make 32 so we should be able to have
chromas in 16 alignment(usefull for sse2), probably this could be avoided when finally this alignment question is finished.

I've modified for my own use a little bit SeparateFieldsYV12 introduced by MarcFD in MPEG2Dec3 0.94
http://ardaversions.iespana.es/SeparateFieldsYV12.cpp
I hope part of this code could be usefull to open an option with new argumentes

Thanks all for your attention.ARDA

[mg262]

Is there a way to write a filter that will work on any planar colour space and can iterate through planes?

[MfA]

Border issues will always exist. You'd also have to duplicate the outer pixels - otherwise you are just reading junk. The overhead for such a solution is far too high.As I said it would be nice if the filters could tell avisynth what they needed ... avisynth can examine the entire filter chain and just size the buffers appropriately, and it would only do border extension for the filters which requested it.

Filters written to make use of this would probably end up faster ... pro/epilogue code tends to be branch heavy, is a headache to write and is an extra source for bugs (I have been tempted to just do 2 framebuffer copies and do border extension myself just to avoid this). All needless really.

Realistically speaking this belongs in Avisynth, maybe not avisynth 2.6 though, because it is efficient to do in there while it is inefficient to do it inside the filter.

[sh0dan]

Is there a way to write a filter that will work on any planar colour space and can iterate through planes?
You can actually do that with the existing framework. You simply check if vi.IsPlanar() instead of vi.IsYV12(). Of course you cannot make any assumptions about chroma placement in relation to luma, but if you iterate through the planes individually you should be ok.

The good old Invert sample does this.

http://www.avisynth.org/TwoFiveInvert

@Wilbert: The idea was to do it as a planar format it that it will be a LOT easier to implement internally. All filters supporting general planar should work directly on the new format. 4:1:1 is only a marginally used format (NTSC DV only AFAIK), so requiring a new interleaved format just isn't worth the effort IMHO.

[mg262]

I really meant the emphasis of the question to be on the iterate part rather than IsPlanar. So at the moment, the only option is to write code for PLANAR_Y, PLANAR_U and PLANAR_V. Apart from the fact that it is a little ugly to call PLANAR_U code on Y8 (even if it will work because height is set to 0), this doesn't leave open the possibility of ensuring code works on RGB planar colourspaces. It would be good to be able to write simple filters (e.g. averaging) that will work on any planar colourspace, YUV or otherwise... but that would require being able to iterate through planes.

[bill_baroud]

Code that is include inline. In this all the filthy code in avisynth.h. The result is all that code is "baked" into all and every plugin instead being called from avisynth.dll.
The problem with baked in code it cannot be changed without recompiling the plugin.IanB

Thanks, now i understand why you are hunting that code down :)
Now i'm trying to keep up with the discussion (a pretty interesting one ... i'm facing all those problems in my internship - borders, alignement ...)

[sh0dan]

if (vi.IsPlanar()) {
for(int plane = 0; plane < 3; plane++) {
int p;
switch (plane) {
case 0:
p = PLANAR_Y; break;
case 1:
p = PLANAR_U; break;
case 2:
p = PLANAR_V; break;
}
int w = src->GetRowSize(p);
int h = src->GetHeight(p);
if (! (w && h))
continue;
[..Your filter code...]
}
}

[MfA]

Two drawbacks I can think of :
* more memory used
* copy won't be as fast ( since the bitblt will never reach the condition width == pitch ). However, that condition can be changed into pitch = pitch, if 32 bytes / lines added don't cost too much.Meh, the memory overhead of 16 pixels margin really isnt an issue in this day and age.

The relative CPU overhead compared to any non trivial filters is such that I find it hard to care either. Even with trivial filters the codecs will be eating so much CPU that this isnt going to make a dent in processing time.

A 16 pixel margin is better than nothing, but some filters need even more than that (FFT for instance). So they would still need to do extra copies.

If the filter could just tell avisynth the margins, everything would be peachy. You could have margins as large as you want, it would all be as efficient as possible ... and you could factorize border extension code into avisynth, instead of repeating it in filters all the time. I really dont think this idea is that bad that it needs to be ignored outright.
For SubFrame : it must be kept : you lose 16 byte alignment, but doing a copy cost more. It's up to the filter's devs to write too versions of their SSE2 code : one aligned and one not aligned ( it'll still be faster than C code, though it might be slower than the mmx one ).Lets take a conservative 1000 MB/s throughput, and a 1kx1k RGB frame ... a copy will take you 6 msec.

Dunno about you, but I dont want to write that much extra code to avoid a 6 msec operation. Maybe add a flag filters can use ... a "Im a masochist, please give me unaligned buffers to work with" flag ;)

[Manao]

Lets take a conservative 1000 MB/s throughput, and a 1kx1k RGB frame ... a copy will take you 6 msec.

Dunno about you, but I dont want to write that much extra code to avoid a 6 msec operation. Maybe add a flag filters can use ... a "Im a masochist, please give me unaligned buffers to work with" flagI agree that I spoke a bit to quickly. Yet it's possible to avoid unnecessary copy, if the filter that needs 16-bytes alignment has a way to request it. Why copy and align on the crop if the next filter in the chain must copy the picture in a new buffer anyway, and don't need 16-bytes alignment ?

[mg262]

Maybe add a flag filters can use ... a "Im a masochist, please give me unaligned buffers to work with" flag In practice, that means adding an extra argument to GetFrame? Or something else? (remember GetFrame doesn't know which filter has called it...)

[MfA]

Well, it would be an extra virtual function call for IClip to set the flag ... unfortunately storing the actual flag somewhere is a bit complex since you can't add private data to the existing classes (a d-pointer/pimpl for every class would have been really nice, if down the line the inlined/baked-in code is removed from the header Id suggest adding them ... although if binary compatibility has to be broken, there will probably be a switch to 3.0 anyway).

Id rather you comment on my serious suggestion though :) Although I guess alignment could be rolled into padding/border extension requests.

Something like

virtual void __stdcall SetBufferAlignment(int align, int pad_x, int mod_x, int pad_y, int mod_y, int extend_borders)

Defaults would be 1, 0, 1, 0, 1, 0

Align would give the modules of alignment of the first byte, pad_x/y is used to get padding for stuff like block based filters and mod_x/y is used to get large moduli for stuff like FFT. Extend borders could have a couple of options from an enum ... border repeat, 2 types of mirror and point symmetric (one type with symmetry around border pixel, other with symmetry around point at half-pel away from border) and repetition.

[mg262]

Id rather you comment on my serious suggestion though Although I guess alignment could be rolled into padding/border extension requests.Oh, I think it's an absolutely brilliant idea. I just don't have anything to say about it that hasn't been said already! More specifically, I agree that it would be great, that padding at top and bottom wouldn't add any overhead because it can be entirely ignored by filters, that in most cases padding of any sort has acceptable overhead (assuming filters don't request an unreasonable amount), and that the amount of work needed to put it in means that its unlikely to happen soon.

Incidentally, on the point of allocation, I was recently told that allocating large amounts of memory can be counterintuitively fast, because the operating system can allocate entire pages at once. (I haven't looked into that further myself.)

Edit -- Minor proviso: I don't know what the cache effects would be of e.g. going through the entire frame and initialising all the padding bytes to zero. I would want to check with kassandro or someone else knowledgeable before trying something like that. (Even if it is expensive, that's not to say that it shouldn't be done or that the option of doing it shouldn't be present... but it's something I would personally want to be aware of.)

[MfA]

Talking about cache, the filter should also indicate if it's in-place ...

virtual void __stdcall SetBufferAlignment(int align, int pad_x, int mod_x, int pad_y, int mod_y, int extend_borders, int in_place)

Defaults would be 1, 0, 1, 0, 1, 0, 1

If the filter didnt ask for border extension it simply wouldnt be done ... caching with this method would be irrelevant.

If you just put a 16 pixel border around everything without such an interface you wouldnt really need to 0 the border either, if the filter wants to do border extension it would do it itself (there are usually better ways to extend borders rather than filling with 0s). It's more about the filter being able to know for sure a border is present, that's all that is needed.

[sh0dan]

Optional padding seems like a good idea. It's safe, as long as it's done in the constructor. Requests for smaller padding should of course be ignored. But from then on ALL new frames created should obey the rules.

env->SetPadding(x,y) - default is 0. It will probably be rounded up to mod 8 internally.

Borders are not in any way guaranteed to be preserved filter to filter, as you might be given a copy. If you need modulo X, just add the needed padding.

Borders will NOT be filled in any way. We can expose helper functions you can call by using env->Invoke(). Invoke doesn't insert any cache or other stuff between the called filter and your, so you'll get the frame (with borders).

Alignment on first pixel would make SubFrame have to switch to blits. I nearly got my head chopped off for suggesting that two pages back. This will a boolean value. It's either no alignment guarantee or mod 16 guaranteed.

[MfA]

Oops, of course that should be global and not in IClip ... dunno what I was thinking there.

[IanB]

@Mfa,

No you were sort of right the first time, padding/alignment want to mutate up the GetFrame() chain so each filter gets it's optimum source data, not be global. env->SetPadding(x,y) in no way precludes this. Probably where to interface this in env->NewVideoFrame() so that the next upstream filter directly writes it's output data into an optimised memory structure.

@sh0dan,

For the Y8 format, how would you feel if both CS_PLANAR and CS_INTERLEAVED were set? It is after all a trivial example of both layouts.

IanB

[mg262]

env->SetPadding(x,y)Is this only to apply to new video frames then requested by the filter from the environment? Or does it apply to every instance of Filter::GetFrame called by the filter? Perhaps a filter might want different amounts of padding on different input clips.

I think I may be getting a bit confused about the big picture here...

[sh0dan]

@sh0dan,

For the Y8 format, how would you feel if both CS_PLANAR and CS_INTERLEAVED were set? It is after all a trivial example of both layouts.

No problem!

No you were sort of right the first time, padding/alignment want to mutate up the GetFrame() chain so each filter gets it's optimum source data, not be global. env->SetPadding(x,y) in no way precludes this. Probably where to interface this in env->NewVideoFrame() so that the next upstream filter directly writes it's output data into an optimised memory structure.

I cannot see how you would pass that upstream. A filter the simply returns child->GetFrame(n); doesn't create any new frames. I can't see padding being done any other way than global padding, where one filter requesting padding would affect ALL frames being created.

[mg262]

I cannot see how you would pass that upstream.You could add a couple of arguments to GetFrame, specifying the (minimum) amount of padding that a frame must have -- this would work; the problem is that it requires a very minor rewrite of all existing plug-ins, which is presumably not an option.

Edit: or: calling GetFrame(..., padding) for a filter that only defines GetFrame(...) will call a default implementation of GetFrame(..., padding), which calls GetFrame(...) and then performs a bitblt. [I'm deliberately being ambiguous about where this default implementation should reside... but since it's clear that we can intercept such calls for caching, etc., perhaps a similar mechanism that is binary compatible could be used? Although extending the vtable for IClip in this way may not be a safe thing to do... .]

I don't follow how putting information into the environment will work unless you are willing to look at the call stack... (but I haven't looked from the code for IScriptEnvironment so I have probably missed something).

Edit 2: Sorry, didn't read your message below before I edited...

[IanB]

@mg262, it ain't writen yet, but it seems to me that the object of the exercise is to have child->GetFrame() calls return a frame with requested padding/alignment.

Now filters do 1 of 2 things.

1. They child->GetFrame, env->NewVideoFrame(), transform the src to dst and return the new dst frame.

2. They child->GetFrame, MakeWritable(), in place edit and return the frame. Now remember MakeWritable() tests isWritable() and if not writable will env->NewVideoFrame() and copy the contents across.

So in 1.5 cases the direct child will return a brand spanking new PVideoFrame so affecting the behaviour of it's env->NewVideoFrame() calls achives the desired result. In the other 0.5 cases you get a PVideoFrame originally from a (grand^n)child. So maybe we need to carry the effect up the tree. Yes this is trickey. As usual much thought needed.

Another issue, how to arbitrate conflicting requests when you get a forked GetFrame chain. Do we just max() the parameters? If some deviant askes for 10K padding there is a good chance of blowing the L1 cache. Do we limit the values?

Whatever happens I believe these padding/alignement parameters can only be best effort hints. We can always expose helpers to test for conditions and blit a new frame when required to provide some transparancy to filter authors.

Lets take a conservative 1000 MB/s throughput, and a 1kx1k RGB frame ... a copy will take you 6 msec.The aim of some here is to process in real time or faster. 30 frames per second is 33msec/frame and 5.5 * 6 = 33. Given the input (Avi, Mpeg2, DirectShow, ...)Source all do a blit and the output AviRead also does a blit you only have 3.5 blit left in your time budget. Food for thort, huh ....

IanB

[mg262]

So, this may or may not be relevant but they can return a frame from any PClip, not just child, so far as I understand it? So it ends up being a dag rather than a tree...

In what I wrote above, I didn't take account of the fact that a frame can be cached and then another filter can request the same frame with different padding. So ignore what I wrote...

How about something like this: a filter is given an extra DemandPadding function, to be called after construction but before any GetFrame calls. So e.g. DemandPadding (x,y) means that every frame it return should have that amount of padding. Now it in turn can call DemandPadding (x,y) on clips it happens to hold pointers to.

So this requires a two-stage construction process -- first construct all the filters, and then call DemandPadding (0,0) on the final output, which will then propagate DemandPadding calls back through the tree.

Backwards compatibility is a separate issue; -- [sorry, just noticed the time -- must run -- apologies for broken message]

[MfA]

I forgot for a moment the caching filter was there, so calls to IClip functions dont necessarily propogate up the chain ... so what I said would work I guess, the caching filter would just intercept the call and pass it upto avisynth proper. Not very neat, but in the end I dont think it would be anymore out of place in IClip than setcachehints is (the fact that the caching filter is situated in between is a matter of implementation, not interface).

Ill behaved filters can cause trouble with any script, I dont see any problem with maxing padding requirements.

[sh0dan]

@IanB: While I see your point, my point is that while I realize it's possible, and it's the optimal solution, it might be shooting birds with cannons.

Global Padding:

Pros:
*) Much less householding. This is simply two variables kept in ScriptEnv, that affects sizes of all new frames. Job done - no place for bugs.
*) Virtually zero cost. We can adjust offset and UVoffset for top/left padding, we simply extend pitch for side padding, keeping RowSize.

Cons:
*) Requires more memory (=less cached frames)

While I could implement global padding in two hours, filter specific padding is not something I'll touch, since I don't think I'll be able to do a stable version of it.

[Bidoche]

@All

I also think you are getting it too far with this padding propagation idea.
It's true that in some case it may save some blits by direclty allocating the borders an upstream filter will need later.

But : - Sometimes you will you waste memory for naught, as you can't be sure which frame will reach and which won't (decimation and the like, blits inbetween ...)
- It's a pain to get it right (propagation, forks...)
- It will break the world


The current solution, ie padding the frame at the time you need it padded, appears satisfactory enough to me.
But maybe some effort could be done on simplifying it (a VideoFrame method taking the pads values)

[mg262]

It will break the world LOL :p

You have a point. (720+32)*(576+32)/(720*576) = 1.10 -- so a 16 pixel margin on every side would cause a 10% overhead, which is substantial. (For small margins the overhead is approximately linear... so 8 pixel margins mean a 5% overhead, etc.) Perhaps we wouldn't be thinking of margins anywhere near as large as that... but I think part of the problem is that we aren't being very clear on when and where these margins would be useful. So, may I ask: who would actually use the padding, how much would you require and what precisely would you use it for?

One other thing that I do think is that expecting people to write out the code 2 or more times (for example for aligned memory and non-aligned memory, or for padding present and padding absent, if this latter example even makes sense.) is generally not a good idea... few people have the patience to do this systematically.

Edit: One other question... how does the L2 cache algorithm work? Is it possible that any left/right padding will be dragged through the cache even though it is not actually accessed?

[Manao]

I would use it each time i've got to write a spatial filter ( convolution / mask & co ). If the paddable area exists, you just have to pad it with the proper values and then use the algorithm on the whole frame without having to care about limit condition.

Mainly, that'd avoid all the ugly stuff ( macros / templates ) you've got to do when you wrote such filters.

Half the filters in the masktools would have a use for such a padding possibility. And that would avoid coders to write a complicated framework each time they have to iterate a spatial function over the whole frame.

It would require only to padd the picture by the width / height of the convolution kernel ( sse2 would require 16 byte alignment in addition, so a minimal padding width of 16.

What concerns me, memory wise, is more the access times for padding the picture than the momery overhead. Padding a picture doesn't require a lot of operation, but even a 1 pixel border would need to access height * 2 cache lines ( roughly 1/5 of the picture ), and is hardly prefetchable ( because padding itself is fast )

Still, it'd be by far faster than a copy, and it would clean the code. I don't want to write such things anymore : MT_LOAD_NORMAL movq , mm0, esi + %4, ebx, mm6, mm7
movq mm1 , mm0
%5 %1 , mm0, esi + %4, ebx, mm6, mm7
%6 %1 , mm0, esi + %4, ebx, mm6, mm7
%3 mm1 , mm5
%7 %1 , mm0, esi + %4, ebx, mm6, mm7
%8 %1 , mm0, esi + %4, ebx, mm6, mm7
%9 %1 , mm0, esi + %4, ebx, mm6, mm7
%10 %1 , mm0, esi + %4, ebx, mm6, mm7
%11 %1 , mm0, esi + %4, ebx, mm6, mm7
%12 %1 , mm0, esi + %4, ebx, mm6, mm7
%2 mm0 , mm1
movq [edi + %4], mm0
(%5 - %12 are macros themselves )

[MfA]

Meh, I couldnt give less about the storage overhead ... but on the other hand I dont care much about processing overhead either, so I personally wouldnt really care if it was done automagically during getframe by a buffer copy.

As for why to use it, just consider the humble neighbourhood filter ... without border extension you always have to deal with the pixels within your filter radius from the border with a seperate code path. More code, more bugs ... and it tends to be branch heavy and non SIMD too, so not efficient either.

Padding being present makes absolutely no difference for filters which dont use it, apart from the extra storage.

[sh0dan]

I tested (and fixed) more filters:

Non trivial filters working:
StackVertical, StackHorizontal, dissolve, Fade, Splice, Weave, Bob, Histogram, Limiter, Merge*, *ReduceBy2(), Blur/Sharpen, TemporalSoften, Flip*, Turn*, Crop, *Resize, All TextOverlay filters, Conditional functions.

We pretty much only need converters and source filters now.

[Bidoche]

After some brainstorming with Manao, I got a solution that can give the same advantage as propagation without the real cost.

Supposing :struct PaddingSettings { /*whatever*/ };

//in the env
//handles of a LIFO stack of settings in the env
//the top one is the one currently used
virtual void PushPaddingSetting(PaddingSetting const& settings);
virtual void PopPaddingSetting();

So a filter who actually cares about padding push a setting before requesting frames, and pop it once done.
Then it will get properly padded frames.

And filters downstream who know they do not need padding (because they make copies) can push a new settings for their own frame requests and still respect parent settings for the frames they create.

[sh0dan]

@Bidoche: Seems like a brilliant solution! Simple and effective. You and Manao should storm your brains some more! :)

[mg262]

It would require only to padd the picture by the width / height of the convolution kernel ( sse2 would require 16 byte alignment in addition, so a minimal padding width of 16.That explains the 16, which had led me to think you were thinking of of more complex operations than convolutions. But (and I know I'm probably missing something simple here) why is not sufficient to take width + padding and round that up to the nearest multiple of 16, rather than rounding the padding up on its own?

i.e. I can't see why it wouldn't work like this: requesting 4 bytes of padding would not cause any increased memory usage in three out of four cases (where width = 4, 8, 12 Mod 16), because the padding goes into the extra space we have already due to alignment -- and in the fourth case (where width = 0 Mod 16), 16 extra bytes are needed... but on average the penalty is still 4 bytes.

Also, of course, the 16 requirement doesn't apply to height and so the overhead will only be half of what I quoted.

[Bidoche]

Correction for a hole I overlooked in my earlier proposal:

//in the env
virtual void AddPaddingSettings(PaddingSettigs const& setting);

use Push when you want to replace the current settings (ie basically because u make copies in général)
use Add when you need padding and will pass frames through (you don't know if previous settings are stricter or not, so settings must be maxed together)

[MfA]

I must say, apart from the binary compatibility issue I like mg262's suggestion of just creating versions of getframe with padding parameters. (Newvideoframe too, though that is less important.) With an inplace flag in the function call you accomplish the same as having seperate push/add calls.

Avisynth can do the adding, pushing and popping ... why expose this to the filter?

[Bidoche]

@MfA

What is the difference between the following and my proposal : //in Clip
PVideoFrame GetFrame(int n, PadSetting const& setting) const
{
env->PushPadSetting(setting);
PVideoFrame result = GetFrame(n);
env->PopPadSetting(setting);
return result;
}That's just syntaxic sugar

[MfA]

Syntactic sugar which reduces two calls to one where it matters, and removes the ability to screw up the stack.

[Bidoche]

Syntactic sugar which reduces two calls to one where it matters, and removes the ability to screw up the stack.Never said it isn't desirable.
Besides this code snipet is not correct as you seem to think, if GetFrame throws, the stack gets wrong.

And second point, this 'helper' should be a method in PadSetting (PVideoFrame GetPaddedFrame(PClip const& clip, int n) const) in order to avoid polluting the Clip interface.

[IanB]

@Bidoche,

Like your idea a lot. This is a little like how I did the LegacyPlanarAlignment trickery.

Extending the idea a little :- we could implement it as a filter that authors can env->Invoke("FramePaddingPlease") This filter would hide all the mundane pushing/poping/adding, optional edge filling and general interfacing with avisynth internals, minimising chances for errors, handling throw's, being totally opaque so we can change/improve the design.

Also with the pushing/poping the scope probably only needs to extend back to the (grand)childs NewVideoFrame call.

IanB

Edit: In fact as a filter we don't need a stack, just set it in the FramePaddingPlease filter and clear it in env->newVideoFrame() all the tracking thru forking etc just happens by virtue of the way the GetFrame chain works.

[sh0dan]

I've got a cold ATM, so rather ironic I found some time for more filter work.

- Took out greyscale and RGB32<->RGB24 from convert.cpp and placed them in separate files.
- Added new FOURCC's to AviSource.
- Added new colorspaces to BlankClip.
- Added more info to Info() [CPU detection seems broken??!?]
- Split up merge and plane Swappers.
- Split up Plane transfers into separate classes.
- Plane splitters working on all internal colorspaces.
- Added UtoY8 and VtoY8.
- Added automatic destination colorspace detection on planar YtoUV.
- Begun Planar Convertion. ConvertToY8 implemented, but not yet activated.
- Merged TSP's Thread safety modifications.


@IanB: I couldn't figure out what all the TEST defines and ifdefs in merge.cpp were doing. I hope you don't mind I took them out, when I split up the code into separate classes.

[tsp]

It seems as if most people suffers from the cold in Denmark right now ;)
would you include all the changes in my current version of the avisynth256MT modification (http://www.tsp.person.dk/avisynth256MT.zip)?
It still needs some modifications but at least it works (and SMP support is cool :)). Maybe at a later time when it is more feature complete.

[Wilbert]

Just curious :)
- Added new FOURCC's to AviSource.
What's this?

[Fizick]

What if I ask for complex float format support? :)
(For fft3dfilter)

[tsp]

maybe just a generic data format?

[sh0dan]

What's this?
I added 'Y800', 'YV24', 'YV16' and 'Y41B' as supported FOURCCs that are accepted by AviSynth.

Sorry. Generic data support is not a priority for 2.6. Maybe it is for 3.0, but I cannot answer that question.

Btw, added Generic planar conversion routines, so we can now convert YV12<->YV24<->YV16<->YV411. Took IanB's excellent suggestion, and used the resizer for chroma. No interlaced support yet, though.

[MfA]

What if I ask for complex float format support? :)
(For fft3dfilter)For stuff like that I wanted the per frame custom data. I guess I should get off my lazy ass and write some code for it if I want it included.

[Fizick]

Per frame custom data will be useful for DePan filter (for frame motion data).

[IanB]

@IanB: I couldn't figure out what all the TEST defines and ifdefs in merge.cpp were doing. I hope you don't mind I took them out, when I split up the code into separate classes.They were a test harness so I could push the code thru all the different paths at will, it's a pity if you have removed it coz making sure all the code paths work correctly is near imposible without it.

The logic of the tests was to mask bits from the control word to force a particular test either always true or always false. Setting the top #if to 0 made all the test code evaluate to nothing.

Adding explicit test hooks is probably something we should all learn to do on programs as complex as avisynth.

IanB

[sh0dan]

They were a test harness so I could push the code thru all the different paths at will, it's a pity if you have removed it coz making sure all the code paths work correctly is near imposible without it.

I'm still not sure I quite get how it works ;) It's still present in all the merge code, but since I had to rework most of the plane transfer functions, I took it out, since I didn't have any idea of what to do with it.

There isn't much assembler in the plane swappers, except for YUY2. For assembler testing I usually use something like:

if ((n&1) && env->GetCPUFlags() & CPUF_MMX)

This way the MMX gets executed every second frame, and errors are quite easy to detect.

[tsp]

Added my multithreading modifications to the CVS.

[mg262]

You may safely access up to pitch (note: pitch may vary from frame to frame). The canvas is height*pitch+(a little bit), but don't ever count on the +(a little bit).May I ask: is it safe to read that extra little bit as long as you don't write to it -- or could this cause a page fault? [The idea being that an SSE2 implementation reads the data in blocks, and the final block might potentially overrun the end of the frame.]

[IanB]

@mg262,

No you cannot safely access the 1st pixel of the (height+1)th line. The last byte that is safe to access is frame->GetReadptr()+(height*pitch)-1; (remember [0..7] is 8 bytes). I take it this is a chroma plane only aligned 8 issue, this was the reason for the LegacyChromaAlignment() feature in 2.5.6+

However as this is one of the subjects of discussion in this thread things may change (for the better). I certainly have a fairly developed idea on how to implement a system to transparently request extended padding and alignment of frames, I just need some time to scratch myself.

If you want to take your life in your hands, windows allocates memory in pages so you probably can read memory upto the end of the current page without getting an access violation.

IanB

[mg262]

Thank you. Please don't feel you have to hurry with the implementation... it was just something I wanted to check. It wasn't actually a chroma issue; each loop processes 64 luma pixels (and as ever the instructions are interleaved to reduce latency, so jumping into the middle isn't an option). It can be worked around without too much complication.

Edit: I have looked back at the code (which I wrote in March) and there was a subsidiary issue, namely that I was loading 8 pixels into a xmm register, and then unpacking each to be a word. Using movdqu meant reading up to 8 pixels further than was necessary, with attendant problems. But I seem to have replaced this with movq+movq2dq instead, on the grounds that it was (empirically) faster.

In any case, I should probably be processing the last line separately in C++ to avoid all these problems...

[IanB]

@mg262,

Ah! Cache Line aligned code, you must be keen :D

Don't be slack :D write the last iteration in asm as well :D

IanB

[mg262]

Cache Line aligned codeNot that, I'm afraid, though I have thought about it... (one place where not having a proper macro assembler hurts). Its a weird side-effect of a trick to do convolutions fast. (One I will document if anyone is interested, although probably after the code is sorted out, which may take a bit.)

>Don't be slack :D write the last iteration in asm as well :D

Slavedriver!

Scary thing? It makes about 4% difference to the function speed.

[bill_baroud]

hey, i'm interested :sly:
now you have to document it :D

[Bidoche]

Finally took the time to post. :)

more details on my proposal :struct PadSetting
{
long top;
long horizontal;
long bottom;
long extra;
};The top and bottom members should be self explanatory.
Horizontal comes from left and right margins merged together, as one can always choose to think of margin from being on left or right.
This fusion incures an extra cost of horizontal (bytes) which I think affordable for the simplification it gives.
extra is the guaranteed amount you can read/write (without fault) after last signifiant byte. (ie width + horizontal + extra - 1 is valid on last scanline)

All of those values counting in bytes, not in pixels.
Because it's simplest this way, it avoids potentially troublesome transformation.

[mg262]

Nice and very clean!

In some cases you might want the mirror of equivalent of extra -- padding at the beginning. These are probably too marginal to be worth including, but I thought I'd mention it in case someone thinks of cases I've missed. At the moment the main one that occurs to me is if you are working in blocks of cache lines (64), where the beginning of the frame might cross one. (On the other hand, that could be more easily solved by aligning the start of the frame on a multiple of 64.)

[Bidoche]

In some cases you might want the mirror of equivalent of extra -- padding at the beginning.I thought of that too, but so far I haven't been able to come up where a single case where it matters, so...

On the other hand, that could be more easily solved by aligning the start of the frame on a multiple of 64.But align is a compile time value... unless we put it there as well :p

[mg262]

True, but the overhead of always aligning the whole frame to 64 is at most 48 bytes per frame, which is (IMO) not something worth worrying about.

[Bidoche]

Isn't each scanline aligned in 2.5 !?

It is in 3.0, so 64 align may cost big.

[mg262]

It is aligned to 16, but that's not what I meant... if you are working in blocks of 64, and the frame boundary lies in the middle of the block, then loading the whole block could cause a page fault. (That is exactly the situation I was referring to above... the code works on blocks of size 64 bytes, because it turns out to be faster to reorder every eight bytes as 73625140 rather than 76543210 for an intermediate stage... I wrote it like this way back when I didn't know much about AVISynth, and now realise that in theory it can cause a page fault... in practice it hasn't happened yet.)

[MfA]

Nm ... reading fault as miss, growing dyslexic in old age.

[mg262]

RRrr... sorry, meant access violation.

[sh0dan]

Did some conversion stuff:

- Added RGB to planar conversion (all formats - all matrices)
- Added YUY2 to Planar conversion (all formats)
- Fixed border issue in text renderer for generic planar formats.


Now we only need Generic Planar -> RGB/YUY2. Shouldn't be much work in either. Then it should actually be quite usable!

[sh0dan]

Finished conversion stuff:
- Added generic planar to rgb conversion.
- Added YV16 to YUY2 conversion.
- Added YUY2 input to UToY8/VtoY8.
- YUY2 to Y8 now working.
- Added Y8 to generic planar conversion.
- Added Direct RGB To Y8 conversion.
- Interlaced support for all modes to/from YV12. (Using field separation)

[Wilbert]

@Shodan,

Could you add interleaved2planar/planar2interleaved conversions? So that YV411 (DV) can be imported and converted to the supported YV411p.

[sh0dan]

What exactly are you thinking of?

- An YUY2 to 4:1:0 planar converter? You know the most about the DV decoders - would it be correct to simply grab the leftmost chroma sample?

I'm thinking the best in the long run would be to modify Cedocida to be able to input / output "Y41B" (planar 4:1:0).

[Wilbert]

What exactly are you thinking of?
I simply meant 4:1:1 interleaved to 4:1:1 planar. Of course the dv decoder should be able to output 4:1:1 interleaved, which is not yet the case.

I'm thinking the best in the long run would be to modify Cedocida to be able to input / output "Y41B" (planar 4:1:1).
That's also an option. I will bug dittrich about it :)

[sh0dan]

There is no 4:1:1 interleaved - only planar. There isn't much point in implementing it interleaved, when we've got it planar. Planar support is virtually free, compared to an interleaved format that would take much longer to support. This goes for filter writing too.

[Wilbert]

There is no 4:1:1 interleaved - only planar.
I'm pretty sure DV/NTSC is stored as 4:1:1 interleaved. Forget about it (i thought such a filter was possible without supporting a new colorformat), i will ask dittrich to add an 'output to Y41P' option.

[sh0dan]

Had a quick look at Cedocida, and it actually seems to be storing YUV 4:1:1 planar, when decoding NTSC, so the change, to output it should be minor.

Regarding FOURCC, it seems to be quite a mess. 'Y41P' is generally considered YUV 4:1:1 PACKED, whereas 'Y41B' is YUV 4:1:1 planar, but without any UV-order specified. What a mess. :(

[sh0dan]

Had a little fun with some assembler:
- Added dynamicly compiled MMX/iSSE for RGB<->YV24 conversions. Speed is approx 200% of C-code.

[IanB]

For our definitive view on FOURCC memory layout we probably should defer to the DirectShow (YUCK!) GUID definitions. They seem to munge the FOURCC into the bits of the GUID. This seems to be where M$ is putting there development/documentation effort.

IanB

[tsp]

more multithreading

- Fixed memoryleak with MTMode 2 and 4
- Added a synchronization class IClipLocalStorage and smartpointer PClipLocalStorage to handle synchronisation between class instances when using MTMode 2 and 4
- Increment sequencecount in MakeWritable to avoid crash when calling GetFrame between MakeWritable() and GetWritePtr()

[sh0dan]

More generic work:
- Added generic planar input to Overlay.
- Fixed VC6 scoping in multithread code.
- Throw error on unsupported colorspaces in Turn*.
- Added MMX YUY2 <-> YV16 from DGDecode.
- Added MMX YUY2 -> Y8 conversion.
- Added Generic Planar -> YUY2 through intermediate YV16.

All colorspace conversions should be done now. It all seem to play pretty well, so I think we are approaching an alpha.

[sh0dan]

Update:

Ian and I have reviewed the colorspace conversion code, and it should be pretty solid by now - and pretty fast even. :)

We have the following things left, before we'll go alpha:

- Add new input types to AviSource/DSS.
- Review output code, so we deliver new formats in a proper way.
- Remove baked code.

Ian is blowing me out of the water on optimizations. It really shows when you've been away from assembly for a year! :)

[Wilbert]

I did some documentation and testing :) I noticed the following (latest CVS):

- *Resize: bug in YV16 mode, lower half contains rainbow (tried bicubicresize and didn't test other resizers)
- CPU detection doesn't work (corresponding info field is empty)
- Turn*: YV16 mode not implemented
- ConvertToY41B not implemented

[sh0dan]

- Resize bug fixed.
- Fixed CPU reporting in Info()
- Added time indicator on audio length and video (current frame & total) in Info()
- Fixed Y8 issue in resizer.
- Added specific error reporting when requesting subsampling on Y8 to avisynth.h.

Turn only works on previous formats, and planar formats, where width and height subsampling match. (Y8, YV12 and YV24)

Edit: Ian has enabled Turn180 on all planar formats.

"Y41B" is called "YV411" everywhere, so you can use ConvertToYV411().

Edit: More stuff:

- Put dynamic matrix conversion into separate file.
- Checked and fixed bugs in Planar output.
- Planar alignment was off on new frametypes. Too much was being allocated.
- Fixed compile warning in MT code.
- Added new formats to AviSource.
- Order of attempts: YV12, YV411, YV16, YV24, YUY2, Y8, RGB32, and RGB24 in turn.
- Fixed crash bug in AviSource error reporting.
- Fixed greyscale not working on new planar formats.
- Video was NOT converted on RGB -> YV12.

Basicly AviSource is now capable of opening all new formats.

[mg262]

Guys, thank you for all the hard work! It is very much appreciated.

[jeffmikels]

Is Avisynth capable of reading dvr-ms files with any current plugins, or is that something that is on the roadmap for the future?

I'd like to request it if it's not...

On another note, I think it would be really powerful if you would extend the DirectShowSource function to allow scripts to specify the exact DirectShow filter chain to use. I know that I could change the priorities of certain filters, but to have manual control over filters through Avisynth would be incredible!

[sh0dan]

Is Avisynth capable of reading dvr-ms files with any current plugins, or is that something that is on the roadmap for the future?

I'd like to request it if it's not...

Even though it's MPEG 2, MS doesn't provide information about the format, since it's DRM'ed. AFAIK there are conversion tools available to convert it to other formats.

If it's playable using DirectShow you should be able to open it using DSS.
On another note, I think it would be really powerful if you would extend the DirectShowSource function to allow scripts to specify the exact DirectShow filter chain to use. I know that I could change the priorities of certain filters, but to have manual control over filters through Avisynth would be incredible!
Use GraphEdit to make your graph, and open the GRF file using DirectShowSource. See the documentation on avisynth.org.

[jeffmikels]

Even though it's MPEG 2, MS doesn't provide information about the format, since it's DRM'ed. AFAIK there are conversion tools available to convert it to other formats.


I don't think the format is DRM. My reading on the subject leads me to believe that the format has the capability of carrying encrypted streams or unencrypted streams and that the format itself is publicly available. I thought DVR-MS was supposed to be simply a wrapper around other streams.

http://msdn.microsoft.com/library/default.asp?url=/archive/en-us/directx9_c_Summer_03/directX/htm/aboutthedvrmsfileformat.asp


If it's playable using DirectShow you should be able to open it using DSS.

Use GraphEdit to make your graph, and open the GRF file using DirectShowSource. See the documentation on avisynth.org.

Opening a DVR-MS file with DSS yields a video stream that plays twice the normal speed and an audio stream that plays at normal speed.

Opening a GRF file in Avisynth has been nearly impossible for me to figure out. Apparently Avisynth doesn't recognize the open pins on the end of my graph even though I only remove the final render filters. On top of it, there's no way I know of to programmatically change the source file (the input source) of a GRF.

[Wilbert]

Opening a DVR-MS file with DSS yields a video stream that plays twice the normal speed and an audio stream that plays at normal speed.
Did you set the fps parameter?

Opening a GRF file in Avisynth has been nearly impossible for me to figure out. Apparently Avisynth doesn't recognize the open pins on the end of my graph even though I only remove the final render filters.
Could you post a screenshot of your graph and state the error message?

[Fizick]

Sorry, I am not sure, if this question was discussed:

Can we give info about internal (and plugin's) functions parameters names (and types) to external programs (like AVSEdit) ?
This info may be somehow used internally too.

[sh0dan]

Sorry, I am not sure, if this question was discussed:

Can we give info about internal (and plugin's) functions parameters names (and types) to external programs (like AVSEdit) ?
This info may be somehow used internally too.
Internal and external (plugin) functions are exported as AviSynth Variables.

$InternalFunctions$ Should contain function names of all internal functions.
$InternalFunctions!Functionname!Param$ Should contain all parameters for each internal function.
$PluginFunctions$ Should contain all plugins.
$Plugin!Functionname!Param$ Should contain all parameters.

Use env->GetVar() to access them.

[Fizick]

sh0dan,
Thanks for good (old ?) news. It was true for Avisynth 2.5.5?
It is interesting, why AVS-editors do not use it, but use some external function descriptions in xml-files.

So, there are no info for parameter's type?

[sh0dan]

External plugins have been exported for several versions, I believe the Internal functions were added by Ian for 2.5.6. Vdubmod use the plugin export for syntax highlighting.

You can use a try/catch mechanism, when you request "$InternalFunctions$". If you get a "NotFound" exception, people are using an earlier version.

Parameter types are passed in the "$Plugin!FUNC!Param$". They are delivered as they are specified in the filters "c[type]s[audio]b", etc.

[Fizick]

I know about syntax highligting in VDMod, (recently i added some new words to new VDMod version) , but it still not use parameters names. It will be very useful hints for user. I hope, somebody read this info and implement it.
So, one more undocumented Avisynth feature is opened. :)

[sh0dan]

Yes - you are right. I actually planned to do a "How to use avisynth directly from your application", but never got around to doing it. I'll put it on my to-do-list (which is basicly scattered in this thread).

While Ian works out if there is a solid solution to the baked code, I'll do some time documenting the new colorspace stuff in 2.6.

[Wilbert]

I'll do some time documenting the new colorspace stuff in 2.6.
I already added much about this at avisynth.org :) I still have to copy it to sourceforge.

[IanB]

I've just finished a session Planarising various bits and pieces, and I found myself coding (vi.IsPlanar() && !vi.IsY8()) a lot more than I would like. I am slowly coming to the realization that IsPlanar maybe should respond false for Y8 data.

What do others think?

And I am becoming convinced that I was right that the core should treat planar formats genericly and just look at the H and V subscaling factor. Using the various IsYV??? seems to paint oneself into a corner quite code wise. I suspect removing them from the API in favour of various representations of subscaling factors i.e. BitShift (0, 1, 2), Mask (0, 1, 3), Divisor/Multiplier (1, 2, 4), etc would be a positive thing.

Thorts?

IanB

[Bidoche]

I like Y8 being planar, after all for some tasks, it can be handled exactly like them.

You may just need a method to get the list of available planes.

[mg262]

I found myself coding (vi.IsPlanar() && !vi.IsY8()) a lot more than I would likeCould you give us some examples?

For the second, IMO the more structural information describing colour spaces available, the better... that number and possibly type of planes, sub sampling factors, and colour depth. By type I mean something like this:

Luma
chroma (in the presence of luma -- e.g. Y)
chroma (in the absence of luma -- e.g. R)
alpha channel

I'm not sure whether this is overkill, but being able to get hold of the number of planes and colour depth are more substantive.

[foxyshadis]

Isn't Y16 also going to be available, in which case that would have to be blocked from multiplane access areas? It seems (vi.IsPlanar() && vi.PlaneCount() > 1) would make more sense. (However you were to implement a # of planes property.)

[Wilbert]

Isn't Y16 also going to be available, in which case that would have to be blocked from multiplane access areas?
It's YV16, which has three planes.

[Bidoche]

Shouldn't be the correct code here be : vi.IsPlanar() && vi.IsYUV() ?

[foxyshadis]

It's YV16, which has three planes.
I must have misread a while back, I thought a 16-bit greyscale was going in as well. Sorry!

[sh0dan]

@IanB: I was also a bit amazed at how much special code Y8 needed. But in reality it's usually pretty easy to handle. Since:

* PVideoFrame->GetWidth(PLANAR_U) and GetHeight return 0. This make 90% of the routines skip chroma.
* Usually just a matter of returning the destination frame when luma processing is done.

I considered if there was another solution, but didn't see any significantly better way. Making Y8 "not planar" would lead to a lot of (vi.IsPlanar() || vi.IsY8()) in the cases where ordinary planar code could be used.

Mask & Divisor/Multiplier are nice helper functions, though I don't want to encourage multiplication/division. :D (Just kidding - they would be nice to have)

[IanB]

Yes, IsPlanar() is not the right test for where I have been working, something like plane enumeration, PlaneCount or IsMultiPlane() is what should be being used. I'll survey the code and think about it a bit more.

Probably helper vi.Validate(const char *messagetext) is what we should have.

[Bidoche]

Probably helper vi.Validate(const char *messagetext) is what we should have.What would you be passing in messagetext ?

[Bidoche]

related link :

http://aegisknight.org/cppinterface.html

[IanB]

> What would you be passing in messagetext ?

Probably the name of the script verb choking on it's parameters.

i.e. "Crop : YV411 images must be Mod 4 width."

> related link :

Pity Ben didn't read this years ago :D

[jeffmikels]

Did you set the fps parameter?



Yes.


Could you post a screenshot of your graph and state the error message?

The error message says:
Video Returned: DirectShowSource: The filter graph manager won't talk to me
Audio Returned: DirectShowSource: The filter graph manager won't talk to me

I'm attaching a screenshot of my graph.

[IanB]

Okay, I have had a quick survey of the IsPlanar() use in the code.I found myself coding (vi.IsPlanar() && !vi.IsY8()) a lot more than I would like. I am slowly coming to the realization that IsPlanar() maybe should respond false for Y8 data.And as I said, I now more strongly believe CS_Y8 should not include the CS_PLANAR bit. Given the current state of play the CS_PLANAR bit would be best utilized to say simply and exclusively This format has separate data planes for each channel!

I like Y8 being planar, after all for some tasks, it can be handled exactly like them.Making Y8 "not planar" would lead to a lot of (vi.IsPlanar() || vi.IsY8()) in the cases where ordinary planar code could be used.This is not a good reason to have Y8 included in the set of Planar formats. I would actually prefer seeing (vi.IsPlanar() || vi.IsY8()) coded in these cases because it states This code is good for both single and multiple plane data. All the current interleaved formats can be treated the same as each plane for vertical operations, and we wouldn't want to declare them planar just because of that. In a lot of cases Y8 data processes equally well thru both the planar and interleaved code paths.

I was also a bit amazed at how much special code Y8 needed.

* PVideoFrame->GetWidth(PLANAR_U) and GetHeight return 0. This make 90% of the routines skip chroma.And that is the crucial rub. Not all code copes with zero dimmension planes. I have always been uneasy trusting routines to correctly do nothing with a plane because the dimensions are zero. Most coding bugs fall under the category of boundary condition errors, off by 1 errors being the most prevelant. And these bite here.

By not pressing the boundary conditions of code more reliable software ensues. If the conditions to expose a bug can never occur then the bug never occurs.


Yes, IsPlanar() is not the right test ..., something like ... IsMultiPlane() is what should be being used. I'll survey the code and ...Much earlier in this thread I proposed that Y8 also include the CS_INTERLEAVED bit as well as the CS_PLANAR bit. Having had some experience actually implementing this now I change my position and believe Y8 format should not include either of these bits and IsPlanar() is the appropriate name and test.

Format Y8 really stands alone, it shares most of the benefits of both planar and interleaved formats but does not suffer from the deficiencies of either. Having a test like IsPlanar() is most beneficial when it can be succesfully used to filter for both benefits or restrictions of a given data set.

Thorts?

IanB

[sh0dan]

New "tweak" functionality by Wilbert:

1) It enables you to adjust the hue and/or saturation in a specific hue range [startHue,endHue] and/or in a specific saturation range [maxSat,minSat]. Some interpolation to the prescribed saturation range is added to prevent color banding. The interpolation can be set with interp (interp=0 won't interpolate, interp=4 is maximal).

2) Added planar support (trivial).

I removed the SSE code. It's not being used since it's slower, and there were many conditions in which it wasn't used. Parameter is still present for script compatibility, but simply ignored.

[mg262]

Planar Tweak -- :)

Ian,
May I go back to Planar/Y8? Specifically: is it that people have previously been using IsPlanar() where they should have been using IsYV12(), or is it that there are genuine cases where code deals with any planar colourspace except Y8?

[Fizick]

All these planars is important...
But may I ask once more about some special arbitrary (not video) format or additional custom data for such plugins as MVTools, Motion (motion vectors), DePan (global motion), DeComb (hints) ? Now they use hacks with frambuffers in various video formats.

[mg262]

May add I something similar to mull over? At present, working with floats, integers or bools on a per-frame basis requires the use of ConditionalFilter/ScriptClip... apart from any other problems, they can impose linear-access requirements if you look at previous frames' info. I claim that it would be preferable to have streams of floats, integers and bools (in exactly the sense that a clip is a stream of videoframes) -- i.e. something that assigned one float to every frame, or integer to every frame, or etc.

I've mocked such things up for my own use (and you can see a little of it here (http://forum.doom9.org/showthread.php?p=728296#post728296)); it takes a little getting used to, just like using Trim(1, 0) to get the "next" frame does, but IMO its works pretty well. It would be nice if the language supported such things, because it would make it much easier to pass information between filters -- for example, scene break information could be passed as a stream of bools, and blend-detection information as a stream of floats.

I suspect that this may be considered too complex ... but it's worth a try :)

[sh0dan]

@Fizick:
The only way I can see meta-data streams is using the method mvtools use for it. You now have Y8, which is a singleplane single-byte array.

You can put whatever data you wish into such a plane, it'll be cached properly, you can use changefps, selectevery and still maintain frame-to-frame mapping with your image data. It does the job, it does it without modifying any code, and it does the job well.

Agreed, Donalds 'hints' was a hack, but using Y8 to stream meta-data seem like a pretty solid way. You don't have to do any code, that you wouldn't have to do on any other meta-stream.

[sh0dan]

@mg262:
While I agree it would be really great, I think it would be hard without rather large redesign. Conditional filtering as it is IS a hack. When used with global variables, it isn't thread safe, which IS bad.

We could add a function to ScriptEnv, that wrapped AVSValues in a Y8 stream. A "conditional" script would than be like this:

Source filter constructor calls env->CreateStream(&vi, number_of_AvsValuesPerFrame, const char* "TypeIdentifier"), which filles out VideoInfo with the appropriate info (Y8, width, height).

GetFrame calls env->WrapValues(PVideoFrame *dst, AVSValue* ArrayOfAvsValues).

The destination filter could use AVSValues* v = PClip->UnWarpValues(PVideoFrame *src);

This would be the basic principle - and all conditional processing could be translated into this. Some "finer details" about passing the size of the AvsValue array is missing, but the principle is there. "TypeIdentifier" should be there so that filters can ensure that they get expected input (if they care).

It does open up a lot of potential bugs, but it definately solves more problems than it creates.

[mg262]

sh0dan,

I think that being able to have streams of AVSValues is more than needed... and it allows creation of e.g. a stream of streams of AVSValues, which is not a sensible type. Streams of structures would be nice, but simply having streams of the three basic types supported by ConditionalReader, namely bool, int, float, would deal with most things.

I haven't looked at the framework enough to know how hard it is. I don't know whether it would be of any interest, but my code to set up four or five different stream types takes 200 lines or less; the basic setup looks like this:*

*This code is not tidied up to a satisfactory standard. I am somewhat embarrassed to post it. But this discussion is happening now, and I won't have time to tidy this for a while...

template <class T> class Stream //behaves like vector<T>
{
protected:
PClip encoded;
public:
Stream(PClip c) : encoded (c) {}
T point(int n, IScriptEnvironment* env) const
{return T(encoded->GetFrame(n, env));}
};

template <class T> class FilterReturning : public GenericVideoFilter
{
public:
//FilterReturning(const FilterReturning<T> &) {}
FilterReturning(PClip _child, IScriptEnvironment* env);
virtual T GetPt(int n, IScriptEnvironment* env) = 0;
virtual PVideoFrame __stdcall GetFrame(int n, IScriptEnvironment* env)
{return GetPt(n, env).toFrame(vi, env);}
};

struct NumberData
{
double value;
//NumberData(double _value) : value (_value) {}
};
class NumberPt : public RawIO<NumberData>
{
public:
NumberPt(double d=0){ value =d;}// : x(_x), y(_y) {}
NumberPt(const RawIO<NumberData> &r) : RawIO<NumberData>(r) {}
};
typedef Stream < NumberPt > NumberStream;
typedef FilterReturning < NumberPt > NumberFilter;
(I've excluded the I/O code as this is relatively long and uninteresting.)

So NumberFilter::GetFrame calls the virtual function
NumberPt NumberFilter::GetPt(int n, IScriptEnvironment* env)
(which is overridden by the plugin author) and stores the returned NumberPt in a frame.

Going the other way, a NumberStream can be constructed from a PClip; and numberstream.point(n, env) extracts the relevant value via GetFrame.

Together, these give almost no overhead over using a clip... the only difference is that as you don't have AVSValue::AsNumberStream(), you need to use NumberStream(args[x].AsClip).

There is more I could say... but I don't want to go on for too long.

Edit: But this is important; having one AVSvalue per frame -- and preferably of a fixed type -- lets you override the inbuilt operators (not necessarily in the first release). So if you have two number streams, a and b, you want to be able write a+b in your script and get another number stream back. It also allows automatic promotion float->floatstream, so you can use constant streams easily.

[Fizick]

sh0dan,
Y8 is a little better, but what if somebody will try for example Resize this meta-clip ?
May be some special mark (CS_METADATA or CS_NONVIDEO) for it?
MVTools still use hack with audiochannels number.

[mg262]

MVTools still use hack with audiochannels number.But that is used to pass information upstream. Even if you had built in support for arbitrary colourspaces and arbitrary information attached to frames, I don't see that you could use them to pass information upstream?

[MfA]

Occasionally you might want filtered output of previous frames from later in the chain as input too, not just numerical values. Ideally you would just loopback a clip in script.

[IanB]

New "tweak" functionality by Wilbert:

1) It enables you to adjust the hue and/or saturation in a specific hue range [startHue,endHue] and/or in a specific saturation range [maxSat,minSat]. Some interpolation to the prescribed saturation range is added to prevent color banding. The interpolation can be set with interp (interp=0 won't interpolate, interp=4 is maximal).I appreciate the new capability and find it quite functional.

:angry: However, I am not pleased with the absolutely massive performance hit this has introduce! :angry:

I put considerable effort into writing and tuning the mapping code to get this filter fast. I was quite surprised at the time how fast the mapping code was. I was astounded that I could not even come close with the MMX/iSSE code, no matter what I did.

I expect to see a replacement version in CVS soon that restores or exceeds the previous level of performance. Hint: Start by moving the if (allPixels) test outside the loops and putting the old code in one branch and the new code in the other. When it's fixed and I have stopped seathing I'll see what can be done to make the new chroma range limited code perform passably.

[IanB]

Planar #*!@$ -- :)

May I go back to Planar/Y8? Specifically: is it that people have previously been using IsPlanar() where they should have been using IsYV12(),There possible are cases in the wild but it is not an issue here.or is it that there are genuine cases where code deals with any planar colourspace except Y8?Well yes, sort of. So far there are no "genuine cases" where the Planar path fails for Y8 data. But that is really by more good luck and carefull programming than good managment.

From a practical point of view the code would on balance be cleaner if IsPlanar() returns false for Y8 format data. Doing this coerces the meaning of Planar to mean "Has multiple data planes" rather than a more literal meaning of "Data is arranged in single planes" which I don't think is really much of a stretch.

[IanB]

@Fizick, mg262, sh0dan, MfA,

Unfortunatly, no matter how we cloak it metadata within the current architecture is always going to be a hack or a really big rewrite. Marking a stream with say CS_METADATA won't stop filters munging with it, most current code just tests for what it needs to be special and does something generic with the rest, i.e. if (x.IsPlanar()) {planar code} else {default code}. David has vastly improved structures for coping with this is 3.0 hopefully the final coding model will enforce against our current limited thinking.

[sh0dan]

I appreciate the new capability and find it quite functional.
[...]
:angry: However, I am not pleased with the absolutely massive performance hit this has introduce! :angry:
[...]
I expect to see a replacement version in CVS soon that restores or exceeds the previous level of performance. Hint: Start by moving the if (allPixels) test outside the loops and putting the old code in one branch and the new code in the other. When it's fixed and I have stopped seathing I'll see what can be done to make the new chroma range limited code perform passably.
Easy now. We are in pre-alpha. No need to throw angry faces at people.

I made UV-processing a single lookup per 2 chroma components in a 128k table. We could compare, but I'd be VERY surprised if it doesn't outperform both of the old versions (C and iSSE).

@Wilbert: You might want to re-test your "interp" code. It wasn't activated in the code, as it was missing in the filter definition - this was also why the isse switch you had at the end was acting up.

[mg262]

The 128k table will play havoc with the cache. (Ian pointed this out to me some time ago.)

On the filter I was looking at this for, the values looked up were luma differences and were typically very small, so you tended to access the same parts of the table, and the paired lookup table had a small edge. Here, where the values would range widely, I'd expect it to do substantially worse due to cache misses.

[sh0dan]

The 128k table will play havoc with the cache. (Ian pointed this out to me some time ago.)
[...]

Thanks for pointing this out.

1) L2 caches are at LEAST 256k on modern processors.
2) Chroma is usually very close to 128.
3) Would you agree that a single lookup in L2 cache is USUALLY faster than:
* 4 lookups in a 256 byte table.
* 1 Addition
* 1 Subtraction
* 2 Multiplications
* 2 Bitshifts
* A min(x,y) function.
* A max(x,y) function.

Or am I just daydreaming?

Edit:
Speed in AviSynth 2.56: 584 fps.
Speed in AviSynth 2.6: 986 fps.

(1265 frames 720x576).

If you want it even faster you could make it skip Y or UV planes if they aren't changed. But this is fast enough for me.

[mg262]

sh0dan,

My mistake -- I apologise. I forgot that output U depends on source U and V [not just source U], so I was thinking of two separate look-ups.

986 FPS is fast enough for anyone!

[Fizick]

IanB,
1) In my opinion, we may use CS_METADATA=CS_UNKNOWN=0 for metadata clips.
Almost all filter will not process such clips (it is not planar, not interleaved, not YV12, not RGB, not YUY2, etc) . The only needed operations with metaclips are probably full-frame manipulations with Changefps, interleave, selectevery and similar (as Sh0dan pointed).
Of course we must change BitsPerPixel default (for CS_UNKNOWN) from 0 to 8 in avisynth.h.
Is it possible?

2)The second question is about MVTools hack with audiochannels number.
As pointed by Mg262, it is used somehow to transmit poiter of MVanalysis data. I do not understand the manao's code . Probably it is for speed increase to do not calculate some values repeatedly.
Here is part of mvanalyse.cpp:

// we'll transmit to the processing filters a handle
// on the analyzing filter itself ( it's own pointer ), in order
// to activate the right parameters.
vi.nchannels = reinterpret_cast<int>(&analysisData);

I do not understand, how good or bad it is (i am not programmer :) ).
Any comments?

[mg262]

On 2, I think that it is to give a speed increase in MVAnalyse by not calculating some values at all. So, for example, MVAnalyse only computes block variance if it is going to be used by some other filter.

But, how is MVAnalyse to know whether block variance is needed? This is difficult, because no data flows from other filters to MVAnalyse, only from MVAnalyse to other filters. So Manao has essentially stuffed a pointer to the MVAnalyse filter somewhere (specifically, vi.nchannels) where it can be picked up by other filters; those other filters then use this pointer to tell MVAnalyse what they need.

This is not guaranteed to work by C++ -- the behaviour is implementation-defined. However, it works with MSVC, at least as long as you don't e.g. put TCPDeliver/TCPSource in. It seems to me that it would be tricky to add this behaviour to the framework.

[MfA]

@Fizick, mg262, sh0dan, MfA,
Unfortunatly, no matter how we cloak it metadata within the current architecture is always going to be a hack or a really big rewrite. Marking a stream with say CS_METADATA won't stop filters munging with it, most current code just tests for what it needs to be special and does something generic with the rest, i.e. if (x.IsPlanar()) {planar code} else {default code}. David has vastly improved structures for coping with this is 3.0 hopefully the final coding model will enforce against our current limited thinking.If you use Shodan's suggestion and use an extra clip for metadata you could add a format none of the standard filters can munch to make sure users dont accidently pass the metadata the wrong way.

That wasn't really what I was talking about though, regardless of how the metadata is passed mg262 seems to want the ability to access (meta)data added to previous frames by filters later in the chain. He only wants it for some numerical values for scenechange detection and such apparently, but if you put metadata in a seperate clip then just looping back a clip as input for a filter earlier in the chain would be a more generic solution (with at least a 1 frame delay to keep things deterministic of course).

[mg262]

mg262 seems to want the ability to access (meta)data added to previous frames by filters later in the chain. He only wants it for some numerical values for scenechange detection and such apparently, but if you put metadata in a seperate clip I want no such thing, nor did I ask for it.

I don't want to add data to frames at all, and in fact I think that metadata of this kind is a subpar solution for a number of reasons. I asked for a type -- probably implemented as 3 extensions to AVSValue that would store an stream of floats (or an stream of integers or and stream of bools). For present purposes, a stream is much like an array, except that it has lazy evaluation, i.e. values are only computed on demand.

Here is another way of putting it:
I want x, where x is to int as a clip is to a video frame.

[MfA]

If the numbers are associated with frames they are frame metadata.

[mg262]

The numbers are not associated with frames any more than video frames are numbered. You can take the third number and treat as if it corresponds to the third frame, and this is often a sensible thing to do, but it's not mandatory. I'm not going to argue terminology -- call it meta-data if you want -- but it's not tied to a particular clip.

I certainly don't only want it for scene change detection; that was one of the examples I gave, together with blend detection, but it's a complete replacement mechanism for ConditionalFilter.

[squid_80]

Here is part of mvanalyse.cpp:

// we'll transmit to the processing filters a handle
// on the analyzing filter itself ( it's own pointer ), in order
// to activate the right parameters.
vi.nchannels = reinterpret_cast<int>(&analysisData);

I do not understand, how good or bad it is (i am not programmer :) ).
Any comments?
It really annoys me when programmers cast pointers to ints.

[Manao]

squid_80 : it's not like I had the choice... I must put the data into the videoinfo, and it is made of int only, so...

his is not guaranteed to work by C++ -- the behaviour is implementation-defined.Why wouldn't it work ?

[IanB]

Easy now. We are in pre-alpha. No need to throw angry faces at people. :cool: :) :D :thanks: No offence intended. There seems to be a poor range of displeasure icons since vBuletin 3, I really wanted :mortified: rather than : angry :

I made UV-processing a single lookup per 2 chroma components in a 128k table. We could compare, but I'd be VERY surprised if it doesn't outperform both of the old versions (C and iSSE).Now this is the ticket. Any half modern processor cache is gonna swallow a 128K table easily these days. Even my old PIImmx, which struggles a little cachewise, runs a whisker faster than before and anyway it can't run the iSSE code. Good job :D

[IanB]

The 128k table will play havoc with the cache. (Ian pointed this out to me some time ago.) :cool: I seem to remember the elements were int's and there were multiple tables, such that the numbers came out about the size of a small L2 cache. I implied it might be borderline and need severe testing.

[IanB]

IanB,
1) In my opinion, we may use CS_METADATA=CS_UNKNOWN=0 for metadata clips. Almost all filter will not process such clips (it is not planar, not interleaved, not YV12, not RGB, not YUY2, etc). ... Is it possible?Unfortunately not all programmer do the right thing. For example slack implementation of vertical filters can just smack the Y, U & V planes, knowing columns are independant and that on non-planar formats the size of the U & V planes are zero and that PLANAR_Y is the same as the "only" plane.

I suppose we could add it to the API to uncourage people out of bad habits but we can't fix it for the legacy world.

:EDIT: If munging of metadata stored in a clip is a problem then check summing it might help. Throw an exception when it's broke.

2)The second question is about MVTools hack with audiochannels number. ... Any comments?It's a hack. As I understand it there is a higher level problem to addressing this properly. As the language stands filters can only return 1 thing, basicly an AVSvalue that is not an Array. Well a filter could return an array but you would have dificulties handling it within the script language, deep cloning of AVSValue arrays is not currently supported, so elements would either get prematurely released or never released i.e. leak.

In this MVTools case adding tagging to PClips would probably be sufficient but it would not be a total solution.

[squid_80]

squid_80 : it's not like I had the choice... I must put the data into the videoinfo, and it is made of int only, so...Fair enough, I do concede that you had no choice. Just wanted to get the word out that it's not something that should be done lightly. Using num_audio_samples instead of nchannels is a quick fix for 64-bitness anyway.

(I think I've turned bitter after wading through the resizing code and the Decomb source code. And the fact that I nearly failed a programming project at university three years ago for trying to be clever and doing the same thing. How was I supposed to know the lecturer would use an UltraSPARC for marking??)

[mg262]

Well a filter could return an array but you would have dificulties handling it within the script language,Do we need to handle such material in the script language? As long as the AVSValue is correctly passed into into the client filter, I would have said that's fine... after all, we're discussing how to stop filters from processing this data anyway!

CS_METADATA: While it would certainly be cleaner to stop most filters from processing metadata, I think that the proper solution is to type all data (as I believe is possible in 3.0). And for what it's worth, in Motion I very deliberately chose a format that could be played with by script authors (for example to binary-mask-merge together two sets of motion vectors); in fact it is precisely a 64-bit single-planar colourspace, and I want to allow any operations that are sensible on this colourspace.

Manao,
the behaviour of disguised pointers is specifically stated to be implementation dependent (page 8?? of 3e C++ book... can't find it at the moment). That wasn't meant to be a criticism, though. squid_80 has just given some examples where it would fail; also garbage collection and maybe distributed processing.

[Manao]

mg262 : I didn't take the remark as a critic, I rather wanted to know what could go wrong if I was doing that. I didn't thought of 64 bits compatibility. But int on 64 bits platform are 64 bits, so it shouldn't matter.

Garbage collector can't work with C++, or am I missing something ?

I know that what I done isn't proper C++ coding. Since I already use a global object, I should have used it to transmit data back and forth between filters, but I initially thought that filters would work outside MVTools dll ( silly dream ).

Anyway, that could be made cleaner.

[mg262]

Garbage collector can't work with C++,I don't know whether anyone has actually written one, but the language states that it's perfectly legitimate.

(I hadn't thought of 64-bits either! But IMO the thing about these mechanisms that look almost-safe... disguised pointers, memory leaks, global data... is that they cause problems in some weird case that is hard to think of.)

[Wilbert]

@Sh0dan,
@Wilbert: You might want to re-test your "interp" code. It wasn't activated in the code, as it was missing in the filter definition - this was also why the isse switch you had at the end was acting up.
I can't believe i missed that :) I will retest the interpolation tomorrow.

@IanB,
I didn't test the speed. I know it would be a bit slower, but i think not that much when processing *all* pixels. Anyway, i hope it's good now with Sh0dan's speed improvements.

[squid_80]

mg262 : I didn't take the remark as a critic, I rather wanted to know what could go wrong if I was doing that. I didn't thought of 64 bits compatibility. But int on 64 bits platform are 64 bits, so it shouldn't matter.No they're not. Ints remain 32 bits wide in both windows and linux. Longs aren't an option since they're 64-bits in linux, 32 in windows. That's why I said a quick fix is to use num_audio_samples(which is int64) instead of nchannels.

[Leak]

Garbage collector can't work with C++, or am I missing something ?
Oh, it can... (http://www.hpl.hp.com/personal/Hans_Boehm/gc/)

It would work better if it actually knew about the datastructures being used, but treating everything as a pointer works too as long as you know all allocated blocks - it just might happen that a bogus pointer (that really is some int value) prevents a block from being deallocated, but that shouldn't happen too often.

np: Bring Back The Buffalo - Breakfast At Sophia's (Bring Back The Buffalo)

[Fizick]

MVtools is for example of this ploblem only.

Probably in MVtools we can simply add some parameters to MVAnalyse, which must be set by user (what additional data will be needed in compensation function).
Or some more info is transmitted?
Probably it is offtopic here. sorry.

About CS_METADATA. May be we can not add it (as alias), but simply use CS_UNKNOWN. Can its BitPerPixels value be changed to 8 in AVisynth 2.5.7 ? :)

[Wilbert]

update 22-11:

bug in AviSource (latest CVS):

AviSource("D:\Temp\andreSample.avi",fourCC="dvsd", pixel_type="yuy2")

reports 'decompressor couldn't produce YV12', but the DV codec can't output YV12 (only YUY2).


if (bOpen && fYV12 && ICERR_OK == ICDecompressQuery(hic, pbiSrc, &biDst)) {
vi.pixel_type = VideoInfo::CS_YV12;
_RPT0(0,"AVISource: Opening as YV12.\n");
bOpen = false; // Skip further attempts
} else if (forcedType) {
env->ThrowError("AVISource: the video decompressor couldn't produce YV12 output");
}

forcedType is set to true (since a pixel_type is specified) hence the error.

It also doesn't work if pixel_type is not specified, since fYV12 is set to true, but it can't be opened as YV12. Since bOpen is set to false, it doesn't check YV411, and so on.

I'm not sure how to change this without using the "old" nested 'if else' statements.

[Wilbert]

bug in Overlay (latest CVS):


c = ImageSource("F:\Pictures\original.jpg").AddBorders(1,0,1,0).ConvertToYUY2
Overlay(c, c, mode="HardLight", opacity=1.0)

The luma (chroma idem) range is [0,255] and not scaled to [16,235].

I checked it for YV24 and YUY2. I didn't check other modes.

[sh0dan]

@Wilbert: This is by design. No coring is applied by itself, if luma (or chroma) are resulting in larger Y-values. The formula for hardlight is:

outY = baseY[x] + overlayY[x]*2 - 256;

The only place where Overlay cares about pc_range/tv_range, is when RGB input and output are converted. Otherwise it doesn't really care about the coring value.

[Wilbert]

@Wilbert: This is by design. No coring is applied by itself, if luma (or chroma) are resulting in larger Y-values.
Ok, i know. But, i assume best is to scale (or clamp ?) to CCIR-601 values when using those modes?

[IanB]

While reviewing some code with Arda, I had pause to think a little harder than usual about this coring issue. I think we are doing ourselves a very great disservice by continually clamping values in YUV processing.

Some background:- The CCIR-601 black=16, white=235 concept dates back to the transition from analogue to digital processing in the TV industry. In the analogue domain particularly when processing video signals with a colour sub carrier present there had to be provision for an amount of "head room" to prevent clipping when instantaneous signal value could exceed the nominal peak white or peak black levels. This often happened when mixing video for special effects or overlaying text titles. The visual results from overloading a video amplifier was quite horrid. The normal solution was to build head room into the video processor and, if required, finally attenuate the signal at the final output to conform to standard levels.

To allow this head room to be carried through a video processing chain including a new digital video processor the engineers of the day cleverly mapped the accepted overload levels for peak super-black=0 and peak super-white=255, which gave standard black=16 and standard white=235.

I propose we now follow this model with the coring option.

i.e. With coring=true we treat black=16 and white=235 BUT we do NOT clip at these levels. Further all 2.6 YUV filer are expected to have stable (documentable) behaviour when processing head room values. If people want clipping then they need to explicitly use the Limiter filter.

For coring=false we simply continue to use PC levels black=0 and white=255 as in previous versions.

Thoughts!

[Wilbert]

i.e. With coring=true we treat black=16 and white=235 BUT we do NOT clip at these levels. Further all 2.6 YUV filer are expected to have stable (documentable) behaviour when processing head room values. If people want clipping then they need to explicitly use the Limiter filter.

For coring=false we simply continue to use PC levels black=0 and white=255 as in previous versions.
Could you give a concrete filter example where this problem exists (ie luma/chroma are clipped while it needn't be)?

For me, when head room values are allowed (and kept), you need to treat it as a [0,255] YUV clip.

[IanB]

@Wilbert,

A [16..235] clip with over-range values is not always the same as a [0..255] clip.

Your ColorMatrix() filter needs to know the black level to work correctly, it does an env->Invoke("Limiter", ...) on the output. If a user would rather use Tweak(Sat=0.98765, Cont=0.94321) to more gracefully scale the possible over-range values they cannot.

It is not really a problem per se, rather more a philosophy that allows greater scope to control values. Clipping is a crude way to protect against over-range values, doing it should be a choice. For some cases scaling is a better choice, a choice users do not have if clipping is built into filters. Other more elabourate options for dealing with over-range values might be some form of squishing i.e. [233..235]->233, [236..245]->234, [246..255]->235, whatever it is it should be a choice.

A more hypothetical case :- Filter A does some image processing that can result in slight over-range values. The algorithm needs to know what the black and white levels are and author of the filter implements a coring option and because he knows he might generate over-range values he does standard output clipping. Filter B does some advanced edge detection, it is level agnostic. A user wants to chain filters A and B because he knows the processing filter A does will help filter B better recognise the edges he is interested in. His input data is standard TV range, so to get the effect he needs he enables the coring option. However the clipping filter A does causes a sharp change in the 1st derivative of the output, this spoils the results from filter B. The user is screwed, with coring off filter A doesn't process correctly, with corring on filter B trips over the abupt delta caused by the clipping.

[sh0dan]

Overlay actually applies an "advanced", though very unscientific method for handling cases, where Y>255 or <0. What I do is, that I gradually fade out chroma, thus creating pure white or pure black. This works surprisingly well visually. Here is the actual algorithm.

if (Y>255) { // Apply overbrightness to UV
int multiplier = max(0,288-Y); // 0 to 32
U = ((U*multiplier) + (128*(32-multiplier)))>>5;
V = ((V*multiplier) + (128*(32-multiplier)))>>5;
Y = 255;
}

Therefore, if Y is 288 or more, U&V will be 128 (neutral).

A similar algorithm could also be applied for coring, thereby neutralizing chroma, if Y is 255. 254 will leave a little of the original chroma, 253 a bit more and so on.

[IanB]

@Sh0dan,

Precisely another example for not arbitarily clipping inside filters, there are just so many ways to deal with over-range values, just given the chance.

We probably should crack a new thread on ways to manage over-range yuv data.

[Fizick]

Sorry for old thread post, but i see it as most appropriate.

I am interested in not-subsampled YUV color format, but not planar (3 planes), but interleaved.
one plane YUV24 like RGB24: YUV YUV YUV ... (or VUY... in memory)
or YUV32 like RGB32 (RGBA): YUVA YUVA (or VUYA ... in memory)

This format is convinient for:
- motion estimation (SAD) (luma with chroma),
- block (pixel) replacement,
- various filters (denoisers, etc) which take into account the color similarity of all pixel color component.

Implementation should be very similar (read "absolutely equal") to current RGB code in filters (besides color conversion).

Comments?

[MfA]

You can get cheap GPUs with multi GB/s transfer speed now. IMO the time is about right to put GPU support in Avisynth (ie. the ability to pass GPU memory reference to frames rather than system memory pointers, and potentially some GPU versions of the internal functions).

[sh0dan]

@MfA, Fizick: 2.6 is already way too feature-creeped, thus we still don't even have an alpha.

@Fiz: Yes, it would be nice, but more colorspaces means more complexity - and since it brings nothing to the end user (since we already have planar 4:4:4) - I cannot support yet another cs.

@MfA: That would IMO belong in a fork. GPU processing is very interesting - but it would be much better to start with a clean slate, and design it entirely for the GPU.