Em 07-06-2011 12:05, Hans Verkuil escreveu:
> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>
> When an application changes a control you want to generate an event.
> However, you want to avoid sending such an event back to the application
> (file handle) that caused the change.
Why?
I can see two usecases for an event-triggered control change:
1) when two applications are used, and one changed a value that could
affect the other;
2) as a way to implement async changes.
However, it seems, from your comments, that you're covering only case (1).
There are several reasons why we need to support case (2):
Some controls may be associated to a servo mechanism (like zoom, optical
focus, etc), or may require some time to happen (like charging a flash device).
So, it makes sense to have events back to the application that caused the change.
Kernel should not assume that the application that requested a change on a control
doesn't want to receive the notification back when the event actually happened.
This way, both cases will be covered.
Yet, I failed to see where, in the code, such restriction were imposed.
>
> Add the filehandle to the various set control functions.
>
> The filehandle isn't used yet, but the control event patches will need
> this.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> ---
> drivers/media/video/v4l2-ctrls.c | 45 ++++++++++++++++++++----------------
> drivers/media/video/v4l2-ioctl.c | 8 +++---
> drivers/media/video/v4l2-subdev.c | 4 +-
> include/media/v4l2-ctrls.h | 8 ++++--
> 4 files changed, 36 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
> index c39ab0c..a38bdf9 100644
> --- a/drivers/media/video/v4l2-ctrls.c
> +++ b/drivers/media/video/v4l2-ctrls.c
> @@ -657,7 +657,8 @@ static int ctrl_is_volatile(struct v4l2_ext_control *c,
> }
>
> /* Copy the new value to the current value. */
> -static void new_to_cur(struct v4l2_ctrl *ctrl, bool update_inactive)
> +static void new_to_cur(struct v4l2_fh *fh, struct v4l2_ctrl *ctrl,
> + bool update_inactive)
> {
> if (ctrl == NULL)
> return;
> @@ -1717,7 +1718,8 @@ EXPORT_SYMBOL(v4l2_ctrl_g_ctrl);
> /* Core function that calls try/s_ctrl and ensures that the new value is
> copied to the current value on a set.
> Must be called with ctrl->handler->lock held. */
> -static int try_or_set_control_cluster(struct v4l2_ctrl *master, bool set)
> +static int try_or_set_control_cluster(struct v4l2_fh *fh,
> + struct v4l2_ctrl *master, bool set)
> {
> bool update_flag;
> bool try = !set;
> @@ -1768,12 +1770,13 @@ static int try_or_set_control_cluster(struct v4l2_ctrl *master, bool set)
>
> update_flag = is_cur_manual(master) != is_new_manual(master);
> for (i = 0; i < master->ncontrols; i++)
> - new_to_cur(master->cluster[i], update_flag && i > 0);
> + new_to_cur(fh, master->cluster[i], update_flag && i > 0);
> return 0;
> }
>
> /* Try or set controls. */
> -static int try_or_set_ext_ctrls(struct v4l2_ctrl_handler *hdl,
> +static int try_or_set_ext_ctrls(struct v4l2_fh *fh,
> + struct v4l2_ctrl_handler *hdl,
> struct v4l2_ext_controls *cs,
> struct ctrl_helper *helpers,
> bool set)
> @@ -1818,7 +1821,7 @@ static int try_or_set_ext_ctrls(struct v4l2_ctrl_handler *hdl,
> ret = cluster_walk(i, cs, helpers, user_to_new);
>
> if (!ret)
> - ret = try_or_set_control_cluster(master, set);
> + ret = try_or_set_control_cluster(fh, master, set);
>
> /* Copy the new values back to userspace. */
> if (!ret)
> @@ -1831,7 +1834,7 @@ static int try_or_set_ext_ctrls(struct v4l2_ctrl_handler *hdl,
> }
>
> /* Try or try-and-set controls */
> -static int try_set_ext_ctrls(struct v4l2_ctrl_handler *hdl,
> +static int try_set_ext_ctrls(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl,
> struct v4l2_ext_controls *cs,
> bool set)
> {
> @@ -1858,7 +1861,7 @@ static int try_set_ext_ctrls(struct v4l2_ctrl_handler *hdl,
>
> /* First 'try' all controls and abort on error */
> if (!ret)
> - ret = try_or_set_ext_ctrls(hdl, cs, helpers, false);
> + ret = try_or_set_ext_ctrls(NULL, hdl, cs, helpers, false);
> /* If this is a 'set' operation and the initial 'try' failed,
> then set error_idx to count to tell the application that no
> controls changed value yet. */
> @@ -1868,7 +1871,7 @@ static int try_set_ext_ctrls(struct v4l2_ctrl_handler *hdl,
> /* Reset 'handled' state */
> for (i = 0; i < cs->count; i++)
> helpers[i].handled = false;
> - ret = try_or_set_ext_ctrls(hdl, cs, helpers, true);
> + ret = try_or_set_ext_ctrls(fh, hdl, cs, helpers, true);
> }
>
> if (cs->count > ARRAY_SIZE(helper))
> @@ -1878,30 +1881,31 @@ static int try_set_ext_ctrls(struct v4l2_ctrl_handler *hdl,
>
> int v4l2_try_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls *cs)
> {
> - return try_set_ext_ctrls(hdl, cs, false);
> + return try_set_ext_ctrls(NULL, hdl, cs, false);
> }
> EXPORT_SYMBOL(v4l2_try_ext_ctrls);
>
> -int v4l2_s_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls *cs)
> +int v4l2_s_ext_ctrls(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl,
> + struct v4l2_ext_controls *cs)
> {
> - return try_set_ext_ctrls(hdl, cs, true);
> + return try_set_ext_ctrls(fh, hdl, cs, true);
> }
> EXPORT_SYMBOL(v4l2_s_ext_ctrls);
>
> int v4l2_subdev_try_ext_ctrls(struct v4l2_subdev *sd, struct v4l2_ext_controls *cs)
> {
> - return try_set_ext_ctrls(sd->ctrl_handler, cs, false);
> + return try_set_ext_ctrls(NULL, sd->ctrl_handler, cs, false);
> }
> EXPORT_SYMBOL(v4l2_subdev_try_ext_ctrls);
>
> int v4l2_subdev_s_ext_ctrls(struct v4l2_subdev *sd, struct v4l2_ext_controls *cs)
> {
> - return try_set_ext_ctrls(sd->ctrl_handler, cs, true);
> + return try_set_ext_ctrls(NULL, sd->ctrl_handler, cs, true);
> }
> EXPORT_SYMBOL(v4l2_subdev_s_ext_ctrls);
>
> /* Helper function for VIDIOC_S_CTRL compatibility */
> -static int set_ctrl(struct v4l2_ctrl *ctrl, s32 *val)
> +static int set_ctrl(struct v4l2_fh *fh, struct v4l2_ctrl *ctrl, s32 *val)
> {
> struct v4l2_ctrl *master = ctrl->cluster[0];
> int ret;
> @@ -1916,15 +1920,16 @@ static int set_ctrl(struct v4l2_ctrl *ctrl, s32 *val)
>
> ctrl->val = *val;
> ctrl->is_new = 1;
> - ret = try_or_set_control_cluster(master, false);
> + ret = try_or_set_control_cluster(NULL, master, false);
> if (!ret)
> - ret = try_or_set_control_cluster(master, true);
> + ret = try_or_set_control_cluster(fh, master, true);
> *val = ctrl->cur.val;
> v4l2_ctrl_unlock(ctrl);
> return ret;
> }
>
> -int v4l2_s_ctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_control *control)
> +int v4l2_s_ctrl(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl,
> + struct v4l2_control *control)
> {
> struct v4l2_ctrl *ctrl = v4l2_ctrl_find(hdl, control->id);
>
> @@ -1934,13 +1939,13 @@ int v4l2_s_ctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_control *control)
> if (ctrl->flags & V4L2_CTRL_FLAG_READ_ONLY)
> return -EACCES;
>
> - return set_ctrl(ctrl, &control->value);
> + return set_ctrl(fh, ctrl, &control->value);
> }
> EXPORT_SYMBOL(v4l2_s_ctrl);
>
> int v4l2_subdev_s_ctrl(struct v4l2_subdev *sd, struct v4l2_control *control)
> {
> - return v4l2_s_ctrl(sd->ctrl_handler, control);
> + return v4l2_s_ctrl(NULL, sd->ctrl_handler, control);
> }
> EXPORT_SYMBOL(v4l2_subdev_s_ctrl);
>
> @@ -1948,6 +1953,6 @@ int v4l2_ctrl_s_ctrl(struct v4l2_ctrl *ctrl, s32 val)
> {
> /* It's a driver bug if this happens. */
> WARN_ON(!type_is_int(ctrl));
> - return set_ctrl(ctrl, &val);
> + return set_ctrl(NULL, ctrl, &val);
> }
> EXPORT_SYMBOL(v4l2_ctrl_s_ctrl);
> diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
> index 9811b1e..32107de 100644
> --- a/drivers/media/video/v4l2-ioctl.c
> +++ b/drivers/media/video/v4l2-ioctl.c
> @@ -1481,11 +1481,11 @@ static long __video_do_ioctl(struct file *file,
> dbgarg(cmd, "id=0x%x, value=%d\n", p->id, p->value);
>
> if (vfh && vfh->ctrl_handler) {
> - ret = v4l2_s_ctrl(vfh->ctrl_handler, p);
> + ret = v4l2_s_ctrl(vfh, vfh->ctrl_handler, p);
> break;
> }
> if (vfd->ctrl_handler) {
> - ret = v4l2_s_ctrl(vfd->ctrl_handler, p);
> + ret = v4l2_s_ctrl(NULL, vfd->ctrl_handler, p);
> break;
> }
> if (ops->vidioc_s_ctrl) {
> @@ -1530,9 +1530,9 @@ static long __video_do_ioctl(struct file *file,
> break;
> v4l_print_ext_ctrls(cmd, vfd, p, 1);
> if (vfh && vfh->ctrl_handler)
> - ret = v4l2_s_ext_ctrls(vfh->ctrl_handler, p);
> + ret = v4l2_s_ext_ctrls(vfh, vfh->ctrl_handler, p);
> else if (vfd->ctrl_handler)
> - ret = v4l2_s_ext_ctrls(vfd->ctrl_handler, p);
> + ret = v4l2_s_ext_ctrls(NULL, vfd->ctrl_handler, p);
> else if (check_ext_ctrls(p, 0))
> ret = ops->vidioc_s_ext_ctrls(file, fh, p);
> break;
> diff --git a/drivers/media/video/v4l2-subdev.c b/drivers/media/video/v4l2-subdev.c
> index f396cc3..fd5dcca 100644
> --- a/drivers/media/video/v4l2-subdev.c
> +++ b/drivers/media/video/v4l2-subdev.c
> @@ -164,13 +164,13 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
> return v4l2_g_ctrl(vfh->ctrl_handler, arg);
>
> case VIDIOC_S_CTRL:
> - return v4l2_s_ctrl(vfh->ctrl_handler, arg);
> + return v4l2_s_ctrl(vfh, vfh->ctrl_handler, arg);
>
> case VIDIOC_G_EXT_CTRLS:
> return v4l2_g_ext_ctrls(vfh->ctrl_handler, arg);
>
> case VIDIOC_S_EXT_CTRLS:
> - return v4l2_s_ext_ctrls(vfh->ctrl_handler, arg);
> + return v4l2_s_ext_ctrls(vfh, vfh->ctrl_handler, arg);
>
> case VIDIOC_TRY_EXT_CTRLS:
> return v4l2_try_ext_ctrls(vfh->ctrl_handler, arg);
> diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> index 56323e3..e720f11 100644
> --- a/include/media/v4l2-ctrls.h
> +++ b/include/media/v4l2-ctrls.h
> @@ -28,6 +28,7 @@
> /* forward references */
> struct v4l2_ctrl_handler;
> struct v4l2_ctrl;
> +struct v4l2_fh;
> struct video_device;
> struct v4l2_subdev;
>
> @@ -485,15 +486,16 @@ s32 v4l2_ctrl_g_ctrl(struct v4l2_ctrl *ctrl);
> */
> int v4l2_ctrl_s_ctrl(struct v4l2_ctrl *ctrl, s32 val);
>
> -
> /* Helpers for ioctl_ops. If hdl == NULL then they will all return -EINVAL. */
> int v4l2_queryctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_queryctrl *qc);
> int v4l2_querymenu(struct v4l2_ctrl_handler *hdl, struct v4l2_querymenu *qm);
> int v4l2_g_ctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_control *ctrl);
> -int v4l2_s_ctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_control *ctrl);
> +int v4l2_s_ctrl(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl,
> + struct v4l2_control *ctrl);
> int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls *c);
> int v4l2_try_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls *c);
> -int v4l2_s_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls *c);
> +int v4l2_s_ext_ctrls(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl,
> + struct v4l2_ext_controls *c);
>
> /* Helpers for subdevices. If the associated ctrl_handler == NULL then they
> will all return -EINVAL. */
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
