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

[Michael Niedermayer]

On Wed, Nov 18, 2009 at 12:47:00AM +0100, Stefano Sabatini wrote:
> 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;

thats not a simplification thats hugely worse code, the compiler
can unroll and optimize away the first but not the second
also it wastes the last spare bit in 16bits

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Avoid a single point of failure, be that a person or equipment.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091118/7b73267a/attachment.pgp>