2.5 and YV12 "best practices" for porting

[SansGrip]

First off I apologise for using the b**t word in the subject ;).

I'm about halfway through my first "proper" port to YV12 and wondered if I was the only one having problems working out a "canonical" GetFrame structure. We all know that


for(int y = 0; y < height; ++y)
{
for(int x = 0; x < row_size; ++x)


and so on is often the "best" way to loop over YUY2 data. Is there an equivalent for YV12? Of course for simple stuff like Invert the two-consecutive-loops approach (one loop for luma, one for chroma) is probably the nicest way of doing it, but this seems to break down with more complex algorithms.

What I've come up with so far (since I need to process all components jointly) is something like this:


const unsigned char* py = frame->GetReadPtr(PLANAR_Y),
ppy = py + pitch_y;
const int pitch_y_2 = pitch_y * 2;
// etc.

unsigned char* dpy = dest->GetWritePtr(PLANAR_Y),
dppy = dpy + dpitch_y;
const int dpitch_y_2 = dpitch_y * 2;
// etc.

for(int y = 0; y < height_uv; ++y)
{
for(int x = 0; i < row_size_uv; ++x)
{
int yx = x << 1;

// Here py[yx], py[yx + 1], ppy[yx],
// ppy[yx + 1], pu[x] and pv[x] are
// magically transformed into new_y1_1,
// new_y2_1, new_y1_2, new_y2_2, new_u and
// new_v respectively

dpy[yx] = new_y1_1;
dpy[yx + 1] = new_y2_1;
dppy[yx] = new_y1_2;
dppy[yx + 1] = new_y1_2;
dpu[x] = new_u;
dpv[x] = new_v;
}

py += pitch_y_2;
ppy += pitch_y_2;
dpy += dpitch_y_2;
dppy += dpitch_y_2;
pu += pitch_uv;
pv += pitch_uv;
dpu += dpitch_uv;
dpv += dpitch_uv;
}


Does this look okay, or is it merely the product of a deranged mind? That's a terrifying number of pointers to maintain...

As I was working all this out it occurred to me that while there was an awful lot of code repetition (and related bugs, e.g. the old using-the-wrong-pitch snafu) writing for 2.0, there's even more with 2.5. I've already fixed two bugs involving switched pitch_y and pitch_uv, which are real easy to mix up if you've been staring at the screen for hours.

To this end I started considering an STL-esque class library for Avisynth filters. By maximizing inlined code and (unfortunately) avoiding polymorphism, etc., it should be possible to maintain performance while dramatically easing code readability and simplifying maintenance. It would also have the virtue of reducing the number of typo bugs because, well, there'd be less typing ;). What's more, judicious use of assert()s would catch buffer overruns first time, every time. Stupid idea? If it is, it's an appealing stupid idea :D.

Anyway, as to the second part of my question: Are there any "best practices" for 2.5 in general? My understanding is that temporal filters should always call SetCacheHints, and that IsYUV() no longer means IsYUY2(), but is there anything other than that? How about the idea of a function to return filter information such as version number? I think it would be very useful.

Note to mods: I considered putting this in the development section but it seemed more pertinent to this one. Feel free to move it there (or merge with another 2.5-related thread) if you think it better.

Thanks for listening :).

[sh0dan]

Best practice is (IMO) _always_ to do two or three seperate loops.
One for Y, and one or two for U/V. The loop setup code is only minimal.
Sometimes I have used a while loop, for routines that work both on RGB, YUY2 and YV12.

So the loop I'd suggest would be something like the one in the Invert example on the alpha page. If you would like to speed up the loops, I think it would be better to do more pixels in parallel i the same plane, to avoid reading pixels as single bytes.

[vlad59]

For some filters working in a planor colorspace is a pleasure :

With C3D I only have a function called ProcessPlane called three times (one for each plane). That's the benefit to have three totally independant process.
In YUV2 I was obliged to use a lots of tricks to avoid to deinterleave the stream.

For others it's quite a pain :

With TemporalCleaner, I need some information of the luma plane for processing the U and V plane. So I build a special Y function wich do normal work for the Y plane and in addition build a byte array with all information required by the chroma planes.
So I got a Y function much more complicated than the original YUV2 one and a UV function simplier.
All that for almost exactly the same speed for both filter.

