Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Steve Longerbeam <slongerbeam@gmail.com>
Subject: Re: [PATCH v4 1/4] gpu: ipu-v3: ipu-ic: Rename yuv2rgb encoding
 matrices
To:     Philipp Zabel <p.zabel@pengutronix.de>, linux-media@vger.kernel.org
Cc:     Tim Harvey <tharvey@gateworks.com>,
        "open list:DRM DRIVERS FOR FREESCALE IMX" 
        <dri-devel@lists.freedesktop.org>,
        open list <linux-kernel@vger.kernel.org>
References: <20190209014748.10427-1-slongerbeam@gmail.com>
 <20190209014748.10427-2-slongerbeam@gmail.com>
 <1549879117.7687.2.camel@pengutronix.de>
 <0f987e19-e6e9-a56e-00ec-61e7e300a92e@gmail.com>
 <1549966666.4800.3.camel@pengutronix.de>
 <7d4c5935-ffa1-2320-1632-136e1ce89350@gmail.com>
 <1550054109.3937.1.camel@pengutronix.de>
Message-ID: <58bf1e34-8b9d-7fec-6ba5-5db1d5c5457d@gmail.com>
Date:   Thu, 14 Feb 2019 10:54:19 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.4.0
MIME-Version: 1.0
In-Reply-To: <1550054109.3937.1.camel@pengutronix.de>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 2/13/19 2:35 AM, Philipp Zabel wrote:
> On Tue, 2019-02-12 at 09:42 -0800, Steve Longerbeam wrote:
> [...]
>>>> But what about this "SAT_MODE" field in the IC task parameter memory?
>>> That just controls the saturation. The result after the matrix
>>> multiplication is either saturated to [0..255] or to [16..235]/[16..240]
>>> when converting from the internal representation to the 8 bit output.
>> By saturation I think you mean clipped to those ranges?
> Yes, thanks. I didn't realize it sounds weird to use saturated this way.
> See:https://en.wikipedia.org/wiki/Saturation_arithmetic

Ok, saturation can mean the same thing as clipping/clamping. Thanks for 
the article.

I tested a RGB->YUV pipeline with the .sat bit set in the BT.601 rgb2yuv 
table, with the following pipeline on the SabreSD:

'ov5640 1-003c':0
         [fmt:RGB565_2X8_LE/1024x768@1/30 field:none colorspace:srgb 
xfer:srgb ycbcr:601 quantization:full-range]

'imx6-mipi-csi2':0
         [fmt:RGB565_2X8_LE/1024x768 field:none colorspace:srgb 
xfer:srgb ycbcr:601 quantization:full-range]

'imx6-mipi-csi2':2
         [fmt:RGB565_2X8_LE/1024x768 field:none colorspace:srgb 
xfer:srgb ycbcr:601 quantization:full-range]

'ipu1_csi1':0
         [fmt:RGB565_2X8_LE/1024x768@1/30 field:none colorspace:srgb 
xfer:srgb ycbcr:601 quantization:full-range
          crop.bounds:(0,0)/1024x768
          crop:(0,0)/1024x768
          compose.bounds:(0,0)/1024x768
          compose:(0,0)/1024x768]

'ipu1_csi1':1
         [fmt:ARGB8888_1X32/1024x768@1/30 field:none colorspace:srgb 
xfer:srgb ycbcr:601 quantization:full-range]

'ipu1_ic_prp':0
         [fmt:ARGB8888_1X32/1024x768@1/30 field:none colorspace:srgb 
xfer:srgb ycbcr:601 quantization:full-range]

'ipu1_ic_prp':1
         [fmt:ARGB8888_1X32/1024x768@1/30 field:none colorspace:srgb 
xfer:srgb ycbcr:601 quantization:full-range]

'ipu1_ic_prpenc':0
         [fmt:ARGB8888_1X32/1024x768@1/30 field:none colorspace:srgb 
xfer:srgb ycbcr:601 quantization:full-range]

'ipu1_ic_prpenc':1
         [fmt:AYUV8_1X32/800x600@1/30 field:none colorspace:srgb 
xfer:srgb ycbcr:601 quantization:lim-range]

/dev/video0:0
Format Video Capture:
     Width/Height      : 800/600
     Pixel Format      : 'YV12'
     Field             : None
     Bytes per Line    : 800
     Size Image        : 720000
     Colorspace        : sRGB
     Transfer Function : sRGB
     YCbCr/HSV Encoding: ITU-R 601
     Quantization      : Limited Range
     Flags             :


The result being that the captured image colors are all off (there's a 
bright pink shade to the images). But I discovered the init_csc() 
function was not setting the saturation bit at the correct bit offset 
within the TPMEM. The saturation bit is bit 42, or bit 10 of the second 
32-bit word. But the code was writing to bit 9 of the second word. After 
correcting this, saturation is working fine. I have added another patch 
that fixes this for v5 series.


>
>>> SAT_MODE should be set for conversions to YUV limited range so that the
>>> coefficients can be rounded to the closest value.
>> Well, we have already rounded the coefficients to the nearest int in the
>> tables. Do you mean the final result (coeff * color component + offset)
>> is rounded?
> The manual says so: "The final calculation result is limited according
> to the SAT_MODE parameter and rounded to 8 bits", but that's not what I
> meant. Still, I might have been mistaken.
>
> I think due to the fact that the coefficients are multiplied by up to
> 255 (max pixel value) and then effectively divided by 256 when
> converting to 8 bit, the only way to overflow limited range is if two
> coefficients are rounded away from zero in the calculation of a single
> component. This doesn't seem to happen in practice.
>
> A constructed example, conversion to YUV limited range with carefully
> chosen coefficients.
>
>    Y = R * .1817 + G * .6153 + B * .0618 + 16;
>
> Note that .1817 + .6153 + .0618 < 219/255.
> With rounded coefficients though:
>
>    Y = (R * 47 + G * 158 + B * 16 + (64 << 6)) / 256 = 236.136

Yes, for a rec.709 conversion and max/worst-case RGB signal = (255,255,255).

But the rec.709 coefficients for Y are actually

Y = (R * 47 + G * 157 + B * 16 + (16 << 8)) / 256


which for RGB = (255,255,255), Y = 235.14, which doesn't overflow 
limited range.

Steve