YUV12 in avisynth

[Marc FD]

Hi :)
I've heards some things about YUV12 support in avisynth, but there is nothing really serious.
(I'd really like my new filters, who are YUV12 native, to work in avisynth)
What do you think of starting YUV12 devellopement ?
What is really needed ?

- Conversion routines :
RGB32/24->YUV12 in ConvertToYUV12 (current YUV-RGB code to adapt)
YUV12->RGB32/24 in ConvertToRGB (current YUV-RGB code to adapt)
YUY2->YUV12 in ConvertToYUV12 (really trivial. deinterleave)
YUV12->YUY2 in ConvertToYUY2 (really trivial. interleave)

- YUV12 archictecture
Avisynth would provide Y,U and V Pointers and Pitchs, when using YUV12.

- YUY2->YUV12 code adaptation.
Porting YUY2 code to YUV12 may not be very easy, because of the different archictecture, but it's not too hard to adapt C code.
For ASM-optimised code, 2D processing is easier (in my point of view) to code in YUV12 than in YUY2. especially YUY2 1D horizontal processing, who is really a pain.

Of course, if the avisynth team need coders, i'm volunteer :D

Cheers,
MarcFD

[Nic]

Id highly recommned the XviD color conversion routines if you want to & from YV12 :)

-Nic

[sh0dan]

@marc:
I've created an Avisynth_2_1 branch in the CVS, but I haven't put anything up yet, since I still have to rewrite audio.cpp for the new audio handling (it doesn't quite compile yet). I'll notify you all here, when a proposal is finished.
YV12 is definately a 2.1 priority IMO - but my time is limited ATM.
YUY2->YV12 seems trivial, but others have done it elsewhere.

@nic:
Yes - the Xvid routines would be preferable, if they are portable (nasm->masm) without too many problems - they are VERY well-written.

[vlad59]

I never read avisynth core sources before today so please forgive me if I say dumb things ;)

My thoughts about adding YV12 colorspace support in avisynth :
- To make things easier we could keep only one VideoFrameBuffer per VideoFrame and have 3 new pointers for Y, U and V plane. offset and pitch don't need to be duplicated because U and V pitch and offset can be calculated with Y pitch and offset. For width and height, it's the same.

- The first avisynth function to port is Avisource (to make test) and it should be better for now to only decode in YV12 if the parameter is set.

I'll make some tests. I have a lot to learn in vfw programming so it can be long

[sh0dan]

Originally posted by vlad59

- To make things easier we could keep only one VideoFrameBuffer per VideoFrame and have 3 new pointers for Y, U and V plane. offset and pitch don't need to be duplicated because U and V pitch and offset can be calculated with Y pitch and offset. For width and height, it's the same.

U-V pitch should be the same, but should not be calculated from Y, since pitch also is dependant on other things, as memoryalignment, etc. If Y-pitch is is 8byte-aligned, UV-pitch will only be 4byte aligned, which will result in penalties. Solutions could be:

- GetPitch() returns Ypitch + (UVpitch<<16);
Hacky, but it would work.
- Implement GetUVPitch();
Probably the most reliable.

Regarding data, can it not be assumed that
U = *(data + height* Ypitch)
V = *(V + UVpitch*(height/2))

This would how it would be returned from vfw (and hopefully any MPEG-decoder) - or am I mistaking?


- The first avisynth function to port is Avisource (to make test) and it should be better for now to only decode in YV12 if the parameter is set.


Sounds like a great idea! I'll also get on it - we should get as many opinions on this as possible, since it's a very big change, that many people will have to work with it!

[Marc FD]

Originally posted by sh0dan

Regarding data, can it not be assumed that
U = *(data + height* Ypitch)
V = *(V + UVpitch*(height/2))

This would how it would be returned from vfw (and hopefully any MPEG-decoder) - or am I mistaking?



no, i don't think so, because of alignement too.
but it might be correct. i'll check that.

BTW, what do you think of :
overload GetReadPtr()/GetWritePtr()
with GetReadPtr(int channel) / GetWritePtr(int channel)
in avisynth.h, add a enum {Ychannel, Uchannel, Vchannel} or 3 define.

