Re: [PATCH 5/6] [media] V4L: Add VP8 encoder controls

[Sylwester Nawrocki <s.nawrocki@samsung.com>]

Hi Arun,

On 06/10/2013 03:23 PM, Arun Kumar K wrote:
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index fccd08b..2cf17d4 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -456,6 +456,23 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>  		"RGB full range (0-255)",
>  		NULL,
>  	};
> +	static const char * const vpx_num_partitions[] = {
> +		"1 partition",
> +		"2 partitions",
> +		"4 partitions",
> +		"8 partitions",
> +		NULL,
> +	};
> +	static const char * const vpx_num_ref_frames[] = {
> +		"1 reference frame",
> +		"2 reference frame",
> +		NULL,
> +	};

Have you considered using V4L2_CTRL_TYPE_INTEGER_MENU control type for this ?
One example is V4L2_CID_ISO_SENSITIVITY control.

> +/*  VPX streams, specific to multiplexed streams */
> +#define V4L2_CID_VPX_NUM_PARTITIONS		(V4L2_CID_VPX_BASE+0)
> +enum v4l2_vp8_num_partitions {
> +	V4L2_VPX_1_PARTITION	= 0,
> +	V4L2_VPX_2_PARTITIONS	= (1 << 1),
> +	V4L2_VPX_4_PARTITIONS	= (1 << 2),
> +	V4L2_VPX_8_PARTITIONS	= (1 << 3),
> +};

I think we could still have such standard value definitions if needed,
but rather in form of:

#define V4L2_VPX_1_PARTITION	1
#define V4L2_VPX_2_PARTITIONS	2
#define V4L2_VPX_4_PARTITIONS	4
#define V4L2_VPX_8_PARTITIONS	8

> +#define V4L2_CID_VPX_IMD_DISABLE_4X4		(V4L2_CID_VPX_BASE+1)
> +#define V4L2_CID_VPX_NUM_REF_FRAMES		(V4L2_CID_VPX_BASE+2)
> +enum v4l2_vp8_num_ref_frames {
> +	V4L2_VPX_1_REF_FRAME	= 0,
> +	V4L2_VPX_2_REF_FRAME	= 1,
> +};

Regards,
Sylwester