Proposal for AviSynth 2.6

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