[RFC/PATCH 0/3] 64-bit integer menus

[RFC/PATCH 0/3] 64-bit integer menus

[Sakari Ailus]

Hi all,

This patchset, which I'm sending as RFC since it has not been really tested
(including compiling the vivi patch), adds 64-bit integer menu controls. The
control items in the integer menu are just like in regular menus but they
are 64-bit integers instead of strings.

I'm also pondering whether to assign 1 to ctrl->step for menu type controls
as well but haven't checked what may have been the original reason to
implement it as it is now implemented.

The reason why I don't use a union for qmenu and qmenu_int in
v4l2_ctrl_config is that a lot of drivers use that field in the initialiser
and GCC < 4.6 does not support initialisers with anonymous unions.

Similar union is created in v4l2_querymenu but I do not see this as a
problem since I do not expect initialisers to be used with this field in the
user space code.

Comments and questions are welcome.

Kind regards,


-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	jabber/XMPP/Gmail: sailus@retiisi.org.uk

[RFC/PATCH 1/3] v4l: Introduce integer menu controls

[Sakari Ailus]

Create a new control type called V4L2_CTRL_TYPE_INTEGER_MENU. Integer menu
controls are just like menu controls but the menu items are 64-bit integers
rather than strings.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 drivers/media/video/v4l2-ctrls.c |   63 +++++++++++++++++++++++++++----------
 include/linux/videodev2.h        |    6 +++-
 include/media/v4l2-ctrls.h       |    6 +++-
 3 files changed, 56 insertions(+), 19 deletions(-)

diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
index 0f415da..609eb31 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -804,7 +804,8 @@ static void fill_event(struct v4l2_event *ev, struct v4l2_ctrl *ctrl, u32 change
 		ev->u.ctrl.value64 = ctrl->cur.val64;
 	ev->u.ctrl.minimum = ctrl->minimum;
 	ev->u.ctrl.maximum = ctrl->maximum;
-	if (ctrl->type == V4L2_CTRL_TYPE_MENU)
+	if (ctrl->type == V4L2_CTRL_TYPE_MENU
+	    || ctrl->type == V4L2_CTRL_TYPE_INTEGER_MENU)
 		ev->u.ctrl.step = 1;
 	else
 		ev->u.ctrl.step = ctrl->step;
@@ -1035,10 +1036,13 @@ static int validate_new_int(const struct v4l2_ctrl *ctrl, s32 *pval)
 		return 0;
 
 	case V4L2_CTRL_TYPE_MENU:
+	case V4L2_CTRL_TYPE_INTEGER_MENU:
 		if (val < ctrl->minimum || val > ctrl->maximum)
 			return -ERANGE;
-		if (ctrl->qmenu[val][0] == '\0' ||
-		    (ctrl->menu_skip_mask & (1 << val)))
+		if (ctrl->menu_skip_mask & (1 << val))
+			return -EINVAL;
+		if (ctrl->type == V4L2_CTRL_TYPE_MENU &&
+		    ctrl->qmenu[val][0] == '\0')
 			return -EINVAL;
 		return 0;
 
@@ -1295,7 +1299,8 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 			const struct v4l2_ctrl_ops *ops,
 			u32 id, const char *name, enum v4l2_ctrl_type type,
 			s32 min, s32 max, u32 step, s32 def,
-			u32 flags, const char * const *qmenu, void *priv)
+			u32 flags, const char * const *qmenu,
+			const s64 *qmenu_int, void *priv)
 {
 	struct v4l2_ctrl *ctrl;
 	unsigned sz_extra = 0;
@@ -1308,6 +1313,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 	    (type == V4L2_CTRL_TYPE_INTEGER && step == 0) ||
 	    (type == V4L2_CTRL_TYPE_BITMASK && max == 0) ||
 	    (type == V4L2_CTRL_TYPE_MENU && qmenu == NULL) ||
+	    (type == V4L2_CTRL_TYPE_INTEGER_MENU && qmenu_int == NULL) ||
 	    (type == V4L2_CTRL_TYPE_STRING && max == 0)) {
 		handler_set_err(hdl, -ERANGE);
 		return NULL;
@@ -1318,6 +1324,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 	}
 	if ((type == V4L2_CTRL_TYPE_INTEGER ||
 	     type == V4L2_CTRL_TYPE_MENU ||
+	     type == V4L2_CTRL_TYPE_INTEGER_MENU ||
 	     type == V4L2_CTRL_TYPE_BOOLEAN) &&
 	    (def < min || def > max)) {
 		handler_set_err(hdl, -ERANGE);
@@ -1352,7 +1359,10 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 	ctrl->minimum = min;
 	ctrl->maximum = max;
 	ctrl->step = step;
-	ctrl->qmenu = qmenu;
+	if (type == V4L2_CTRL_TYPE_MENU)
+		ctrl->qmenu = qmenu;
+	else if (type == V4L2_CTRL_TYPE_INTEGER_MENU)
+		ctrl->qmenu_int = qmenu_int;
 	ctrl->priv = priv;
 	ctrl->cur.val = ctrl->val = ctrl->default_value = def;
 
@@ -1379,6 +1389,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_custom(struct v4l2_ctrl_handler *hdl,
 	struct v4l2_ctrl *ctrl;
 	const char *name = cfg->name;
 	const char * const *qmenu = cfg->qmenu;
+	const s64 *qmenu_int = cfg->qmenu_int;
 	enum v4l2_ctrl_type type = cfg->type;
 	u32 flags = cfg->flags;
 	s32 min = cfg->min;
@@ -1390,18 +1401,24 @@ struct v4l2_ctrl *v4l2_ctrl_new_custom(struct v4l2_ctrl_handler *hdl,
 		v4l2_ctrl_fill(cfg->id, &name, &type, &min, &max, &step,
 								&def, &flags);
 
-	is_menu = (cfg->type == V4L2_CTRL_TYPE_MENU);
+	is_menu = (cfg->type == V4L2_CTRL_TYPE_MENU ||
+		   cfg->type == V4L2_CTRL_TYPE_INTEGER_MENU);
 	if (is_menu)
 		WARN_ON(step);
 	else
 		WARN_ON(cfg->menu_skip_mask);
-	if (is_menu && qmenu == NULL)
+	if (cfg->type == V4L2_CTRL_TYPE_MENU && qmenu == NULL)
 		qmenu = v4l2_ctrl_get_menu(cfg->id);
+	else if (cfg->type == V4L2_CTRL_TYPE_INTEGER_MENU &&
+		 qmenu_int == NULL) {
+		handler_set_err(hdl, -EINVAL);
+		return NULL;
+	}
 
 	ctrl = v4l2_ctrl_new(hdl, cfg->ops, cfg->id, name,
 			type, min, max,
 			is_menu ? cfg->menu_skip_mask : step,
-			def, flags, qmenu, priv);
+			def, flags, qmenu, qmenu_int, priv);
 	if (ctrl)
 		ctrl->is_private = cfg->is_private;
 	return ctrl;
@@ -1418,12 +1435,13 @@ struct v4l2_ctrl *v4l2_ctrl_new_std(struct v4l2_ctrl_handler *hdl,
 	u32 flags;
 
 	v4l2_ctrl_fill(id, &name, &type, &min, &max, &step, &def, &flags);
-	if (type == V4L2_CTRL_TYPE_MENU) {
+	if (type == V4L2_CTRL_TYPE_MENU
+	    || type == V4L2_CTRL_TYPE_INTEGER_MENU) {
 		handler_set_err(hdl, -EINVAL);
 		return NULL;
 	}
 	return v4l2_ctrl_new(hdl, ops, id, name, type,
-				    min, max, step, def, flags, NULL, NULL);
+			     min, max, step, def, flags, NULL, NULL, NULL);
 }
 EXPORT_SYMBOL(v4l2_ctrl_new_std);
 
@@ -1440,12 +1458,13 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct v4l2_ctrl_handler *hdl,
 	u32 flags;
 
 	v4l2_ctrl_fill(id, &name, &type, &min, &max, &step, &def, &flags);
-	if (type != V4L2_CTRL_TYPE_MENU) {
+	if (type != V4L2_CTRL_TYPE_MENU
+	    && type != V4L2_CTRL_TYPE_INTEGER_MENU) {
 		handler_set_err(hdl, -EINVAL);
 		return NULL;
 	}
 	return v4l2_ctrl_new(hdl, ops, id, name, type,
-				    0, max, mask, def, flags, qmenu, NULL);
+			     0, max, mask, def, flags, qmenu, NULL, NULL);
 }
 EXPORT_SYMBOL(v4l2_ctrl_new_std_menu);
 
@@ -1609,6 +1628,9 @@ static void log_ctrl(const struct v4l2_ctrl *ctrl,
 	case V4L2_CTRL_TYPE_MENU:
 		printk(KERN_CONT "%s", ctrl->qmenu[ctrl->cur.val]);
 		break;
+	case V4L2_CTRL_TYPE_INTEGER_MENU:
+		printk(KERN_CONT "%lld", ctrl->qmenu_int[ctrl->cur.val]);
+		break;
 	case V4L2_CTRL_TYPE_BITMASK:
 		printk(KERN_CONT "0x%08x", ctrl->cur.val);
 		break;
@@ -1745,7 +1767,8 @@ int v4l2_queryctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_queryctrl *qc)
 	qc->minimum = ctrl->minimum;
 	qc->maximum = ctrl->maximum;
 	qc->default_value = ctrl->default_value;
-	if (ctrl->type == V4L2_CTRL_TYPE_MENU)
+	if (ctrl->type == V4L2_CTRL_TYPE_MENU
+	    || ctrl->type == V4L2_CTRL_TYPE_INTEGER_MENU)
 		qc->step = 1;
 	else
 		qc->step = ctrl->step;
@@ -1775,16 +1798,22 @@ int v4l2_querymenu(struct v4l2_ctrl_handler *hdl, struct v4l2_querymenu *qm)
 
 	qm->reserved = 0;
 	/* Sanity checks */
-	if (ctrl->qmenu == NULL ||
+	if ((ctrl->type == V4L2_CTRL_TYPE_MENU && ctrl->qmenu == NULL) ||
+	    (ctrl->type == V4L2_CTRL_TYPE_INTEGER_MENU
+	     && ctrl->qmenu_int == NULL) ||
 	    i < ctrl->minimum || i > ctrl->maximum)
 		return -EINVAL;
 	/* Use mask to see if this menu item should be skipped */
 	if (ctrl->menu_skip_mask & (1 << i))
 		return -EINVAL;
 	/* Empty menu items should also be skipped */
-	if (ctrl->qmenu[i] == NULL || ctrl->qmenu[i][0] == '\0')
-		return -EINVAL;
-	strlcpy(qm->name, ctrl->qmenu[i], sizeof(qm->name));
+	if (ctrl->type == V4L2_CTRL_TYPE_MENU) {
+		if (ctrl->qmenu[i] == NULL || ctrl->qmenu[i][0] == '\0')
+			return -EINVAL;
+		strlcpy(qm->name, ctrl->qmenu[i], sizeof(qm->name));
+	} else if (ctrl->type == V4L2_CTRL_TYPE_INTEGER_MENU) {
+		qm->value = ctrl->qmenu_int[i];
+	}
 	return 0;
 }
 EXPORT_SYMBOL(v4l2_querymenu);
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 4b752d5..9633c69 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -1094,6 +1094,7 @@ enum v4l2_ctrl_type {
 	V4L2_CTRL_TYPE_CTRL_CLASS    = 6,
 	V4L2_CTRL_TYPE_STRING        = 7,
 	V4L2_CTRL_TYPE_BITMASK       = 8,
+	V4L2_CTRL_TYPE_INTEGER_MENU = 9,
 };
 
 /*  Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
@@ -1113,7 +1114,10 @@ struct v4l2_queryctrl {
 struct v4l2_querymenu {
 	__u32		id;
 	__u32		index;
-	__u8		name[32];	/* Whatever */
+	union {
+		__u8	name[32];	/* Whatever */
+		__s64	value;
+	};
 	__u32		reserved;
 };
 
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index eeb3df6..f7819e7 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -129,7 +129,10 @@ struct v4l2_ctrl {
 		u32 step;
 		u32 menu_skip_mask;
 	};
