[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 Wed, Aug 25, 2010 at 07:36:40PM +0200, Stefano Sabatini wrote:
> On date Wednesday 2010-08-25 16:07:48 +0200, Michael Niedermayer encoded:
> > On Wed, Aug 25, 2010 at 03:35:38PM +0200, Stefano Sabatini wrote:
> > > On date Wednesday 2010-08-25 10:10:17 +0200, Michael Niedermayer encoded:
> > > > On Wed, Aug 25, 2010 at 12:30:38AM +0200, Stefano Sabatini wrote:
> > > > [...]
> > > > > @@ -120,3 +120,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 av_copy_image_plane(uint8_t       *dst, int dst_linesize,
> > > > > +                         const uint8_t *src, int src_linesize,
> > > > > +                         int width, int height)
> > > > > +{
> > > > > +    if (!dst || !src)
> > > > > +        return;
> > > > > +    for (;height > 0; height--) {
> > > > > +        memcpy(dst, src, width);
> > > > > +        dst += dst_linesize;
> > > > > +        src += src_linesize;
> > > > > +    }
> > > > > +}
> > > > 
> > > > thats definitly not working with half of the pixel formats and i would
> > > > expect that you test code like this
> > > 
> > > Can you elaborate on that?
> > 
> > think of 16bit, think of 1bit think of any packed format bgr32, bgr24, yuy2
> > then think of width
> 
> Indeed the name 'width' is poorly chosen, it should rather be "bytewidth".
> 
> > > Also note that here I'm not adding or
> > > changing the logic, I'm just *moving* the code.
> > 
> > you are making internal stuff part of the public api
> > 
> > thats like
> > 1. no need to be so picky its just internal
> > 2. no need to be so picky i am just moving it
> > ;)
> 
> OK, I see two possible solutions, and since I'm lazy I'll let the
> choice to you:
> 
> 1) pick a better name for the param var (width -> bytewidth) and
>   clearly document it
> 
> 2) leave the function internal to lavcore and name it
>    ff_copy_image_plane

if nothing outside needs it then no need to make it public and as is
the param should be renamed

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

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
-------------- 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/20100826/1c90af28/attachment.pgp>