[Libva] [PATCH] shaders: gen[789]: fix Y coefficient in YUV->RGB conversion

[Gwenole Beauchesne]

2015-10-21 12:13 GMT+02:00 Lionel Landwerlin <lionel.g.landwerlin at intel.com>:
> On 21/10/15 01:57, Zhao Yakui wrote:
>>
>> On 10/21/2015 12:48 AM, Lionel Landwerlin wrote:
>>>
>>> In the following formula for the conversion :
>>>
>>> R = Clamp ( 1.164(Y-16/255) + 1.596(Cr-128/255))
>>> G = Clamp ( 1.164(Y-16/255) - 0.813(Cr-128/255) - 0.392(Cb-128/255))
>>> B = Clamp ( 1.164(Y-16/255) + 2.017(Cb-128/255))
>>>
>>> we must substract 16 (or 16/255 if dealing with [0.0, 1.0] floats) to
>>> Y before applying the multiplier coefficent. The shader was missing
>>> the substraction.
>>
>>
>> Hi,
>>
>>    Thanks for your patch.
>>    The issue comes from the confusing formula between YUV and RGB.
>> As you see from the below link, it has several different formulas about
>> YUV to RGB.
>>     >http://www.equasys.de/colorconversion.html
>>
>>     And ITU has three standards for the conversion between YUV and RGB.
>> (BT601, 709, 2020).
>>     Even for BT601, it will also have the different conversion if the
>> footroom/headroom is considered.
>>
>>     If you hope to fix the conversion issue, I prefer that the shader is
>> based on the generic conversion matrix and the conversion matrix is passed.
>> In such case we can pass the different conversion matrix for the different
>> standard.
>
>
> Yeah, I was wondering because the driver's code base includes more generic
> shaders that work with a matrix passed from userspace :
> http://cgit.freedesktop.org/vaapi/intel-driver/tree/src/shaders/render
>
> Why isn't the driver using those shaders?
>
> Indeed having a matrix would be best. What the current conversion being used
> right now?
> Reading your link it seems it's neither BT601 nor BT709.

YPbPr (analog), full range.

More correct equations should involve Kr, Kg, Kb, where Kg = 1 - (Kr + Kb). And,
- For BT.601: Kr = 0.299, Kb = 0.114
- For BT.709: Kr = 0.2126,  Kb = 0.0722
- For SMPTE.240M: Kr = 0.212, Kb = 0.087

IMHO, we should use equations for YCbCr (digital) video, and support
both limited [16..235] and full [0..255] ranges. There is additional
VA-API on the staging branch.

Having said that, an initial patch where you move to YCbCr (limited
range) first looks fine to me. i.e. your original one + fixes to the
other coefficients. Then, generic expansion could be added afterwards,
as proper support would require integration of staging vpp apis
anyway.

>
>>
>> Thanks
>>    Yakui
>>>
>>> ---
>>>   src/shaders/post_processing/gen7/YUV_to_RGB.g4a   | 16 ++++++++++++++++
>>>   src/shaders/post_processing/gen7/pl2_to_rgbx.g75b | 16 ++++++++++++++++
>>>   src/shaders/post_processing/gen7/pl2_to_rgbx.g7b  | 16 ++++++++++++++++
>>>   src/shaders/post_processing/gen8/YUV_to_RGB.g8a   | 16 ++++++++++++++++
>>>   src/shaders/post_processing/gen8/pl2_to_rgbx.g8b  | 16 ++++++++++++++++
>>>   src/shaders/post_processing/gen9/pl2_to_rgbx.g9b  | 16 ++++++++++++++++
>>>   6 files changed, 96 insertions(+)
>>
>>
>
> _______________________________________________
> Libva mailing list
> Libva at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libva


Regards,
-- 
Gwenole Beauchesne
Intel Corporation SAS / 2 rue de Paris, 92196 Meudon Cedex, France
Registration Number (RCS): Nanterre B 302 456 199