Re: [PATCH v16 5/7] drm/vkms: Create KUnit tests for YUV conversions

Maxime Ripard
2025-02-05T08:55:36Z

[-- Attachment #1: Type: text/plain, Size: 2102 bytes --]

On Mon, Jan 27, 2025 at 11:48:23AM +0100, Louis Chauvet wrote:
> On 26/01/25 - 18:06, Maxime Ripard wrote:
> > On Tue, Jan 21, 2025 at 11:48:06AM +0100, Louis Chauvet wrote:
> > > +static struct yuv_u8_to_argb_u16_case yuv_u8_to_argb_u16_cases[] = {
> > > + /*
> > > + * colour.RGB_to_YCbCr(<rgb color in 16 bit form>,
> > > + * K=colour.WEIGHTS_YCBCR["ITU-R BT.601"],
> > > + * in_bits = 16,
> > > + * in_legal = False,
> > > + * in_int = True,
> > > + * out_bits = 8,
> > > + * out_legal = False,
> > > + * out_int = True)
> > > + *
> > > + * Test cases for conversion between YUV BT601 full range and RGB
> > > + * using the ITU-R BT.601 weights.
> > > + */
> >
> > What are the input and output formats?
> >
> > Ditto for all the other tests.
>
> There is no really "input" and "output" format, they are reference values
> for conversion, you should be able to use it in both direction. They are
> generated by RGB_to_YCbCr (RGB input, YUV output) just because it was
> easier to create the colors from RGB values.

RGB and YUV aren't formats, they are color models. XRGB8888 is a format.
NV12 is a format.

> If you think we should specify what is was used as input and output to
> generate those values, I can modify the comment to:
>
> Tests cases for color conversion generated by converting RGB
> values to YUV BT601 full range using the ITU-R BT.601 weights.

My point is that those comments should provide a way to reimplement the
test from scratch, and compare to the actual implementation. It's useful
when you have a test failure and start to wonder if the implementation
or the test is at fault.

By saying only RGB and YUV, you can't possibly do that.

> Beside that modification, did you notice anything else on the series that
> require more work before adding your Ack-by/Reviewed-by on the other
> patches?

The rest looked good to me the last time I looked.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]