Re: [PATCH v2 4/6] media: vicodec: Add pixel encoding flags to fwht header

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/16/19 4:25 PM, Dafna Hirschfeld wrote:
> Add flags indicating the pixel encoding - yuv/rgb/hsv to
> fwht header and to the pixel info. Use it to enumerate
> the supported pixel formats.
> 
> Signed-off-by: Dafna Hirschfeld <dafna3@xxxxxxxxx>
> ---
>  drivers/media/platform/vicodec/codec-fwht.h   |  5 ++
>  .../media/platform/vicodec/codec-v4l2-fwht.c  | 76 +++++++++++++------
>  .../media/platform/vicodec/codec-v4l2-fwht.h  |  7 ++
>  drivers/media/platform/vicodec/vicodec-core.c | 20 +++--
>  4 files changed, 78 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/media/platform/vicodec/codec-fwht.h b/drivers/media/platform/vicodec/codec-fwht.h
> index 6d230f5e9d60..b3f1389256df 100644
> --- a/drivers/media/platform/vicodec/codec-fwht.h
> +++ b/drivers/media/platform/vicodec/codec-fwht.h
> @@ -79,6 +79,11 @@
>  
>  /* A 4-values flag - the number of components - 1 */
>  #define FWHT_FL_COMPONENTS_NUM_MSK	GENMASK(17, 16)
> +#define FWHT_FL_PIXENC_MSK	GENMASK(19, 18)

I think we should reserve 3 bits for this, so use GENMASK(20, 18).

> +#define FWHT_FL_PIXENC_YUV	0UL
> +#define FWHT_FL_PIXENC_RGB	BIT(18)
> +#define FWHT_FL_PIXENC_HSV	(BIT(18) | BIT(19))

I'd change this to:

YUV: (1 << 18)
RGB: (2 << 18)
HSV: (3 << 18)

