[FFmpeg-devel] [RFC] Request for pixdesc API review

[Stefano Sabatini]

On date Tuesday 2009-11-17 16:24:16 +0100, Stefano Sabatini encoded:
> On date Tuesday 2009-11-17 14:51:09 +0100, Michael Niedermayer encoded:
> > On Mon, Nov 16, 2009 at 12:43:02AM +0100, Stefano Sabatini wrote:
> > [...]
> > >  pixdesc.h |    4 ++++
> > >  1 file changed, 4 insertions(+)
> > > d60486d284ea2688ea83ef088ef8d858fbb1a367  clarify-log2-chroma.patch
> > > Index: ffmpeg/libavutil/pixdesc.h
> > > ===================================================================
> > > --- ffmpeg.orig/libavutil/pixdesc.h	2009-11-16 00:09:44.000000000 +0100
> > > +++ ffmpeg/libavutil/pixdesc.h	2009-11-16 00:11:10.000000000 +0100
> > > @@ -62,6 +62,8 @@
> > >       * For YV12 this is 1 for example.
> > >       * chroma_width = -((-luma_width) >> log2_chroma_w)
> > >       * The note above is needed to ensure rounding up.
> > > +     * This value only refers to the components 1 and 2 in the
> > > +     * descriptor.
> > >       */
> > >      uint8_t log2_chroma_w;      ///< chroma_width = -((-luma_width )>>log2_chroma_w)
> > 
> > it refers to chroma as its name says, i think thats clear already
> 
> But it's not immediately obvious that the chroma components are stored
> in comp[1] and comp[2].
> 
> Alternatively we could have a bit in the component telling if the
> component is a chroma component, what about it?

I thought a little more about this other option, I have a slight
preference for it since it provides more expressivity to the pixdesc,
allowing to say for example if a pixel format contains chroma
components.

Also it *slightly* simplifies expressions of the kind:
   s = (i == 1 || i == 2) ? expr(log2_chroma) : 0;

which become:
   s = pixdesc->comp[i]->is_chroma ? expr(log2_chroma) ? 0;

or simply:
   s = comp->is_chroma ? expr(log2_chroma) : 0;

I prefer the latter forms since are more explicit, and so more
readable.

Sample patch attached.

Regards.
-- 
FFmpeg = Fostering Fierce Moronic Pure Evanescent God
-------------- next part --------------
A non-text attachment was scrubbed...
Name: use-chr-in-comp.patch
Type: text/x-diff
Size: 9861 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091118/36e47268/attachment.patch>