[FFmpeg-devel] [PATCH] Implement av_get_pix_fmt_chroma_shift()

[Stefano Sabatini]

On date Sunday 2010-01-31 02:55:25 +0100, Michael Niedermayer encoded:
> On Sun, Jan 31, 2010 at 02:26:52AM +0100, Stefano Sabatini wrote:
> > On date Sunday 2010-01-31 02:12:17 +0100, Michael Niedermayer encoded:
> > > On Sat, Jan 30, 2010 at 04:06:50PM +0100, Stefano Sabatini wrote:
> > > [...]
> > > > Index: ffmpeg/libavutil/pixdesc.c
> > > > ===================================================================
> > > > --- ffmpeg.orig/libavutil/pixdesc.c	2010-01-30 15:32:48.000000000 +0100
> > > > +++ ffmpeg/libavutil/pixdesc.c	2010-01-30 15:57:39.000000000 +0100
> > > > @@ -703,3 +703,11 @@
> > > >  
> > > >      return bits >> log2_pixels;
> > > >  }
> > > > +
> > > > +void av_get_pix_fmt_chroma_shift(int *w_shift, int *h_shift, enum PixelFormat pix_fmt)
> > > > +{
> > > > +    if (w_shift)
> > > > +        *w_shift = av_pix_fmt_descriptors[pix_fmt].log2_chroma_w;
> > > > +    if (h_shift)
> > > > +        *h_shift = av_pix_fmt_descriptors[pix_fmt].log2_chroma_h;
> > > > +}
> > > 
> > > what about a
> > > 
> > > int get_true(){
> > >     return 1;
> > > }
> > 
> > Not that I care so much about it, but this operation is performed *a
> > lot* in the codebase, so adding a very specialized function for
> > simplifying the code doesn't look so weird to me.
> 
> what looks weird to me is that you want to put it in libavutil.
> 
> quoting doc/avutil.txt:
>     AVUtil
>     ======
>     libavutil is a small lightweight library of generally useful functions.
>     It is not a library for code needed by both libavcodec and libavformat.

This always looked weird to me, the two statements are contradictory,
a general function *may* be needed by the other non-lavu libraries,
and if two non-lavu libraries need some code maybe it is because that
code is of general utility.

If you want to keep lavu small, and yet want to be able to make
general code shareable, then maybe we need another lib.

Just consider opt / eval code, these provide very general functions,
which are not even related to multimedia, being that it's natural that
many libs would need them, and having that in a shareable lib would
avoid code/work duplication.

> besides this having these 2 line wraper functions used across libs creates ABI
> issues, we cannot change the function anymore without a major version bump.

This is not going to happen very likely for this function.

Regards.
-- 
FFmpeg = Fierce and Furious Multimedia Powerful EntanGlement