GetReadPtr(Uchannel) would give you a pointer to the U block, ect...

[vlad59]

@MarcFD

Yes your idea to overload GetReadPtr()/GetWritePtr() is great and with that we could imagine to have pointer to H/S/L plane one day.

About Sh0dan formulas :
U = *(data + height* Ypitch)
V = *(V + UVpitch*(height/2))

Just to be sure I'm not wrong
Width and height must be mod 4 so we really must care about memory alignement and make 16bits aligned buffer when possible to allow further SSE2 optimizations. Now I think (But I can be wrong, I haven't read all avisynth core code) alignement is done by the pitch wich will always be mod 8 or mod 16.
So this formula should be correct.
U = *(data + height* Ypitch)
V = *(V + UVpitch*(height/2))

For vfw I have no idea of how it work.

[trbarry]

I maybe haven't thought this through properly but you might get more compatibility with less Avisynth code changes if you just arranged the buffer like:


yyyy ... yyyy ...
yyyy ... yyyy
yyyy ... yyyy
yyyy ... yyyy
uuuu ... vvvv
uuuu ... vvvv ...

That is, only one pointer to the data and only one pitch and line count, like now. I believe 4:2:0 is only valid for even pixel widths and heights so it would be easy to calculate where the U and V blocks begin.

But I am thinking only a way to handle 4:2:0 planar data. I don't know what a YV12 file format looks like so maybe this isn't compatible.

Also be aware that a conversion from 4:2:0 to 4:2:2 (or RGB) needs to now whether it is a frame or field picture, that is, whether it was interlaced. Ignoring this is one of the sources of chroma delay causing so much flap with DVD players and decoders recently. So that info has to be dragged along with the data as long as it stays in 4:2:0 format.

- Tom

edit: Oops, forgot about the multiple of 8 thing mentioned above for performance. So it's better to waste the space and use:

yyyy...
uu...
vv...

[Marc FD]

i just etudied some cases :

- In XviD :
the IMAGE struct holds 3 pointers : y,u and v
stride is used for Y and stride>>1 or stride/2 for U and V

- In DVD2AVI MPEG-2 Decoder (MPEG2Dec)
it uses an array of 3 pointers. ex : src[0] is Y src[1] is U and src[2]
stride is used for Y and stride2 for UV. but stride2 is everywhere computed this way : stride2 = stride/2

But in these cases it's an internal architecture.
( i see avisynth more like a video processing platform )
so avisynth should handle YUV12 then cleanest way.

even 100 bytes spoiled for each frame is really ridiculous regarding the quantity of data we are processing, no ?

[-h]

even 100 bytes spoiled for each frame is really ridiculous regarding the quantity of data we are processing, no ?

Most applications will pass YV12 data to you in the form (Yplane with Ystride), (Uplane with Ystride/2), (Vplane with Ystride/2). You can enforce some larger U/V strides internally, it'll just mean a bunch of extra memcpy()'s per every frame and a cache hit by doing so.

-h

[Marc FD]

of course. i never said the opposite :)

the idea is, avisynth should use Ypitch/Ywidth/Yheight and UVpitch/UVwidth/UVheight, like ffdshow does !!
+ 3 pointers for Y,U and V.

with this toolbox, avisynth filters could be adpated to any configuration :
- Y, U and V aligned (classic) or Y, U and V non aligned !
- UVstride == Ystride/2 or UVstride != Ystride/2
or any other possibility.
the *source filter will fill this with the specific info.
then the filter will work the cleanest way possible.
when we alloc a new image, then we could align the Y,U and V blocks for optimal performance. All blocks should be 16 bytes aligned when
possible in avisynth.

BTW, a usefull debuging function to add in avisynth i had : KillCache()

PVideoFrame KillCache::GetFrame(n,IScriptEnv env)
{
PVideoFrame src = child->GetFrame(n);
PVideoFrame dst = env->NewVideoFrame(vi);
env->BitBlt(<src to dst>); // (i'm too lazy to write the whole code)
return dst;
}

this way, debugging of filters is much easier :

