[PATCH v6 08/11] media: rkvdec: Add image format concept

Jonas Karlman posted 11 patches 2 months, 3 weeks ago
[PATCH v6 08/11] media: rkvdec: Add image format concept
Posted by Jonas Karlman 2 months, 3 weeks ago
Add an enum rkvdec_image_fmt used to signal an image format, e.g.
4:2:0 8-bit, 4:2:0 10-bit or any.

Tag each supported CAPUTRE format with an image format and use this tag
to filter out unsupported CAPTURE formats.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Tested-by: Christopher Obbard <chris.obbard@collabora.com>
---
v6:
- Change to use fmt_idx instead of j++ tucked inside a condition (Dan)

v5:
- Collect t-b tags

v4:
- Change fmt_opaque into an image format
- Split patch into two

v3:
- New patch
---
 drivers/staging/media/rkvdec/rkvdec.c | 48 ++++++++++++++++++++-------
 drivers/staging/media/rkvdec/rkvdec.h | 13 +++++++-
 2 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c
index efbf9aa578ae..467fc05b347a 100644
--- a/drivers/staging/media/rkvdec/rkvdec.c
+++ b/drivers/staging/media/rkvdec/rkvdec.c
@@ -27,26 +27,45 @@
 #include "rkvdec.h"
 #include "rkvdec-regs.h"
 
-static u32 rkvdec_enum_decoded_fmt(struct rkvdec_ctx *ctx, int index)
+static inline bool rkvdec_image_fmt_match(enum rkvdec_image_fmt fmt1,
+					  enum rkvdec_image_fmt fmt2)
+{
+	return fmt1 == fmt2 || fmt2 == RKVDEC_IMG_FMT_ANY ||
+	       fmt1 == RKVDEC_IMG_FMT_ANY;
+}
+
+static u32 rkvdec_enum_decoded_fmt(struct rkvdec_ctx *ctx, int index,
+				   enum rkvdec_image_fmt image_fmt)
 {
 	const struct rkvdec_coded_fmt_desc *desc = ctx->coded_fmt_desc;
+	int fmt_idx = -1;
+	unsigned int i;
 
 	if (WARN_ON(!desc))
 		return 0;
 
-	if (index >= desc->num_decoded_fmts)
-		return 0;
+	for (i = 0; i < desc->num_decoded_fmts; i++) {
+		if (!rkvdec_image_fmt_match(desc->decoded_fmts[i].image_fmt,
+					    image_fmt))
+			continue;
+		fmt_idx++;
+		if (index == fmt_idx)
+			return desc->decoded_fmts[i].fourcc;
+	}
 
-	return desc->decoded_fmts[index];
+	return 0;
 }
 
-static bool rkvdec_is_valid_fmt(struct rkvdec_ctx *ctx, u32 fourcc)
+static bool rkvdec_is_valid_fmt(struct rkvdec_ctx *ctx, u32 fourcc,
+				enum rkvdec_image_fmt image_fmt)
 {
 	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)
+		if (rkvdec_image_fmt_match(desc->decoded_fmts[i].image_fmt,
+					   image_fmt) &&
+		    desc->decoded_fmts[i].fourcc == fourcc)
 			return true;
 	}
 
@@ -80,7 +99,7 @@ static void rkvdec_reset_decoded_fmt(struct rkvdec_ctx *ctx)
 	struct v4l2_format *f = &ctx->decoded_fmt;
 	u32 fourcc;
 
-	fourcc = rkvdec_enum_decoded_fmt(ctx, 0);
+	fourcc = rkvdec_enum_decoded_fmt(ctx, 0, ctx->image_fmt);
 	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;
@@ -149,8 +168,11 @@ static const struct rkvdec_ctrls rkvdec_h264_ctrls = {
 	.num_ctrls = ARRAY_SIZE(rkvdec_h264_ctrl_descs),
 };
 
