[FFmpeg-devel] [PATCH 1/1] Reimplement ff_img_copy_plane() as av_img_copy_plane() in libavcore, and deprecate the old function.

[Michael Niedermayer]

On Tue, Aug 24, 2010 at 08:25:30PM +0200, Stefano Sabatini wrote:
> On date Tuesday 2010-08-24 19:52:04 +0200, Michael Niedermayer encoded:
> > On Tue, Aug 24, 2010 at 05:18:20PM +0200, Stefano Sabatini wrote:
> > > On date Tuesday 2010-08-24 13:06:45 +0200, Michael Niedermayer encoded:
> > > > On Fri, Aug 20, 2010 at 07:49:14PM +0200, Stefano Sabatini wrote:
> > > > > On date Friday 2010-08-20 00:38:16 +0200, Michael Niedermayer encoded:
> > > > > > On Thu, Aug 19, 2010 at 12:54:27PM +0200, Stefano Sabatini wrote:
> > > > > > > On date Thursday 2010-08-19 12:44:34 +0200, Michael Niedermayer encoded:
> > > > > > > > On Thu, Aug 19, 2010 at 12:37:39PM +0200, Stefano Sabatini wrote:
> > > > > > > [...]
> > > > > > > > > diff --git a/libavcore/imgutils.h b/libavcore/imgutils.h
> > > > > > > > > index 874bef1..03e2e88 100644
> > > > > > > > > --- a/libavcore/imgutils.h
> > > > > > > > > +++ b/libavcore/imgutils.h
> > > > > > > > > @@ -104,4 +104,24 @@ int av_fill_image_pointers(uint8_t *data[4], enum PixelFormat pix_fmt, int heigh
> > > > > > > > >   */
> > > > > > > > >  int av_check_image_size(unsigned int w, unsigned int h, int log_offset, void *log_ctx);
> > > > > > > > >  
> > > > > > > > > +/**
> > > > > > > > > + * Copy image plane data in src_data to dst_data.
> > > > > > > > > + *
> > > > > > > > > + * @param dst_linesize linesizes for the image in dst_data
> > > > > > > > > + * @param src_linesize linesizes for the image in src_data
> > > > > > > > > + */
> > > > > > > > > +static inline
> > > > > > > > > +void av_copy_image_plane(uint8_t       *dst_data, int dst_linesize,
> > > > > > > > 
> > > > > > > > why static inline?
> > > > > > > 
> > > > > > > Should be faster, and the function looks small enough, if you prefer
> > > > > > > to keep it non-static inline I'm fine as well.
> > > > > > 
> > > > > > i prefer non static non inline
> > > > > 
> > > > > Updated.
> > > > > 
> > > > > BTW do you want to remove the static inline to
> > > > > av_fill_image_max_pixsteps() as well?
> > > > > 
> > > > > Regards.
> > > > > -- 
> > > > > FFmpeg = Friendly and Fabulous Magical Powered Entertaining Gnome
> > > > 
> > > > >  libavcodec/dsputil.c    |    3 ++-
> > > > >  libavcodec/dsputil.h    |    7 +++++++
> > > > >  libavcodec/imgconvert.c |   14 ++++----------
> > > > >  libavcore/imgutils.c    |   13 +++++++++++++
> > > > >  libavcore/imgutils.h    |   10 ++++++++++
> > > > >  5 files changed, 36 insertions(+), 11 deletions(-)
> > > > > 1df6f1b8bb80c899d76182d274c89341880c6b1e  0001-Reimplement-ff_img_copy_plane-as-av_copy_image_plane.patch
> > > > > From c4ea0edd25c06ee1cd757bdb545bc513143756cc Mon Sep 17 00:00:00 2001
> > > > > From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> > > > > Date: Thu, 19 Aug 2010 12:22:08 +0200
> > > > > Subject: [PATCH 1/5] Reimplement ff_img_copy_plane() as av_copy_image_plane() in libavcore,
> > > > >  and deprecate the old function.
> > > > > 
> > > > > ---
> > > > >  libavcodec/dsputil.c    |    3 ++-
> > > > >  libavcodec/dsputil.h    |    7 +++++++
> > > > >  libavcodec/imgconvert.c |   14 ++++----------
> > > > >  libavcore/imgutils.c    |   13 +++++++++++++
> > > > >  libavcore/imgutils.h    |   10 ++++++++++
> > > > >  5 files changed, 36 insertions(+), 11 deletions(-)
> > > > > 
> > > > > diff --git a/libavcodec/dsputil.c b/libavcodec/dsputil.c
> > > > > index 534f03f..57ea273 100644
> > > > > --- a/libavcodec/dsputil.c
> > > > > +++ b/libavcodec/dsputil.c
> > > > > @@ -27,6 +27,7 @@
> > > > >   * DSP utils
> > > > >   */
> > > > >  
> > > > > +#include "libavcore/imgutils.h"
> > > > >  #include "avcodec.h"
> > > > >  #include "dsputil.h"
> > > > >  #include "simple_idct.h"
> > > > > @@ -4466,7 +4467,7 @@ av_cold void dsputil_init(DSPContext* c, AVCodecContext *avctx)
> > > > >      c->sv_fmul_scalar[0] = sv_fmul_scalar_2_c;
> > > > >      c->sv_fmul_scalar[1] = sv_fmul_scalar_4_c;
> > > > >  
> > > > > -    c->shrink[0]= ff_img_copy_plane;
> > > > > +    c->shrink[0]= av_copy_image_plane;
> > > > 
> > > > for consistency i would suggest that all these functions be named like
> > > > av_image_* or av_pic_*
> > > 
> > > Currently I already have these functions:
> > > av_fill_image_max_pixsteps()
> > > av_get_image_linesize()
> > 
> > > av_fill_image_linesizes()
> > 
> > isnt that just
> > for(4)
> >     av_get_image_linesize()
> 
> Currently... slightly more efficient than that.

being completely speed irrelevant, its duplicated bloat and complicates
api

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100824/1af8102b/attachment.pgp>