*source().Crop,trim,resize,ect....
KillCache()
SuspiciousOrNotStableFilter() #mainly mine ;)

because the blocks are as thin as they can be, any slight memory acces violation will be sanctionned by an exeption.
and a frame processed through KillCache should always be aligned.
adding a align check in end of it seems a good idea.

and, while i'm here, i've a (silly?) idea for avisynth 2.1 :
adding a AVSvalue in the VideoFrame Struct.
any filter could acces it by GetFrameInfo.
this way we can do cheap filter communication.
of course it's only to give some usefull hints (decomb patterns, scenes-changes, this kind of info...) not to store big masks.
maybe an extention of AVSvalue (i think of a Ptr type) could help to handle masks gracefully.

What do you think ?

[WarpEnterprises]

Could somebody please sum up the advantage of a third color format being used in AviSynth?

[trbarry]

Could somebody please sum up the advantage of a third color format being used in AviSynth?

Don't know about others but my own reasons are fairly simple.

1) Color conversions are potentially lossy, and slow.

2) Most of my video input comes from DVD's or HDTV and both of those are already in YV12 format.

3) Most of my video output goes to MPEG2 or MPEG4 (Divx or Xvid) also in YV12 format.

4) Some algorithms run faster or are easier to write when working with planar 4:2:0 data.

- Tom

[Guest]

4) Some algorithms run faster or are easier to write when working with planar 4:2:0 data.
Indulging in understatement, eh Tom? YUY2 is a giant pain in the you-know-what. Let's go planar!

[vlad59]

As -h said, I think we must stay simple and choose an already working solution. Extra optimizations can come later.

[sh0dan]

Fast response (I'll do a more detailed later)

- YV12 is easier and faster to process than YUY2, since it's planar and is source of MPEG2 decoding, and destination format of MPEG4 anyway. And it's faster because it has half the UV-information of YUY2.

- Pitch/Stride is mostly (by default) 8-byte-aligned , but that's easy to change. AVI always delivered in 4-byte-aligned frames. Perhaps the default alignment should be autodetected, and set to 16 if a P4 is detected. It seems like most apps use UV-pitch=Ypitch/2, which makes sense. but all frames are BitBlitted on output anyway, so output wouldn't mind if we have two different pitches.
Both pros and cons.
Two pitches = A little faster, less compatible (when converting filters from other apps).
One pitch = A little slower, easier integration of existing filters (ffdshow for instance).


- Seperate YUV pointers. Not IMO! They are easy to calculate, and seperate pointers, where they point to different memory-locations would involve copying the data when YV12 is delivered, and general confusion. Seperate pointers would also make YV12 too much a special case.
We could make helper functions that return the UV-pointers, but they should always be placed after Y with the correct pitch.

[trbarry]

Perhaps the default alignment should be autodetected, and set to 16 if a P4 is detected.

I think that if the default alignment in Avisynth was always 16 bytes then everything would run faster even on Athlon's and P3's, because data would more often be in a single cache line.

With memory sizes the way they are these days nobody gets enough small memory chunks that it is even worth while worrying about wasting a few bytes to get the alignment. So we should maybe just align everything in Avisynth to at least 16 bytes. Life would be simpler.

- Tom

[Marc FD]

Originally posted by sh0dan

One pitch = A little slower, easier integration of existing filters (ffdshow for instance).

ffdsow uses 2 pitches, i pointed it out some posts before.


- Seperate YUV pointers. Not IMO! They are easy to calculate, and seperate pointers, where they point to different memory-locations would involve copying the data when YV12 is delivered, and general confusion. Seperate pointers would also make YV12 too much a special case.
We could make helper functions that return the UV-pointers, but they should always be placed after Y with the correct pitch.


maybe i'm wrong :), but :

1) The decoder i know the much in _deep_ detail is the MPEG2 decoder of DVD2AVI :
I'm sure of one thing : it uses 3 pointers, and it _NEEDS_ 3 pointers because each block as it own malloc (assuming they are contiguous is dangerous) and the output is done in the "assembleframe" function (the name speaks for itself) who just assemble the 3 blocks in one YUY2 or RGB32 block.

