[x265] [PATCH] added C code for chroma filter_vpp function

[Steve Borho]

On Mon, Oct 21, 2013 at 8:17 AM, <praveen at multicorewareinc.com> wrote:

> # HG changeset patch
> # User Praveen Tiwari
> # Date 1382361413 -19800
> # Node ID b1b531241d70d96d09436fecad4c55879048c126
> # Parent  5fbedc9da1c7201e1cac1c76fdbc5334e349a29d
> added C code for chroma filter_vpp function
>
> diff -r 5fbedc9da1c7 -r b1b531241d70 source/common/ipfilter.cpp
> --- a/source/common/ipfilter.cpp        Mon Oct 21 18:27:13 2013 +0530
> +++ b/source/common/ipfilter.cpp        Mon Oct 21 18:46:53 2013 +0530
> @@ -491,6 +491,80 @@
>          dst += dstStride;
>      }
>  }
> +
> +template<int N, int width, int height>
> +void interp_vert_pp_c(pixel *src, intptr_t srcStride, pixel *dst,
> intptr_t dstStride, int coeffIdx)
> +{
> +    short c[8];
> +    int16_t const * coeff = (N == 4) ? g_chromaFilter[coeffIdx] :
> g_lumaFilter[coeffIdx];
>

I know you're copying this code from the other primitive, but this code
below is just odd.

Why is it making a copy from an int16_t pointer to a short array?  this is
entirely pointless.


> +
> +    c[0] = coeff[0];
> +    c[1] = coeff[1];
> +    if (N >= 4)
> +    {
> +        c[2] = coeff[2];
> +        c[3] = coeff[3];
> +    }
> +    if (N >= 6)
> +    {
> +        c[4] = coeff[4];
> +        c[5] = coeff[5];
> +    }
> +    if (N == 8)
> +    {
> +        c[6] = coeff[6];
> +        c[7] = coeff[7];
> +    }
> +
> +    src -= (N / 2 - 1) * srcStride;
> +
> +    int offset;
> +    short maxVal;
> +    //int headRoom = IF_INTERNAL_PREC - X265_DEPTH;
> +    int shift = IF_FILTER_PREC;
> +
> +    //shift += headRoom;
> +    offset = 1 << (shift - 1);
> +    //offset += IF_INTERNAL_OFFS << IF_FILTER_PREC;
> +    maxVal = (1 << X265_DEPTH) - 1;
>

and all these commented lines are also just clutter.


> +
> +    int row, col;
> +
> +    for (row = 0; row < height; row++)
> +    {
> +        for (col = 0; col < width; col++)
> +        {
> +            int sum;
> +
> +            sum  = src[col + 0 * srcStride] * c[0];
> +            sum += src[col + 1 * srcStride] * c[1];
> +            if (N >= 4)
> +            {
> +                sum += src[col + 2 * srcStride] * c[2];
> +                sum += src[col + 3 * srcStride] * c[3];
> +            }
> +            if (N >= 6)
> +            {
> +                sum += src[col + 4 * srcStride] * c[4];
> +                sum += src[col + 5 * srcStride] * c[5];
> +            }
>

N is always 4 or 8; why are there three checks?


> +            if (N == 8)
> +            {
> +                sum += src[col + 6 * srcStride] * c[6];
> +                sum += src[col + 7 * srcStride] * c[7];
> +            }
> +
> +            short val = (short)((sum + offset) >> shift);
> +            val = (val < 0) ? 0 : val;
> +            val = (val > maxVal) ? maxVal : val;
>

we have Clip functions; but they evaluate to the same logic so I guess this
is ok for a C primitive.


> +
> +            dst[col] = (pixel)val;
> +        }
> +
> +        src += srcStride;
> +        dst += dstStride;
> +    }
> +}
>  }
>

Queueing after cleaning all this up


>
>  namespace x265 {
> _______________________________________________
> x265-devel mailing list
> x265-devel at videolan.org
> https://mailman.videolan.org/listinfo/x265-devel
>



-- 
Steve Borho
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/x265-devel/attachments/20131021/39650519/attachment.html>