-	const char * const *qmenu;
+	union {
+		const char * const *qmenu;
+		const s64 *qmenu_int;
+	};
 	unsigned long flags;
 	union {
 		s32 val;
@@ -219,6 +222,7 @@ struct v4l2_ctrl_config {
 	u32 flags;
 	u32 menu_skip_mask;
 	const char * const *qmenu;
+	const s64 *qmenu_int;
 	unsigned int is_private:1;
 };
 
-- 
1.7.2.5

Re: [RFC/PATCH 1/3] v4l: Introduce integer menu controls

[Laurent Pinchart]

Hi Sakari,

Thanks for the patch.

On Thursday 24 November 2011 17:12:50 Sakari Ailus wrote:
> Create a new control type called V4L2_CTRL_TYPE_INTEGER_MENU. Integer menu
> controls are just like menu controls but the menu items are 64-bit integers
> rather than strings.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> ---
>  drivers/media/video/v4l2-ctrls.c |   63
> +++++++++++++++++++++++++++---------- include/linux/videodev2.h        |  
>  6 +++-
>  include/media/v4l2-ctrls.h       |    6 +++-
>  3 files changed, 56 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/media/video/v4l2-ctrls.c
> b/drivers/media/video/v4l2-ctrls.c index 0f415da..609eb31 100644
> --- a/drivers/media/video/v4l2-ctrls.c
> +++ b/drivers/media/video/v4l2-ctrls.c
> @@ -804,7 +804,8 @@ static void fill_event(struct v4l2_event *ev, struct
> v4l2_ctrl *ctrl, u32 change ev->u.ctrl.value64 = ctrl->cur.val64;
>  	ev->u.ctrl.minimum = ctrl->minimum;
>  	ev->u.ctrl.maximum = ctrl->maximum;
> -	if (ctrl->type == V4L2_CTRL_TYPE_MENU)
> +	if (ctrl->type == V4L2_CTRL_TYPE_MENU
> +	    || ctrl->type == V4L2_CTRL_TYPE_INTEGER_MENU)
>  		ev->u.ctrl.step = 1;
>  	else
>  		ev->u.ctrl.step = ctrl->step;
> @@ -1035,10 +1036,13 @@ static int validate_new_int(const struct v4l2_ctrl
> *ctrl, s32 *pval) return 0;
> 
>  	case V4L2_CTRL_TYPE_MENU:
> +	case V4L2_CTRL_TYPE_INTEGER_MENU:
>  		if (val < ctrl->minimum || val > ctrl->maximum)
>  			return -ERANGE;
> -		if (ctrl->qmenu[val][0] == '\0' ||
> -		    (ctrl->menu_skip_mask & (1 << val)))
> +		if (ctrl->menu_skip_mask & (1 << val))
> +			return -EINVAL;
> +		if (ctrl->type == V4L2_CTRL_TYPE_MENU &&
> +		    ctrl->qmenu[val][0] == '\0')
>  			return -EINVAL;
>  		return 0;
> 
> @@ -1295,7 +1299,8 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
> v4l2_ctrl_handler *hdl, const struct v4l2_ctrl_ops *ops,
>  			u32 id, const char *name, enum v4l2_ctrl_type type,
>  			s32 min, s32 max, u32 step, s32 def,
> -			u32 flags, const char * const *qmenu, void *priv)
> +			u32 flags, const char * const *qmenu,
> +			const s64 *qmenu_int, void *priv)
>  {
>  	struct v4l2_ctrl *ctrl;
>  	unsigned sz_extra = 0;
> @@ -1308,6 +1313,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
> v4l2_ctrl_handler *hdl, (type == V4L2_CTRL_TYPE_INTEGER && step == 0) ||
>  	    (type == V4L2_CTRL_TYPE_BITMASK && max == 0) ||
>  	    (type == V4L2_CTRL_TYPE_MENU && qmenu == NULL) ||
> +	    (type == V4L2_CTRL_TYPE_INTEGER_MENU && qmenu_int == NULL) ||
>  	    (type == V4L2_CTRL_TYPE_STRING && max == 0)) {
>  		handler_set_err(hdl, -ERANGE);
>  		return NULL;
> @@ -1318,6 +1324,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
> v4l2_ctrl_handler *hdl, }
>  	if ((type == V4L2_CTRL_TYPE_INTEGER ||
>  	     type == V4L2_CTRL_TYPE_MENU ||
> +	     type == V4L2_CTRL_TYPE_INTEGER_MENU ||
>  	     type == V4L2_CTRL_TYPE_BOOLEAN) &&
>  	    (def < min || def > max)) {
>  		handler_set_err(hdl, -ERANGE);
> @@ -1352,7 +1359,10 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
> v4l2_ctrl_handler *hdl, ctrl->minimum = min;
>  	ctrl->maximum = max;
>  	ctrl->step = step;
> -	ctrl->qmenu = qmenu;
> +	if (type == V4L2_CTRL_TYPE_MENU)
> +		ctrl->qmenu = qmenu;
> +	else if (type == V4L2_CTRL_TYPE_INTEGER_MENU)
> +		ctrl->qmenu_int = qmenu_int;
>  	ctrl->priv = priv;
>  	ctrl->cur.val = ctrl->val = ctrl->default_value = def;
> 
> @@ -1379,6 +1389,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_custom(struct
> v4l2_ctrl_handler *hdl, struct v4l2_ctrl *ctrl;
>  	const char *name = cfg->name;
>  	const char * const *qmenu = cfg->qmenu;
> +	const s64 *qmenu_int = cfg->qmenu_int;
>  	enum v4l2_ctrl_type type = cfg->type;
>  	u32 flags = cfg->flags;
>  	s32 min = cfg->min;
> @@ -1390,18 +1401,24 @@ struct v4l2_ctrl *v4l2_ctrl_new_custom(struct
> v4l2_ctrl_handler *hdl, v4l2_ctrl_fill(cfg->id, &name, &type, &min, &max,
> &step,
>  								&def, &flags);
> 
> -	is_menu = (cfg->type == V4L2_CTRL_TYPE_MENU);
> +	is_menu = (cfg->type == V4L2_CTRL_TYPE_MENU ||
> +		   cfg->type == V4L2_CTRL_TYPE_INTEGER_MENU);
>  	if (is_menu)
>  		WARN_ON(step);
>  	else
>  		WARN_ON(cfg->menu_skip_mask);
> -	if (is_menu && qmenu == NULL)
> +	if (cfg->type == V4L2_CTRL_TYPE_MENU && qmenu == NULL)
>  		qmenu = v4l2_ctrl_get_menu(cfg->id);
> +	else if (cfg->type == V4L2_CTRL_TYPE_INTEGER_MENU &&
> +		 qmenu_int == NULL) {
> +		handler_set_err(hdl, -EINVAL);
> +		return NULL;
> +	}
> 
>  	ctrl = v4l2_ctrl_new(hdl, cfg->ops, cfg->id, name,
>  			type, min, max,
>  			is_menu ? cfg->menu_skip_mask : step,
> -			def, flags, qmenu, priv);
> +			def, flags, qmenu, qmenu_int, priv);
>  	if (ctrl)
>  		ctrl->is_private = cfg->is_private;
>  	return ctrl;
> @@ -1418,12 +1435,13 @@ struct v4l2_ctrl *v4l2_ctrl_new_std(struct
> v4l2_ctrl_handler *hdl, u32 flags;
> 
>  	v4l2_ctrl_fill(id, &name, &type, &min, &max, &step, &def, &flags);
> -	if (type == V4L2_CTRL_TYPE_MENU) {
> +	if (type == V4L2_CTRL_TYPE_MENU
> +	    || type == V4L2_CTRL_TYPE_INTEGER_MENU) {
>  		handler_set_err(hdl, -EINVAL);
>  		return NULL;
>  	}
>  	return v4l2_ctrl_new(hdl, ops, id, name, type,
> -				    min, max, step, def, flags, NULL, NULL);
> +			     min, max, step, def, flags, NULL, NULL, NULL);
>  }
>  EXPORT_SYMBOL(v4l2_ctrl_new_std);
> 
> @@ -1440,12 +1458,13 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct
> v4l2_ctrl_handler *hdl, u32 flags;
> 
>  	v4l2_ctrl_fill(id, &name, &type, &min, &max, &step, &def, &flags);
> -	if (type != V4L2_CTRL_TYPE_MENU) {
> +	if (type != V4L2_CTRL_TYPE_MENU
> +	    && type != V4L2_CTRL_TYPE_INTEGER_MENU) {
>  		handler_set_err(hdl, -EINVAL);
>  		return NULL;
>  	}
>  	return v4l2_ctrl_new(hdl, ops, id, name, type,
> -				    0, max, mask, def, flags, qmenu, NULL);
> +			     0, max, mask, def, flags, qmenu, NULL, NULL);

You pass NULL to the v4l2_ctrl_new() qmenu_int argument, which will make the 
function fail for integer menu controls. Do you expect standard integer menu 
controls to share a list of values ? If not, what about modifying 
v4l2_ctrl_new_std_menu() to take a list of values (or alternatively forbidding 
the function from being used for integer menu controls) ?

>  }
>  EXPORT_SYMBOL(v4l2_ctrl_new_std_menu);
> 
> @@ -1609,6 +1628,9 @@ static void log_ctrl(const struct v4l2_ctrl *ctrl,
>  	case V4L2_CTRL_TYPE_MENU:
>  		printk(KERN_CONT "%s", ctrl->qmenu[ctrl->cur.val]);
>  		break;
> +	case V4L2_CTRL_TYPE_INTEGER_MENU:
> +		printk(KERN_CONT "%lld", ctrl->qmenu_int[ctrl->cur.val]);
> +		break;
>  	case V4L2_CTRL_TYPE_BITMASK:
>  		printk(KERN_CONT "0x%08x", ctrl->cur.val);
>  		break;
> @@ -1745,7 +1767,8 @@ int v4l2_queryctrl(struct v4l2_ctrl_handler *hdl,
> struct v4l2_queryctrl *qc) qc->minimum = ctrl->minimum;
>  	qc->maximum = ctrl->maximum;
>  	qc->default_value = ctrl->default_value;
> -	if (ctrl->type == V4L2_CTRL_TYPE_MENU)
> +	if (ctrl->type == V4L2_CTRL_TYPE_MENU
> +	    || ctrl->type == V4L2_CTRL_TYPE_INTEGER_MENU)
>  		qc->step = 1;
>  	else
>  		qc->step = ctrl->step;
> @@ -1775,16 +1798,22 @@ int v4l2_querymenu(struct v4l2_ctrl_handler *hdl,
> struct v4l2_querymenu *qm)
> 
>  	qm->reserved = 0;
>  	/* Sanity checks */
> -	if (ctrl->qmenu == NULL ||
> +	if ((ctrl->type == V4L2_CTRL_TYPE_MENU && ctrl->qmenu == NULL) ||
> +	    (ctrl->type == V4L2_CTRL_TYPE_INTEGER_MENU
> +	     && ctrl->qmenu_int == NULL) ||
>  	    i < ctrl->minimum || i > ctrl->maximum)
>  		return -EINVAL;
>  	/* Use mask to see if this menu item should be skipped */
>  	if (ctrl->menu_skip_mask & (1 << i))
>  		return -EINVAL;
>  	/* Empty menu items should also be skipped */
> -	if (ctrl->qmenu[i] == NULL || ctrl->qmenu[i][0] == '\0')
> -		return -EINVAL;
> -	strlcpy(qm->name, ctrl->qmenu[i], sizeof(qm->name));
> +	if (ctrl->type == V4L2_CTRL_TYPE_MENU) {
> +		if (ctrl->qmenu[i] == NULL || ctrl->qmenu[i][0] == '\0')
> +			return -EINVAL;
> +		strlcpy(qm->name, ctrl->qmenu[i], sizeof(qm->name));
> +	} else if (ctrl->type == V4L2_CTRL_TYPE_INTEGER_MENU) {
> +		qm->value = ctrl->qmenu_int[i];
> +	}
>  	return 0;
>  }
>  EXPORT_SYMBOL(v4l2_querymenu);

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH 1/3] v4l: Introduce integer menu controls

[Sakari Ailus]

On Fri, Nov 25, 2011 at 11:28:46AM +0100, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thanks for the patch.

Hi Laurent,

Thanks for the comments!

> On Thursday 24 November 2011 17:12:50 Sakari Ailus wrote:
...
> > @@ -1440,12 +1458,13 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct
> > v4l2_ctrl_handler *hdl, u32 flags;
> > 
> >  	v4l2_ctrl_fill(id, &name, &type, &min, &max, &step, &def, &flags);
> > -	if (type != V4L2_CTRL_TYPE_MENU) {
> > +	if (type != V4L2_CTRL_TYPE_MENU
> > +	    && type != V4L2_CTRL_TYPE_INTEGER_MENU) {
> >  		handler_set_err(hdl, -EINVAL);
> >  		return NULL;
> >  	}
> >  	return v4l2_ctrl_new(hdl, ops, id, name, type,
> > -				    0, max, mask, def, flags, qmenu, NULL);
> > +			     0, max, mask, def, flags, qmenu, NULL, NULL);
> 
> You pass NULL to the v4l2_ctrl_new() qmenu_int argument, which will make the 
> function fail for integer menu controls. Do you expect standard integer menu 
> controls to share a list of values ? If not, what about modifying 
> v4l2_ctrl_new_std_menu() to take a list of values (or alternatively forbidding 
> the function from being used for integer menu controls) ?

We currently have no integer menu controls, let alone one which would have a
set of standard values. We need same functionality as in
v4l2_ctrl_get_menu() for integer menus when we add the first standardised
integer menu control. I think it could be added at that time, or I could
implement a v4l2_ctrl_get_integer_menu() which would do nothing.

What do you think?

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	jabber/XMPP/Gmail: sailus@retiisi.org.uk

Re: [RFC/PATCH 1/3] v4l: Introduce integer menu controls

[Laurent Pinchart]

Hi Sakari,

On Friday 25 November 2011 13:02:02 Sakari Ailus wrote:
> On Fri, Nov 25, 2011 at 11:28:46AM +0100, Laurent Pinchart wrote:
> > On Thursday 24 November 2011 17:12:50 Sakari Ailus wrote:
> ...
> 
> > > @@ -1440,12 +1458,13 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct
> > > v4l2_ctrl_handler *hdl, u32 flags;
> > > 
> > >  	v4l2_ctrl_fill(id, &name, &type, &min, &max, &step, &def, &flags);
> > > 
> > > -	if (type != V4L2_CTRL_TYPE_MENU) {
> > > +	if (type != V4L2_CTRL_TYPE_MENU
> > > +	    && type != V4L2_CTRL_TYPE_INTEGER_MENU) {
> > > 
> > >  		handler_set_err(hdl, -EINVAL);
> > >  		return NULL;
> > >  	
> > >  	}
> > >  	return v4l2_ctrl_new(hdl, ops, id, name, type,
> > > 
> > > -				    0, max, mask, def, flags, qmenu, NULL);
> > > +			     0, max, mask, def, flags, qmenu, NULL, NULL);
> > 
> > You pass NULL to the v4l2_ctrl_new() qmenu_int argument, which will make
> > the function fail for integer menu controls. Do you expect standard
> > integer menu controls to share a list of values ? If not, what about
> > modifying v4l2_ctrl_new_std_menu() to take a list of values (or
> > alternatively forbidding the function from being used for integer menu
> > controls) ?
> 
> We currently have no integer menu controls, let alone one which would have
> a set of standard values. We need same functionality as in
> v4l2_ctrl_get_menu() for integer menus when we add the first standardised
> integer menu control. I think it could be added at that time, or I could
> implement a v4l2_ctrl_get_integer_menu() which would do nothing.
> 
> What do you think?

I was just wondering if we will ever have a standard menu control with 
standard integer items. If that never happens, v4l2_ctrl_new_std_menu() needs 
to either take a qmenu_int array, or reject integer menu controls completely. 
I would thus delay adding the V4L2_CTRL_TYPE_INTEGER_MENU check to the 
function as it wouldn't work anyway (or, alternatively, we would add the 
qmenu_int argument now).

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH 1/3] v4l: Introduce integer menu controls

