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

Nicolas Dufresne
2023-11-08T02:45:25Z

Le dimanche 05 novembre 2023 à 16:55 +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.

Is this only refactoring to reduce duplicated code ? I haven't seen
what is new in there yet, maybe the commit message could clarify.

regards,
Nicolas

>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
> 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 3a4b15a98e02..834b426da8b1 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -350,6 +350,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,
> + 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)
> {
> @@ -385,37 +413,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;
> @@ -439,22 +449,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);