> +
>  #define FWHT_FL_COMPONENTS_NUM_OFFSET	16
>  
>  /*
> diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.c b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
> index 143af8c587b3..3df51d47674b 100644
> --- a/drivers/media/platform/vicodec/codec-v4l2-fwht.c
> +++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
> @@ -11,32 +11,53 @@
>  #include "codec-v4l2-fwht.h"
>  
>  static const struct v4l2_fwht_pixfmt_info v4l2_fwht_pixfmts[] = {
> -	{ V4L2_PIX_FMT_YUV420,  1, 3, 2, 1, 1, 2, 2, 3, 3},
> -	{ V4L2_PIX_FMT_YVU420,  1, 3, 2, 1, 1, 2, 2, 3, 3},
> -	{ V4L2_PIX_FMT_YUV422P, 1, 2, 1, 1, 1, 2, 1, 3, 3},
> -	{ V4L2_PIX_FMT_NV12,    1, 3, 2, 1, 2, 2, 2, 3, 2},
> -	{ V4L2_PIX_FMT_NV21,    1, 3, 2, 1, 2, 2, 2, 3, 2},
> -	{ V4L2_PIX_FMT_NV16,    1, 2, 1, 1, 2, 2, 1, 3, 2},
> -	{ V4L2_PIX_FMT_NV61,    1, 2, 1, 1, 2, 2, 1, 3, 2},
> -	{ V4L2_PIX_FMT_NV24,    1, 3, 1, 1, 2, 1, 1, 3, 2},
> -	{ V4L2_PIX_FMT_NV42,    1, 3, 1, 1, 2, 1, 1, 3, 2},
> -	{ V4L2_PIX_FMT_YUYV,    2, 2, 1, 2, 4, 2, 1, 3, 1},
> -	{ V4L2_PIX_FMT_YVYU,    2, 2, 1, 2, 4, 2, 1, 3, 1},
> -	{ V4L2_PIX_FMT_UYVY,    2, 2, 1, 2, 4, 2, 1, 3, 1},
> -	{ V4L2_PIX_FMT_VYUY,    2, 2, 1, 2, 4, 2, 1, 3, 1},
> -	{ V4L2_PIX_FMT_BGR24,   3, 3, 1, 3, 3, 1, 1, 3, 1},
> -	{ V4L2_PIX_FMT_RGB24,   3, 3, 1, 3, 3, 1, 1, 3, 1},
> -	{ V4L2_PIX_FMT_HSV24,   3, 3, 1, 3, 3, 1, 1, 3, 1},
> -	{ V4L2_PIX_FMT_BGR32,   4, 4, 1, 4, 4, 1, 1, 3, 1},
> -	{ V4L2_PIX_FMT_XBGR32,  4, 4, 1, 4, 4, 1, 1, 3, 1},
> -	{ V4L2_PIX_FMT_RGB32,   4, 4, 1, 4, 4, 1, 1, 3, 1},
> -	{ V4L2_PIX_FMT_XRGB32,  4, 4, 1, 4, 4, 1, 1, 3, 1},
> -	{ V4L2_PIX_FMT_HSV32,   4, 4, 1, 4, 4, 1, 1, 3, 1},
> -	{ V4L2_PIX_FMT_ARGB32,  4, 4, 1, 4, 4, 1, 1, 4, 1},
> -	{ V4L2_PIX_FMT_ABGR32,  4, 4, 1, 4, 4, 1, 1, 4, 1},
> -	{ V4L2_PIX_FMT_GREY,    1, 1, 1, 1, 0, 1, 1, 1, 1},
> +	{ V4L2_PIX_FMT_YUV420,  1, 3, 2, 1, 1, 2, 2, 3, 3, FWHT_FL_PIXENC_YUV},
> +	{ V4L2_PIX_FMT_YVU420,  1, 3, 2, 1, 1, 2, 2, 3, 3, FWHT_FL_PIXENC_YUV},
> +	{ V4L2_PIX_FMT_YUV422P, 1, 2, 1, 1, 1, 2, 1, 3, 3, FWHT_FL_PIXENC_YUV},
> +	{ V4L2_PIX_FMT_NV12,    1, 3, 2, 1, 2, 2, 2, 3, 2, FWHT_FL_PIXENC_YUV},
> +	{ V4L2_PIX_FMT_NV21,    1, 3, 2, 1, 2, 2, 2, 3, 2, FWHT_FL_PIXENC_YUV},
> +	{ V4L2_PIX_FMT_NV16,    1, 2, 1, 1, 2, 2, 1, 3, 2, FWHT_FL_PIXENC_YUV},
> +	{ V4L2_PIX_FMT_NV61,    1, 2, 1, 1, 2, 2, 1, 3, 2, FWHT_FL_PIXENC_YUV},
> +	{ V4L2_PIX_FMT_NV24,    1, 3, 1, 1, 2, 1, 1, 3, 2, FWHT_FL_PIXENC_YUV},
> +	{ V4L2_PIX_FMT_NV42,    1, 3, 1, 1, 2, 1, 1, 3, 2, FWHT_FL_PIXENC_YUV},
> +	{ V4L2_PIX_FMT_YUYV,    2, 2, 1, 2, 4, 2, 1, 3, 1, FWHT_FL_PIXENC_YUV},
> +	{ V4L2_PIX_FMT_YVYU,    2, 2, 1, 2, 4, 2, 1, 3, 1, FWHT_FL_PIXENC_YUV},
> +	{ V4L2_PIX_FMT_UYVY,    2, 2, 1, 2, 4, 2, 1, 3, 1, FWHT_FL_PIXENC_YUV},
> +	{ V4L2_PIX_FMT_VYUY,    2, 2, 1, 2, 4, 2, 1, 3, 1, FWHT_FL_PIXENC_YUV},
> +	{ V4L2_PIX_FMT_BGR24,   3, 3, 1, 3, 3, 1, 1, 3, 1, FWHT_FL_PIXENC_RGB},
> +	{ V4L2_PIX_FMT_RGB24,   3, 3, 1, 3, 3, 1, 1, 3, 1, FWHT_FL_PIXENC_RGB},
> +	{ V4L2_PIX_FMT_HSV24,   3, 3, 1, 3, 3, 1, 1, 3, 1, FWHT_FL_PIXENC_HSV},
> +	{ V4L2_PIX_FMT_BGR32,   4, 4, 1, 4, 4, 1, 1, 3, 1, FWHT_FL_PIXENC_RGB},
> +	{ V4L2_PIX_FMT_XBGR32,  4, 4, 1, 4, 4, 1, 1, 3, 1, FWHT_FL_PIXENC_RGB},
> +	{ V4L2_PIX_FMT_RGB32,   4, 4, 1, 4, 4, 1, 1, 3, 1, FWHT_FL_PIXENC_RGB},
> +	{ V4L2_PIX_FMT_XRGB32,  4, 4, 1, 4, 4, 1, 1, 3, 1, FWHT_FL_PIXENC_RGB},
> +	{ V4L2_PIX_FMT_HSV32,   4, 4, 1, 4, 4, 1, 1, 3, 1, FWHT_FL_PIXENC_HSV},
> +	{ V4L2_PIX_FMT_ARGB32,  4, 4, 1, 4, 4, 1, 1, 4, 1, FWHT_FL_PIXENC_RGB},
> +	{ V4L2_PIX_FMT_ABGR32,  4, 4, 1, 4, 4, 1, 1, 4, 1, FWHT_FL_PIXENC_RGB},
> +	{ V4L2_PIX_FMT_GREY,    1, 1, 1, 1, 0, 1, 1, 1, 1, FWHT_FL_PIXENC_RGB},
>  };
>  
> +const struct v4l2_fwht_pixfmt_info *v4l2_fwht_default_fmt(u32 width_div, u32 height_div,
> +							  u32 version,
> +							  u32 components_num,
> +							  u32 pixenc,
> +							  unsigned int start_idx)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(v4l2_fwht_pixfmts); i++) {
> +		if (v4l2_fwht_pixfmts[i].width_div == width_div &&
> +		    v4l2_fwht_pixfmts[i].height_div == height_div &&
> +		    (version == 1 || v4l2_fwht_pixfmts[i].pixenc == pixenc) &&

A pixenc value of 0 means that we are dealing with an old header. So we can
replace the version check with:

	(!pixenc || v4l2_fwht_pixfmts[i].pixenc == pixenc) &&



> +		    (version == 1 || v4l2_fwht_pixfmts[i].components_num == components_num)) {

I don't think this is right. If this is an old header version, then we should only match
formats with 3 components. So I'd drop the version check here and just ensure that
components_num is always 3 if we have an old header.

Note that with these changes the version argument can be dropped as well.

> +			if (start_idx == 0)
> +				return v4l2_fwht_pixfmts + i;
> +			start_idx--;
> +		}
> +	}
> +	return NULL;
> +}
> +
>  const struct v4l2_fwht_pixfmt_info *v4l2_fwht_find_pixfmt(u32 pixelformat)
>  {
>  	unsigned int i;
> @@ -187,6 +208,7 @@ int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
>  	p_hdr->width = htonl(state->visible_width);
>  	p_hdr->height = htonl(state->visible_height);
>  	flags |= (info->components_num - 1) << FWHT_FL_COMPONENTS_NUM_OFFSET;
> +	flags |= info->pixenc;
>  	if (encoding & FWHT_LUMA_UNENCODED)
>  		flags |= FWHT_FL_LUMA_IS_UNCOMPRESSED;
>  	if (encoding & FWHT_CB_UNENCODED)
> @@ -245,8 +267,14 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
>  	flags = ntohl(p_hdr->flags);
>  
>  	if (version == FWHT_VERSION) {
> +		u32 pixenc = flags & FWHT_FL_PIXENC_MSK;
> +
>  		components_num = 1 + ((flags & FWHT_FL_COMPONENTS_NUM_MSK) >>
>  			FWHT_FL_COMPONENTS_NUM_OFFSET);
> +
> +		if (components_num != info->components_num ||

The components check should be done after this 'if'. Since it also is
valid for older headers.

> +		    pixenc != info->pixenc)
> +			return -EINVAL;
>  	}
>  
>  	state->colorspace = ntohl(p_hdr->colorspace);
> diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.h b/drivers/media/platform/vicodec/codec-v4l2-fwht.h
> index 203c45d98905..5787d4e6822b 100644
> --- a/drivers/media/platform/vicodec/codec-v4l2-fwht.h
> +++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.h
> @@ -20,6 +20,7 @@ struct v4l2_fwht_pixfmt_info {
>  	unsigned int height_div;
>  	unsigned int components_num;
>  	unsigned int planes_num;
> +	unsigned int pixenc;
>  };
>  
>  struct v4l2_fwht_state {
> @@ -45,6 +46,12 @@ struct v4l2_fwht_state {
>  
>  const struct v4l2_fwht_pixfmt_info *v4l2_fwht_find_pixfmt(u32 pixelformat);
>  const struct v4l2_fwht_pixfmt_info *v4l2_fwht_get_pixfmt(u32 idx);
> +const struct v4l2_fwht_pixfmt_info *v4l2_fwht_default_fmt(u32 width_div,
> +							  u32 height_div,
> +							  u32 version,
> +							  u32 components_num,
> +							  u32 pixenc,
> +							  unsigned int start_idx);
>  
>  int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out);
>  int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out);
> diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
> index 51053d5d630a..0df8d3509144 100644
> --- a/drivers/media/platform/vicodec/vicodec-core.c
> +++ b/drivers/media/platform/vicodec/vicodec-core.c
> @@ -395,9 +395,9 @@ static int vidioc_querycap(struct file *file, void *priv,
>  	return 0;
>  }
>  
> -static int enum_fmt(struct v4l2_fmtdesc *f, bool is_enc, bool is_out)
> +static int enum_fmt(struct v4l2_fmtdesc *f, struct vicodec_ctx *ctx, bool is_out)
>  {
> -	bool is_uncomp = (is_enc && is_out) || (!is_enc && !is_out);
> +	bool is_uncomp = (ctx->is_enc && is_out) || (!ctx->is_enc && !is_out);
>  
>  	if (V4L2_TYPE_IS_MULTIPLANAR(f->type) && !multiplanar)
>  		return -EINVAL;
> @@ -405,9 +405,17 @@ static int enum_fmt(struct v4l2_fmtdesc *f, bool is_enc, bool is_out)
>  		return -EINVAL;
>  
>  	if (is_uncomp) {
> -		const struct v4l2_fwht_pixfmt_info *info =
> -			v4l2_fwht_get_pixfmt(f->index);
> +		const struct v4l2_fwht_pixfmt_info *info = get_q_data(ctx, f->type)->info;
>  
> +		if (ctx->is_enc)
> +			info = v4l2_fwht_get_pixfmt(f->index);
> +		else
> +			info = v4l2_fwht_default_fmt(info->width_div,
> +						     info->height_div,
> +						     FWHT_VERSION,
> +						     info->components_num,
> +						     info->pixenc,
> +						     f->index);
>  		if (!info)
>  			return -EINVAL;
>  		f->pixelformat = info->id;
> @@ -424,7 +432,7 @@ static int vidioc_enum_fmt_vid_cap(struct file *file, void *priv,
>  {
>  	struct vicodec_ctx *ctx = file2ctx(file);
>  
> -	return enum_fmt(f, ctx->is_enc, false);
> +	return enum_fmt(f, ctx, false);
>  }
>  
>  static int vidioc_enum_fmt_vid_out(struct file *file, void *priv,
> @@ -432,7 +440,7 @@ static int vidioc_enum_fmt_vid_out(struct file *file, void *priv,
>  {
>  	struct vicodec_ctx *ctx = file2ctx(file);
>  
> -	return enum_fmt(f, ctx->is_enc, true);
> +	return enum_fmt(f, ctx, true);
>  }
>  
>  static int vidioc_g_fmt(struct vicodec_ctx *ctx, struct v4l2_format *f)
> 

Regards,

	Hans



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux