diff mbox series

[4/9] media: v4l2: Add JPEG pixel format to v4l2 format info

Message ID 20230324151228.2778112-5-paul.kocialkowski@bootlin.com
State New
Headers
Series media: sun6i-csi/isp: Implement MC I/O support |

Commit Message

Paul Kocialkowski March 24, 2023, 3:12 p.m. UTC 
  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@bootlin.com>
---
 drivers/media/v4l2-core/v4l2-common.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Jernej Škrabec March 25, 2023, 7:08 a.m. UTC | #1 
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 <paul.kocialkowski@bootlin.com>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej
Nicolas Dufresne March 31, 2023, 7:07 p.m. UTC | #2 
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@bootlin.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;
>
diff mbox series

Patch

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;