Re: [RFCv2 PATCH 07/11] v4l2-ctrls: add control events.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Hans,

On Wed, May 25, 2011 at 03:33:51PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> 
> Whenever a control changes value or state an event is sent to anyone
> that subscribed to it.
> 
> This functionality is useful for control panels but also for applications
> that need to wait for (usually status) controls to change value.

Thanks for the patch!

I agree that it's good to pass more information of the control (min, max
etc.) to the user space with the event. However, to support events arriving
from interrupt context which we've discussed in the past, such information
must be also accessible in those situations.

What do you think about more fine-grained locking of controls, say, spinlock
for each control (cluster) as an idea?

> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> ---
>  drivers/media/video/v4l2-ctrls.c |  101 ++++++++++++++++++++++++++++++
>  drivers/media/video/v4l2-event.c |  126 +++++++++++++++++++++++++++-----------
>  drivers/media/video/v4l2-fh.c    |    4 +-
>  include/linux/videodev2.h        |   29 ++++++++-
>  include/media/v4l2-ctrls.h       |   16 +++++-
>  include/media/v4l2-event.h       |    2 +
>  6 files changed, 237 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
> index 3e0d8ab..e2a7ac7 100644
> --- a/drivers/media/video/v4l2-ctrls.c
> +++ b/drivers/media/video/v4l2-ctrls.c
> @@ -23,6 +23,7 @@
>  #include <media/v4l2-ioctl.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-ctrls.h>
> +#include <media/v4l2-event.h>
>  #include <media/v4l2-dev.h>
>  
>  #define call_op(master, op) \
> @@ -540,6 +541,44 @@ static bool type_is_int(const struct v4l2_ctrl *ctrl)
>  	}
>  }
>  
> +static void fill_event(struct v4l2_event *ev, struct v4l2_ctrl *ctrl, u32 changes)
> +{
> +	memset(ev->reserved, 0, sizeof(ev->reserved));
> +	ev->type = V4L2_EVENT_CTRL;
> +	ev->id = ctrl->id;
> +	ev->u.ctrl.changes = changes;
> +	ev->u.ctrl.type = ctrl->type;
> +	ev->u.ctrl.flags = ctrl->flags;
> +	if (ctrl->type == V4L2_CTRL_TYPE_STRING)
> +		ev->u.ctrl.value64 = 0;
> +	else
> +		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)
> +		ev->u.ctrl.step = 1;
> +	else
> +		ev->u.ctrl.step = ctrl->step;
> +	ev->u.ctrl.default_value = ctrl->default_value;
> +}
> +
> +static void send_event(struct v4l2_fh *fh, struct v4l2_ctrl *ctrl, u32 changes)
> +{
> +	struct v4l2_event ev;
> +	struct v4l2_ctrl_fh *pos;
> +
> +	if (list_empty(&ctrl->fhs))
> +			return;
> +	fill_event(&ev, ctrl, changes);
> +
> +	list_for_each_entry(pos, &ctrl->fhs, node) {
> +		if (pos->fh == fh)
> +			continue;
> +		ev.u.ctrl.flags = ctrl->flags;

What's the purpose of setting flags here as well? fill_event() above already
does it.

> +		v4l2_event_queue_fh(pos->fh, &ev);
> +	}
> +}
> +
>  /* Helper function: copy the current control value back to the caller */
>  static int cur_to_user(struct v4l2_ext_control *c,
>  		       struct v4l2_ctrl *ctrl)

Regards,

-- 
Sakari Ailus
sakari dot ailus at iki dot fi
--
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


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux