Re: [PATCH v4 08/11] media: rkvdec: Extract decoded format enumeration into helper

[Nicolas Dufresne <nicolas.dufresne@collabora.com>]

Le dimanche 05 novembre 2023 à 16:55 +0000, Jonas Karlman a écrit :
> Add a rkvdec_is_valid_fmt() helper that check if a fourcc is a supported
> CAPTURE format, and a rkvdec_enum_decoded_fmt() helper that enumerate
                                                              enumerates
> valid formats.
> 
> This move current code into helper functions in preparation for adding
       moves
> CAPTURE format filtering and validation in next patch.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>

With the fixed,
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
> v4:
> - Rename rkvdec_decoded_fmts() to rkvdec_enum_decoded_fmt()
> - Rename rkvdec_valid_fmt() to rkvdec_is_valid_fmt()
> 
> v3:
> - New patch
> 
>  drivers/staging/media/rkvdec/rkvdec.c | 49 +++++++++++++++++++--------
>  1 file changed, 35 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c
> index 7a79840470e1..c3aede94c872 100644
> --- a/drivers/staging/media/rkvdec/rkvdec.c
> +++ b/drivers/staging/media/rkvdec/rkvdec.c
> @@ -27,6 +27,32 @@
>  #include "rkvdec.h"
>  #include "rkvdec-regs.h"
>  
> +static u32 rkvdec_enum_decoded_fmt(struct rkvdec_ctx *ctx, int index)
> +{
> +	const struct rkvdec_coded_fmt_desc *desc = ctx->coded_fmt_desc;
> +
> +	if (WARN_ON(!desc))
> +		return 0;
> +
> +	if (index >= desc->num_decoded_fmts)
> +		return 0;
> +
> +	return desc->decoded_fmts[index];
> +}
> +
> +static bool rkvdec_is_valid_fmt(struct rkvdec_ctx *ctx, u32 fourcc)
> +{
> +	const struct rkvdec_coded_fmt_desc *desc = ctx->coded_fmt_desc;
> +	unsigned int i;
> +
> +	for (i = 0; i < desc->num_decoded_fmts; i++) {
> +		if (desc->decoded_fmts[i] == fourcc)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
>  static void rkvdec_fill_decoded_pixfmt(struct rkvdec_ctx *ctx,
>  				       struct v4l2_pix_format_mplane *pix_mp)
>  {
> @@ -52,8 +78,10 @@ static void rkvdec_reset_fmt(struct rkvdec_ctx *ctx, struct v4l2_format *f,
>  static void rkvdec_reset_decoded_fmt(struct rkvdec_ctx *ctx)
>  {
>  	struct v4l2_format *f = &ctx->decoded_fmt;
> +	u32 fourcc;
>  
> -	rkvdec_reset_fmt(ctx, f, ctx->coded_fmt_desc->decoded_fmts[0]);
> +	fourcc = rkvdec_enum_decoded_fmt(ctx, 0);
> +	rkvdec_reset_fmt(ctx, f, fourcc);
>  	f->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
>  	f->fmt.pix_mp.width = ctx->coded_fmt.fmt.pix_mp.width;
>  	f->fmt.pix_mp.height = ctx->coded_fmt.fmt.pix_mp.height;
> @@ -244,7 +272,6 @@ static int rkvdec_try_capture_fmt(struct file *file, void *priv,
>  	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
>  	struct rkvdec_ctx *ctx = fh_to_rkvdec_ctx(priv);
>  	const struct rkvdec_coded_fmt_desc *coded_desc;
> -	unsigned int i;
>  
>  	/*
>  	 * The codec context should point to a coded format desc, if the format
> @@ -255,13 +282,8 @@ static int rkvdec_try_capture_fmt(struct file *file, void *priv,
>  	if (WARN_ON(!coded_desc))
>  		return -EINVAL;
>  
> -	for (i = 0; i < coded_desc->num_decoded_fmts; i++) {
> -		if (coded_desc->decoded_fmts[i] == pix_mp->pixelformat)
> -			break;
> -	}
> -
> -	if (i == coded_desc->num_decoded_fmts)
> -		pix_mp->pixelformat = coded_desc->decoded_fmts[0];
> +	if (!rkvdec_is_valid_fmt(ctx, pix_mp->pixelformat))
> +		pix_mp->pixelformat = rkvdec_enum_decoded_fmt(ctx, 0);
>  
>  	/* Always apply the frmsize constraint of the coded end. */
>  	pix_mp->width = max(pix_mp->width, ctx->coded_fmt.fmt.pix_mp.width);
> @@ -425,14 +447,13 @@ static int rkvdec_enum_capture_fmt(struct file *file, void *priv,
>  				   struct v4l2_fmtdesc *f)
>  {
>  	struct rkvdec_ctx *ctx = fh_to_rkvdec_ctx(priv);
> +	u32 fourcc;
>  
> -	if (WARN_ON(!ctx->coded_fmt_desc))
> -		return -EINVAL;
> -
> -	if (f->index >= ctx->coded_fmt_desc->num_decoded_fmts)
> +	fourcc = rkvdec_enum_decoded_fmt(ctx, f->index);
> +	if (!fourcc)
>  		return -EINVAL;
>  
> -	f->pixelformat = ctx->coded_fmt_desc->decoded_fmts[f->index];
> +	f->pixelformat = fourcc;
>  	return 0;
>  }
>