Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1258712 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/6] Support getting default values from any control Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> - 2015-10-29 11:20 +0100
    [PATCH v2 4/6] usb/uvc: Support for V4L2_CTRL_WHICH_DEF_VAL Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> - 2015-10-29 11:20 +0100
      Re: [PATCH v2 4/6] usb/uvc: Support for V4L2_CTRL_WHICH_DEF_VAL Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2015-10-30 15:30 +0100
    [PATCH v2 5/6] media/usb/pvrusb2: Support for V4L2_CTRL_WHICH_DEF_VAL Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> - 2015-10-29 11:20 +0100
      Re: [PATCH v2 5/6] media/usb/pvrusb2: Support for  V4L2_CTRL_WHICH_DEF_VAL Mike Isely <isely@isely.net> - 2015-10-29 16:40 +0100

#1258712 — [PATCH v2 0/6] Support getting default values from any control

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_v5



Changelog v2 (compared to Support getting default values from any control

Rebase to latest version
Remove saa7164-* patches (Hans already ported them to v4l2-ctrl)
Update doc dates

Suggested by Laurent Pinchart <laurent.pinchart@ideasonboard.com>:

Fixes for the uvc implementation (error in lock handling) (Thanks!)


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 (6):
  videodev2.h: Extend struct v4l2_ext_controls
  media/core: Replace ctrl_class with which
  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
  Docbook: media: Document changes on struct v4l2_ext_controls

 Documentation/DocBook/media/v4l/v4l2.xml           | 10 ++++
 .../DocBook/media/v4l/vidioc-g-ext-ctrls.xml       | 28 +++++++++--
 drivers/media/platform/s5p-mfc/s5p_mfc_dec.c       |  2 +-
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c       |  2 +-
 drivers/media/usb/pvrusb2/pvrusb2-v4l2.c           | 16 ++++++-
 drivers/media/usb/uvc/uvc_v4l2.c                   | 20 ++++++++
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c      |  6 +--
 drivers/media/v4l2-core/v4l2-ctrls.c               | 54 ++++++++++++++++------
 drivers/media/v4l2-core/v4l2-ioctl.c               | 14 +++---
 include/uapi/linux/videodev2.h                     | 12 ++++-
 10 files changed, 131 insertions(+), 33 deletions(-)

-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1258713 — [PATCH v2 4/6] usb/uvc: Support for V4L2_CTRL_WHICH_DEF_VAL

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 | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 2764f43607c1..d7723ce772b3 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -983,6 +983,22 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh,
 	unsigned int i;
 	int ret;
 
+	if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) {
+		for (i = 0; i < ctrls->count; ++ctrl, ++i) {
+			struct v4l2_queryctrl qc = { .id = ctrl->id };
+
+			ret = uvc_query_v4l2_ctrl(chain, &qc);
+			if (ret < 0) {
+				ctrls->error_idx = i;
+				return ret;
+			}
+
+			ctrl->value = qc.default_value;
+		}
+
+		return 0;
+	}
+
 	ret = uvc_ctrl_begin(chain);
 	if (ret < 0)
 		return ret;
@@ -1010,6 +1026,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.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1259536 — Re: [PATCH v2 4/6] usb/uvc: Support for V4L2_CTRL_WHICH_DEF_VAL

Hi Ricardo,

Thank you for the patch.

On Thursday 29 October 2015 11:10:30 Ricardo Ribalda Delgado wrote:
> 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>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/usb/uvc/uvc_v4l2.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
> b/drivers/media/usb/uvc/uvc_v4l2.c index 2764f43607c1..d7723ce772b3 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -983,6 +983,22 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file,
> void *fh, unsigned int i;
>  	int ret;
> 
> +	if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) {
> +		for (i = 0; i < ctrls->count; ++ctrl, ++i) {
> +			struct v4l2_queryctrl qc = { .id = ctrl->id };
> +
> +			ret = uvc_query_v4l2_ctrl(chain, &qc);
> +			if (ret < 0) {
> +				ctrls->error_idx = i;
> +				return ret;
> +			}
> +
> +			ctrl->value = qc.default_value;
> +		}
> +
> +		return 0;
> +	}
> +
>  	ret = uvc_ctrl_begin(chain);
>  	if (ret < 0)
>  		return ret;
> @@ -1010,6 +1026,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;

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1258714 — [PATCH v2 5/6] media/usb/pvrusb2: Support for V4L2_CTRL_WHICH_DEF_VAL

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

Acked-by: Mike Isely <isely@pobox.com>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
index 1c5f85bf7ed4..81f788b7b242 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,15 @@ 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(
-				pvr2_hdw_get_ctrl_v4l(hdw, ctrl->id), &val);
+		cptr = pvr2_hdw_get_ctrl_v4l(hdw, ctrl->id);
+		if (cptr) {
+			if (ctls->which == V4L2_CTRL_WHICH_DEF_VAL)
+				pvr2_ctrl_get_def(cptr, &val);
+			else
+				ret = pvr2_ctrl_get_value(cptr, &val);
+		} else
+			ret = -EINVAL;
+
 		if (ret) {
 			ctls->error_idx = idx;
 			return ret;
@@ -658,6 +666,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.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1258870 — Re: [PATCH v2 5/6] media/usb/pvrusb2: Support for V4L2_CTRL_WHICH_DEF_VAL

Looks good to me (still), including now the change I had previously 
suggested.  For the record, the ack still applies.  (I guess you can 
consider this to be an ack of the ack...)

  -Mike


On Thu, 29 Oct 2015, Ricardo Ribalda Delgado wrote:

> This driver does not use the control infrastructure.
> Add support for the new field which on structure
>  v4l2_ext_controls
> 
> Acked-by: Mike Isely <isely@pobox.com>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
>  drivers/media/usb/pvrusb2/pvrusb2-v4l2.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
> index 1c5f85bf7ed4..81f788b7b242 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,15 @@ 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(
> -				pvr2_hdw_get_ctrl_v4l(hdw, ctrl->id), &val);
> +		cptr = pvr2_hdw_get_ctrl_v4l(hdw, ctrl->id);
> +		if (cptr) {
> +			if (ctls->which == V4L2_CTRL_WHICH_DEF_VAL)
> +				pvr2_ctrl_get_def(cptr, &val);
> +			else
> +				ret = pvr2_ctrl_get_value(cptr, &val);
> +		} else
> +			ret = -EINVAL;
> +
>  		if (ret) {
>  			ctls->error_idx = idx;
>  			return ret;
> @@ -658,6 +666,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;
> 

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web