[Mesa-dev] [PATCH] nir/lower_tex: Fix minor error in YUV color conversion matrix

[Kenneth Graunke]

On Wednesday, May 3, 2017 11:37:52 PM PDT Johnson Lin wrote:
> The matrix used for YCbCr to RGB is listed in:
> 
> https://en.wikipedia.org/wiki/YCbCr
> 
> There was an error in converting the offsets from integers to unorm
> values: 0.0625=16/256 should be 16.0/255,and 0.5=128.0/256 should be
> 128.0/255.  With this fix, the CSC result is bit aligned with wikipedia's
> conversion result and FFMPeg's result.
> ---
>  src/compiler/nir/nir_lower_tex.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/compiler/nir/nir_lower_tex.c b/src/compiler/nir/nir_lower_tex.c
> index 352d1499bc8d..4ef81955513e 100644
> --- a/src/compiler/nir/nir_lower_tex.c
> +++ b/src/compiler/nir/nir_lower_tex.c
> @@ -244,9 +244,9 @@ convert_yuv_to_rgb(nir_builder *b, nir_tex_instr *tex,
>     nir_ssa_def *yuv =
>        nir_vec4(b,
>                 nir_fmul(b, nir_imm_float(b, 1.16438356f),
> -                        nir_fadd(b, y, nir_imm_float(b, -0.0625f))),
> -               nir_channel(b, nir_fadd(b, u, nir_imm_float(b, -0.5f)), 0),
> -               nir_channel(b, nir_fadd(b, v, nir_imm_float(b, -0.5f)), 0),
> +                        nir_fadd(b, y, nir_imm_float(b, -16.0f / 255.0f))),
> +               nir_channel(b, nir_fadd(b, u, nir_imm_float(b, -128.0f / 255.0f)), 0),
> +               nir_channel(b, nir_fadd(b, v, nir_imm_float(b, -128.0f / 255.0f)), 0),
>                 nir_imm_float(b, 0.0));
> 
>     nir_ssa_def *red = nir_fdot4(b, yuv, nir_build_imm(b, 4, 32, m[0]));
> --
> 1.9.1
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 

I scooped up the various tags:

    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100854
    Reviewed-by: Eric Anholt <eric at anholt.net>
    Reviewed-by: Kristian H. Kristensen <hoegsberg at google.com>

and pushed the patch.  Thanks for fixing this!

Kristian had suggested adding the problematic value to Piglit's
tests/spec/ext_image_dma_buf_import/sample_yuv.c.  If you have time
to write a patch for it, that would be very appreciated.  Having a
Piglit test would ensure this doesn't regress in the future.

--Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170503/6d93a7fe/attachment.sig>