[PATCH 0/8] Support getting default values from any control

[PATCH 0/8] Support getting default values from any control

[Ricardo Ribalda Delgado]

Integer controls provide a way to get their default/initial value, but
any other control (p_u32, p_u8.....) provide no other way to get the
initial value than unloading the module and loading it back.

*What is the actual problem?
I have a custom control with WIDTH integer values. Every value
represents the calibrated FPN (fixed pattern noise) correction value for that
column
-Application A changes the FPN correction value
-Application B wants to restore the calibrated value but it cant :(

*What is the proposed solution?

(Kudos to Hans Verkuil!!!)

The key change is in struct v4l2_ext_controls where the __u32 ctrl_class field
is changed to:

        union {
                __u32 ctrl_class;
                __u32 which;
        };

And two new defines are added:

#define V4L2_CTRL_WHICH_CUR_VAL        0
#define V4L2_CTRL_WHICH_DEF_VAL        0x0f000000

The 'which' field tells you which controls are get/set/tried.

V4L2_CTRL_WHICH_CUR_VAL: the current value of the controls
V4L2_CTRL_WHICH_DEF_VAL: the default value of the controls
V4L2_CTRL_CLASS_*: the current value of the controls belonging to the specified class.
        Note: this is deprecated usage and is only there for backwards compatibility.
        Which is also why I don't think there is a need to add V4L2_CTRL_WHICH_
        aliases for these defines.


I have posted a copy of my working tree to

https://github.com/ribalda/linux/tree/which_def

Changelog v1 (compared to v5 of New ioct VIDIOC_G_DEF_EXT_CTRLS):

Suggested by Hans Verkuil <hverkuil@xs4all.nl>

Replace ioctl implementation with a new union on the struct v4l2_ext_controls
THANKS!

Ricardo Ribalda Delgado (8):
  videodev2.h: Fix typo in comment
  videodev2.h: Extend struct v4l2_ext_controls
  media/v4l2-core: struct struct v4l2_ext_controls param which
  usb/uvc: Support for V4L2_CTRL_WHICH_DEF_VAL
  media/usb/pvrusb2: Support for V4L2_CTRL_WHICH_DEF_VAL
  media/pci/saa7164-encoder Support for V4L2_CTRL_WHICH_DEF_VAL
  media/pci/saa7164-vbi Support for V4L2_CTRL_WHICH_DEF_VAL
  Docbook: media: Document changes on struct v4l2_ext_controls

 Documentation/DocBook/media/v4l/v4l2.xml           |  9 ++++
 .../DocBook/media/v4l/vidioc-g-ext-ctrls.xml       | 14 ++++++
 drivers/media/pci/saa7164/saa7164-encoder.c        | 55 ++++++++++++---------
 drivers/media/pci/saa7164/saa7164-vbi.c            | 57 +++++++++++++---------
 drivers/media/usb/pvrusb2/pvrusb2-v4l2.c           | 17 ++++++-
 drivers/media/usb/uvc/uvc_v4l2.c                   | 14 +++++-
 drivers/media/v4l2-core/v4l2-ctrls.c               | 35 +++++++++++--
 include/uapi/linux/videodev2.h                     |  9 +++-
 8 files changed, 153 insertions(+), 57 deletions(-)

-- 
2.5.0

[PATCH 1/8] videodev2.h: Fix typo in comment

[Ricardo Ribalda Delgado]

Referenced file has moved

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 include/uapi/linux/videodev2.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 3228fbebcd63..72fa3e490e30 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -2271,7 +2271,7 @@ struct v4l2_create_buffers {
 #define VIDIOC_QUERY_EXT_CTRL	_IOWR('V', 103, struct v4l2_query_ext_ctrl)
 
 /* Reminder: when adding new ioctls please add support for them to
-   drivers/media/video/v4l2-compat-ioctl32.c as well! */
+   drivers/media/v4l2-core/v4l2-compat-ioctl32.c as well! */
 
 #define BASE_VIDIOC_PRIVATE	192		/* 192-255 are private */
 
-- 
2.5.0

[PATCH 2/8] videodev2.h: Extend struct v4l2_ext_controls

[Ricardo Ribalda Delgado]

So it can be used to get the default value of a control.

Without this change it is not possible to get  get the
default value of array controls.

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 include/uapi/linux/videodev2.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 72fa3e490e30..2e857b19a155 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1461,7 +1461,10 @@ struct v4l2_ext_control {
 } __attribute__ ((packed));
 
 struct v4l2_ext_controls {
-	__u32 ctrl_class;
+	union {
+		__u32 ctrl_class;
+		__u32 which;
+	};
 	__u32 count;
 	__u32 error_idx;
 	__u32 reserved[2];
@@ -1472,6 +1475,8 @@ struct v4l2_ext_controls {
 #define V4L2_CTRL_ID2CLASS(id)    ((id) & 0x0fff0000UL)
 #define V4L2_CTRL_DRIVER_PRIV(id) (((id) & 0xffff) >= 0x1000)
 #define V4L2_CTRL_MAX_DIMS	  (4)
+#define V4L2_CTRL_WHICH_CUR_VAL   0
+#define V4L2_CTRL_WHICH_DEF_VAL   0x0f000000
 
 enum v4l2_ctrl_type {
 	V4L2_CTRL_TYPE_INTEGER	     = 1,
-- 
2.5.0

[PATCH 3/8] media/v4l2-core: struct struct v4l2_ext_controls param which

[Ricardo Ribalda Delgado]

Support for new field which on v4l2_ext_controls, used to get the
default value of one or more controls.

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index b6b7dcc1b77d..23a69f637f6d 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1489,6 +1489,17 @@ static int new_to_user(struct v4l2_ext_control *c,
 	return ptr_to_user(c, ctrl, ctrl->p_new);
 }
 
+/* Helper function: copy the initial control value back to the caller */
+static int def_to_user(struct v4l2_ext_control *c, struct v4l2_ctrl *ctrl)
+{
+	int idx;
+
+	for (idx = 0; idx < ctrl->elems; idx++)
+		ctrl->type_ops->init(ctrl, idx, ctrl->p_new);
+
+	return ptr_to_user(c, ctrl, ctrl->p_new);
+}
+
 /* Helper function: copy the caller-provider value to the given control value */
 static int user_to_ptr(struct v4l2_ext_control *c,
 		       struct v4l2_ctrl *ctrl,
@@ -2708,7 +2719,9 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl,
 
 		cs->error_idx = i;
 
-		if (cs->ctrl_class && V4L2_CTRL_ID2CLASS(id) != cs->ctrl_class)
+		if (cs->ctrl_class &&
+		    cs->which != V4L2_CTRL_WHICH_DEF_VAL &&
+		    V4L2_CTRL_ID2CLASS(id) != cs->ctrl_class)
 			return -EINVAL;
 
 		/* Old-style private controls are not allowed for
@@ -2787,7 +2800,7 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl,
    whether there are any controls at all. */
 static int class_check(struct v4l2_ctrl_handler *hdl, u32 ctrl_class)
 {
-	if (ctrl_class == 0)
+	if (ctrl_class == 0 || ctrl_class == V4L2_CTRL_WHICH_DEF_VAL)
 		return list_empty(&hdl->ctrl_refs) ? -EINVAL : 0;
 	return find_ref_lock(hdl, ctrl_class | 1) ? 0 : -EINVAL;
 }
@@ -2801,10 +2814,14 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls *cs
 	struct v4l2_ctrl_helper *helpers = helper;
 	int ret;
 	int i, j;
+	bool def_value = false;
 
 	cs->error_idx = cs->count;
 	cs->ctrl_class = V4L2_CTRL_ID2CLASS(cs->ctrl_class);
 
+	if (cs->which == V4L2_CTRL_WHICH_DEF_VAL)
+		def_value = true;
+
 	if (hdl == NULL)
 		return -EINVAL;
 
@@ -2827,9 +2844,11 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls *cs
 
 	for (i = 0; !ret && i < cs->count; i++) {
 		int (*ctrl_to_user)(struct v4l2_ext_control *c,
-				    struct v4l2_ctrl *ctrl) = cur_to_user;
+				    struct v4l2_ctrl *ctrl);
 		struct v4l2_ctrl *master;
 
+		ctrl_to_user = def_value ? def_to_user : cur_to_user;
+
 		if (helpers[i].mref == NULL)
 			continue;
 
@@ -2839,8 +2858,9 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls *cs
 		v4l2_ctrl_lock(master);
 
 		/* g_volatile_ctrl will update the new control values */
-		if ((master->flags & V4L2_CTRL_FLAG_VOLATILE) ||
-			(master->has_volatiles && !is_cur_manual(master))) {
+		if (!def_value &&
+		    ((master->flags & V4L2_CTRL_FLAG_VOLATILE) ||
+		    (master->has_volatiles && !is_cur_manual(master)))) {
 			for (j = 0; j < master->ncontrols; j++)
 				cur_to_new(master->cluster[j]);
 			ret = call_op(master, g_volatile_ctrl);
@@ -3062,6 +3082,11 @@ static int try_set_ext_ctrls(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl,
 	int ret;
 
 	cs->error_idx = cs->count;
+
+	/* Default value cannot be changed */
+	if (cs->which == V4L2_CTRL_WHICH_DEF_VAL)
+		return -EINVAL;
+
 	cs->ctrl_class = V4L2_CTRL_ID2CLASS(cs->ctrl_class);
 
 	if (hdl == NULL)
-- 
2.5.0

[PATCH 4/8] usb/uvc: Support for V4L2_CTRL_WHICH_DEF_VAL

[Ricardo Ribalda Delgado]

This driver does not use the control infrastructure.
Add support for the new field which on structure
 v4l2_ext_controls

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/media/usb/uvc/uvc_v4l2.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 2764f43607c1..e6d3a1bcfa2f 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -980,6 +980,7 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh,
 	struct uvc_fh *handle = fh;
 	struct uvc_video_chain *chain = handle->chain;
 	struct v4l2_ext_control *ctrl = ctrls->controls;
+	struct v4l2_queryctrl qc;
 	unsigned int i;
 	int ret;
 
@@ -988,7 +989,14 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh,
 		return ret;
 
 	for (i = 0; i < ctrls->count; ++ctrl, ++i) {
-		ret = uvc_ctrl_get(chain, ctrl);
+		if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) {
+			qc.id = ctrl->id;
+			ret = uvc_query_v4l2_ctrl(chain, &qc);
+			if (!ret)
+				ctrl->value = qc.default_value;
+		} else
+			ret = uvc_ctrl_get(chain, ctrl);
+
 		if (ret < 0) {
 			uvc_ctrl_rollback(handle);
 			ctrls->error_idx = i;
@@ -1010,6 +1018,10 @@ static int uvc_ioctl_s_try_ext_ctrls(struct uvc_fh *handle,
 	unsigned int i;
 	int ret;
 
+	/* Default value cannot be changed */
+	if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL)
+		return -EINVAL;
+
 	ret = uvc_ctrl_begin(chain);
 	if (ret < 0)
 		return ret;
-- 
2.5.0

[PATCH 5/8] media/usb/pvrusb2: Support for V4L2_CTRL_WHICH_DEF_VAL

[Ricardo Ribalda Delgado]

This driver does not use the control infrastructure.
Add support for the new field which on structure
 v4l2_ext_controls

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
index 1c5f85bf7ed4..43b2f2214798 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
@@ -628,6 +628,7 @@ static int pvr2_g_ext_ctrls(struct file *file, void *priv,
 	struct pvr2_v4l2_fh *fh = file->private_data;
 	struct pvr2_hdw *hdw = fh->channel.mc_head->hdw;
 	struct v4l2_ext_control *ctrl;
+	struct pvr2_ctrl *cptr;
 	unsigned int idx;
 	int val;
 	int ret;
@@ -635,8 +636,18 @@ static int pvr2_g_ext_ctrls(struct file *file, void *priv,
 	ret = 0;
 	for (idx = 0; idx < ctls->count; idx++) {
 		ctrl = ctls->controls + idx;
-		ret = pvr2_ctrl_get_value(
+		if (ctls->which == V4L2_CTRL_WHICH_DEF_VAL) {
+			cptr = pvr2_hdw_get_ctrl_v4l(hdw, ctrl->id);
+			if (cptr)
+				pvr2_ctrl_get_def(cptr, &val);
+			else
+				ret = -EINVAL;
+
+
+		} else
+			ret = pvr2_ctrl_get_value(
 				pvr2_hdw_get_ctrl_v4l(hdw, ctrl->id), &val);
+
 		if (ret) {
 			ctls->error_idx = idx;
 			return ret;
@@ -658,6 +669,10 @@ static int pvr2_s_ext_ctrls(struct file *file, void *priv,
 	unsigned int idx;
 	int ret;
 
+	/* Default value cannot be changed */
+	if (ctls->which == V4L2_CTRL_WHICH_DEF_VAL)
+		return -EINVAL;
+
 	ret = 0;
 	for (idx = 0; idx < ctls->count; idx++) {
 		ctrl = ctls->controls + idx;
-- 
2.5.0

[PATCH 6/8] media/pci/saa7164-encoder Support for V4L2_CTRL_WHICH_DEF_VAL

[Ricardo Ribalda Delgado]

This driver does not use the control infrastructure.
Add support for the new field which on structure
 v4l2_ext_controls

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/media/pci/saa7164/saa7164-encoder.c | 55 ++++++++++++++++-------------
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/drivers/media/pci/saa7164/saa7164-encoder.c b/drivers/media/pci/saa7164/saa7164-encoder.c
index 4434e0f28c26..e897b83b66dd 100644
--- a/drivers/media/pci/saa7164/saa7164-encoder.c
+++ b/drivers/media/pci/saa7164/saa7164-encoder.c
@@ -531,30 +531,6 @@ static int saa7164_get_ctrl(struct saa7164_port *port,
 	return 0;
 }
 
-static int vidioc_g_ext_ctrls(struct file *file, void *priv,
-	struct v4l2_ext_controls *ctrls)
-{
-	struct saa7164_encoder_fh *fh = file->private_data;
-	struct saa7164_port *port = fh->port;
-	int i, err = 0;
-
-	if (ctrls->ctrl_class == V4L2_CTRL_CLASS_MPEG) {
-		for (i = 0; i < ctrls->count; i++) {
-			struct v4l2_ext_control *ctrl = ctrls->controls + i;
-
-			err = saa7164_get_ctrl(port, ctrl);
-			if (err) {
-				ctrls->error_idx = i;
-				break;
-			}
-		}
-		return err;
-
-	}
-
-	return -EINVAL;
-}
-
 static int saa7164_try_ctrl(struct v4l2_ext_control *ctrl, int ac3)
 {
 	int ret = -EINVAL;
@@ -884,6 +860,37 @@ static int vidioc_queryctrl(struct file *file, void *priv,
 	return -EINVAL;
 }
 
+static int vidioc_g_ext_ctrls(struct file *file, void *priv,
+	struct v4l2_ext_controls *ctrls)
+{
+	struct saa7164_encoder_fh *fh = file->private_data;
+	struct saa7164_port *port = fh->port;
+	int i, err = 0;
+
+	if (ctrls->ctrl_class != V4L2_CTRL_CLASS_MPEG &&
+		ctrls->which != V4L2_CTRL_WHICH_DEF_VAL)
+		return -EINVAL;
+
+	for (i = 0; i < ctrls->count; i++) {
+		struct v4l2_ext_control *ctrl = ctrls->controls + i;
+
+		if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) {
+			struct v4l2_queryctrl q;
+
+			err = fill_queryctrl(&port->encoder_params, &q);
+			if (!err)
+				ctrl->value = q.default_value;
+		} else
+			err = saa7164_get_ctrl(port, ctrl);
+
+		if (err) {
+			ctrls->error_idx = i;
+			break;
+		}
+	}
+	return err;
+}
+
 static int saa7164_encoder_stop_port(struct saa7164_port *port)
 {
 	struct saa7164_dev *dev = port->dev;
-- 
2.5.0

[PATCH 7/8] media/pci/saa7164-vbi Support for V4L2_CTRL_WHICH_DEF_VAL

[Ricardo Ribalda Delgado]

This driver does not use the control infrastructure.
Add support for the new field which on structure
 v4l2_ext_controls

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/media/pci/saa7164/saa7164-vbi.c | 57 +++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 24 deletions(-)

diff --git a/drivers/media/pci/saa7164/saa7164-vbi.c b/drivers/media/pci/saa7164/saa7164-vbi.c
index 859fd03d82f9..e8723102a4d2 100644
--- a/drivers/media/pci/saa7164/saa7164-vbi.c
+++ b/drivers/media/pci/saa7164/saa7164-vbi.c
@@ -494,30 +494,6 @@ static int saa7164_get_ctrl(struct saa7164_port *port,
 	return 0;
 }
 
-static int vidioc_g_ext_ctrls(struct file *file, void *priv,
-	struct v4l2_ext_controls *ctrls)
-{
-	struct saa7164_vbi_fh *fh = file->private_data;
-	struct saa7164_port *port = fh->port;
-	int i, err = 0;
-
-	if (ctrls->ctrl_class == V4L2_CTRL_CLASS_MPEG) {
-		for (i = 0; i < ctrls->count; i++) {
-			struct v4l2_ext_control *ctrl = ctrls->controls + i;
-
-			err = saa7164_get_ctrl(port, ctrl);
-			if (err) {
-				ctrls->error_idx = i;
-				break;
-			}
-		}
-		return err;
-
-	}
-
-	return -EINVAL;
-}
-
 static int saa7164_try_ctrl(struct v4l2_ext_control *ctrl, int ac3)
 {
 	int ret = -EINVAL;
@@ -810,6 +786,39 @@ static int vidioc_queryctrl(struct file *file, void *priv,
 	return -EINVAL;
 }
 
+static int vidioc_g_ext_ctrls(struct file *file, void *priv,
+	struct v4l2_ext_controls *ctrls)
+{
+	struct saa7164_vbi_fh *fh = file->private_data;
+	struct saa7164_port *port = fh->port;
+	int i, err = 0;
+
+	if (ctrls->ctrl_class != V4L2_CTRL_CLASS_MPEG &&
+		ctrls->which != V4L2_CTRL_WHICH_DEF_VAL)
+		return -EINVAL;
+
+	for (i = 0; i < ctrls->count; i++) {
+		struct v4l2_ext_control *ctrl = ctrls->controls + i;
+
+		if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) {
+			struct v4l2_queryctrl q;
+
+			err = fill_queryctrl(&port->vbi_params, &q);
+			if (!err)
+				ctrl->value = q.default_value;
+		} else
+			err = saa7164_get_ctrl(port, ctrl);
+
+		if (err) {
+			ctrls->error_idx = i;
+			break;
+		}
+	}
+
+	return err;
+}
+
+
 static int saa7164_vbi_stop_port(struct saa7164_port *port)
 {
 	struct saa7164_dev *dev = port->dev;
-- 
2.5.0

[PATCH 8/8] Docbook: media: Document changes on struct v4l2_ext_controls

[Ricardo Ribalda Delgado]

Vidioc-g-ext-ctrls can now be used to get the default value of the
controls.

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 Documentation/DocBook/media/v4l/v4l2.xml               |  9 +++++++++
 Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml | 14 ++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
index e98caa1c39bd..be52bd2fb335 100644
--- a/Documentation/DocBook/media/v4l/v4l2.xml
+++ b/Documentation/DocBook/media/v4l/v4l2.xml
@@ -153,6 +153,15 @@ structs, ioctls) must be noted in more detail in the history chapter
 applications. -->
 
       <revision>
+	<revnumber>4.4</revnumber>
+	<date>2015-08-20</date>
+	<authorinitials>rr</authorinitials>
+	<revremark>Extend vidioc-g-ext-ctrls;. Replace ctrl_class with a new
+union with ctrl_class and which. Which is used to select the current value of
+the control or the default value.
+	</revremark>
+      </revision>
+      <revision>
 	<revnumber>3.21</revnumber>
 	<date>2015-02-13</date>
 	<authorinitials>mcc</authorinitials>
diff --git a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
index c5bdbfcc42b3..224fa2bd1481 100644
--- a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
@@ -216,7 +216,12 @@ Valid if <constant>V4L2_CTRL_FLAG_HAS_PAYLOAD</constant> is set for this control
       <tgroup cols="3">
 	&cs-str;
 	<tbody valign="top">
+	 <row>
+	    <entry>union</entry>
+	    <entry>(anonymous)</entry>
+	  </row>
 	  <row>
+	    <entry></entry>
 	    <entry>__u32</entry>
 	    <entry><structfield>ctrl_class</structfield></entry>
 	    <entry>The control class to which all controls belong, see
@@ -228,6 +233,15 @@ with a <structfield>count</structfield> of 0. If that succeeds, then the driver
 supports this feature.</entry>
 	  </row>
 	  <row>
+	    <entry></entry>
+	    <entry>__u32</entry>
+	    <entry><structfield>which</structfield></entry>
+	    <entry> Which control are get/set/tried. <constant>V4L2_CTRL_WHICH_CUR_VAL</constant>
+will return the current value of the control and <constant>V4L2_CTRL_WHICH_DEF_VAL</constant> will
+return the default value of the control. Please note that the default value of the control cannot
+be set or tried, only get.</entry>
+	  </row>
+	  <row>
 	    <entry>__u32</entry>
 	    <entry><structfield>count</structfield></entry>
 	    <entry>The number of controls in the controls array. May
-- 
2.5.0

Re: [PATCH 3/8] media/v4l2-core: struct struct v4l2_ext_controls param which

[Hans Verkuil]

On 08/21/2015 11:29 AM, Ricardo Ribalda Delgado wrote:
> Support for new field which on v4l2_ext_controls, used to get the
> default value of one or more controls.
> 
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
>  drivers/media/v4l2-core/v4l2-ctrls.c | 35 ++++++++++++++++++++++++++++++-----
>  1 file changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index b6b7dcc1b77d..23a69f637f6d 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -1489,6 +1489,17 @@ static int new_to_user(struct v4l2_ext_control *c,
>  	return ptr_to_user(c, ctrl, ctrl->p_new);
>  }
>  
> +/* Helper function: copy the initial control value back to the caller */
> +static int def_to_user(struct v4l2_ext_control *c, struct v4l2_ctrl *ctrl)
> +{
> +	int idx;
> +
> +	for (idx = 0; idx < ctrl->elems; idx++)
> +		ctrl->type_ops->init(ctrl, idx, ctrl->p_new);
> +
> +	return ptr_to_user(c, ctrl, ctrl->p_new);
> +}
> +
>  /* Helper function: copy the caller-provider value to the given control value */
>  static int user_to_ptr(struct v4l2_ext_control *c,
>  		       struct v4l2_ctrl *ctrl,
> @@ -2708,7 +2719,9 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl,
>  
>  		cs->error_idx = i;
>  
> -		if (cs->ctrl_class && V4L2_CTRL_ID2CLASS(id) != cs->ctrl_class)
> +		if (cs->ctrl_class &&
> +		    cs->which != V4L2_CTRL_WHICH_DEF_VAL &&
> +		    V4L2_CTRL_ID2CLASS(id) != cs->ctrl_class)
>  			return -EINVAL;
>  
>  		/* Old-style private controls are not allowed for
> @@ -2787,7 +2800,7 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl,
>     whether there are any controls at all. */
>  static int class_check(struct v4l2_ctrl_handler *hdl, u32 ctrl_class)
>  {
> -	if (ctrl_class == 0)
> +	if (ctrl_class == 0 || ctrl_class == V4L2_CTRL_WHICH_DEF_VAL)
>  		return list_empty(&hdl->ctrl_refs) ? -EINVAL : 0;
>  	return find_ref_lock(hdl, ctrl_class | 1) ? 0 : -EINVAL;
>  }
> @@ -2801,10 +2814,14 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls *cs
>  	struct v4l2_ctrl_helper *helpers = helper;
>  	int ret;
>  	int i, j;
> +	bool def_value = false;
>  
>  	cs->error_idx = cs->count;
>  	cs->ctrl_class = V4L2_CTRL_ID2CLASS(cs->ctrl_class);
>  
> +	if (cs->which == V4L2_CTRL_WHICH_DEF_VAL)
> +		def_value = true;
> +

Ah, this is confusing. First assigning to ctrl_class, then checking 'which'.

I would add a patch after patch 2/8 that replaces all occurrences of ctrl_class in
by 'which'. It's only used in v4l2-core, the saa7164 driver and in Documentation.
The old 'ctrl_class' shouldn't be used in the kernel anymore.

It is probably a good idea to put #ifndef __KERNEL__ around the ctrl_class field in
the header. That way it isn't visible in the kernel at all.

I would also rename V4L2_CTRL_ID2CLASS to V4L2_CTRL_ID2WHICH (and keep the old define
as #define V4L2_CTRL_ID2CLASS V4L2_CTRL_ID2WHICH under #ifndef __KERNEL__).

>  	if (hdl == NULL)
>  		return -EINVAL;
>  
> @@ -2827,9 +2844,11 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls *cs
>  
>  	for (i = 0; !ret && i < cs->count; i++) {
>  		int (*ctrl_to_user)(struct v4l2_ext_control *c,
> -				    struct v4l2_ctrl *ctrl) = cur_to_user;
> +				    struct v4l2_ctrl *ctrl);
>  		struct v4l2_ctrl *master;
>  
> +		ctrl_to_user = def_value ? def_to_user : cur_to_user;
> +
>  		if (helpers[i].mref == NULL)
>  			continue;
>  
> @@ -2839,8 +2858,9 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls *cs
>  		v4l2_ctrl_lock(master);
>  
>  		/* g_volatile_ctrl will update the new control values */
> -		if ((master->flags & V4L2_CTRL_FLAG_VOLATILE) ||
> -			(master->has_volatiles && !is_cur_manual(master))) {
> +		if (!def_value &&
> +		    ((master->flags & V4L2_CTRL_FLAG_VOLATILE) ||
> +		    (master->has_volatiles && !is_cur_manual(master)))) {
>  			for (j = 0; j < master->ncontrols; j++)
>  				cur_to_new(master->cluster[j]);
>  			ret = call_op(master, g_volatile_ctrl);
> @@ -3062,6 +3082,11 @@ static int try_set_ext_ctrls(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl,
>  	int ret;
>  
>  	cs->error_idx = cs->count;
> +
> +	/* Default value cannot be changed */
> +	if (cs->which == V4L2_CTRL_WHICH_DEF_VAL)
> +		return -EINVAL;
> +
>  	cs->ctrl_class = V4L2_CTRL_ID2CLASS(cs->ctrl_class);
>  
>  	if (hdl == NULL)
> 

Regards,

	Hans

Re: [PATCH 8/8] Docbook: media: Document changes on struct v4l2_ext_controls

[Hans Verkuil]

On 08/21/2015 11:29 AM, Ricardo Ribalda Delgado wrote:
> Vidioc-g-ext-ctrls can now be used to get the default value of the
> controls.
> 
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
>  Documentation/DocBook/media/v4l/v4l2.xml               |  9 +++++++++
>  Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml | 14 ++++++++++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
> index e98caa1c39bd..be52bd2fb335 100644
> --- a/Documentation/DocBook/media/v4l/v4l2.xml
> +++ b/Documentation/DocBook/media/v4l/v4l2.xml
> @@ -153,6 +153,15 @@ structs, ioctls) must be noted in more detail in the history chapter
>  applications. -->
>  
>        <revision>
> +	<revnumber>4.4</revnumber>
> +	<date>2015-08-20</date>
> +	<authorinitials>rr</authorinitials>
> +	<revremark>Extend vidioc-g-ext-ctrls;. Replace ctrl_class with a new
> +union with ctrl_class and which. Which is used to select the current value of
> +the control or the default value.
> +	</revremark>
> +      </revision>
> +      <revision>
>  	<revnumber>3.21</revnumber>
>  	<date>2015-02-13</date>
>  	<authorinitials>mcc</authorinitials>
> diff --git a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
> index c5bdbfcc42b3..224fa2bd1481 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-g-ext-ctrls.xml
> @@ -216,7 +216,12 @@ Valid if <constant>V4L2_CTRL_FLAG_HAS_PAYLOAD</constant> is set for this control
>        <tgroup cols="3">
>  	&cs-str;
>  	<tbody valign="top">
> +	 <row>
> +	    <entry>union</entry>
> +	    <entry>(anonymous)</entry>
> +	  </row>
>  	  <row>
> +	    <entry></entry>
>  	    <entry>__u32</entry>
>  	    <entry><structfield>ctrl_class</structfield></entry>
>  	    <entry>The control class to which all controls belong, see
> @@ -228,6 +233,15 @@ with a <structfield>count</structfield> of 0. If that succeeds, then the driver
>  supports this feature.</entry>

All I would say here is that ctrl_class is an alias for 'which', kept for backwards compatibility.
Applications should use 'which' instead.


>  	  </row>
>  	  <row>
> +	    <entry></entry>
> +	    <entry>__u32</entry>
> +	    <entry><structfield>which</structfield></entry>
> +	    <entry> Which control are get/set/tried. <constant>V4L2_CTRL_WHICH_CUR_VAL</constant>

I'd say: "Which value of the control to get/set/try."

> +will return the current value of the control and <constant>V4L2_CTRL_WHICH_DEF_VAL</constant> will
> +return the default value of the control. Please note that the default value of the control cannot
> +be set or tried, only get.</entry>

I'd rephrase that:

"Please note that you can only get the default value of the control, you cannot
set or try it."

Add this:

"For backwards compatibility you can also use a control class here (see
<xref linkend="ctrl-class" />. In that case all controls have to belong to that
control class. This usage is deprecated, instead just use <constant>V4L2_CTRL_WHICH_CUR_VAL</constant>.
There are some very old drivers that do not yet support <constant>V4L2_CTRL_WHICH_CUR_VAL</constant>
and that require a control class here. You can test for such drivers by setting ctrl_class to
<constant>V4L2_CTRL_WHICH_CUR_VAL</constant> and calling VIDIOC_TRY_EXT_CTRLS with a count of 0.
If that fails, then the driver does not support <constant>V4L2_CTRL_WHICH_CUR_VAL</constant>."

I think the only driver that still doesn't support this is saa7164. I really need
to convert it to the control framework.

Regards,

	Hans

> +	  </row>
> +	  <row>
>  	    <entry>__u32</entry>
>  	    <entry><structfield>count</structfield></entry>
>  	    <entry>The number of controls in the controls array. May
>