[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <70a13bcfda7565efc1cf7117a559edfed5318819.camel@collabora.com>
Date: Fri, 25 Oct 2024 13:20:55 -0400
From: Nicolas Dufresne <nicolas.dufresne@...labora.com>
To: Jonas Karlman <jonas@...boo.se>, Sebastian Fricke
<sebastian.fricke@...labora.com>, Ezequiel Garcia
<ezequiel@...guardiasur.com.ar>, Mauro Carvalho Chehab <mchehab@...nel.org>
Cc: Alex Bee <knaerzche@...il.com>, Benjamin Gaignard
<benjamin.gaignard@...labora.com>, Detlev Casanova
<detlev.casanova@...labora.com>, Dan Carpenter <dan.carpenter@...aro.org>,
linux-media@...r.kernel.org, linux-rockchip@...ts.infradead.org,
linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org, Christopher
Obbard <chris.obbard@...labora.com>
Subject: Re: [PATCH v6 01/11] media: v4l2-common: Add helpers to calculate
bytesperline and sizeimage
Le lundi 09 septembre 2024 à 19:24 +0000, Jonas Karlman a écrit :
> Add helper functions to calculate plane bytesperline and sizeimage,
> these new helpers consider bpp div, block width and height when
> calculating plane bytesperline and sizeimage.
>
> Signed-off-by: Jonas Karlman <jonas@...boo.se>
> Tested-by: Nicolas Dufresne <nicolas.dufresne@...labora.com>
> Tested-by: Christopher Obbard <chris.obbard@...labora.com>
> ---
> v6:
> - No change
>
> v5:
> - Collect t-b tags
>
> v4:
> - No change
>
> v3:
> - Consider bpp_div in calculation
> ---
> drivers/media/v4l2-core/v4l2-common.c | 78 +++++++++++++--------------
> 1 file changed, 39 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 0a2f4f0d0a07..8ad04d48a023 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -357,6 +357,34 @@ static inline unsigned int v4l2_format_block_height(const struct v4l2_format_inf
> return info->block_h[plane];
> }
>
> +static inline unsigned int v4l2_format_plane_width(const struct v4l2_format_info *info, int plane,
nit: What would you think of naming this one v4l2_format_plane_stride() ? As the
returned value is in bytes, not in pixels. (I'm not a huge fan of
_bytesperline(), but would seem valid and consistent too).
With or without this, I'm happy to see this land:
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@...labora.com>
> + unsigned int width)
> +{
> + unsigned int hdiv = plane ? info->hdiv : 1;
> + unsigned int aligned_width =
> + ALIGN(width, v4l2_format_block_width(info, plane));
> +
> + return DIV_ROUND_UP(aligned_width, hdiv) *
> + info->bpp[plane] / info->bpp_div[plane];
> +}
> +
> +static inline unsigned int v4l2_format_plane_height(const struct v4l2_format_info *info, int plane,
> + unsigned int height)
> +{
> + unsigned int vdiv = plane ? info->vdiv : 1;
> + unsigned int aligned_height =
> + ALIGN(height, v4l2_format_block_height(info, plane));
> +
> + return DIV_ROUND_UP(aligned_height, vdiv);
> +}
> +
> +static inline unsigned int v4l2_format_plane_size(const struct v4l2_format_info *info, int plane,
> + unsigned int width, unsigned int height)
> +{
> + return v4l2_format_plane_width(info, plane, width) *
> + v4l2_format_plane_height(info, plane, height);
> +}
> +
> void v4l2_apply_frmsize_constraints(u32 *width, u32 *height,
> const struct v4l2_frmsize_stepwise *frmsize)
> {
> @@ -392,37 +420,19 @@ int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt,
>
> if (info->mem_planes == 1) {
> plane = &pixfmt->plane_fmt[0];
> - plane->bytesperline = ALIGN(width, v4l2_format_block_width(info, 0)) * info->bpp[0] / info->bpp_div[0];
> + plane->bytesperline = v4l2_format_plane_width(info, 0, width);
> plane->sizeimage = 0;
>
> - for (i = 0; i < info->comp_planes; i++) {
> - unsigned int hdiv = (i == 0) ? 1 : info->hdiv;
> - unsigned int vdiv = (i == 0) ? 1 : info->vdiv;
> - unsigned int aligned_width;
> - unsigned int aligned_height;
> -
> - aligned_width = ALIGN(width, v4l2_format_block_width(info, i));
> - aligned_height = ALIGN(height, v4l2_format_block_height(info, i));
> -
> - plane->sizeimage += info->bpp[i] *
> - DIV_ROUND_UP(aligned_width, hdiv) *
> - DIV_ROUND_UP(aligned_height, vdiv) / info->bpp_div[i];
> - }
> + for (i = 0; i < info->comp_planes; i++)
> + plane->sizeimage +=
> + v4l2_format_plane_size(info, i, width, height);
> } else {
> for (i = 0; i < info->comp_planes; i++) {
> - unsigned int hdiv = (i == 0) ? 1 : info->hdiv;
> - unsigned int vdiv = (i == 0) ? 1 : info->vdiv;
> - unsigned int aligned_width;
> - unsigned int aligned_height;
> -
> - aligned_width = ALIGN(width, v4l2_format_block_width(info, i));
> - aligned_height = ALIGN(height, v4l2_format_block_height(info, i));
> -
> plane = &pixfmt->plane_fmt[i];
> plane->bytesperline =
> - info->bpp[i] * DIV_ROUND_UP(aligned_width, hdiv) / info->bpp_div[i];
> - plane->sizeimage =
> - plane->bytesperline * DIV_ROUND_UP(aligned_height, vdiv);
> + v4l2_format_plane_width(info, i, width);
> + plane->sizeimage = plane->bytesperline *
> + v4l2_format_plane_height(info, i, height);
> }
> }
> return 0;
> @@ -446,22 +456,12 @@ int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
> pixfmt->width = width;
> pixfmt->height = height;
> pixfmt->pixelformat = pixelformat;
> - pixfmt->bytesperline = ALIGN(width, v4l2_format_block_width(info, 0)) * info->bpp[0] / info->bpp_div[0];
> + pixfmt->bytesperline = v4l2_format_plane_width(info, 0, width);
> pixfmt->sizeimage = 0;
>
> - for (i = 0; i < info->comp_planes; i++) {
> - unsigned int hdiv = (i == 0) ? 1 : info->hdiv;
> - unsigned int vdiv = (i == 0) ? 1 : info->vdiv;
> - unsigned int aligned_width;
> - unsigned int aligned_height;
> -
> - aligned_width = ALIGN(width, v4l2_format_block_width(info, i));
> - aligned_height = ALIGN(height, v4l2_format_block_height(info, i));
> -
> - pixfmt->sizeimage += info->bpp[i] *
> - DIV_ROUND_UP(aligned_width, hdiv) *
> - DIV_ROUND_UP(aligned_height, vdiv) / info->bpp_div[i];
> - }
> + for (i = 0; i < info->comp_planes; i++)
> + pixfmt->sizeimage +=
> + v4l2_format_plane_size(info, i, width, height);
> return 0;
> }
> EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt);
Powered by blists - more mailing lists