Re: [PATCH v7 01/12] media: v4l2-common: Add helpers to calculate bytesperline and sizeimage

[Hans Verkuil <hverkuil@xs4all.nl>]

On 2/25/25 10:40, Sebastian Fricke wrote:
> From: Jonas Karlman <jonas@kwiboo.se>
> 
> 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@kwiboo.se>
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Tested-by: Christopher Obbard <chris.obbard@collabora.com>
> ---
>  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 e4b2de3833ee3d440493f94fca5ee2049b013ef0..fdab16b9d481300eecf2202b3930fdf0a97a3023 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_stride(const struct v4l2_format_info *info, int plane,
> +						   unsigned int width)

Note that there was a small fixup for this function here:

https://patchwork.linuxtv.org/project/linux-media/patch/20250225124008.195405-1-sebastian.fricke@collabora.com/

Just so it isn't forgotten when this series is merged.

Regards,

	Hans

> +{
> +	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_stride(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_stride(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_stride(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_stride(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);
>