linux-kernel - Re: [PATCH v4 1/4] gpu: ipu-v3: ipu-ic: Rename yuv2rgb encoding matrices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1549879117.7687.2.camel@pengutronix.de>
Date:   Mon, 11 Feb 2019 10:58:37 +0100
From:   Philipp Zabel <p.zabel@...gutronix.de>
To:     Steve Longerbeam <slongerbeam@...il.com>,
        linux-media@...r.kernel.org
Cc:     Tim Harvey <tharvey@...eworks.com>,
        "open list:DRM DRIVERS FOR FREESCALE IMX" 
        <dri-devel@...ts.freedesktop.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 1/4] gpu: ipu-v3: ipu-ic: Rename yuv2rgb encoding
 matrices

On Fri, 2019-02-08 at 17:47 -0800, Steve Longerbeam wrote:
> The ycbcr2rgb and inverse rgb2ycbcr matrices define the BT.601 encoding
> coefficients, so rename them to indicate that. And add some comments
> to make clear these are BT.601 coefficients encoding between YUV limited
> range and RGB full range. The ic_csc_rgb2rgb matrix is just an identity
> matrix, so rename to ic_csc_identity. No functional changes.
> 
> Signed-off-by: Steve Longerbeam <slongerbeam@...il.com>
> ---
> Changes in v2:
> - rename ic_csc_rgb2rgb matrix to ic_csc_identity.
> ---
>  drivers/gpu/ipu-v3/ipu-ic.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/ipu-v3/ipu-ic.c b/drivers/gpu/ipu-v3/ipu-ic.c
> index 594c3cbc8291..3ef61f0b509b 100644
> --- a/drivers/gpu/ipu-v3/ipu-ic.c
> +++ b/drivers/gpu/ipu-v3/ipu-ic.c
> @@ -183,11 +183,13 @@ struct ic_csc_params {
>  };
>  
>  /*
> + * BT.601 encoding from RGB full range to YUV limited range:
> + *
>   * Y = R *  .299 + G *  .587 + B *  .114;
>   * U = R * -.169 + G * -.332 + B *  .500 + 128.;
>   * V = R *  .500 + G * -.419 + B * -.0813 + 128.;

Hm, this is a conversion to full range BT.601. For limited range, the
matrix coefficients

   0.2990  0.5870  0.1140
  -0.1687 -0.3313  0.5000
   0.5000 -0.4187 -0.0813

should be multiplied with 219/255 (Y) and 224/255 (U,V), respectively:

  Y = R *  .2568 + G *  .5041 + B *  .0979 + 16;
  U = R * -.1482 + G * -.2910 + B *  .4392 + 128;
  V = R *  .4392 + G * -.3678 + B * -.0714 + 128;

>   */
> -static const struct ic_csc_params ic_csc_rgb2ycbcr = {
> +static const struct ic_csc_params ic_csc_rgb2ycbcr_bt601 = {
>  	.coeff = {
>  		{ 77, 150, 29 },
>  		{ 469, 427, 128 },
> @@ -197,8 +199,11 @@ static const struct ic_csc_params ic_csc_rgb2ycbcr = {
>  	.scale = 1,
>  };
>  
> -/* transparent RGB->RGB matrix for graphics combining */
> -static const struct ic_csc_params ic_csc_rgb2rgb = {
> +/*
> + * identity matrix, used for transparent RGB->RGB graphics
> + * combining.
> + */
> +static const struct ic_csc_params ic_csc_identity = {
>  	.coeff = {
>  		{ 128, 0, 0 },
>  		{ 0, 128, 0 },
> @@ -208,11 +213,13 @@ static const struct ic_csc_params ic_csc_rgb2rgb = {
>  };
>  
>  /*
> + * Inverse BT.601 encoding from YUV limited range to RGB full range:
> + *
>   * R = (1.164 * (Y - 16)) + (1.596 * (Cr - 128));
>   * G = (1.164 * (Y - 16)) - (0.392 * (Cb - 128)) - (0.813 * (Cr - 128));
>   * B = (1.164 * (Y - 16)) + (2.017 * (Cb - 128);
>   */

This looks correct.

> -static const struct ic_csc_params ic_csc_ycbcr2rgb = {
> +static const struct ic_csc_params ic_csc_ycbcr2rgb_bt601 = {
>  	.coeff = {
>  		{ 149, 0, 204 },
>  		{ 149, 462, 408 },
> @@ -238,11 +245,11 @@ static int init_csc(struct ipu_ic *ic,
>  		(priv->tpmem_base + ic->reg->tpmem_csc[csc_index]);
>  
>  	if (inf == IPUV3_COLORSPACE_YUV && outf == IPUV3_COLORSPACE_RGB)
> -		params = &ic_csc_ycbcr2rgb;
> +		params = &ic_csc_ycbcr2rgb_bt601;
>  	else if (inf == IPUV3_COLORSPACE_RGB && outf == IPUV3_COLORSPACE_YUV)
> -		params = &ic_csc_rgb2ycbcr;
> +		params = &ic_csc_rgb2ycbcr_bt601;
>  	else if (inf == IPUV3_COLORSPACE_RGB && outf == IPUV3_COLORSPACE_RGB)
> -		params = &ic_csc_rgb2rgb;
> +		params = &ic_csc_identity;
>  	else {
>  		dev_err(priv->ipu->dev, "Unsupported color space conversion\n");
>  		return -EINVAL;

regards
Philipp