From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-pa0-f46.google.com ([209.85.220.46]:51368 "EHLO mail-pa0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752651AbaELMQ2 (ORCPT ); Mon, 12 May 2014 08:16:28 -0400 Message-ID: <5370BB96.9060300@samsung.com> Date: Mon, 12 May 2014 17:46:22 +0530 From: Arun Kumar K MIME-Version: 1.0 To: Hans Verkuil , Laurent Pinchart CC: linux-media@vger.kernel.org, linux-samsung-soc@vger.kernel.org, k.debski@samsung.com, s.nawrocki@samsung.com, posciak@chromium.org, arunkk.samsung@gmail.com Subject: Re: [PATCH v3 1/2] [media] v4l: Add source change event References: <1399549756-3743-1-git-send-email-arun.kk@samsung.com> <1399549756-3743-2-git-send-email-arun.kk@samsung.com> <3628141.bT6snEDtDi@avalon> <536CD4DD.1080101@xs4all.nl> In-Reply-To: <536CD4DD.1080101@xs4all.nl> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org List-ID: Hi Hans, Laurent, On 05/09/14 18:45, Hans Verkuil wrote: > On 05/09/2014 03:09 PM, Laurent Pinchart wrote: >> Hi Arun, >> >> Thank you for the patch. We're slowly getting there :-) >> >> On Thursday 08 May 2014 17:19:15 Arun Kumar K wrote: >>> This event indicates that the video device has encountered >>> a source parameter change during runtime. This can typically be a >>> resolution change detected by a video decoder OR a format change >>> detected by an HDMI connector. >>> >>> This needs to be nofified to the userspace and the application may >>> be expected to reallocate buffers before proceeding. The application >>> can subscribe to events on a specific pad or input/output port which >>> it is interested in. >>> >>> Signed-off-by: Arun Kumar K >>> --- >>> Documentation/DocBook/media/v4l/vidioc-dqevent.xml | 32 +++++++++++++++++ >>> .../DocBook/media/v4l/vidioc-subscribe-event.xml | 19 +++++++++++ >>> drivers/media/v4l2-core/v4l2-event.c | 36 +++++++++++++++++ >>> include/media/v4l2-event.h | 4 +++ >>> include/uapi/linux/videodev2.h | 8 +++++ >>> 5 files changed, 99 insertions(+) >>> >>> diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml >>> b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml index 89891ad..6afabaa >>> 100644 >>> --- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml >>> +++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml >>> @@ -242,6 +242,22 @@ >>> >>> >>> >>> + >>> + struct <structname>v4l2_event_src_change</structname> >>> + >>> + &cs-str; >>> + >>> + >>> + __u32 >>> + changes >>> + >>> + A bitmask that tells what has changed. See >> linkend="src-changes-flags" />. + >>> + >>> + >>> + >>> +
>>> + >>> >>> Changes >>> >>> @@ -270,6 +286,22 @@ >>> >>> >>>
>>> + >>> + >>> + Source Changes >>> + >>> + &cs-def; >>> + >>> + >>> + V4L2_EVENT_SRC_CH_RESOLUTION >>> + 0x0001 >>> + This event gets triggered when a resolution change is >>> + detected at runtime. This can typically come from a video decoder. >>> + >>> + >>> + >>> + >>> +
>>> >>> >>> &return-value; >>> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml >>> b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml index >>> 5c70b61..8012829 100644 >>> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml >>> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml >>> @@ -155,6 +155,25 @@ >>> >>> >>> >>> + V4L2_EVENT_SOURCE_CHANGE >>> + 5 >>> + >>> + This event is triggered when a format change is >>> + detected during runtime by the video device. It can be a >>> + runtime resolution change triggered by a video decoder or the >>> + format change happening on an HDMI connector. >>> + This event requires that the id >>> + matches the pad/input/output index from which you want to >>> + receive events. >>> + >>> + This event has a &v4l2-event-source-change; associated >>> + with it. The changes bitfield denotes >>> + what has changed for the subscribed pad. If multiple events >>> + occured before application could dequeue them, then the changes >>> + will have the ORed value of all the events generated. >>> + >>> + >>> + >>> V4L2_EVENT_PRIVATE_START >>> 0x08000000 >>> Base event number for driver-private events. >>> diff --git a/drivers/media/v4l2-core/v4l2-event.c >>> b/drivers/media/v4l2-core/v4l2-event.c index 86dcb54..8761aab 100644 >>> --- a/drivers/media/v4l2-core/v4l2-event.c >>> +++ b/drivers/media/v4l2-core/v4l2-event.c >>> @@ -318,3 +318,39 @@ int v4l2_event_subdev_unsubscribe(struct v4l2_subdev >>> *sd, struct v4l2_fh *fh, return v4l2_event_unsubscribe(fh, sub); >>> } >>> EXPORT_SYMBOL_GPL(v4l2_event_subdev_unsubscribe); >>> + >>> +static void v4l2_event_src_replace(struct v4l2_event *old, >>> + const struct v4l2_event *new) >>> +{ >>> + u32 old_changes = old->u.src_change.changes; >>> + >>> + old->u.src_change = new->u.src_change; >>> + old->u.src_change.changes |= old_changes; >>> +} >>> + >>> +static void v4l2_event_src_merge(const struct v4l2_event *old, >>> + struct v4l2_event *new) >>> +{ >>> + new->u.src_change.changes |= old->u.src_change.changes; >>> +} >>> + >>> +static const struct v4l2_subscribed_event_ops v4l2_event_src_ch_ops = { >>> + .replace = v4l2_event_src_replace, >>> + .merge = v4l2_event_src_merge, >>> +}; >>> + >>> +int v4l2_src_change_event_subscribe(struct v4l2_fh *fh, >>> + const struct v4l2_event_subscription *sub) >>> +{ >>> + if (sub->type == V4L2_EVENT_SOURCE_CHANGE) >>> + return v4l2_event_subscribe(fh, sub, 0, &v4l2_event_src_ch_ops); >>> + return -EINVAL; >>> +} >>> +EXPORT_SYMBOL_GPL(v4l2_src_change_event_subscribe); >>> + >>> +int v4l2_src_change_event_subdev_subscribe(struct v4l2_subdev *sd, >>> + struct v4l2_fh *fh, struct v4l2_event_subscription *sub) >>> +{ >>> + return v4l2_src_change_event_subscribe(fh, sub); >>> +} >>> +EXPORT_SYMBOL_GPL(v4l2_src_change_event_subdev_subscribe); >>> diff --git a/include/media/v4l2-event.h b/include/media/v4l2-event.h >>> index be05d01..1ab9045 100644 >>> --- a/include/media/v4l2-event.h >>> +++ b/include/media/v4l2-event.h >>> @@ -132,4 +132,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh, >>> void v4l2_event_unsubscribe_all(struct v4l2_fh *fh); >>> int v4l2_event_subdev_unsubscribe(struct v4l2_subdev *sd, struct v4l2_fh >>> *fh, struct v4l2_event_subscription *sub); >>> +int v4l2_src_change_event_subscribe(struct v4l2_fh *fh, >>> + const struct v4l2_event_subscription *sub); >>> +int v4l2_src_change_event_subdev_subscribe(struct v4l2_subdev *sd, >>> + struct v4l2_fh *fh, struct v4l2_event_subscription *sub); >>> #endif /* V4L2_EVENT_H */ >>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h >>> index ea468ee..b923d91 100644 >>> --- a/include/uapi/linux/videodev2.h >>> +++ b/include/uapi/linux/videodev2.h >>> @@ -1765,6 +1765,7 @@ struct v4l2_streamparm { >>> #define V4L2_EVENT_EOS 2 >>> #define V4L2_EVENT_CTRL 3 >>> #define V4L2_EVENT_FRAME_SYNC 4 >>> +#define V4L2_EVENT_SOURCE_CHANGE 5 >> >> My last concern is about the event name (and the name of the related >> structure). Either this event applies to inputs only, in which case the >> documentation shouldn't mention the output number as it does above, or the >> event applies to both inputs and outputs, in which case the name "source" is >> too restrictive. I'd be tempted to go for the second option, even though I >> have less use cases in mind on the output side. > > I don't think it makes sense to support this for outputs (I think I originally > suggested that, but I didn't think that through). One practical problem with > that approach would be with m2m devices because that makes it impossible to > tell the difference between a change on the capture side and a change on the > output side, the other is that normally outputs do not change since you are in > control w.r.t. to the format/resolution, etc. > > If there is a need for format change events on an output, then I would expect > to see a separate event for that. > Ok. I will modify the documentation to mention only about pad / input index and keep the same name. Regards Arun > Regards, > > Hans > >> >>> #define V4L2_EVENT_PRIVATE_START 0x08000000 >>> >>> /* Payload for V4L2_EVENT_VSYNC */ >>> @@ -1796,12 +1797,19 @@ struct v4l2_event_frame_sync { >>> __u32 frame_sequence; >>> }; >>> >>> +#define V4L2_EVENT_SRC_CH_RESOLUTION (1 << 0) >>> + >>> +struct v4l2_event_src_change { >>> + __u32 changes; >>> +}; >>> + >>> struct v4l2_event { >>> __u32 type; >>> union { >>> struct v4l2_event_vsync vsync; >>> struct v4l2_event_ctrl ctrl; >>> struct v4l2_event_frame_sync frame_sync; >>> + struct v4l2_event_src_change src_change; >>> __u8 data[64]; >>> } u; >>> __u32 pending; >> >