* [PATCH for v3.5] v4l2-event: fix regression with initial event handling.
@ 2012-05-07 20:53 Hans Verkuil
2012-05-08 7:57 ` Hans de Goede
0 siblings, 1 reply; 2+ messages in thread
From: Hans Verkuil @ 2012-05-07 20:53 UTC (permalink / raw)
To: linux-media; +Cc: Hans de Goede, Laurent Pinchart, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
If the V4L2_EVENT_SUB_FL_SEND_INITIAL was set, then the application expects
to receive an initial event of the initial value of the control.
However, commit c53c2549333b340e2662dc64ec81323476b69a97 that added the new
v4l2_subscribed_event_ops introduced a regression: while the code still queued
that initial event the __v4l2_event_queue_fh() function was modified to ignore
such requests if sev->elems was 0 (meaning that the event subscription wasn't
finished yet).
And sev->elems was only set to a non-zero value after the add operation
returned.
This patch fixes this by passing the elems value to the add function. Then the
add function can set it before queuing the initial event.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/video/uvc/uvc_ctrl.c | 5 ++++-
drivers/media/video/v4l2-ctrls.c | 5 ++++-
drivers/media/video/v4l2-event.c | 2 +-
include/media/v4l2-event.h | 2 +-
4 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/media/video/uvc/uvc_ctrl.c b/drivers/media/video/uvc/uvc_ctrl.c
index 28363b7..f3bd66c 100644
--- a/drivers/media/video/uvc/uvc_ctrl.c
+++ b/drivers/media/video/uvc/uvc_ctrl.c
@@ -1250,7 +1250,7 @@ static void uvc_ctrl_send_events(struct uvc_fh *handle,
}
}
-static int uvc_ctrl_add_event(struct v4l2_subscribed_event *sev)
+static int uvc_ctrl_add_event(struct v4l2_subscribed_event *sev, unsigned elems)
{
struct uvc_fh *handle = container_of(sev->fh, struct uvc_fh, vfh);
struct uvc_control_mapping *mapping;
@@ -1278,6 +1278,9 @@ static int uvc_ctrl_add_event(struct v4l2_subscribed_event *sev)
uvc_ctrl_fill_event(handle->chain, &ev, ctrl, mapping, val,
changes);
+ /* Mark the queue as active, allowing this initial
+ event to be accepted. */
+ sev->elems = elems;
v4l2_event_queue_fh(sev->fh, &ev);
}
diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
index ae544d8..20873c2 100644
--- a/drivers/media/video/v4l2-ctrls.c
+++ b/drivers/media/video/v4l2-ctrls.c
@@ -2424,7 +2424,7 @@ int v4l2_ctrl_s_ctrl(struct v4l2_ctrl *ctrl, s32 val)
}
EXPORT_SYMBOL(v4l2_ctrl_s_ctrl);
-static int v4l2_ctrl_add_event(struct v4l2_subscribed_event *sev)
+static int v4l2_ctrl_add_event(struct v4l2_subscribed_event *sev, unsigned elems)
{
struct v4l2_ctrl *ctrl = v4l2_ctrl_find(sev->fh->ctrl_handler, sev->id);
@@ -2441,6 +2441,9 @@ static int v4l2_ctrl_add_event(struct v4l2_subscribed_event *sev)
if (!(ctrl->flags & V4L2_CTRL_FLAG_WRITE_ONLY))
changes |= V4L2_EVENT_CTRL_CH_VALUE;
fill_event(&ev, ctrl, changes);
+ /* Mark the queue as active, allowing this initial
+ event to be accepted. */
+ sev->elems = elems;
v4l2_event_queue_fh(sev->fh, &ev);
}
v4l2_ctrl_unlock(ctrl);
diff --git a/drivers/media/video/v4l2-event.c b/drivers/media/video/v4l2-event.c
index 60b4e2e..ef2a33c 100644
--- a/drivers/media/video/v4l2-event.c
+++ b/drivers/media/video/v4l2-event.c
@@ -239,7 +239,7 @@ int v4l2_event_subscribe(struct v4l2_fh *fh,
}
if (sev->ops && sev->ops->add) {
- int ret = sev->ops->add(sev);
+ int ret = sev->ops->add(sev, elems);
if (ret) {
sev->ops = NULL;
v4l2_event_unsubscribe(fh, sub);
diff --git a/include/media/v4l2-event.h b/include/media/v4l2-event.h
index 88fa9a1..2885a81 100644
--- a/include/media/v4l2-event.h
+++ b/include/media/v4l2-event.h
@@ -85,7 +85,7 @@ struct v4l2_kevent {
* @merge: Optional callback that can merge event 'old' into event 'new'.
*/
struct v4l2_subscribed_event_ops {
- int (*add)(struct v4l2_subscribed_event *sev);
+ int (*add)(struct v4l2_subscribed_event *sev, unsigned elems);
void (*del)(struct v4l2_subscribed_event *sev);
void (*replace)(struct v4l2_event *old, const struct v4l2_event *new);
void (*merge)(const struct v4l2_event *old, struct v4l2_event *new);
--
1.7.10
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH for v3.5] v4l2-event: fix regression with initial event handling.
2012-05-07 20:53 [PATCH for v3.5] v4l2-event: fix regression with initial event handling Hans Verkuil
@ 2012-05-08 7:57 ` Hans de Goede
0 siblings, 0 replies; 2+ messages in thread
From: Hans de Goede @ 2012-05-08 7:57 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Laurent Pinchart, Hans Verkuil
Hi,
Good one, ACK.
Acked-by: Hans de Goede <hdegoede@redhat.com>
Regards,
Hans
On 05/07/2012 10:53 PM, Hans Verkuil wrote:
> From: Hans Verkuil<hans.verkuil@cisco.com>
>
> If the V4L2_EVENT_SUB_FL_SEND_INITIAL was set, then the application expects
> to receive an initial event of the initial value of the control.
>
> However, commit c53c2549333b340e2662dc64ec81323476b69a97 that added the new
> v4l2_subscribed_event_ops introduced a regression: while the code still queued
> that initial event the __v4l2_event_queue_fh() function was modified to ignore
> such requests if sev->elems was 0 (meaning that the event subscription wasn't
> finished yet).
>
> And sev->elems was only set to a non-zero value after the add operation
> returned.
>
> This patch fixes this by passing the elems value to the add function. Then the
> add function can set it before queuing the initial event.
>
> Signed-off-by: Hans Verkuil<hans.verkuil@cisco.com>
> ---
> drivers/media/video/uvc/uvc_ctrl.c | 5 ++++-
> drivers/media/video/v4l2-ctrls.c | 5 ++++-
> drivers/media/video/v4l2-event.c | 2 +-
> include/media/v4l2-event.h | 2 +-
> 4 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/video/uvc/uvc_ctrl.c b/drivers/media/video/uvc/uvc_ctrl.c
> index 28363b7..f3bd66c 100644
> --- a/drivers/media/video/uvc/uvc_ctrl.c
> +++ b/drivers/media/video/uvc/uvc_ctrl.c
> @@ -1250,7 +1250,7 @@ static void uvc_ctrl_send_events(struct uvc_fh *handle,
> }
> }
>
> -static int uvc_ctrl_add_event(struct v4l2_subscribed_event *sev)
> +static int uvc_ctrl_add_event(struct v4l2_subscribed_event *sev, unsigned elems)
> {
> struct uvc_fh *handle = container_of(sev->fh, struct uvc_fh, vfh);
> struct uvc_control_mapping *mapping;
> @@ -1278,6 +1278,9 @@ static int uvc_ctrl_add_event(struct v4l2_subscribed_event *sev)
>
> uvc_ctrl_fill_event(handle->chain,&ev, ctrl, mapping, val,
> changes);
> + /* Mark the queue as active, allowing this initial
> + event to be accepted. */
> + sev->elems = elems;
> v4l2_event_queue_fh(sev->fh,&ev);
> }
>
> diff --git a/drivers/media/video/v4l2-ctrls.c b/drivers/media/video/v4l2-ctrls.c
> index ae544d8..20873c2 100644
> --- a/drivers/media/video/v4l2-ctrls.c
> +++ b/drivers/media/video/v4l2-ctrls.c
> @@ -2424,7 +2424,7 @@ int v4l2_ctrl_s_ctrl(struct v4l2_ctrl *ctrl, s32 val)
> }
> EXPORT_SYMBOL(v4l2_ctrl_s_ctrl);
>
> -static int v4l2_ctrl_add_event(struct v4l2_subscribed_event *sev)
> +static int v4l2_ctrl_add_event(struct v4l2_subscribed_event *sev, unsigned elems)
> {
> struct v4l2_ctrl *ctrl = v4l2_ctrl_find(sev->fh->ctrl_handler, sev->id);
>
> @@ -2441,6 +2441,9 @@ static int v4l2_ctrl_add_event(struct v4l2_subscribed_event *sev)
> if (!(ctrl->flags& V4L2_CTRL_FLAG_WRITE_ONLY))
> changes |= V4L2_EVENT_CTRL_CH_VALUE;
> fill_event(&ev, ctrl, changes);
> + /* Mark the queue as active, allowing this initial
> + event to be accepted. */
> + sev->elems = elems;
> v4l2_event_queue_fh(sev->fh,&ev);
> }
> v4l2_ctrl_unlock(ctrl);
> diff --git a/drivers/media/video/v4l2-event.c b/drivers/media/video/v4l2-event.c
> index 60b4e2e..ef2a33c 100644
> --- a/drivers/media/video/v4l2-event.c
> +++ b/drivers/media/video/v4l2-event.c
> @@ -239,7 +239,7 @@ int v4l2_event_subscribe(struct v4l2_fh *fh,
> }
>
> if (sev->ops&& sev->ops->add) {
> - int ret = sev->ops->add(sev);
> + int ret = sev->ops->add(sev, elems);
> if (ret) {
> sev->ops = NULL;
> v4l2_event_unsubscribe(fh, sub);
> diff --git a/include/media/v4l2-event.h b/include/media/v4l2-event.h
> index 88fa9a1..2885a81 100644
> --- a/include/media/v4l2-event.h
> +++ b/include/media/v4l2-event.h
> @@ -85,7 +85,7 @@ struct v4l2_kevent {
> * @merge: Optional callback that can merge event 'old' into event 'new'.
> */
> struct v4l2_subscribed_event_ops {
> - int (*add)(struct v4l2_subscribed_event *sev);
> + int (*add)(struct v4l2_subscribed_event *sev, unsigned elems);
> void (*del)(struct v4l2_subscribed_event *sev);
> void (*replace)(struct v4l2_event *old, const struct v4l2_event *new);
> void (*merge)(const struct v4l2_event *old, struct v4l2_event *new);
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-05-08 7:57 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-07 20:53 [PATCH for v3.5] v4l2-event: fix regression with initial event handling Hans Verkuil
2012-05-08 7:57 ` Hans de Goede
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.