[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 Sat, Sep 04, 2010 at 10:52:08AM +0200, Stefano Sabatini wrote:
> On date Saturday 2010-09-04 01:27:41 +0200, Michael Niedermayer encoded:
> > On Fri, Aug 27, 2010 at 01:47:26AM +0200, Stefano Sabatini wrote:
> > > On date Thursday 2010-08-26 21:40:32 +0200, Michael Niedermayer encoded:
> > > > On Thu, Aug 26, 2010 at 02:27:27AM +0200, Stefano Sabatini wrote:
> > > > > On date Thursday 2010-08-26 00:44:12 +0200, Michael Niedermayer encoded:
> > > > [...]
> > > > 
> > > > > > if nothing outside needs it then no need to make it public and as is
> > > > > > the param should be renamed
> > > > 
> > > > after your patch it definitly is used outside.
> > > > 
> > > > 
> > > > > 
> > > > > Updated, regards.
> > > > 
> > > > 
> > > > [...]
> > > > > --- a/libavcodec/dsputil.c
> > > > > +++ b/libavcodec/dsputil.c
> > > > > @@ -27,6 +27,7 @@
> > > > >   * DSP utils
> > > > >   */
> > > > >  
> > > > > +#include "libavcore/internal.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]= ff_copy_image_plane;
> > > > [...]
> > > > > --- a/libavcore/imgutils.c
> > > > > +++ b/libavcore/imgutils.c
> > > > > @@ -21,6 +21,7 @@
> > > > >   * misc image utilities
> > > > >   */
> > > > >  
> > > > > +#include "internal.h"
> > > > >  #include "imgutils.h"
> > > > >  #include "libavutil/pixdesc.h"
> > > > >  
> > > > > @@ -120,3 +121,16 @@ int av_check_image_size(unsigned int w, unsigned int h, int log_offset, void *lo
> > > > >      av_log(&imgutils, AV_LOG_ERROR, "Picture size %ux%u is invalid\n", w, h);
> > > > >      return AVERROR(EINVAL);
> > > > >  }
> > > > > +
> > > > > +void ff_copy_image_plane(uint8_t       *dst, int dst_linesize,
> > > > > +                         const uint8_t *src, int src_linesize,
> > > > > +                         int bytewidth, int height)
> > > > > +{
> > > > > +    if (!dst || !src)
> > > 
> > > Yes but this is an internal dependancy, so no exposed to the general
> > > public, that was what I meant since the beginning.
> > 
> > if its used by a differnt lib then it is external and part of the ABI/API
> > it also then requires all the versioning, major and minor and soname bumping
> > if it changes. Thus the API has to be reviewed more throughout than if its
> > just internal where we can change it trivially
> 
> So tell what's wrong with the following and possibly suggest what you
> prefer instead, thus avoiding guessloops to me.

the doxy is poor, the name is poor
call it av_image_copy_plane() for example

and doxy could be

Copy image plane from src to dst.
That is, copy "height" number of lines of "bytewidth" bytes each.
The first byte of each successive line is seperated by *_linesize bytes.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- 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/20100904/42e74876/attachment.pgp>