[Sakari Ailus]

On Fri, Nov 25, 2011 at 01:43:12PM +0100, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Friday 25 November 2011 13:02:02 Sakari Ailus wrote:
> > On Fri, Nov 25, 2011 at 11:28:46AM +0100, Laurent Pinchart wrote:
> > > On Thursday 24 November 2011 17:12:50 Sakari Ailus wrote:
> > ...
> > 
> > > > @@ -1440,12 +1458,13 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct
> > > > v4l2_ctrl_handler *hdl, u32 flags;
> > > > 
> > > >  	v4l2_ctrl_fill(id, &name, &type, &min, &max, &step, &def, &flags);
> > > > 
> > > > -	if (type != V4L2_CTRL_TYPE_MENU) {
> > > > +	if (type != V4L2_CTRL_TYPE_MENU
> > > > +	    && type != V4L2_CTRL_TYPE_INTEGER_MENU) {
> > > > 
> > > >  		handler_set_err(hdl, -EINVAL);
> > > >  		return NULL;
> > > >  	
> > > >  	}
> > > >  	return v4l2_ctrl_new(hdl, ops, id, name, type,
> > > > 
> > > > -				    0, max, mask, def, flags, qmenu, NULL);
> > > > +			     0, max, mask, def, flags, qmenu, NULL, NULL);
> > > 
> > > You pass NULL to the v4l2_ctrl_new() qmenu_int argument, which will make
> > > the function fail for integer menu controls. Do you expect standard
> > > integer menu controls to share a list of values ? If not, what about
> > > modifying v4l2_ctrl_new_std_menu() to take a list of values (or
> > > alternatively forbidding the function from being used for integer menu
> > > controls) ?
> > 
> > We currently have no integer menu controls, let alone one which would have
> > a set of standard values. We need same functionality as in
> > v4l2_ctrl_get_menu() for integer menus when we add the first standardised
> > integer menu control. I think it could be added at that time, or I could
> > implement a v4l2_ctrl_get_integer_menu() which would do nothing.
> > 
> > What do you think?
> 
> I was just wondering if we will ever have a standard menu control with 
> standard integer items. If that never happens, v4l2_ctrl_new_std_menu() needs 
> to either take a qmenu_int array, or reject integer menu controls completely. 
> I would thus delay adding the V4L2_CTRL_TYPE_INTEGER_MENU check to the 
> function as it wouldn't work anyway (or, alternatively, we would add the 
> qmenu_int argument now).

