Johnson Lin <johnson....@intel.com> writes: > The matrix used for YCbCr to RGB is listed in Wiki > https://en.wikipedia.org/wiki/YCbCr; > There is minor error in the matrix constant: 0.0625=16/256 should be 16.0/255, > and 0.5=128.0/256 should be 128.0/255. > Note that conversion from a 0-255 byte number to 0-1.0 float is to divide by > 255 > instead of 256. That's we get 255=1.0f. > By the constant change we can see the CSC result is bit aligned with > Wiki conversion result and FFMPeg result. > Otherwise in some situation, there will be one bit difference
Thanks for fixing this. Try to wrap the the commit message so it fits in 80 columns, eg: The matrix used for YCbCr to RGB is listed in: https://en.wikipedia.org/wiki/YCbCr; There is minor error in the matrix constant: 0.0625=16/256 should be 16.0/255, and 0.5=128.0/256 should be 128.0/255. Note that conversion from a 0-255 byte number to 0-1.0 float is to divide by 255 instead of 256. That's we get 255=1.0f. By the constant change we can see the CSC result is bit aligned with Wiki conversion result and FFMPeg result. Otherwise in some situation, there will be one bit difference > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100854 > --- > 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..f20425e84aab 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))), > + nir_channel(b, nir_fadd(b, u, nir_imm_float(b, -128.0f/255)), > 0), > + nir_channel(b, nir_fadd(b, v, nir_imm_float(b, -128.0f/255)), > 0), I'd like to make two changes here: use a float constant (255.0f instead of 255) and add spaces around the '/'. That is: nir_channel(b, nir_fadd(b, v, nir_imm_float(b, -128.0f / 255.0f)), 0), with that and the 'nir:' prefix in the commit subject: Reviewed-by: Kristian H. Kristensen <hoegsb...@google.com> > 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
