[Intel-gfx] [PATCH 05/13] Xv: introduce planar memcpy helper

[Simon Farnsworth]

Daniel Vetter wrote:
> Reduced 3 copies of the same code to one.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  src/i830_video.c |  172 +++++++++++++++++-------------------------------------
>  1 files changed, 53 insertions(+), 119 deletions(-)
> 
> diff --git a/src/i830_video.c b/src/i830_video.c
> index 5085bdd..93df761 100644
> --- a/src/i830_video.c
> +++ b/src/i830_video.c
> @@ -1358,6 +1358,54 @@ I830CopyPackedData(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv,
>  	drm_intel_bo_unmap(pPriv->buf);
>  }
>  
> +static void i830_memcpy_plane(unsigned char *dst, unsigned char *src,
> +		int height, int width,
> +		int dstPitch, int srcPitch, Rotation rotation)
> +{
> +    int i, j = 0;
> +    unsigned char *s;
> +
> +    switch (rotation) {
> +    case RR_Rotate_0:
> +	if (srcPitch == dstPitch)
> +	    memcpy (dst, src, srcPitch * height);

This is wrong, and will sometimes crash. See Barry Scott's patch in the
message "[Intel-gfx] [PATCH] Fix segv for clipped movie window".

You need the test to be:
if (srcPitch == dstPitch && srcPitch == width)

Otherwise, in the case where I have covered part of the left edge of the
video, and part of the right edge of the video, but not covered the
bottom line of the video, you will walk off the end of the buffer, and
dereference memory you can't read.

> +	else
> +	    for (i = 0; i < height; i++) {
> +		memcpy(dst, src, width);
> +		src += srcPitch;
> +		dst += dstPitch;
> +	    }
> +	break;
> +    case RR_Rotate_90:
> +	for (i = 0; i < height; i++) {
> +	    s = src;
> +	    for (j = 0; j < width; j++) {
> +		dst[(i) + ((width - j - 1) * dstPitch)] = *s++;
> +	    }
> +	    src += srcPitch;
> +	}
> +	break;
> +    case RR_Rotate_180:
> +	for (i = 0; i < height; i++) {
> +	    s = src;
> +	    for (j = 0; j < width; j++) {
> +		dst[(width - j - 1) + ((height - i - 1) * dstPitch)] = *s++;
> +	    }
> +	    src += srcPitch;
> +	}
> +	break;
> +    case RR_Rotate_270:
> +	for (i = 0; i < height; i++) {
> +	    s = src;
> +	    for (j = 0; j < width; j++) {
> +		dst[(height - i - 1) + (j * dstPitch)] = *s++;
> +	    }
> +	    src += srcPitch;
> +	}
> +	break;
> +    }
> +}
> +
>  static void
>  I830CopyPlanarData(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv,
>  		   unsigned char *buf, int srcPitch,
> @@ -1365,9 +1413,7 @@ I830CopyPlanarData(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv,
>  		   int h, int w, int id)
>  {
>      I830Ptr pI830 = I830PTR(pScrn);
> -    int i, j = 0;
>      unsigned char *src1, *src2, *src3, *dst_base, *dst1, *dst2, *dst3;
> -    unsigned char *s;
>      int dstPitch2 = dstPitch << 1;
>  
>  #if 0
> @@ -1397,45 +1443,7 @@ I830CopyPlanarData(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv,
>      else
>  	dst1 = dst_base + pPriv->YBuf1offset;
>  
> -    switch (pPriv->rotation) {
> -    case RR_Rotate_0:
> -	if (srcPitch == dstPitch2)
> -	    memcpy (dst1, src1, srcPitch * h);
> -	else
> -	    for (i = 0; i < h; i++) {
> -		memcpy(dst1, src1, w);
> -		src1 += srcPitch;
> -		dst1 += dstPitch2;
> -	    }
> -	break;
> -    case RR_Rotate_90:
> -	for (i = 0; i < h; i++) {
> -	    s = src1;
> -	    for (j = 0; j < w; j++) {
> -		dst1[(i) + ((w - j - 1) * dstPitch2)] = *s++;
> -	    }
> -	    src1 += srcPitch;
> -	}
> -	break;
> -    case RR_Rotate_180:
> -	for (i = 0; i < h; i++) {
> -	    s = src1;
> -	    for (j = 0; j < w; j++) {
> -		dst1[(w - j - 1) + ((h - i - 1) * dstPitch2)] = *s++;
> -	    }
> -	    src1 += srcPitch;
> -	}
> -	break;
> -    case RR_Rotate_270:
> -	for (i = 0; i < h; i++) {
> -	    s = src1;
> -	    for (j = 0; j < w; j++) {
> -		dst1[(h - i - 1) + (j * dstPitch2)] = *s++;
> -	    }
> -	    src1 += srcPitch;
> -	}
> -	break;
> -    }
> +    i830_memcpy_plane(dst1, src1, h, w, dstPitch2, srcPitch, pPriv->rotation);
>  
>      /* Copy V data for YV12, or U data for I420 */
>      src2 = buf + (srcH * srcPitch) + ((top * srcPitch) >> 2) + (left >> 1);
> @@ -1455,45 +1463,8 @@ I830CopyPlanarData(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv,
>  	    dst2 = dst_base + pPriv->VBuf1offset;
>      }
>  
> -    switch (pPriv->rotation) {
> -    case RR_Rotate_0:
> -	if (srcPitch2 == dstPitch)
> -	    memcpy (dst2, src2, h/2 * srcPitch2);
> -	else
> -	    for (i = 0; i < h / 2; i++) {
> -		memcpy(dst2, src2, w / 2);
> -		src2 += srcPitch2;
> -		dst2 += dstPitch;
> -	    }
> -	break;
> -    case RR_Rotate_90:
> -	for (i = 0; i < (h/2); i++) {
> -	    s = src2;
> -	    for (j = 0; j < (w/2); j++) {
> -		dst2[(i) + (((w/2) - j - 1) * (dstPitch))] = *s++;
> -	    }
> -	    src2 += srcPitch2;
> -	}
> -	break;
> -    case RR_Rotate_180:
> -	for (i = 0; i < (h/2); i++) {
> -	    s = src2;
> -	    for (j = 0; j < (w/2); j++) {
> -		dst2[((w/2) - j - 1) + (((h/2) - i - 1) * dstPitch)] = *s++;
> -	    }
> -	    src2 += srcPitch2;
> -	}
> -	break;
> -    case RR_Rotate_270:
> -	for (i = 0; i < (h/2); i++) {
> -	    s = src2;
> -	    for (j = 0; j < (w/2); j++) {
> -		dst2[((h/2) - i - 1) + (j * dstPitch)] = *s++;
> -	    }
> -	    src2 += srcPitch2;
> -	}
> -	break;
> -    }
> +    i830_memcpy_plane(dst2, src2, h/2, w/2,
> +		    dstPitch, srcPitch2, pPriv->rotation);
>  
>      /* Copy U data for YV12, or V data for I420 */
>      src3 = buf + (srcH * srcPitch) + ((srcH >> 1) * srcPitch2) +
> @@ -1514,45 +1485,8 @@ I830CopyPlanarData(ScrnInfoPtr pScrn, I830PortPrivPtr pPriv,
>  	    dst3 = dst_base + pPriv->UBuf1offset;
>      }
>  
> -    switch (pPriv->rotation) {
> -    case RR_Rotate_0:
> -	if (srcPitch2 == dstPitch)
> -	    memcpy (dst3, src3, srcPitch2 * h/2);
> -	else
> -	    for (i = 0; i < h / 2; i++) {
> -		memcpy(dst3, src3, w / 2);
> -		src3 += srcPitch2;
> -		dst3 += dstPitch;
> -	    }
> -	break;
> -    case RR_Rotate_90:
> -	for (i = 0; i < (h/2); i++) {
> -	    s = src3;
> -	    for (j = 0; j < (w/2); j++) {
> -		dst3[(i) + (((w/2) - j - 1) * (dstPitch))] = *s++;
> -	    }
> -	    src3 += srcPitch2;
> -	}
> -	break;
> -    case RR_Rotate_180:
> -	for (i = 0; i < (h/2); i++) {
> -	    s = src3;
> -	    for (j = 0; j < (w/2); j++) {
> -		dst3[((w/2) - j - 1) + (((h/2) - i - 1) * dstPitch)] = *s++;
> -	    }
> -	    src3 += srcPitch2;
> -	}
> -	break;
> -    case RR_Rotate_270:
> -	for (i = 0; i < (h/2); i++) {
> -	    s = src3;
> -	    for (j = 0; j < (w/2); j++) {
> -		dst3[((h/2) - i - 1) + (j * dstPitch)] = *s++;
> -	    }
> -	    src3 += srcPitch2;
> -	}
> -	break;
> -    }
> +    i830_memcpy_plane(dst3, src3, h/2, w/2,
> +		    dstPitch, srcPitch2, pPriv->rotation);
>  
>      if (pPriv->textured)
>  	drm_intel_bo_unmap(pPriv->buf);


-- 
Simon Farnsworth
Software Engineer

ONELAN Limited
1st Floor Andersen House
Newtown Road
Henley-on-Thames, OXON
RG9 1HG
United Kingdom

Tel:    +44(0)1491 411400
Fax:    +44(0)1491 579254
Support:+44(0)1491 845282

www.onelan.com