Either one, yes. I think I'll add a separate patch adding standard integer
menus and remove the check from this one.

There'll definitely be a need for them. For example, there are bit rate
menus in the standard menu type controls that ideally should be integers. We
won't change them but there will be others. Or I'd be very surprised if
there were not!

Cheers,

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	jabber/XMPP/Gmail: sailus@retiisi.org.uk

Re: [RFC/PATCH 1/3] v4l: Introduce integer menu controls

[Laurent Pinchart]

Hi Sakari,

On Friday 25 November 2011 13:56:50 Sakari Ailus wrote:
> On Fri, Nov 25, 2011 at 01:43:12PM +0100, Laurent Pinchart wrote:
> > On Friday 25 November 2011 13:02:02 Sakari Ailus wrote:
> > > On Fri, Nov 25, 2011 at 11:28:46AM +0100, Laurent Pinchart wrote:
> > > > On Thursday 24 November 2011 17:12:50 Sakari Ailus wrote:
> > > ...
> > > 
> > > > > @@ -1440,12 +1458,13 @@ struct v4l2_ctrl
> > > > > *v4l2_ctrl_new_std_menu(struct v4l2_ctrl_handler *hdl, u32 flags;
> > > > > 
> > > > >  	v4l2_ctrl_fill(id, &name, &type, &min, &max, &step, &def,
> > > > >  	&flags);
> > > > > 
> > > > > -	if (type != V4L2_CTRL_TYPE_MENU) {
> > > > > +	if (type != V4L2_CTRL_TYPE_MENU
> > > > > +	    && type != V4L2_CTRL_TYPE_INTEGER_MENU) {
> > > > > 
> > > > >  		handler_set_err(hdl, -EINVAL);
> > > > >  		return NULL;
> > > > >  	
> > > > >  	}
> > > > >  	return v4l2_ctrl_new(hdl, ops, id, name, type,
> > > > > 
> > > > > -				    0, max, mask, def, flags, qmenu, NULL);
> > > > > +			     0, max, mask, def, flags, qmenu, NULL, NULL);
> > > > 
> > > > You pass NULL to the v4l2_ctrl_new() qmenu_int argument, which will
> > > > make the function fail for integer menu controls. Do you expect
> > > > standard integer menu controls to share a list of values ? If not,
> > > > what about modifying v4l2_ctrl_new_std_menu() to take a list of
> > > > values (or alternatively forbidding the function from being used for
> > > > integer menu controls) ?
> > > 
> > > We currently have no integer menu controls, let alone one which would
> > > have a set of standard values. We need same functionality as in
> > > v4l2_ctrl_get_menu() for integer menus when we add the first
> > > standardised integer menu control. I think it could be added at that
> > > time, or I could implement a v4l2_ctrl_get_integer_menu() which would
> > > do nothing.
> > > 
> > > What do you think?
> > 
> > I was just wondering if we will ever have a standard menu control with
> > standard integer items. If that never happens, v4l2_ctrl_new_std_menu()
> > needs to either take a qmenu_int array, or reject integer menu controls
> > completely. I would thus delay adding the V4L2_CTRL_TYPE_INTEGER_MENU
> > check to the function as it wouldn't work anyway (or, alternatively, we
> > would add the qmenu_int argument now).
> 
> Either one, yes. I think I'll add a separate patch adding standard integer
> menus and remove the check from this one.
> 
> There'll definitely be a need for them. For example, there are bit rate
> menus in the standard menu type controls that ideally should be integers.

