Frame types for AVS 3.0

[sh0dan]

Continued from this thread (http://forum.doom9.org/showthread.php?s=&threadid=48261).

@sh0dan

Please don't tell me 0 pitch is forbidden.

No - 0 pitch is no problem. Can't see any problems arising from it.

Regarding your hierarchy - problem is that it's not an easy distinction - is seems ok. But it should also be remembered, that both YV12 and YUY2 are YUV formats - however I still like your distinction better.

So the complete hierarchy could look like this:


General --- IsInterLeaved() --------+--- IsYUV() -- YUY2
\ \
\ IsPlanar() -- IsRGB() -- RGB24 / RGB32
|-- IsRGB()
|__ IsYUV() - IsYV12() - YV12 / I420


I still don't like that you using switch to choose an algorithm - IMO using the functions are both more readable, and safer. For instance, your filter will NOT process I420 material, because the enum is different, for internal purposes that filter writers shouldn't care about.

[Bidoche]

No - 0 pitch is no problem. Can't see any problems arising from it Cool, then BitBlit can be used to fill the squares, just have to create a row containing the good data.

Regarding your hierarchy - problem is that it's not an easy distinction - is seems ok. But it should also be remembered, that both YV12 and YUY2 are YUV formats - however I still like your distinction better. Generally in the processing, being interleaved makes YUY2 closer to RGB than to YV12.
But in some cases you would want to group YUY2 with YV12, for Crop for example, they have the same horizontal restrictions who can be shared...

I still don't like that you using switch to choose an algorithm - IMO using the functions are both more readable, and safer. For instance, your filter will NOT process I420 material, because the enum is different, for internal purposes that filter writers shouldn't care about.
I don't see how a list of ifs can be more readable than a switch...

And for safety concern, of course if we are willing to add many colorspace clones, with enums we will be sorry.
At least If they are differencied, I mean YV12 and I420 are the same internally. So let's them be the same.

So there are no such thing as I_I420 in 3.0 and my switchs are safe.

[sh0dan]

At least If they are differencied, I mean YV12 and I420 are the same internally. So let's them be the same.

Problem is, that for filters they are - for AviSynth it isn't. MPEG2DEC3 for instance decodes to I420, and it is marked as such in AviSynth.
AviSynth delivers the proper pointers to filters, so that the image appears as YV12 - even though it is arranged as I420 in memory. To filters I420 is fully transparent, as long as IsYV12() is used.

I420 is a good feature for input/output relying on being able to construct a correct I420 frame.

[Bidoche]

I don't get your point.
Filters who decode I420 could fill data as YV12 from the start, no ?

When you have I420 input you fill plane data accordingly.
If it's not the same way to fill as YV12 we don't care.

[sh0dan]

It is to avoid BitBlits. MPEG2DEC and AviSource (and DirectShowSource IIRC) decodes directly to an AviSynth framebuffer, and the pointers are adjusted according to how the data is aligned.

[Bidoche]

Decode directly to a framebuffer, that's evil :p

You mean for YV12/I420 it's fill all the data in one block, so plane order is an issue...

[sh0dan]

You have to supply a buffer for AviSource for instance.
The pitch/pointers are adjusted to match what we can expect from VFW.

However, if pitch is not mod-16 (as required) on YV12 (actually quite rare) the data is blitted into a larger buffer, by the AlignPlanar filter.

This is the "legacy" (read: BenRG) way of doing this - and even though it isn't pretty it works - and it works fast. IIRC DirectShowSource() does the same thing.

[Bidoche]

Hum, of course now that I think about it, vfw HAD to fill data in one block..

Guess I will have to make some VFW_YV12VideoFrame and VFW_I420VideoFrame who will initially allocate all planes into the first buffer and reallocate into others as needed....
But after that they will report being plain YV12.
Thanks to polymorphism it must be doable.

The pitch/pointers are adjusted to match what we can expect from VFW. And how is that ? If you can point me to the code, it would help

[sh0dan]

Sounds great!

In source.cpp (l:409) this frame is allocated:

PVideoFrame frame = env->NewVideoFrame(vi, -4);

This enforces a 4 byte alignment (which is always the same as VFW delivers). This is where the AlignPlanar filter is applied to all frames (source.cpp, l:88):

return AlignPlanar::Create(result);

AlignPlanar does nothing, if the pitch of a frame is at least mod16. Otherwise it realigns the image, by blitting it to a larger buffer.
In avisynth.cpp, line 754 it checks if V-plane should be placed first, using this line:

return ScriptEnvironment::NewPlanarVideoFrame(vi.width, vi.height, align, !vi.IsVPlaneFirst()); // If planar, maybe swap U&V

Around line 700 it then swaps the pointers, based on this test:

if (U_first) {
Uoffset = offset + pitch * height;
Voffset = offset + pitch * height + UVpitch * (height>>1);
} else {
Voffset = offset + pitch * height;
Uoffset = offset + pitch * height + UVpitch * (height>>1);
}

[Bidoche]

However, if pitch is not mod-16 (as required) This enforces a 4 byte alignment (which is always the same as VFW delivers). How can we get mod-16 when we start specifying mod-4.... ?

[sh0dan]

By applying the AlignPlanar filter. As long as a clip doesn't deliver non-mod16 clips, we're ok. Internally filters are allowed to create "forced size" frames, but they must _always_ apply the AlignPlanar before passing on the frame. This is why it is publicly available in avisynth.h

[Bidoche]

In fact, it's the "(actually quite rare)" part I was wondering about. :p

About all this align stuff, if 4 and 16 are the only used values, maybe it should be made this way.
I mean, can anybosy find a reason where a filter would want a greater align than 16 !?

[sh0dan]

Yes - raising the limit to mod32 is actually something I have been planning to do for some time. This will ensure mod16 on luma, which is needed for SSE2 optimizations on P4 and the upcoming Athlon 64.

(Unless you want to change the way it is now, and just let chroma and luma pitch be independant).

[Bidoche]

I provide same align on all planes, and YUY2 and RGB are aligned too (somehow the interleaved block in handled internally as a plane)

[sh0dan]

Sounds fair enough - let's keep planes separate, and do workarounds when video is delivered as a compact frame.

[Bidoche]

Is there a reason for chroma align to be the half of luma ?
If not, I would simply make them the same. :p

Btw could you install STLPort and boost ? (www.stlport.or and www.boost.org respectively)
3.0 need them now, but 2.x can use the help.

boost::scoped_ptr, for example can simplify (make shorter) and safer lots of code.
I would like to introduce it someplaces, but if it's break your compile, you won't like it :p

[sh0dan]

Are there too many files for it to be included in the CVS tree?

What do they do?

[Bidoche]

STLPort and boost are source libraries, there is no point in including them header by header in the cvs.

STLPort is a STL implementation to replace the one shipped with VC6 who is kinda old (I don't know for you, but I don't like seeing Copyright 95 in the headers).
Besides it adds some few things too (like hash_map which I need to do the FunctionTable)

And boost is a set of cool libraries for various needs, when things are already done in boost, I don't see the point doing them again (probably not as good).

Just take a look at boost::bll and boost::spirit.

[Bidoche]

And for outputting frames, are they align conditions too ?
Or does it accomodate the actual frame pitch(s).

[sh0dan]

Regarding STL / Boost: Of course I'd have liked to hear that "STL-port makes it possible for us to design generic plugin interfaces that will allow plugins to remain through API changes" .. or .. "Boost will enable me to quickly create the best working cache implementation in 2.5 without breaking plugins". :)

Seriously - I have no problem adding these two to the "compilation dependencies", if there is a good point to this. I do trust you, when you say they could be good tools to have - I'd just like to know what the advantages are (explained for us more "casual" programmers).

I assume they will not be needed for plugin compilation!?!

Output:
For "AVI output" we are given a framebuffer by VFW, so we have to bitblit here anyway. I could however imagine that the image writer could use "specific pitch" images. Some way to create a new frame, where you can force pitch would be nice.

[Bidoche]

design generic plugin interfaces that will allow plugins to remain through API changes That would be the work of COM.
But for now I ain't the guy to tell you much about it.
Avisynth already uses COM (from vfw), but does not exposes itself as COM objects.

the best working cache implementation in 2.5 without breaking plugins Arghhh...

I assume they will not be needed for plugin compilation!?! As long as we don't use them in avisynth.h (for 2.5)
For 3.0 it doesn't matter, plugins will be linked throught COM.

For "AVI output" we are given a framebuffer by VFW, so we have to bitblit here anyway. I could however imagine that the image writer could use "specific pitch" images. Some way to create a new frame, where you can force pitch would be nice. Can you be more descriptive.

I'd just like to know what the advantages are
Here a simple example: comparison of T * and (one of the possible replacement) boost::scoped_ptr.

scoped_ptr is a template class, ie its needs a class parameter to be fully qualified, the same way when you define a pointer you precise to which object it points.

You write it like it : scoped_ptr<T> pT; //T is the template parameter
where before you wrote: T * pT; //T was a parameter too :p

Before you had to explicitely initialize pT to 0 avoid garbage.
scoped_ptr has a default constructor who set itself to 0, so you don't have to care about that.
(Already one line less to write).

In destructor, you had to do : if (pT) delete pT; (or just delete if you know != 0)
scoped_ptr automatically deletes the owned object when it leaves the context.
That means you don't have to worry about deletion of the object in the destructor (or elsewhere) (one more line less).

But there is more, it makes it exception-safe :
If you construct an object with new, then call some code who throws an exception.
If you excepted the exception for some treatment, you catch it, do what must and go on.
If you excepted the exception but cannot handle it, you catch it, delete your pointer (otherwise it's leaked) and rethrow the exception. It works, but it's a costly solution (in developpement time).
If the exception caught you unaware, your pointer is leaked and there is nothing you can do about it. :/

On the contrary, with a scoped_ptr, no possible leak.
The scoped_ptr destructor (which delete the object) will be automatically called in the stack unwinding process.


Conclusion : Besides being safer and faster to use than a regular ptr, so why not use it.
Imagine there were no PVideoFrame and you had to use VideoFrame * and handle the AddRef/Release calls by yourselves. That's the same idea.

NB: scoped_ptr is only one of the possible smart ptr , who do not share ownership, there are others.

[Bidoche]

On a completely different subject, I suggest we add this method somewhere :
//paint a memory rectangle of given width, height, pitch
//with the values of its first line
//(pitch allowed negative to paint from bottom to top)
void RectanglePaint(BYTE * dstp, int width, int height, int pitch)
{
BitBlit(dstp, pitch, dstp, pitch, width, height);
}

I have continued reading of source.cpp and this would be an appreciable factorisation for ColorBars, BlankClip (maybe others)