On Wednesday 10 February 2010 15:58:09 Sakari Ailus wrote:
> Add support for subscribing all events with a special id V4L2_EVENT_ALL. If
> V4L2_EVENT_ALL is subscribed, no other events may be subscribed. Otherwise
> V4L2_EVENT_ALL is considered just as any other event.
We should do this differently. I think that EVENT_ALL should not be used
internally (i.e. in the actual list of subscribed events), but just as a
special value for the subscribe and unsubscribe ioctls. So when used with
unsubscribe you can just unsubscribe all subscribed events and when used
with subscribe, then you just subscribe all valid events (valid for that
device node).
So in v4l2-event.c you will have a v4l2_event_unsubscribe_all() to quickly
unsubscribe all events.
In order to easily add all events from the driver it would help if the
v4l2_event_subscribe and v4l2_event_unsubscribe just take the event type
as argument rather than the whole v4l2_event_subscription struct.
You will then get something like this in the driver:
if (sub->type == V4L2_EVENT_ALL) {
int ret = v4l2_event_alloc(fh, 60);
ret = ret ? ret : v4l2_event_subscribe(fh, V4L2_EVENT_EOS);
ret = ret ? ret : v4l2_event_subscribe(fh, V4L2_EVENT_VSYNC);
return ret;
}
An alternative might be to add a v4l2_event_subscribe_all(fh, const u32 *events)
where 'events' is a 0 terminated list of events that need to be subscribed.
For each event this function would then call:
fh->vdev->ioctl_ops->vidioc_subscribe_event(fh, sub);
The nice thing about that is that in the driver you have a minimum of fuss.
I'm leaning towards this second solution due to the simple driver implementation.
Handling EVENT_ALL will simplify things substantially IMHO.
Regards,
Hans
>
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxxxxxxxxxxxxx>
> ---
> drivers/media/video/v4l2-event.c | 13 ++++++++++++-
> include/linux/videodev2.h | 1 +
> 2 files changed, 13 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/media/video/v4l2-event.c b/drivers/media/video/v4l2-event.c
> index 0af0de5..68b3cf4 100644
> --- a/drivers/media/video/v4l2-event.c
> +++ b/drivers/media/video/v4l2-event.c
> @@ -139,6 +139,14 @@ static struct v4l2_subscribed_event *__v4l2_event_subscribed(
> struct v4l2_events *events = fh->events;
> struct v4l2_subscribed_event *sev;
>
> + if (list_empty(&events->subscribed))
> + return NULL;
> +
> + sev = list_entry(events->subscribed.next,
> + struct v4l2_subscribed_event, list);
> + if (sev->type == V4L2_EVENT_ALL)
> + return sev;
> +
> list_for_each_entry(sev, &events->subscribed, list) {
> if (sev->type == type)
> return sev;
> @@ -222,6 +230,8 @@ int v4l2_event_subscribe(struct v4l2_fh *fh,
> /* Allow subscribing to valid events only. */
> if (sub->type < V4L2_EVENT_PRIVATE_START)
> switch (sub->type) {
> + case V4L2_EVENT_ALL:
> + break;
> default:
> return -EINVAL;
> }
> @@ -262,7 +272,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
>
> sev = __v4l2_event_subscribed(fh, sub->type);
>
> - if (sev == NULL) {
> + if (sev == NULL ||
> + (sub->type != V4L2_EVENT_ALL && sev->type == V4L2_EVENT_ALL)) {
> spin_unlock_irqrestore(&fh->vdev->fh_lock, flags);
> return -EINVAL;
> }
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index a19ae89..9ae9a1c 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -1553,6 +1553,7 @@ struct v4l2_event_subscription {
> __u32 reserved[7];
> };
>
> +#define V4L2_EVENT_ALL 0
> #define V4L2_EVENT_PRIVATE_START 0x08000000
>
> /*
>
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG
--
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
