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@xxxxxxxxxxx>
> ---
> 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;
>