Sure, but I doubt that the bit rates themselves will be standard.

> We won't change them but there will be others. Or I'd be very surprised if
> there were not!

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH 1/3] v4l: Introduce integer menu controls

[Hans Verkuil]

On Friday 25 November 2011 13:58:27 Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Friday 25 November 2011 13:56:50 Sakari Ailus wrote:
> > On Fri, Nov 25, 2011 at 01:43:12PM +0100, Laurent Pinchart wrote:
> > > On Friday 25 November 2011 13:02:02 Sakari Ailus wrote:
> > > > On Fri, Nov 25, 2011 at 11:28:46AM +0100, Laurent Pinchart wrote:
> > > > > On Thursday 24 November 2011 17:12:50 Sakari Ailus wrote:
> > > > ...
> > > > 
> > > > > > @@ -1440,12 +1458,13 @@ struct v4l2_ctrl
> > > > > > *v4l2_ctrl_new_std_menu(struct v4l2_ctrl_handler *hdl, u32 flags;
> > > > > > 
> > > > > >  	v4l2_ctrl_fill(id, &name, &type, &min, &max, &step, &def,
> > > > > >  	&flags);
> > > > > > 
> > > > > > -	if (type != V4L2_CTRL_TYPE_MENU) {
> > > > > > +	if (type != V4L2_CTRL_TYPE_MENU
> > > > > > +	    && type != V4L2_CTRL_TYPE_INTEGER_MENU) {
> > > > > > 
> > > > > >  		handler_set_err(hdl, -EINVAL);
> > > > > >  		return NULL;
> > > > > >  	
> > > > > >  	}
> > > > > >  	return v4l2_ctrl_new(hdl, ops, id, name, type,
> > > > > > 
> > > > > > -				    0, max, mask, def, flags, qmenu, 
NULL);
> > > > > > +			     0, max, mask, def, flags, qmenu, NULL, 
NULL);
> > > > > 
> > > > > You pass NULL to the v4l2_ctrl_new() qmenu_int argument, which will
> > > > > make the function fail for integer menu controls. Do you expect
> > > > > standard integer menu controls to share a list of values ? If not,
> > > > > what about modifying v4l2_ctrl_new_std_menu() to take a list of
> > > > > values (or alternatively forbidding the function from being used
> > > > > for integer menu controls) ?
> > > > 
> > > > We currently have no integer menu controls, let alone one which would
> > > > have a set of standard values. We need same functionality as in
> > > > v4l2_ctrl_get_menu() for integer menus when we add the first
> > > > standardised integer menu control. I think it could be added at that
> > > > time, or I could implement a v4l2_ctrl_get_integer_menu() which would
> > > > do nothing.
> > > > 
> > > > What do you think?
> > > 
> > > I was just wondering if we will ever have a standard menu control with
> > > standard integer items. If that never happens, v4l2_ctrl_new_std_menu()
> > > needs to either take a qmenu_int array, or reject integer menu controls
> > > completely. I would thus delay adding the V4L2_CTRL_TYPE_INTEGER_MENU
> > > check to the function as it wouldn't work anyway (or, alternatively, we
> > > would add the qmenu_int argument now).
> > 
> > Either one, yes. I think I'll add a separate patch adding standard
> > integer menus and remove the check from this one.
> > 
> > There'll definitely be a need for them. For example, there are bit rate
> > menus in the standard menu type controls that ideally should be integers.
> 
> Sure, but I doubt that the bit rates themselves will be standard.