2) ffdshow filters :
3ptrs, 2 heigths, 2 widths, 2 pitchs.
see "TimgFilter"
it uses XviD (see below) for assembling/csp convertions.

3) XviD internally :
3ptrs, 1 heigth, 1 width, 2 pitchs.
see "IMAGE", "FRAME" and "DEC_FRAME"
it assembles them in "image_output",
depending of the colospace

4) ffdshow using XviD :
see "TmovieSourceXviD::getframe"
ffdshow uses XVID_CSP_USER to avoid any
assembling. this way, ffdshow is able to directly use the
internal image of XviD.
colorspace convertion is simply ignored.

5) in Nic's PP :
3 ptrs, 1 heigth, 1 width, 1 pitch.


Seperate pointers would also make YV12 too much a special case.


But IT IS a special case. RGB32,RGB24,YUY2 are NOT planar formats.
to handle a planar format, you need OF COURSE a new archictecture.

i think it's why VitualDub/Avisynth are currently not using it.

[sh0dan]

I think we're getting to the point, where I think it's arguments for the sake of arguments.

The compromise:

- 2 pitches. int GetUVPitch() Gets implemented into VideoFrame.
- VideoFrameBuffer hold ONE (1) pointer to data, as always, but gets a new function to return other pointers. Implemented as const BYTE* GetChannelReadPtr(byte channel) const { return data+(stuff); }. That way full compatibility is retained.

Any objections?

[Marc FD]

Originally posted by sh0dan
- 2 pitches. int GetUVPitch() Gets implemented into VideoFrame.
- VideoFrameBuffer hold ONE (1) pointer to data, as always, but gets a new function to return other pointers. Implemented as const BYTE* GetChannelReadPtr(byte channel) const { return data+(stuff); }. That way full compatibility is retained.
[/B]

what is (stuff) ??

BTW, i absolutly knows nothing about avisynth internals, but if the 3 blocks aren't contiguous, how do you free/copy the data :confused:

[vlad59]

@MarcFD

Of course as it's the first planar format in avisynth we'll have to change a little bit the architecture.

But I think we have to use a maximum current avisynth code when possible to use stable code a maximum.

I personnaly it's a good idea to keep only one VideoFrameBuffer* in VideoFrame so we don't have to change anything in cache specific code.

For now we just have to be talk about new functions of VideoFrame not the internal code (wich can evolve in the future).

So my proposition is :

class VideoFrame {
int refcount;
VideoFrameBuffer* const vfb;
const int offset, pitch, row_size, height;

friend class PVideoFrame;
void AddRef() { ++refcount; }
void Release() { if (refcount==1) --vfb->refcount; --refcount; }

friend class ScriptEnvironment;
friend class Cache;

VideoFrame(VideoFrameBuffer* _vfb, int _offset, int _pitch, int _row_size, int _height);

void* operator new(unsigned size);

public:
int GetPitch() const { return pitch; }
int GetYPitch () const {return pitch;}
int GetUPitch () const {return pitch>>1;}
int GetVPitch () const {return pitch>>1;}
// we could have done a int GetUVPitch () const {return pitch>>1;}

int GetRowSize() const { return row_size; }
int GetHeight() const { return height; }

// generally you shouldn't use these two
VideoFrameBuffer* GetFrameBuffer() const { return vfb; }
int GetOffset() const { return offset; }

// in plugins use env->SubFrame()
VideoFrame* Subframe(int rel_offset, int new_pitch, int new_row_size, int new_height) const;

const BYTE* GetReadPtr() const { return vfb->GetReadPtr() + offset; }
const BYTE* GetReadPtr(BYTE p_channel) const { return vfb->GetReadPtr() + ???? (to be defined); }


bool IsWritable() const { return (refcount == 1 && vfb->refcount == 1); }

BYTE* GetWritePtr() const {
return IsWritable() ? (vfb->GetWritePtr() + offset) : 0;
}

~VideoFrame() { --vfb->refcount; }
};



the code defined can be changed of course it's only my current sources.

