2023-03-24 15:16:03

by Paul Kocialkowski

[permalink] [raw]
Subject: [PATCH 4/9] media: v4l2: Add JPEG pixel format to v4l2 format info

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 <[email protected]>
---
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 },
};
unsigned int i;

--
2.39.2


2023-03-25 07:09:03

by Jernej Škrabec

[permalink] [raw]
Subject: Re: [PATCH 4/9] media: v4l2: Add JPEG pixel format to v4l2 format info

Dne petek, 24. marec 2023 ob 16:12:23 CET je Paul Kocialkowski napisal(a):
> 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 <[email protected]>

Acked-by: Jernej Skrabec <[email protected]>

Best regards,
Jernej


2023-03-31 19:15:01

by Nicolas Dufresne

[permalink] [raw]
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 <[email protected]>
> ---
> 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;
>