Actually, they are. MPEG audio level 1, 2, 3 and AC3 audio all have their own
standardized set of possible bitrates. If I had an integer menu at the time
I'm sure I would have used it.

Regards,

	Hans

> 
> > We won't change them but there will be others. Or I'd be very surprised
> > if there were not!

[RFC/PATCH 2/3] v4l: Document integer menu controls

[Sakari Ailus]

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 Documentation/DocBook/media/v4l/compat.xml         |   10 +++++
 Documentation/DocBook/media/v4l/v4l2.xml           |    7 ++++
 .../DocBook/media/v4l/vidioc-queryctrl.xml         |   39 +++++++++++++++++++-
 3 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml
index b68698f..569efd1 100644
--- a/Documentation/DocBook/media/v4l/compat.xml
+++ b/Documentation/DocBook/media/v4l/compat.xml
@@ -2379,6 +2379,16 @@ that used it. It was originally scheduled for removal in 2.6.35.
       </orderedlist>
     </section>
 
+    <section>
+      <title>V4L2 in Linux 3.3</title>
+      <orderedlist>
+        <listitem>
+	  <para>Added integer menus, the new type will be
+	  V4L2_CTRL_TYPE_INTEGER_MENU.</para>
+        </listitem>
+      </orderedlist>
+    </section>
+
     <section id="other">
       <title>Relation of V4L2 to other Linux multimedia APIs</title>
 
diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
index 2ab365c..affe1ba 100644
--- a/Documentation/DocBook/media/v4l/v4l2.xml
+++ b/Documentation/DocBook/media/v4l/v4l2.xml
@@ -128,6 +128,13 @@ structs, ioctls) must be noted in more detail in the history chapter
 applications. -->
 
       <revision>
+	<revnumber>3.3</revnumber>
+	<date>2011-11-24</date>
+	<authorinitials>sa</authorinitials>
+	<revremark>Added V4L2_CTRL_TYPE_INTEGER_MENU.</revremark>
+      </revision>
+
+      <revision>
 	<revnumber>3.2</revnumber>
 	<date>2011-08-26</date>
 	<authorinitials>hv</authorinitials>
diff --git a/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml b/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml
index 0ac0057..049cd46 100644
--- a/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml
@@ -215,11 +215,12 @@ the array to zero.</entry>
 
     <table pgwide="1" frame="none" id="v4l2-querymenu">
       <title>struct <structname>v4l2_querymenu</structname></title>
-      <tgroup cols="3">
+      <tgroup cols="4">
 	&cs-str;
 	<tbody valign="top">
 	  <row>
 	    <entry>__u32</entry>
+	    <entry></entry>
 	    <entry><structfield>id</structfield></entry>
 	    <entry>Identifies the control, set by the application
 from the respective &v4l2-queryctrl;
@@ -227,18 +228,38 @@ from the respective &v4l2-queryctrl;
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
+	    <entry></entry>
 	    <entry><structfield>index</structfield></entry>
 	    <entry>Index of the menu item, starting at zero, set by
 	    the application.</entry>
 	  </row>
 	  <row>
+	    <entry>union</entry>
+	    <entry></entry>
+	    <entry></entry>
+	    <entry></entry>
+	  </row>
+	  <row>
+	    <entry></entry>
 	    <entry>__u8</entry>
 	    <entry><structfield>name</structfield>[32]</entry>
 	    <entry>Name of the menu item, a NUL-terminated ASCII
-string. This information is intended for the user.</entry>
+string. This information is intended for the user. This field is valid
+for <constant>V4L2_CTRL_FLAG_MENU</constant> type controls.</entry>
+	  </row>
+	  <row>
+	    <entry></entry>
+	    <entry>__s64</entry>
+	    <entry><structfield>value</structfield></entry>
+	    <entry>
+              Value of the integer menu item. This field is valid for
+              <constant>V4L2_CTRL_FLAG_INTEGER_MENU</constant> type
+              controls.
+            </entry>
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
+	    <entry></entry>
 	    <entry><structfield>reserved</structfield></entry>
 	    <entry>Reserved for future extensions. Drivers must set
 the array to zero.</entry>
@@ -292,6 +313,20 @@ the menu items can be enumerated with the
 <constant>VIDIOC_QUERYMENU</constant> ioctl.</entry>
 	  </row>
 	  <row>
+	    <entry><constant>V4L2_CTRL_TYPE_INTEGER_MENU</constant></entry>
+	    <entry>&ge; 0</entry>
+	    <entry>1</entry>
+	    <entry>N-1</entry>
+	    <entry>
+              The control has a menu of N choices. The names of the
+              menu items can be enumerated with the
+              <constant>VIDIOC_QUERYMENU</constant> ioctl. This is
+              similar to <constant>V4L2_CTRL_TYPE_MENU</constant>
+              except that instead of integers, the menu items are
+              signed 64-bit integers.
+            </entry>
+	  </row>
+	  <row>
 	    <entry><constant>V4L2_CTRL_TYPE_BITMASK</constant></entry>
 	    <entry>0</entry>
 	    <entry>n/a</entry>
-- 
1.7.2.5

Re: [RFC/PATCH 2/3] v4l: Document integer menu controls

[Sylwester Nawrocki]

