[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zwk2RSgfV75LVLpR@louis-chauvet-laptop>
Date: Fri, 11 Oct 2024 16:29:25 +0200
From: Louis Chauvet <louis.chauvet@...tlin.com>
To: Maxime Ripard <mripard@...nel.org>
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@...il.com>,
Melissa Wen <melissa.srw@...il.com>,
Maaara Canal <mairacanal@...eup.net>,
Haneen Mohammed <hamohammed.sa@...il.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Jonathan Corbet <corbet@....net>,
Simona Vetter <simona@...ll.ch>, rdunlap@...radead.org,
arthurgrillo@...eup.net, pekka.paalanen@...oniitty.fi,
Simona Vetter <simona.vetter@...ll.ch>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
linux-doc@...r.kernel.org, thomas.petazzoni@...tlin.com,
jeremie.dautheribes@...tlin.com, miquel.raynal@...tlin.com,
seanpaul@...gle.com, marcheu@...gle.com, nicolejadeyee@...gle.com,
Pekka Paalanen <pekka.paalanen@...labora.com>
Subject: Re: [PATCH v12 13/15] drm/vkms: Create KUnit tests for YUV
conversions
On 11/10/24 - 12:49, Maxime Ripard wrote:
> On Tue, Oct 08, 2024 at 11:23:22AM GMT, Louis Chauvet wrote:
> >
> > Hi,
> >
> > > > + * The YUV color representation were acquired via the colour python framework.
> > > > + * Below are the function calls used for generating each case.
> > > > + *
> > > > + * For more information got to the docs:
> > > > + * https://colour.readthedocs.io/en/master/generated/colour.RGB_to_YCbCr.html
> > > > + */
> > > > +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)
> > > > + */
> > >
> > > We should really detail what the intent and expected outcome is supposed
> > > to be here. Relying on a third-party python library call for
> > > documentation isn't great.
> >
> > This was requested by Pekka in the [v2] of this series.
>
> Ok.
>
> > I can add something like this before each tests, but I think having the
> > exact python code used may help people to understand what should be the
> > behavior, and refering to the python code to understand the conversion.
>
> Help, sure. Be the *only* documentation, absolutely not.
>
> Let's turn this around. You run kunit, one of these tests fail:
>
> - It adds cognitive load to try to identify and make sense of an
> unknown lib.
>
> - How can we check that the arguments you provided there are the one
> you actually wanted to provide, and you didn't make a typo?
>
> > I can add something like this before each tests to clarify the tested
> > case:
> >
> > Test cases for conversion between YUV BT601 limited range and
> > RGB using the ITU-R BT.601 weights.
> >
> > Or maybe just documenting the structure yuv_u8_to_argb_u16_case:
> >
> > @encoding: Encoding used to convert RGB to YUV
> > @range: Range used to convert RGB to YUV
> > @n_colors: Count of test colors in this case
> > @format_pair.name: Name used for this color conversion, used to
> > clarify the test results
> > @format_pair.rgb: RGB color tested
> > @format_pair.yuv: Same color as @format_pair.rgb, but converted to
> > YUV using @encoding and @range.
> >
> > What do you think?
>
> That it's welcome, but it still doesn't allow to figure out what your
> intent was with this test 2 years from now.
I don't really understand what you want to add. Can you explain what you
expect here? Did you mean you want a description like this above the test
function?
/*
* vkms_format_test_yuv_u8_to_argb_u16 - Testing the conversion between YUV
* colors to ARGB colors in VKMS
*
* This test will use the functions get_conversion_matrix_to_argb_u16 and
* argb_u16_from_yuv888 to convert YUV colors (stored in
* yuv_u8_to_argb_u16_cases) into ARGB colors.
*
* As there is a different range between YUV input (8 bits) and RGB output (16
* bits), the values are not checked exactly but ensured that they are within
* the uncertainty range.
*/
Thanks,
Louis Chauvet
> Maxime
Powered by blists - more mailing lists