The problem will be eventually with U or V pointer wich could be only 2bytes aligned depending of the cropping values.

EDIT : Sorry Sh0dan and MarcFd I've waited too long before send my answer, I haven't read your last posts.
I also forgot to check for pixel_type in specific Get_Pitch.

[Marc FD]

the question is :
do you want YUV 4:2:0 (I420/YV12) planar support in avisynth (3 blocks),
or you just want block YV12 (1 block) ??

i'd prefer full planar support.

(XviD/ffdshow/MPEG2Dec/ect.. uses full YUV 4:2:0 _planar_ format)

i don't know how to use polymorphism in C++, but if it's like in Delphi, should be possible to implement a child of VideoFrame, like PlanarVideoFrame, and then use it if you have YV12 colorspace, no ??

maybe it's impossible :(

[sh0dan]

Originally posted by Marc FD

what is (stuff) ??

Offset (see next post)

BTW, i absolutly knows nothing about avisynth internals, but if the 3 blocks aren't contiguous, how do you free/copy the data :confused:

You don't - as with the existing framework, you never allocate or deallocate a frame yourself. Allocation is done using NewVideoFrame(), and Avisynth does the deallocation.
As now, you cannot write to the pointers yourself, you can only request an empty frame and write to it. If you want 3 pointers, you have to request three pointers, my point is, that you could do this by only requesting one (as now), and easily calculate the rest yourself.

Therefore we can force data to be continuous, I can't see any real arguments for not having this, honestly.

[sh0dan]

Originally posted by vlad59

So my proposition is :
[...]

With small changes:


class VideoFrame {
int refcount;
VideoFrameBuffer* const vfb;
const int offset, pitch, row_size, height, int UV_pitch;

friend class PVideoFrame;
void AddRef() { ++refcount; }
void Release() { if (refcount==1) --vfb->refcount; --refcount; }

friend class ScriptEnvironment;
friend class Cache;

VideoFrame(VideoFrameBuffer* _vfb, int _offset, int _pitch, int _row_size, int _height);

void* operator new(unsigned size);
enum {YV12_CHANNEL_Y=0;YV12_CHANNEL_U=1;YV12_CHANNEL_V=2}; // for planar modes only.
public:
int GetPitch() const { return pitch; }
int GetYPitch () const {return pitch;}
int GetUVPitch () const {return uv_pitch;}

int GetRowSize() const { return row_size; }
int GetHeight() const { return height; }

// generally you shouldn't use these two
VideoFrameBuffer* GetFrameBuffer() const { return vfb; }
int GetOffset() const { return offset; }

// in plugins use env->SubFrame()
VideoFrame* Subframe(int rel_offset, int new_pitch, int new_row_size, int new_height) const;

const BYTE* GetReadPtr() const { return vfb->GetReadPtr() + offset; }
const BYTE* GetReadPtr(BYTE p_channel) const {
switch (p_channel) {
case YV12_CHANNEL_Y:
return vfb->GetReadPtr();
case YV12_CHANNEL_U:
return vfb->GetReadPtr()+(height*pitch);
case YV12_CHANNEL_V:
return vfb->GetReadPtr()+(height*pitch)+((height>>1)*uv_pitch);
default:
_ASSERT(FALSE);
}
return vfb->GetReadPtr();
}


bool IsWritable() const { return (refcount == 1 && vfb->refcount == 1); }

BYTE* GetWritePtr() const {
return IsWritable() ? (vfb->GetWritePtr() + offset) : 0;
}

~VideoFrame() { --vfb->refcount; }
};


Would it make more sense to overload GetPitch, and make different types, so it could return even more types of pitches?

[EDIT]"Full planar" is still possible, since the enums just could be expanded - perhaps the GetPitch should also be like this:

int GetPitch (BYTE p_channel) const {
switch (p_channel) {
case YV12_CHANNEL_Y:
return pitch;
case YV12_CHANNEL_U:
return uv_pitch;
case YV12_CHANNEL_V:
return uv_pitch;
default:
_ASSERT(FALSE);
}
return uv_pitch;
}

[-h]