On 11/24/2011 05:12 PM, Sakari Ailus wrote:
> Signed-off-by: Sakari Ailus<sakari.ailus@iki.fi>
> ---
>   Documentation/DocBook/media/v4l/compat.xml         |   10 +++++
>   Documentation/DocBook/media/v4l/v4l2.xml           |    7 ++++
>   .../DocBook/media/v4l/vidioc-queryctrl.xml         |   39 +++++++++++++++++++-
>   3 files changed, 54 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml
> index b68698f..569efd1 100644
> --- a/Documentation/DocBook/media/v4l/compat.xml
> +++ b/Documentation/DocBook/media/v4l/compat.xml
> @@ -2379,6 +2379,16 @@ that used it. It was originally scheduled for removal in 2.6.35.
>         </orderedlist>
>       </section>
> 
> +<section>
> +<title>V4L2 in Linux 3.3</title>
> +<orderedlist>
> +<listitem>
> +	<para>Added integer menus, the new type will be
> +	  V4L2_CTRL_TYPE_INTEGER_MENU.</para>
> +</listitem>
> +</orderedlist>
> +</section>
> +
>       <section id="other">
>         <title>Relation of V4L2 to other Linux multimedia APIs</title>
> 
> diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
> index 2ab365c..affe1ba 100644
> --- a/Documentation/DocBook/media/v4l/v4l2.xml
> +++ b/Documentation/DocBook/media/v4l/v4l2.xml
> @@ -128,6 +128,13 @@ structs, ioctls) must be noted in more detail in the history chapter
>   applications. -->
> 
>         <revision>
> +	<revnumber>3.3</revnumber>
> +	<date>2011-11-24</date>
> +	<authorinitials>sa</authorinitials>
> +	<revremark>Added V4L2_CTRL_TYPE_INTEGER_MENU.</revremark>
> +</revision>
> +
> +<revision>
>   	<revnumber>3.2</revnumber>
>   	<date>2011-08-26</date>
>   	<authorinitials>hv</authorinitials>
> diff --git a/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml b/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml
> index 0ac0057..049cd46 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml
> @@ -215,11 +215,12 @@ the array to zero.</entry>
> 
>       <table pgwide="1" frame="none" id="v4l2-querymenu">
>         <title>struct<structname>v4l2_querymenu</structname></title>
> -<tgroup cols="3">
> +<tgroup cols="4">
>   	&cs-str;
>   	<tbody valign="top">
>   	<row>
>   	<entry>__u32</entry>
> +	<entry></entry>
>   	<entry><structfield>id</structfield></entry>
>   	<entry>Identifies the control, set by the application
>   from the respective&v4l2-queryctrl;
> @@ -227,18 +228,38 @@ from the respective&v4l2-queryctrl;
>   	</row>
>   	<row>
>   	<entry>__u32</entry>
> +	<entry></entry>
>   	<entry><structfield>index</structfield></entry>
>   	<entry>Index of the menu item, starting at zero, set by
>   	    the application.</entry>
>   	</row>
>   	<row>
> +	<entry>union</entry>
> +	<entry></entry>
> +	<entry></entry>
> +	<entry></entry>
> +	</row>
> +	<row>
> +	<entry></entry>
>   	<entry>__u8</entry>
>   	<entry><structfield>name</structfield>[32]</entry>
>   	<entry>Name of the menu item, a NUL-terminated ASCII
> -string. This information is intended for the user.</entry>
> +string. This information is intended for the user. This field is valid
> +for<constant>V4L2_CTRL_FLAG_MENU</constant>  type controls.</entry>
> +	</row>
> +	<row>
> +	<entry></entry>
> +	<entry>__s64</entry>
> +	<entry><structfield>value</structfield></entry>
> +	<entry>
> +              Value of the integer menu item. This field is valid for
> +<constant>V4L2_CTRL_FLAG_INTEGER_MENU</constant>  type
> +              controls.
> +</entry>
>   	</row>
>   	<row>
>   	<entry>__u32</entry>
> +	<entry></entry>
>   	<entry><structfield>reserved</structfield></entry>
>   	<entry>Reserved for future extensions. Drivers must set
>   the array to zero.</entry>
> @@ -292,6 +313,20 @@ the menu items can be enumerated with the
>   <constant>VIDIOC_QUERYMENU</constant>  ioctl.</entry>
>   	</row>
>   	<row>
> +	<entry><constant>V4L2_CTRL_TYPE_INTEGER_MENU</constant></entry>
> +	<entry>&ge; 0</entry>
> +	<entry>1</entry>
> +	<entry>N-1</entry>
> +	<entry>
> +              The control has a menu of N choices. The names of the
> +              menu items can be enumerated with the

Shouldn't it be something along the lines of "The integer values of
the menu items can be enumerated..." ?

> +<constant>VIDIOC_QUERYMENU</constant>  ioctl. This is
> +              similar to<constant>V4L2_CTRL_TYPE_MENU</constant>
> +              except that instead of integers, the menu items are
> +              signed 64-bit integers.
> +</entry>
> +	</row>
> +	<row>
>   	<entry><constant>V4L2_CTRL_TYPE_BITMASK</constant></entry>
>   	<entry>0</entry>
>   	<entry>n/a</entry>


-- 
Regards,
Sylwester

Re: [RFC/PATCH 2/3] v4l: Document integer menu controls

[Sakari Ailus]

Hi Sylwester,

On Fri, Nov 25, 2011 at 12:17:58AM +0100, Sylwester Nawrocki wrote:
...
> > @@ -292,6 +313,20 @@ the menu items can be enumerated with the
> >   <constant>VIDIOC_QUERYMENU</constant>  ioctl.</entry>
> >   	</row>
> >   	<row>
> > +	<entry><constant>V4L2_CTRL_TYPE_INTEGER_MENU</constant></entry>
> > +	<entry>&ge; 0</entry>
> > +	<entry>1</entry>
> > +	<entry>N-1</entry>
> > +	<entry>
> > +              The control has a menu of N choices. The names of the
> > +              menu items can be enumerated with the
> 
> Shouldn't it be something along the lines of "The integer values of
> the menu items can be enumerated..." ?

Right. This is left from the original menu control definition I copied. I'll
fix this in the next version of the patchset.

Cheers,

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	jabber/XMPP/Gmail: sailus@retiisi.org.uk

Re: [RFC/PATCH 2/3] v4l: Document integer menu controls

[Laurent Pinchart]

Hi Sakari,

Thanks for the patch.

On Thursday 24 November 2011 17:12:51 Sakari Ailus wrote:
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> ---
>  Documentation/DocBook/media/v4l/compat.xml         |   10 +++++
>  Documentation/DocBook/media/v4l/v4l2.xml           |    7 ++++
>  .../DocBook/media/v4l/vidioc-queryctrl.xml         |   39
> +++++++++++++++++++- 3 files changed, 54 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/compat.xml
> b/Documentation/DocBook/media/v4l/compat.xml index b68698f..569efd1 100644
> --- a/Documentation/DocBook/media/v4l/compat.xml
> +++ b/Documentation/DocBook/media/v4l/compat.xml
> @@ -2379,6 +2379,16 @@ that used it. It was originally scheduled for
> removal in 2.6.35. </orderedlist>
>      </section>
> 
> +    <section>
> +      <title>V4L2 in Linux 3.3</title>
> +      <orderedlist>
> +        <listitem>
> +	  <para>Added integer menus, the new type will be
> +	  V4L2_CTRL_TYPE_INTEGER_MENU.</para>
> +        </listitem>
> +      </orderedlist>
> +    </section>
> +
>      <section id="other">
>        <title>Relation of V4L2 to other Linux multimedia APIs</title>
> 
> diff --git a/Documentation/DocBook/media/v4l/v4l2.xml
> b/Documentation/DocBook/media/v4l/v4l2.xml index 2ab365c..affe1ba 100644
> --- a/Documentation/DocBook/media/v4l/v4l2.xml
> +++ b/Documentation/DocBook/media/v4l/v4l2.xml
> @@ -128,6 +128,13 @@ structs, ioctls) must be noted in more detail in the
> history chapter applications. -->
> 
>        <revision>
> +	<revnumber>3.3</revnumber>
> +	<date>2011-11-24</date>
> +	<authorinitials>sa</authorinitials>
> +	<revremark>Added V4L2_CTRL_TYPE_INTEGER_MENU.</revremark>
> +      </revision>
> +
> +      <revision>
>  	<revnumber>3.2</revnumber>
>  	<date>2011-08-26</date>
>  	<authorinitials>hv</authorinitials>
> diff --git a/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml
> b/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml index
> 0ac0057..049cd46 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-queryctrl.xml
> @@ -215,11 +215,12 @@ the array to zero.</entry>
> 
>      <table pgwide="1" frame="none" id="v4l2-querymenu">
>        <title>struct <structname>v4l2_querymenu</structname></title>
> -      <tgroup cols="3">
> +      <tgroup cols="4">
>  	&cs-str;
>  	<tbody valign="top">
>  	  <row>
>  	    <entry>__u32</entry>
> +	    <entry></entry>
>  	    <entry><structfield>id</structfield></entry>
>  	    <entry>Identifies the control, set by the application
>  from the respective &v4l2-queryctrl;
> @@ -227,18 +228,38 @@ from the respective &v4l2-queryctrl;
>  	  </row>
>  	  <row>
>  	    <entry>__u32</entry>
> +	    <entry></entry>
>  	    <entry><structfield>index</structfield></entry>
>  	    <entry>Index of the menu item, starting at zero, set by
>  	    the application.</entry>
>  	  </row>
>  	  <row>
> +	    <entry>union</entry>
> +	    <entry></entry>
> +	    <entry></entry>
> +	    <entry></entry>
> +	  </row>
> +	  <row>
> +	    <entry></entry>
>  	    <entry>__u8</entry>
>  	    <entry><structfield>name</structfield>[32]</entry>
>  	    <entry>Name of the menu item, a NUL-terminated ASCII
> -string. This information is intended for the user.</entry>
> +string. This information is intended for the user. This field is valid
> +for <constant>V4L2_CTRL_FLAG_MENU</constant> type controls.</entry>
> +	  </row>
> +	  <row>
> +	    <entry></entry>
> +	    <entry>__s64</entry>
> +	    <entry><structfield>value</structfield></entry>
> +	    <entry>
> +              Value of the integer menu item. This field is valid for
> +              <constant>V4L2_CTRL_FLAG_INTEGER_MENU</constant> type
> +              controls.
> +            </entry>
>  	  </row>
>  	  <row>
>  	    <entry>__u32</entry>
> +	    <entry></entry>
>  	    <entry><structfield>reserved</structfield></entry>
>  	    <entry>Reserved for future extensions. Drivers must set
>  the array to zero.</entry>
> @@ -292,6 +313,20 @@ the menu items can be enumerated with the
>  <constant>VIDIOC_QUERYMENU</constant> ioctl.</entry>
>  	  </row>
>  	  <row>
> +	    <entry><constant>V4L2_CTRL_TYPE_INTEGER_MENU</constant></entry>
> +	    <entry>&ge; 0</entry>
> +	    <entry>1</entry>
> +	    <entry>N-1</entry>
> +	    <entry>
> +              The control has a menu of N choices. The names of the
> +              menu items can be enumerated with the
> +              <constant>VIDIOC_QUERYMENU</constant> ioctl. This is
> +              similar to <constant>V4L2_CTRL_TYPE_MENU</constant>
> +              except that instead of integers, the menu items are

