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).