do you want YUV 4:2:0 (I420/YV12) planar support in avisynth (3 blocks), or you just want block YV12 (1 block) ??

Remember, the only reason that programs like DVD2AVI/MPEG2DEC/XviD/libavcodec use separate planes is because of the image edging that the MPEG standards necessitate. Any program that desires YV12 input is going to want it in a single block, and will output it as a similar block, since that's what the standard requires (see hardware overlay support for why this is important). If you want SSE2-compatible alignment, you could do that with a single block by aligning it on 32 bytes, giving the UV planes 16-byte alignment.

I guess my biggest qualm with separate blocks is that you will have to blit the input into separate buffers at the start, then blit it back into a single block at the end, when you could avoid both. Though you could resolve this by making sure any yv12 input source is block-aware, and fills the pointers for you (i.e. a special build of mpeg2dec or something).

-h

[Marc FD]

Originally posted by sh0dan

Therefore we can force data to be continuous, I can't see any real arguments for not having this, honestly.

you are right.

i've taken a look in the avisynth internals (the first time i do this ;) ) and i see know what you think. it make sense for me now :D

sorry if i didn't understood you immediatly :)

Okay, so when we use I420 colorspace with a bit alignement.
you need a new pitch,row_size and height.

my suggestion


class VideoFrame {
int refcount;
VideoFrameBuffer* const vfb;
const int offset, pitch, row_size, height;
const int pitch2, row_size2, height2; // needed for I420 alignement

friend class PVideoFrame;
void AddRef() { ++refcount; }
void Release() { if (refcount==1) --vfb->refcount; --refcount; }

friend class ScriptEnvironment;
friend class Cache;

VideoFrame(VideoFrameBuffer* _vfb, int _offset, int _pitch, int _row_size, int _height);

void* operator new(unsigned size);

public:
int GetPitch() const { return pitch; }
int GetPitch2() const {return pitch2;}

int GetRowSize() const { return row_size; }
int GetRowSize2() const { return row_size2; }
int GetHeight() const { return height; }
int GetHeight2() const { return height2; }

// generally you shouldn't use these two
VideoFrameBuffer* GetFrameBuffer() const { return vfb; }
int GetOffset() const { return offset; }

// in plugins use env->SubFrame()
VideoFrame* Subframe(int rel_offset, int new_pitch, int new_row_size, int new_height) const;

const BYTE* GetReadPtr() const { return vfb->GetReadPtr() + offset; }
const BYTE* GetReadPtr(BYTE plane) const {
if (plane==Y_PLANE) {
return vfb->GetReadPtr() + offset;
} else if (plane==U_PLANE) {
return vfp->GetReadPtr() + offset + pitch*height(+alignement trick)
} else if (plane==V_PLANE) {
return vfp->GetReadPtr() + offset + pitch*height(+alignement trick) + pitch2*height2(+alignement trick)
} else {
return vfb->GetReadPtr() + offset; // default
}
}

vfp->GetReadPtr() + offset + pitch*height + pitch2*height2 +


bool IsWritable() const { return (refcount == 1 && vfb->refcount == 1); }

BYTE* GetWritePtr() const {
return IsWritable() ? (vfb->GetWritePtr() + offset) : 0;
}

~VideoFrame() { --vfb->refcount; }
};


seems logical, no ?

EDIT : sorry didn't see yours ;)

[vlad59]

Originally posted by sh0dan
Would it make more sense to overload GetPitch, and make different types, so it could return even more types of pitches?

[EDIT]"Full planar" is still possible, since the enums just could be expanded - perhaps the GetPitch should also be like this:

int GetPitch (BYTE p_channel) const {
switch (p_channel) {
case YV12_CHANNEL_Y:
return pitch;
case YV12_CHANNEL_U:
return uv_pitch;
case YV12_CHANNEL_V:
return uv_pitch;
default:
_ASSERT(FALSE);
}
return uv_pitch;
}

[/B]

Yes I think we must use the same trick on GetReadPtr, GetPitch and even GetRowSize and GetHeight.

And it must also be the same with GetWritePtr. About GetWritePtr I think we will be obliged to provide standard function to copy one plane as Bitblt will copy the three planes.

