linux-kernel - Re: [PATCH v12 13/15] drm/vkms: Create KUnit tests for YUV conversions

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: <20241011-shiny-skua-of-authority-998ad3@houat>
Date: Fri, 11 Oct 2024 12:49:43 +0200
From: Maxime Ripard <mripard@...nel.org>
To: 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 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.

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)