My 2 euro cents

[SansGrip]

Best practice is (IMO) _always_ to do two or three seperate loops.
One for Y, and one or two for U/V.

But what of filters that need to process all components simultaneously? Wouldn't two or three loops be much more complicated?

[SansGrip]

For some filters working in a planor colorspace is a pleasure

For others it's quite a pain

Sounds like a certain packed format we're familiar with... ;)

With TemporalCleaner, I need some information of the luma plane for processing the U and V plane.

Yes, this is what I'm up against right now. When using the components together planar is definitely not the optimal scheme (at least as far as development goes).

So I build a special Y function wich do normal work for the Y plane and in addition build a byte array with all information required by the chroma planes.

Interesting, I'll have to think about that one.

[SansGrip]

An example of one of the problems I'm having is moving a 3x3 window over both luma and chroma components simultaneously. Am I missing something obvious or is it really that complicated?

[sh0dan]

Originally posted by SansGrip
[i]But what of filters that need to process all components simultaneously? Wouldn't two or three loops be much more complicated?
If they are interdependant, yes, if not, no.

An example of one of the problems I'm having is moving a 3x3 window over both luma and chroma components simultaneously. Am I missing something obvious or is it really that complicated?

In that case I would only process chroma every four (or maybe two) pixels.

BYTE* srcpY, srcpU, srcpV = source planes
int PitchY, PitchUV;
height=src->GetHeight(PLANAR_Y)/2;
int xloops=src->GetRowSize(PLANAR_Y)/2;
for (int y=0;y<height;y++){
for (int x=0;x<xloops;x++) {
//Process upper pixel 1 WITH chroma
srcpY[0]=srcpY[0];
srcpU[0]=srcpU[0];
srcpV[0]=srcpV[0];
// Process upper pixel 2 luma
srcpY[1]=srcpY[1];
// Process lower pixel 1 luma
srcpY[PitchY] = srcpY[PitchY];
// Process lower pixel 2 luma
srcpY[PitchY+1] = srcpY[PitchY+1];
}
srcpY+=(pitchY*2);
srcpU+=(pitchUV);
srcpV+=(pitchUV);
}

This will probably be the best, that would process 4 pixels interdependant luma/chroma at 4 pixels per loops. The biggest problem will be alignement, but if you use aligned rowwidths, you can get 8, 16 or even 32 pixels per loop. (Reading single bytes is quite slow - reading full 4-byte ints is much faster).

[SansGrip]

@sh0dan

Thanks for taking the time to think about this. Your code is definitely better than mine.

However, one of the things making a 3x3 window difficult for me is that there are four different configurations which must be taken into account. In the following, O represents chroma and X luma. Obviously the current pixel is the centre one.


O X O X O X X X X X X X
X X X X X X O X O X O X
O X O X O X X X X X X X

Is there a solution to this that I'm missing other than four code paths?

Edit: At the moment I'm actually thinking about some kind of iterator class...

[Bidoche]

@SansGrip

I may be mistaken,

but i Think sh0dan is proposing to process pixels per group of 4, (the four ones associtaed with one chroma sample)

therefore only one code path where you handle jointly the four cases

[SansGrip]

I may be mistaken

As may I :).

but i Think sh0dan is proposing to process pixels per group of 4, (the four ones associtaed with one chroma sample)

I typed out a paragraph but self-doubt crept over me. I'm going to take my youngest to school and think about it, then edit :).

[sh0dan]

Originally posted by Bidoche
but i Think sh0dan is proposing to process pixels per group of 4, (the four ones associtaed with one chroma sample)


Yes. These should be the only four cases you should be able to end up in (except border cases of course).

My point was, that for optimzation purposes you should only care about luma in case 4 (which is the same as the first part of the loop) - otherwise just rely on the chroma results from that test - that way you shouldn't care about luma for the next three pixels.

It may not be 100% correct, but on the other hand it is never correct because if you _really_ should do it right, chroma is placed _between_ the Y-lines downwards (but it IS connected to the leftmost pixel, just to make it more confusing).