@MarcFD
I globally agree with your code example (althought I prefer uv_pitch than pitch2 ;)).
But IMHO height2 and rowsize2 are not usefull as
height2 = height/2 and
rowsize2 = rowsize/2

Instead we need specific offset for U and V plane (to crop and keep correct alignment).

I'll have to think more about it.

[sh0dan]

@marcfd:

- Pitches are already aligned - no need to adjust them. Consider pitches aligned widths.

- No need for seperate height and row-size - they are always half of the Y-original.

- We should use existing naming convensions. In half a year, people will wonder what the difference is between GetPitch() and GetPitch2() - avisynth.h should be as meaningful as possible.

[Marc FD]

Originally posted by vlad59

But IMHO height2 and rowsize2 and not usefull as
height2 = height/2 and
rowsize2 = rowsize/2


oups i forgot we have mod2 restricition :)
yes of course you don't need them.
uv_pitch is not bad but then y_pitch is logical.
it's why i prefer the pitch/pitch2 pair or (compromise) a pitch/pitchUV

[sh0dan]

Originally posted by vlad59

Instead we need specific offset for U and V plane (to crop and keep correct alignment).


You're right!! I had completely forgotten about crop!

Wouldn't this hold up? Or is my math bad?

const BYTE* GetReadPtr(BYTE p_channel) const {
switch (p_channel) {
case YV12_CHANNEL_Y:
return vfb->GetReadPtr();
case YV12_CHANNEL_U:
return vfb->GetReadPtr()+(height*pitch)+(offset>>2);
case YV12_CHANNEL_V:
return vfb->GetReadPtr()+(height*pitch)+(offset>>1)+((height>>1)*uv_pitch);
default:
_ASSERT(FALSE);
}

[vlad59]

Originally posted by sh0dan


You're right!! I had completely forgotten about crop!

Wouldn't this hold up? Or is my math bad?

const BYTE* GetReadPtr(BYTE p_channel) const {
switch (p_channel) {
case YV12_CHANNEL_Y:
return vfb->GetReadPtr();
case YV12_CHANNEL_U:
return vfb->GetReadPtr()+(height*pitch)+(offset>>2);
case YV12_CHANNEL_V:
return vfb->GetReadPtr()+(height*pitch)+(offset>>1)+((height>>1)*uv_pitch);
default:
_ASSERT(FALSE);
}


No I think it should be :

const BYTE* GetReadPtr(BYTE p_channel) const {
switch (p_channel) {
case YV12_CHANNEL_Y:
return vfb->GetReadPtr() + offset; // at least I'm sure of that
case YV12_CHANNEL_U:
return vfb->GetReadPtr+(height*(pitch + offset))+(offset>>1);
case YV12_CHANNEL_V:
return vfb->GetReadPtr()+(height*(pitch + offset))+((height>>1)*(uv_pitch + offset>>1))+ offset>>1;
default:
_ASSERT(FALSE);
}



I'm not 100% sure. But I think it's correct (I'm often wrong ;)).

We must also add some check to allow the use of this function only with YV12.
So I have many questions :
in

enum { UNKNOWN=0, BGR24=0x13, BGR32=0x14, YUY2=0x22, YV12=0x?? };

what values should we use :
0x2? to make isYUV answer true
or
0x3? to make a future isPlanar answer true

With we should have :

const BYTE* GetReadPtr(BYTE p_channel) const {
if (!(pixel_type&0x30))
Throw error
or
if (pixel_type != YV12)
Throw error



Lots of things to think about :o

EDIT : @Sh0dan forget about my correction, it's wrong I have misunderstood the use of offset, sorry :eek:

[sh0dan]

Originally posted by vlad59
No I think it should be :

const BYTE* GetReadPtr(BYTE p_channel) const {
switch (p_channel) {
case YV12_CHANNEL_Y:
return vfb->GetReadPtr() + offset; // at least I'm sure of that
case YV12_CHANNEL_U:
return vfb->GetReadPtr+(height*(pitch + offset))+(offset>>1);
case YV12_CHANNEL_V:
return vfb->GetReadPtr()+(height*(pitch + offset))+((height>>1)*(uv_pitch + offset>>1))+ offset>>1;
default:
_ASSERT(FALSE);
}


ok - I'll do the math (just as exited as you are) :scared:

YPitch = 100
UVPitch = 50
We crop: top = 4 pixels, left= 6 pixels.

Y offset = (4 * 1 byte * Ypitch) + 6 * 1 byte = 406 bytes (not mod4 <shivers>)
U offset = (4 pixels * 1 byte * UVpitch)/2 + (6 pixels * 1 byte) /2 = 103

Surprise! None of us was right. Conclusion - we need to store one offset for Y and one for each U and V.

I'm not 100% sure. But I think it's correct (I'm often wrong ;)).

Well - that goes for both of us :)

We must also add some check to allow the use of this function only with YV12.
So I have many questions :
in

enum { UNKNOWN=0, BGR24=0x13, BGR32=0x14, YUY2=0x22, YV12=0x?? };

what values should we use :
0x2? to make isYUV answer true
or
0x3? to make a future isPlanar answer true


Bens raionale seems to be 0x"RGB=1, YUV=2""how many bytes per pixel" - this just doesn't make sense when dealing with YV12. A way of dealing with it, could be to redefine them as masks - not the best way, but better.
So:

enum { ISBGR=1<<1, ISYUV=1<<2, ISPLANAR=1<<3 };


But then, how do we know if it is RGB24 or RGB32, and how do we handle BytesFromPixels? Should we add a "byte bitsperpixel" to the struct, and derive BytesFromPixels from that?

Lots of things to think about :o

Yeah - too many things are still unclear!

I hope you all see why we can't rush into doing YV12 - there are many considerations we have to do before implementing it. :scared:

[Marc FD]

Originally posted by sh0dan


enum { ISBGR=1<<1, ISYUV=1<<2, ISPLANAR=1<<3 };



what do you think of 0x4xx for planar colorspace ?

4:2:0 -> 0x420 / 4:2:2 -> 0x422 / 4:4:4 -> 0x444

it makes sense ??


Yeah - too many things are still unclear!

I hope you all see why we can't rush into doing YV12 - there are many considerations we have to do before implementing it. :scared:

sure.

let's discuss of everything we need :)

[vlad59]

Originally posted by sh0dan

Surprise! None of us was right. Conclusion - we need to store one offset for Y and one for each U and V.



Yes I went to the same conclusion before going out for lunch. We must think quickly of how to crop and resize to test our ideas.



Bens raionale seems to be 0x"RGB=1, YUV=2""how many bytes per pixel" - this just doesn't make sense when dealing with YV12. A way of dealing with it, could be to redefine them as masks - not the best way, but better.
So:

enum { ISBGR=1<<1, ISYUV=1<<2, ISPLANAR=1<<3 };


But then, how do we know if it is RGB24 or RGB32, and how do we handle BytesFromPixels? Should we add a "byte bitsperpixel" to the struct, and derive BytesFromPixels from that?



No we could keep the same structure and add the planar information
So we could have

enum { UNKNOWN=0, BGR24=0x013, BGR32=0x014, YUY2=0x022, YV12=0x122 };

Here we loose space as we consider that YV12 use 2 byte per pixel instead of 1.5 (wich is not so bad as a beginning.

But MarcFD's idea is a good one too. we really need to share our ideas



Yeah - too many things are still unclear!

I hope you all see why we can't rush into doing YV12 - there are many considerations we have to do before implementing it. :scared:

I agree we really need to take enought time to have clear specs.

I need some sleep after that ;) :o

[sh0dan]

A little OT. Took some time, and finished first proposal for new sample support in 2.1, with float samples and multiple channels. It compiles, and has AVI in/out working. Some audio filters have been converted, but not tested. Plugins seems to be a problem - we'll have to see about that - they seem to break sound(?)

Most interesting changes are in avisynth.h

You can see the branch at: http://cvs.sourceforge.net/cgi-bin/viewcvs.cgi/avisynth2/avisynth/?only_with_tag=avisynth_2_1