Hi Antonio,
My apologies for the late review, I missed this the first time around.
Since I am the 'control guy' :-) I thought I should give my view on this.
And that is the real long-term approach to implementing extended controls
in gspca is to add support in gspca for the control framework (see
Documentation/video4linux/v4l2-controls.txt). That will probably simplify
control handling in gspca anyway. The main problem is how to do it in such
a manner that we can convert gspca drivers to the control framework one by
one.
I am inclined to NACK code that adds a driver-specific extended control
implementation instead of going to the control framework because I know
from painful personal experience that it is very hard to get it right.
I might have some time (no guarantees yet) to help with this. It would
certainly be interesting to add support for the control framework in the
gspca core. Hmm, perhaps that's a job for the weekend...
Regards,
Hans
On Friday, April 20, 2012 17:19:11 Antonio Ospite wrote:
> This makes it possible for applications to handle controls with a class
> different than V4L2_CTRL_CLASS_USER for gspca subdevices, like for
> instance V4L2_CID_EXPOSURE_AUTO which some subdrivers use and which
> can't be controlled right now.
>
> See
> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/47010
> for an example of a problem fixed by this change.
>
> NOTE: gspca currently won't handle control types like
> V4L2_CTRL_TYPE_INTEGER64 or V4L2_CTRL_TYPE_STRING, so just the
> __s32 field 'value' of 'struct v4l2_ext_control' is handled for now.
>
> Signed-off-by: Antonio Ospite <osp...@studenti.unina.it>
> ---
> drivers/media/video/gspca/gspca.c | 42
> +++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/drivers/media/video/gspca/gspca.c
> b/drivers/media/video/gspca/gspca.c
> index ba1bda9..7906093 100644
> --- a/drivers/media/video/gspca/gspca.c
> +++ b/drivers/media/video/gspca/gspca.c
> @@ -1567,6 +1567,46 @@ static int vidioc_g_ctrl(struct file *file, void *priv,
> return gspca_get_ctrl(gspca_dev, ctrl->id, &ctrl->value);
> }
>
> +static int vidioc_s_ext_ctrls(struct file *file, void *priv,
> + struct v4l2_ext_controls *ext_ctrls)
> +{
> + struct gspca_dev *gspca_dev = priv;
> + int ret = 0;
> + int i;
> +
> + for (i = 0; i < ext_ctrls->count; i++) {
> + struct v4l2_ext_control *ctrl;
> +
> + ctrl = ext_ctrls->controls + i;
> + ret = gspca_set_ctrl(gspca_dev, ctrl->id, ctrl->value);
> + if (ret < 0) {
> + ext_ctrls->error_idx = i;
> + return ret;
> + }
> + }
> + return ret;
> +}
> +
> +static int vidioc_g_ext_ctrls(struct file *file, void *priv,
> + struct v4l2_ext_controls *ext_ctrls)
> +{
> + struct gspca_dev *gspca_dev = priv;
> + int i;
> + int ret = 0;
> +
> + for (i = 0; i < ext_ctrls->count; i++) {
> + struct v4l2_ext_control *ctrl;
> +
> + ctrl = ext_ctrls->controls + i;
> + ret = gspca_get_ctrl(gspca_dev, ctrl->id, &ctrl->value);
> + if (ret < 0) {
> + ext_ctrls->error_idx = i;
> + return ret;
> + }
> + }
> + return ret;
> +}
> +
> static int vidioc_querymenu(struct file *file, void *priv,
> struct v4l2_querymenu *qmenu)
> {
> @@ -2260,6 +2300,8 @@ static const struct v4l2_ioctl_ops dev_ioctl_ops = {
> .vidioc_queryctrl = vidioc_queryctrl,
> .vidioc_g_ctrl = vidioc_g_ctrl,
> .vidioc_s_ctrl = vidioc_s_ctrl,
> + .vidioc_g_ext_ctrls = vidioc_g_ext_ctrls,
> + .vidioc_s_ext_ctrls = vidioc_s_ext_ctrls,
> .vidioc_querymenu = vidioc_querymenu,
> .vidioc_enum_input = vidioc_enum_input,
> .vidioc_g_input = vidioc_g_input,
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