-static const u32 rkvdec_h264_vp9_decoded_fmts[] = {
-	V4L2_PIX_FMT_NV12,
+static const struct rkvdec_decoded_fmt_desc rkvdec_h264_vp9_decoded_fmts[] = {
+	{
+		.fourcc = V4L2_PIX_FMT_NV12,
+		.image_fmt = RKVDEC_IMG_FMT_420_8BIT,
+	},
 };
 
 static const struct rkvdec_ctrl_desc rkvdec_vp9_ctrl_descs[] = {
@@ -282,8 +304,9 @@ static int rkvdec_try_capture_fmt(struct file *file, void *priv,
 	if (WARN_ON(!coded_desc))
 		return -EINVAL;
 
-	if (!rkvdec_is_valid_fmt(ctx, pix_mp->pixelformat))
-		pix_mp->pixelformat = rkvdec_enum_decoded_fmt(ctx, 0);
+	if (!rkvdec_is_valid_fmt(ctx, pix_mp->pixelformat, ctx->image_fmt))
+		pix_mp->pixelformat = rkvdec_enum_decoded_fmt(ctx, 0,
+							      ctx->image_fmt);
 
 	/* Always apply the frmsize constraint of the coded end. */
 	pix_mp->width = max(pix_mp->width, ctx->coded_fmt.fmt.pix_mp.width);
@@ -400,6 +423,7 @@ static int rkvdec_s_output_fmt(struct file *file, void *priv,
 	 *
 	 * Note that this will propagates any size changes to the decoded format.
 	 */
+	ctx->image_fmt = RKVDEC_IMG_FMT_ANY;
 	rkvdec_reset_decoded_fmt(ctx);
 
 	/* Propagate colorspace information to capture. */
@@ -449,7 +473,7 @@ static int rkvdec_enum_capture_fmt(struct file *file, void *priv,
 	struct rkvdec_ctx *ctx = fh_to_rkvdec_ctx(priv);
 	u32 fourcc;
 
-	fourcc = rkvdec_enum_decoded_fmt(ctx, f->index);
+	fourcc = rkvdec_enum_decoded_fmt(ctx, f->index, ctx->image_fmt);
 	if (!fourcc)
 		return -EINVAL;
 
diff --git a/drivers/staging/media/rkvdec/rkvdec.h b/drivers/staging/media/rkvdec/rkvdec.h
index 633335ebb9c4..6f8cf50c5d99 100644
--- a/drivers/staging/media/rkvdec/rkvdec.h
+++ b/drivers/staging/media/rkvdec/rkvdec.h
@@ -75,13 +75,23 @@ struct rkvdec_coded_fmt_ops {
 	int (*try_ctrl)(struct rkvdec_ctx *ctx, struct v4l2_ctrl *ctrl);
 };
 
+enum rkvdec_image_fmt {
+	RKVDEC_IMG_FMT_ANY = 0,
+	RKVDEC_IMG_FMT_420_8BIT,
+};
+
+struct rkvdec_decoded_fmt_desc {
+	u32 fourcc;
+	enum rkvdec_image_fmt image_fmt;
+};
+
 struct rkvdec_coded_fmt_desc {
 	u32 fourcc;
 	struct v4l2_frmsize_stepwise frmsize;
 	const struct rkvdec_ctrls *ctrls;
 	const struct rkvdec_coded_fmt_ops *ops;
 	unsigned int num_decoded_fmts;
-	const u32 *decoded_fmts;
+	const struct rkvdec_decoded_fmt_desc *decoded_fmts;
 	u32 subsystem_flags;
 };
 
@@ -104,6 +114,7 @@ struct rkvdec_ctx {
 	const struct rkvdec_coded_fmt_desc *coded_fmt_desc;
 	struct v4l2_ctrl_handler ctrl_hdl;
 	struct rkvdec_dev *dev;
+	enum rkvdec_image_fmt image_fmt;
 	void *priv;
 };
 
-- 
2.46.0
Re: [PATCH v6 08/11] media: rkvdec: Add image format concept
Posted by Nicolas Dufresne 1 month ago
Le lundi 09 septembre 2024 à 19:25 +0000, Jonas Karlman a écrit :
> Add an enum rkvdec_image_fmt used to signal an image format, e.g.
> 4:2:0 8-bit, 4:2:0 10-bit or any.
> 
> Tag each supported CAPUTRE format with an image format and use this tag
> to filter out unsupported CAPTURE formats.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Tested-by: Christopher Obbard <chris.obbard@collabora.com>
> ---
> v6:
> - Change to use fmt_idx instead of j++ tucked inside a condition (Dan)
> 
> v5:
> - Collect t-b tags
> 
> v4:
> - Change fmt_opaque into an image format
> - Split patch into two
> 
> v3:
> - New patch
> ---
>  drivers/staging/media/rkvdec/rkvdec.c | 48 ++++++++++++++++++++-------
>  drivers/staging/media/rkvdec/rkvdec.h | 13 +++++++-
>  2 files changed, 48 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c
> index efbf9aa578ae..467fc05b347a 100644
> --- a/drivers/staging/media/rkvdec/rkvdec.c
> +++ b/drivers/staging/media/rkvdec/rkvdec.c
> @@ -27,26 +27,45 @@
>  #include "rkvdec.h"
>  #include "rkvdec-regs.h"
>  
> -static u32 rkvdec_enum_decoded_fmt(struct rkvdec_ctx *ctx, int index)
> +static inline bool rkvdec_image_fmt_match(enum rkvdec_image_fmt fmt1,
> +					  enum rkvdec_image_fmt fmt2)

nit: We usually let the compiler inline function, specially when small and
static.

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> +{
> +	return fmt1 == fmt2 || fmt2 == RKVDEC_IMG_FMT_ANY ||
> +	       fmt1 == RKVDEC_IMG_FMT_ANY;
> +}
> +
> +static u32 rkvdec_enum_decoded_fmt(struct rkvdec_ctx *ctx, int index,
> +				   enum rkvdec_image_fmt image_fmt)
>  {
>  	const struct rkvdec_coded_fmt_desc *desc = ctx->coded_fmt_desc;
> +	int fmt_idx = -1;
> +	unsigned int i;
>  
>  	if (WARN_ON(!desc))
>  		return 0;
>  
> -	if (index >= desc->num_decoded_fmts)
> -		return 0;
> +	for (i = 0; i < desc->num_decoded_fmts; i++) {
> +		if (!rkvdec_image_fmt_match(desc->decoded_fmts[i].image_fmt,
> +					    image_fmt))
> +			continue;
> +		fmt_idx++;
> +		if (index == fmt_idx)
> +			return desc->decoded_fmts[i].fourcc;
> +	}
>  
> -	return desc->decoded_fmts[index];
> +	return 0;
>  }
>  
> -static bool rkvdec_is_valid_fmt(struct rkvdec_ctx *ctx, u32 fourcc)
> +static bool rkvdec_is_valid_fmt(struct rkvdec_ctx *ctx, u32 fourcc,
> +				enum rkvdec_image_fmt image_fmt)
>  {
>  	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)
> +		if (rkvdec_image_fmt_match(desc->decoded_fmts[i].image_fmt,
> +					   image_fmt) &&
> +		    desc->decoded_fmts[i].fourcc == fourcc)
>  			return true;
>  	}
>  
> @@ -80,7 +99,7 @@ static void rkvdec_reset_decoded_fmt(struct rkvdec_ctx *ctx)
>  	struct v4l2_format *f = &ctx->decoded_fmt;
>  	u32 fourcc;
>  
> -	fourcc = rkvdec_enum_decoded_fmt(ctx, 0);
> +	fourcc = rkvdec_enum_decoded_fmt(ctx, 0, ctx->image_fmt);
>  	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;
> @@ -149,8 +168,11 @@ static const struct rkvdec_ctrls rkvdec_h264_ctrls = {
>  	.num_ctrls = ARRAY_SIZE(rkvdec_h264_ctrl_descs),
>  };
>  
> -static const u32 rkvdec_h264_vp9_decoded_fmts[] = {
> -	V4L2_PIX_FMT_NV12,
> +static const struct rkvdec_decoded_fmt_desc rkvdec_h264_vp9_decoded_fmts[] = {
> +	{
> +		.fourcc = V4L2_PIX_FMT_NV12,
> +		.image_fmt = RKVDEC_IMG_FMT_420_8BIT,
> +	},
>  };
>  
>  static const struct rkvdec_ctrl_desc rkvdec_vp9_ctrl_descs[] = {
> @@ -282,8 +304,9 @@ static int rkvdec_try_capture_fmt(struct file *file, void *priv,
>  	if (WARN_ON(!coded_desc))
>  		return -EINVAL;
>  
> -	if (!rkvdec_is_valid_fmt(ctx, pix_mp->pixelformat))
> -		pix_mp->pixelformat = rkvdec_enum_decoded_fmt(ctx, 0);
> +	if (!rkvdec_is_valid_fmt(ctx, pix_mp->pixelformat, ctx->image_fmt))
> +		pix_mp->pixelformat = rkvdec_enum_decoded_fmt(ctx, 0,
> +							      ctx->image_fmt);
>  
>  	/* Always apply the frmsize constraint of the coded end. */
>  	pix_mp->width = max(pix_mp->width, ctx->coded_fmt.fmt.pix_mp.width);
> @@ -400,6 +423,7 @@ static int rkvdec_s_output_fmt(struct file *file, void *priv,
>  	 *
>  	 * Note that this will propagates any size changes to the decoded format.
>  	 */
> +	ctx->image_fmt = RKVDEC_IMG_FMT_ANY;
>  	rkvdec_reset_decoded_fmt(ctx);
>  
>  	/* Propagate colorspace information to capture. */
> @@ -449,7 +473,7 @@ static int rkvdec_enum_capture_fmt(struct file *file, void *priv,
>  	struct rkvdec_ctx *ctx = fh_to_rkvdec_ctx(priv);
>  	u32 fourcc;
>  
> -	fourcc = rkvdec_enum_decoded_fmt(ctx, f->index);
> +	fourcc = rkvdec_enum_decoded_fmt(ctx, f->index, ctx->image_fmt);
>  	if (!fourcc)
>  		return -EINVAL;
>  
> diff --git a/drivers/staging/media/rkvdec/rkvdec.h b/drivers/staging/media/rkvdec/rkvdec.h
> index 633335ebb9c4..6f8cf50c5d99 100644
> --- a/drivers/staging/media/rkvdec/rkvdec.h
> +++ b/drivers/staging/media/rkvdec/rkvdec.h
> @@ -75,13 +75,23 @@ struct rkvdec_coded_fmt_ops {
>  	int (*try_ctrl)(struct rkvdec_ctx *ctx, struct v4l2_ctrl *ctrl);
>  };
>  
> +enum rkvdec_image_fmt {
> +	RKVDEC_IMG_FMT_ANY = 0,
> +	RKVDEC_IMG_FMT_420_8BIT,
> +};
> +
> +struct rkvdec_decoded_fmt_desc {
> +	u32 fourcc;
> +	enum rkvdec_image_fmt image_fmt;
> +};
> +
>  struct rkvdec_coded_fmt_desc {
>  	u32 fourcc;
>  	struct v4l2_frmsize_stepwise frmsize;
>  	const struct rkvdec_ctrls *ctrls;
>  	const struct rkvdec_coded_fmt_ops *ops;
>  	unsigned int num_decoded_fmts;
> -	const u32 *decoded_fmts;
> +	const struct rkvdec_decoded_fmt_desc *decoded_fmts;
>  	u32 subsystem_flags;
>  };
>  
> @@ -104,6 +114,7 @@ struct rkvdec_ctx {
>  	const struct rkvdec_coded_fmt_desc *coded_fmt_desc;
>  	struct v4l2_ctrl_handler ctrl_hdl;
>  	struct rkvdec_dev *dev;
> +	enum rkvdec_image_fmt image_fmt;
>  	void *priv;
>  };
>  

Re: [PATCH v6 08/11] media: rkvdec: Add image format concept
Posted by Dan Carpenter 2 months, 3 weeks ago
On Mon, Sep 09, 2024 at 07:25:06PM +0000, Jonas Karlman wrote:
> Add an enum rkvdec_image_fmt used to signal an image format, e.g.
> 4:2:0 8-bit, 4:2:0 10-bit or any.
> 
> Tag each supported CAPUTRE format with an image format and use this tag
> to filter out unsupported CAPTURE formats.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> Tested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Tested-by: Christopher Obbard <chris.obbard@collabora.com>
> ---
> v6:
> - Change to use fmt_idx instead of j++ tucked inside a condition (Dan)

Thanks!

regards,
dan carpenter