Do you mean "instead of strings" ?

> +              signed 64-bit integers.
> +            </entry>
> +	  </row>
> +	  <row>
>  	    <entry><constant>V4L2_CTRL_TYPE_BITMASK</constant></entry>
>  	    <entry>0</entry>
>  	    <entry>n/a</entry>

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH 2/3] v4l: Document integer menu controls

[Sakari Ailus]

On Fri, Nov 25, 2011 at 11:30:32AM +0100, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thanks for the patch.

Hi Laurent,

Thanks for the comments!

> On Thursday 24 November 2011 17:12:51 Sakari Ailus wrote:
...
> > @@ -292,6 +313,20 @@ the menu items can be enumerated with the
> >  <constant>VIDIOC_QUERYMENU</constant> ioctl.</entry>
> >  	  </row>
> >  	  <row>
> > +	    <entry><constant>V4L2_CTRL_TYPE_INTEGER_MENU</constant></entry>
> > +	    <entry>&ge; 0</entry>
> > +	    <entry>1</entry>
> > +	    <entry>N-1</entry>
> > +	    <entry>
> > +              The control has a menu of N choices. The names of the
> > +              menu items can be enumerated with the
> > +              <constant>VIDIOC_QUERYMENU</constant> ioctl. This is
> > +              similar to <constant>V4L2_CTRL_TYPE_MENU</constant>
> > +              except that instead of integers, the menu items are
> 
> Do you mean "instead of strings" ?

Yes, I do. Will fix this.

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	jabber/XMPP/Gmail: sailus@retiisi.org.uk

[RFC/PATCH 3/3] vivi: Add an integer menu test control

[Sakari Ailus]

Add an integer menu test control for the vivi driver.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 drivers/media/video/vivi.c |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/vivi.c b/drivers/media/video/vivi.c
index 7d754fb..763ec23 100644
--- a/drivers/media/video/vivi.c
+++ b/drivers/media/video/vivi.c
@@ -177,6 +177,7 @@ struct vivi_dev {
 	struct v4l2_ctrl	   *menu;
 	struct v4l2_ctrl	   *string;
 	struct v4l2_ctrl	   *bitmask;
+	struct v4l2_ctrl	   *int_menu;
 
 	spinlock_t                 slock;
 	struct mutex		   mutex;
@@ -503,6 +504,10 @@ static void vivi_fillbuff(struct vivi_dev *dev, struct vivi_buffer *buf)
 			dev->boolean->cur.val,
 			dev->menu->qmenu[dev->menu->cur.val],
 			dev->string->cur.string);
+	snprintf(str, sizeof(str), " integer_menu %s, value %lld ",
+			dev->int_menu->qmenu[dev->int_menu->cur.val],
+			dev->int64->cur.val64);
+	gen_text(dev, vbuf, line++ * 16, 16, str);
 	mutex_unlock(&dev->ctrl_handler.lock);
 	gen_text(dev, vbuf, line++ * 16, 16, str);
 	if (dev->button_pressed) {
@@ -1183,6 +1188,22 @@ static const struct v4l2_ctrl_config vivi_ctrl_bitmask = {
 	.step = 0,
 };
 
+static const s64 * const vivi_ctrl_int_menu_values[] = {
+	1, 1, 2, 3, 5, 8, 13, 21, 42,
+};
+
+static const struct v4l2_ctrl_config vivi_ctrl_string = {
+	.ops = &vivi_ctrl_ops,
+	.id = VIDI_CID_CUSTOM_BASE + 7
+	.name = "Integer menu",
+	.type = V4L2_CTRL_TYPE_INTEGER_MENU,
+	.min = 1,
+	.max = 8,
+	.def = 4,
+	.menu_skip_mask = 0x02,
+	.qmenu_int = &vivi_ctrl_int_menu_values,
+};
+
 static const struct v4l2_file_operations vivi_fops = {
 	.owner		= THIS_MODULE,
 	.open           = v4l2_fh_open,
-- 
1.7.2.5

Re: [RFC/PATCH 0/3] 64-bit integer menus

[Sylwester Nawrocki]

Hi Sakari,

thanks for the patches.

On 11/24/2011 05:12 PM, Sakari Ailus wrote:
> Hi all,
> 
> This patchset, which I'm sending as RFC since it has not been really tested
> (including compiling the vivi patch), adds 64-bit integer menu controls. The
> control items in the integer menu are just like in regular menus but they
> are 64-bit integers instead of strings.
> 
> I'm also pondering whether to assign 1 to ctrl->step for menu type controls
> as well but haven't checked what may have been the original reason to
> implement it as it is now implemented.
> 
> The reason why I don't use a union for qmenu and qmenu_int in
> v4l2_ctrl_config is that a lot of drivers use that field in the initialiser
> and GCC<  4.6 does not support initialisers with anonymous unions.
> 
> Similar union is created in v4l2_querymenu but I do not see this as a
> problem since I do not expect initialisers to be used with this field in the
> user space code.
> 
> Comments and questions are welcome.

I've gone briefly through the patches and they seem to realize exactly what
I needed. I think we've discussed the integer menu controls during the Cambourne 
meeting, however I wasn't sure yesterday if it was just this.

I'll try and implement some of the controls for m5mols based on your patches.
Cannot guarantee I'll manage to have something ready for 3.3, I need to finish 
a few other things before I get to this. 

-- 
Thanks,
Sylwester