linux-kernel - Re: [PATCH 4/9] media: v4l2: Add JPEG pixel format to v4l2 format info

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: <57144a4a5dc9994319bb1f7c3a75a1943227417c.camel@ndufresne.ca>
Date:   Fri, 31 Mar 2023 15:07:23 -0400
From:   Nicolas Dufresne <nicolas@...fresne.ca>
To:     Paul Kocialkowski <paul.kocialkowski@...tlin.com>,
        linux-media@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-sunxi@...ts.linux.dev, linux-kernel@...r.kernel.org,
        linux-staging@...ts.linux.dev
Cc:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        Chen-Yu Tsai <wens@...e.org>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Samuel Holland <samuel@...lland.org>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Adam Pigg <adam@...gz.co.uk>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH 4/9] media: v4l2: Add JPEG pixel format to v4l2 format
 info

Le vendredi 24 mars 2023 à 16:12 +0100, Paul Kocialkowski a écrit :
> Represent the JPEG pixel format in the v4l2 format info table.
> 
> Note that the bpp is set to 1 which is not a proper way to estimate
> the needed buffer size for a given JPEG image. However the compression
> ratios of JPEG typically allow fitting the image in a smaller size,
> even though extra metadata could increase the total size by an
> arbitrary amount. Thus it is not a perfectly safe way to calculate the
> size of a JPEG buffer for given dimensions but it still provides a
> reasonable approach.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@...tlin.com>
> ---
>  drivers/media/v4l2-core/v4l2-common.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 5101989716aa..8b2f201a8918 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -317,6 +317,9 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
>  		{ .format = V4L2_PIX_FMT_SGBRG12,	.pixel_enc = V4L2_PIXEL_ENC_BAYER, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>  		{ .format = V4L2_PIX_FMT_SGRBG12,	.pixel_enc = V4L2_PIXEL_ENC_BAYER, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
>  		{ .format = V4L2_PIX_FMT_SRGGB12,	.pixel_enc = V4L2_PIXEL_ENC_BAYER, .mem_planes = 1, .comp_planes = 1, .bpp = { 2, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },
> +
> +		/* Compressed formats */
> +		{ .format = V4L2_PIX_FMT_JPEG,	.pixel_enc = V4L2_PIXEL_ENC_COMPRESSED, .mem_planes = 1, .comp_planes = 1, .bpp = { 1, 0, 0, 0 }, .hdiv = 1, .vdiv = 1 },

This is ackward, at best. Guesstimating the compressed buffer size is a valid
use case, but this table and the related helper function seems badly suited.
When I look at the stateless decoders that do that, they take into account the
fact that the compression can handle different bit depth, and difference
subsampling (choma idc). In this implementation, JPEG is reduce to 4:4:4, 8bit.

I'd like to reject this change, and recommand coming back with a suitable
integration, so that it can be special cased and the driver can communicate the
required information to narrow down the estimate. And this way, you are actually
making it usable for other compressed format like H.264, HEVC, VP8, VP9 AV1 etc.

Nicolas

Nacked in that form.

> 
>  	};
>  	unsigned int i;
>