[FFmpeg-devel] [PATCH] av_picture_copy misses pixels on packed planar AVPictures with odd width or height

[Michael Niedermayer]

On Tue, Oct 28, 2008 at 11:52:46AM -0700, Art Clarke wrote:
> To reproduce:
> 1) Create a AVPicture with an odd width and height (e.g. 503w x 251h)
> in a packed pixel format (e.g. YUV_420P).

P stands for planar


> 2) Use the av_picture_copy method
> 3) Check if all bytes are the same in the copied AVPicture: you'll be
> missing one U and V values at the end of each line, and all the U and
> V values for last line.
> 
> Cause:
> A rounding error computing the width and height to copy.  A picture
> 503w x 251h pixels should have U and V plane widths of 252, and 126
> total rows of u and v data but due to the rounding error the
> av_picture_copy only copes over 251 bytes for each row's width, and
> calculates 125 total rows of data.  ff_fill_* methods do the correct
> math.
> 
> Fix:
> 1) Make sure we compute plane width and height by rounding up with
> adjusting for chroma_shifts (as in the ff_fill_linesinze methods)
> 2) remove an unused "width" calculation that was never used.
> 
> - Art

> Index: libavcodec/imgconvert.c
> ===================================================================
> --- libavcodec/imgconvert.c	(revision 15735)
> +++ libavcodec/imgconvert.c	(working copy)
> @@ -870,7 +870,7 @@
>          break;
>      case FF_PIXEL_PLANAR:
>              if (plane == 1 || plane == 2)
> -                width >>= pf->x_chroma_shift;
> +                width = (width + (1 << pf->x_chroma_shift) - 1) >> pf->x_chroma_shift;

width= -((-width)>>pf->x_chroma_shift);


>  
>              return (width * pf->depth + 7) >> 3;
>          break;

> @@ -894,13 +894,11 @@
>      case FF_PIXEL_PACKED:
>      case FF_PIXEL_PLANAR:
>          for(i = 0; i < pf->nb_channels; i++) {
> -            int w, h;
> +            int h;
>              int bwidth = ff_get_plane_bytewidth(pix_fmt, width, i);
> -            w = width;
>              h = height;
>              if (i == 1 || i == 2) {
> -                w >>= pf->x_chroma_shift;
> -                h >>= pf->y_chroma_shift;
> +                h = (height + (1 << pf->y_chroma_shift) - 1) >> pf->y_chroma_shift;
>              }
>              ff_img_copy_plane(dst->data[i], dst->linesize[i],
>                             src->data[i], src->linesize[i],

this seems to contain an unrelated change

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

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- 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/20081029/6600a175/attachment.pgp>