Proposal for AviSynth 2.6

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