* [PATCH 1/1] v4l: Add frame end event
@ 2013-11-01 13:10 Sakari Ailus
2013-11-11 12:16 ` Hans Verkuil
2013-11-12 13:47 ` [PATCH v2 " Sakari Ailus
0 siblings, 2 replies; 4+ messages in thread
From: Sakari Ailus @ 2013-11-01 13:10 UTC (permalink / raw)
To: linux-media; +Cc: vinod.govindapillai
Add an event to signal frame end. This is not necessarily the same timestamp
as the video buffer done timestamp, and can be also subscribed by other
processes than the one controlling streaming and buffer (de)queueing.
Also make all event type constants appear as constants in documentation and
move frame sync event struct documentation after all control event
documentation.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
Hi folks,
As we have a frame sync event that's being used to tell about the frame
start, I thought of having a frame end event as well. This isn't exactly the
same as a buffer ready event which could take place already earlier for
instance if cropping is being done.
I propose to use the id field for the purpose (V4L2_EVENT_FRAME_SYNC_START /
V4L2_EVENT_FRAME_SYNC_END). The frame start event will retain its old id
zero.
Originally I think we thought of using the id field for the line, but
now I think it's worth adding a distinct event for that purpose: line based
events are typically triggered from other sources than line based events.
Frame sync, in my opinion, matches better with frame start and frame end
than to anything related to lines.
So should line based events be supported, they should have their own event
type and use the id field as the line number.
Kind regards,
Sakari
Documentation/DocBook/media/v4l/vidioc-dqevent.xml | 58 +++++++++++++++-------
.../DocBook/media/v4l/vidioc-subscribe-event.xml | 11 +++-
include/uapi/linux/videodev2.h | 3 ++
3 files changed, 51 insertions(+), 21 deletions(-)
diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
index 89891ad..e258c6e 100644
--- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
@@ -76,21 +76,22 @@
<entry></entry>
<entry>&v4l2-event-vsync;</entry>
<entry><structfield>vsync</structfield></entry>
- <entry>Event data for event V4L2_EVENT_VSYNC.
+ <entry>Event data for event <constant>V4L2_EVENT_VSYNC</constant>.
</entry>
</row>
<row>
<entry></entry>
<entry>&v4l2-event-ctrl;</entry>
<entry><structfield>ctrl</structfield></entry>
- <entry>Event data for event V4L2_EVENT_CTRL.
+ <entry>Event data for event <constant>V4L2_EVENT_CTRL</constant>.
</entry>
</row>
<row>
<entry></entry>
<entry>&v4l2-event-frame-sync;</entry>
<entry><structfield>frame_sync</structfield></entry>
- <entry>Event data for event V4L2_EVENT_FRAME_SYNC.</entry>
+ <entry>Event data for event <constant>V4L2_EVENT_FRAME_SYNC
+ </constant>.</entry>
</row>
<row>
<entry></entry>
@@ -226,22 +227,6 @@
</tgroup>
</table>
- <table frame="none" pgwide="1" id="v4l2-event-frame-sync">
- <title>struct <structname>v4l2_event_frame_sync</structname></title>
- <tgroup cols="3">
- &cs-str;
- <tbody valign="top">
- <row>
- <entry>__u32</entry>
- <entry><structfield>frame_sequence</structfield></entry>
- <entry>
- The sequence number of the frame being received.
- </entry>
- </row>
- </tbody>
- </tgroup>
- </table>
-
<table pgwide="1" frame="none" id="changes-flags">
<title>Changes</title>
<tgroup cols="3">
@@ -270,6 +255,41 @@
</tbody>
</tgroup>
</table>
+
+ <table frame="none" pgwide="1" id="v4l2-event-frame-sync">
+ <title>struct <structname>v4l2_event_frame_sync</structname></title>
+ <tgroup cols="3">
+ &cs-str;
+ <tbody valign="top">
+ <row>
+ <entry>__u32</entry>
+ <entry><structfield>frame_sequence</structfield></entry>
+ <entry>
+ The sequence number of the frame being received.
+ </entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
+
+ <table pgwide="1" frame="none" id="v4l2-event-frame-sync-ids">
+ <title>Frame sync event IDs</title>
+ <tgroup cols="3">
+ &cs-def;
+ <tbody valign="top">
+ <row>
+ <entry><constant>V4L2_EVENT_FRAME_SYNC_START</constant></entry>
+ <entry>0x0000</entry>
+ <entry>Frame sync event delivered at frame start.</entry>
+ </row>
+ <row>
+ <entry><constant>V4L2_EVENT_FRAME_SYNC_END</constant></entry>
+ <entry>0x0001</entry>
+ <entry>Frame sync event delivered at frame end.</entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
</refsect1>
<refsect1>
&return-value;
diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
index 5c70b61..aaa6fe1 100644
--- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
@@ -143,15 +143,22 @@
<entry>4</entry>
<entry>
<para>Triggered immediately when the reception of a
- frame has begun. This event has a
+ frame has begun or ended. This event has a
&v4l2-event-frame-sync; associated with it.</para>
<para>If the hardware needs to be stopped in the case of a
buffer underrun it might not be able to generate this event.
In such cases the <structfield>frame_sequence</structfield>
field in &v4l2-event-frame-sync; will not be incremented. This
- causes two consecutive frame sequence numbers to have n times
+ causes two consecutive frame sync events to have n times
frame interval in between them.</para>
+
+ <para>The <structfield>id</structfield> field must be set to
+ either <constant>V4L2_EVENT_FRAME_SYNC_START</constant> or
+ <constant>V4L2_EVENT_FRAME_SYNC_END</constant> which signify
+ the start of frame and end of frame, respectively. See
+ <xref linkend="v4l2-event-frame-sync-ids" /> for more
+ information.</para>
</entry>
</row>
<row>
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 437f1b0..133957a 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1760,6 +1760,9 @@ struct v4l2_event_ctrl {
__s32 default_value;
};
+#define V4L2_EVENT_FRAME_SYNC_START 0
+#define V4L2_EVENT_FRAME_SYNC_END 1
+
struct v4l2_event_frame_sync {
__u32 frame_sequence;
};
--
1.8.3.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 1/1] v4l: Add frame end event
2013-11-01 13:10 [PATCH 1/1] v4l: Add frame end event Sakari Ailus
@ 2013-11-11 12:16 ` Hans Verkuil
2013-11-11 17:27 ` Sakari Ailus
2013-11-12 13:47 ` [PATCH v2 " Sakari Ailus
1 sibling, 1 reply; 4+ messages in thread
From: Hans Verkuil @ 2013-11-11 12:16 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media, vinod.govindapillai
Hi Sakari,
On 11/01/2013 02:10 PM, Sakari Ailus wrote:
> Add an event to signal frame end. This is not necessarily the same timestamp
> as the video buffer done timestamp, and can be also subscribed by other
> processes than the one controlling streaming and buffer (de)queueing.
>
> Also make all event type constants appear as constants in documentation and
> move frame sync event struct documentation after all control event
> documentation.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> Hi folks,
>
> As we have a frame sync event that's being used to tell about the frame
> start, I thought of having a frame end event as well. This isn't exactly the
> same as a buffer ready event which could take place already earlier for
> instance if cropping is being done.
>
> I propose to use the id field for the purpose (V4L2_EVENT_FRAME_SYNC_START /
> V4L2_EVENT_FRAME_SYNC_END). The frame start event will retain its old id
> zero.
>
> Originally I think we thought of using the id field for the line, but
> now I think it's worth adding a distinct event for that purpose: line based
> events are typically triggered from other sources than line based events.
> Frame sync, in my opinion, matches better with frame start and frame end
> than to anything related to lines.
>
> So should line based events be supported, they should have their own event
> type and use the id field as the line number.
I have no objections to this patch. You do need to adapt drivers/media/platform/omap3isp/ispccdc.c
a bit since it is using the FRAME_SYNC event and so it should check the id field.
But will you also be upstreaming a driver that uses the SYNC_END?
I don't really want to merge this if nobody is using it.
Regards,
Hans
>
> Kind regards,
> Sakari
>
> Documentation/DocBook/media/v4l/vidioc-dqevent.xml | 58 +++++++++++++++-------
> .../DocBook/media/v4l/vidioc-subscribe-event.xml | 11 +++-
> include/uapi/linux/videodev2.h | 3 ++
> 3 files changed, 51 insertions(+), 21 deletions(-)
>
> diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> index 89891ad..e258c6e 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
> @@ -76,21 +76,22 @@
> <entry></entry>
> <entry>&v4l2-event-vsync;</entry>
> <entry><structfield>vsync</structfield></entry>
> - <entry>Event data for event V4L2_EVENT_VSYNC.
> + <entry>Event data for event <constant>V4L2_EVENT_VSYNC</constant>.
> </entry>
> </row>
> <row>
> <entry></entry>
> <entry>&v4l2-event-ctrl;</entry>
> <entry><structfield>ctrl</structfield></entry>
> - <entry>Event data for event V4L2_EVENT_CTRL.
> + <entry>Event data for event <constant>V4L2_EVENT_CTRL</constant>.
> </entry>
> </row>
> <row>
> <entry></entry>
> <entry>&v4l2-event-frame-sync;</entry>
> <entry><structfield>frame_sync</structfield></entry>
> - <entry>Event data for event V4L2_EVENT_FRAME_SYNC.</entry>
> + <entry>Event data for event <constant>V4L2_EVENT_FRAME_SYNC
> + </constant>.</entry>
> </row>
> <row>
> <entry></entry>
> @@ -226,22 +227,6 @@
> </tgroup>
> </table>
>
> - <table frame="none" pgwide="1" id="v4l2-event-frame-sync">
> - <title>struct <structname>v4l2_event_frame_sync</structname></title>
> - <tgroup cols="3">
> - &cs-str;
> - <tbody valign="top">
> - <row>
> - <entry>__u32</entry>
> - <entry><structfield>frame_sequence</structfield></entry>
> - <entry>
> - The sequence number of the frame being received.
> - </entry>
> - </row>
> - </tbody>
> - </tgroup>
> - </table>
> -
> <table pgwide="1" frame="none" id="changes-flags">
> <title>Changes</title>
> <tgroup cols="3">
> @@ -270,6 +255,41 @@
> </tbody>
> </tgroup>
> </table>
> +
> + <table frame="none" pgwide="1" id="v4l2-event-frame-sync">
> + <title>struct <structname>v4l2_event_frame_sync</structname></title>
> + <tgroup cols="3">
> + &cs-str;
> + <tbody valign="top">
> + <row>
> + <entry>__u32</entry>
> + <entry><structfield>frame_sequence</structfield></entry>
> + <entry>
> + The sequence number of the frame being received.
> + </entry>
> + </row>
> + </tbody>
> + </tgroup>
> + </table>
> +
> + <table pgwide="1" frame="none" id="v4l2-event-frame-sync-ids">
> + <title>Frame sync event IDs</title>
> + <tgroup cols="3">
> + &cs-def;
> + <tbody valign="top">
> + <row>
> + <entry><constant>V4L2_EVENT_FRAME_SYNC_START</constant></entry>
> + <entry>0x0000</entry>
> + <entry>Frame sync event delivered at frame start.</entry>
> + </row>
> + <row>
> + <entry><constant>V4L2_EVENT_FRAME_SYNC_END</constant></entry>
> + <entry>0x0001</entry>
> + <entry>Frame sync event delivered at frame end.</entry>
> + </row>
> + </tbody>
> + </tgroup>
> + </table>
> </refsect1>
> <refsect1>
> &return-value;
> diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> index 5c70b61..aaa6fe1 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
> @@ -143,15 +143,22 @@
> <entry>4</entry>
> <entry>
> <para>Triggered immediately when the reception of a
> - frame has begun. This event has a
> + frame has begun or ended. This event has a
> &v4l2-event-frame-sync; associated with it.</para>
>
> <para>If the hardware needs to be stopped in the case of a
> buffer underrun it might not be able to generate this event.
> In such cases the <structfield>frame_sequence</structfield>
> field in &v4l2-event-frame-sync; will not be incremented. This
> - causes two consecutive frame sequence numbers to have n times
> + causes two consecutive frame sync events to have n times
> frame interval in between them.</para>
> +
> + <para>The <structfield>id</structfield> field must be set to
> + either <constant>V4L2_EVENT_FRAME_SYNC_START</constant> or
> + <constant>V4L2_EVENT_FRAME_SYNC_END</constant> which signify
> + the start of frame and end of frame, respectively. See
> + <xref linkend="v4l2-event-frame-sync-ids" /> for more
> + information.</para>
> </entry>
> </row>
> <row>
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 437f1b0..133957a 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -1760,6 +1760,9 @@ struct v4l2_event_ctrl {
> __s32 default_value;
> };
>
> +#define V4L2_EVENT_FRAME_SYNC_START 0
> +#define V4L2_EVENT_FRAME_SYNC_END 1
> +
> struct v4l2_event_frame_sync {
> __u32 frame_sequence;
> };
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 1/1] v4l: Add frame end event
2013-11-11 12:16 ` Hans Verkuil
@ 2013-11-11 17:27 ` Sakari Ailus
0 siblings, 0 replies; 4+ messages in thread
From: Sakari Ailus @ 2013-11-11 17:27 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, vinod.govindapillai
Hi Hans,
Thanks for your comments.
Hans Verkuil wrote:
...
> I have no objections to this patch. You do need to adapt drivers/media/platform/omap3isp/ispccdc.c
> a bit since it is using the FRAME_SYNC event and so it should check the id field.
Good point.
> But will you also be upstreaming a driver that uses the SYNC_END?
>
> I don't really want to merge this if nobody is using it.
I agree --- I can't say right now when there could be an upstreamable
driver using that event. Let's keep it out of the tree for now.
Especially that after giving some thought to the multi stream use cases
--- now arguing against my own proposal ;-) --- the "id" field would be
better used to make a difference between different streams, especially
for the frame start event. We're not exactly running out of possible
values for the type field.
So I'd now prefer an entirely separate event to tell about the frame
end, and perhaps add an alias for the frame sync event (FRAME_START).
(This again is a proof of why things that are not going to get used
pretty much immediately should almost never be merged.) I can send a
patch on that as well, and, should someone else need it, that one can be
merged, naturally after a review.
--
Kind regards,
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/1] v4l: Add frame end event
2013-11-01 13:10 [PATCH 1/1] v4l: Add frame end event Sakari Ailus
2013-11-11 12:16 ` Hans Verkuil
@ 2013-11-12 13:47 ` Sakari Ailus
1 sibling, 0 replies; 4+ messages in thread
From: Sakari Ailus @ 2013-11-12 13:47 UTC (permalink / raw)
To: linux-media; +Cc: hverkuil, vinod.govindapillai
Add an event to signal frame end. This is not necessarily the same timestamp
as the video buffer done timestamp, and can be also subscribed by other
processes than the one controlling streaming and buffer (de)queueing.
Also make all event type constants appear as constants in documentation and
move frame sync event struct documentation after all control event
documentation.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
Hi,
I decided it'd be better not to use the id field to tell it's frame end ---
that field would be better used for stream id (and we're not exactly running
out of possible values for type).
As discussed previously, I don't know when this would be needed in mainline.
Regards,
Sakari
Documentation/DocBook/media/v4l/vidioc-dqevent.xml | 39 +++++++++++-----------
.../DocBook/media/v4l/vidioc-subscribe-event.xml | 18 +++++++++-
include/uapi/linux/videodev2.h | 2 ++
3 files changed, 39 insertions(+), 20 deletions(-)
diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
index 89891ad..0fdaa2e 100644
--- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml
@@ -76,21 +76,22 @@
<entry></entry>
<entry>&v4l2-event-vsync;</entry>
<entry><structfield>vsync</structfield></entry>
- <entry>Event data for event V4L2_EVENT_VSYNC.
+ <entry>Event data for event <constant>V4L2_EVENT_VSYNC</constant>.
</entry>
</row>
<row>
<entry></entry>
<entry>&v4l2-event-ctrl;</entry>
<entry><structfield>ctrl</structfield></entry>
- <entry>Event data for event V4L2_EVENT_CTRL.
+ <entry>Event data for event <constant>V4L2_EVENT_CTRL</constant>.
</entry>
</row>
<row>
<entry></entry>
<entry>&v4l2-event-frame-sync;</entry>
<entry><structfield>frame_sync</structfield></entry>
- <entry>Event data for event V4L2_EVENT_FRAME_SYNC.</entry>
+ <entry>Event data for event <constant>V4L2_EVENT_FRAME_SYNC
+ </constant> and <constant>V4L2_EVENT_FRAME_END</constant>.</entry>
</row>
<row>
<entry></entry>
@@ -226,22 +227,6 @@
</tgroup>
</table>
- <table frame="none" pgwide="1" id="v4l2-event-frame-sync">
- <title>struct <structname>v4l2_event_frame_sync</structname></title>
- <tgroup cols="3">
- &cs-str;
- <tbody valign="top">
- <row>
- <entry>__u32</entry>
- <entry><structfield>frame_sequence</structfield></entry>
- <entry>
- The sequence number of the frame being received.
- </entry>
- </row>
- </tbody>
- </tgroup>
- </table>
-
<table pgwide="1" frame="none" id="changes-flags">
<title>Changes</title>
<tgroup cols="3">
@@ -270,6 +255,22 @@
</tbody>
</tgroup>
</table>
+
+ <table frame="none" pgwide="1" id="v4l2-event-frame-sync">
+ <title>struct <structname>v4l2_event_frame_sync</structname></title>
+ <tgroup cols="3">
+ &cs-str;
+ <tbody valign="top">
+ <row>
+ <entry>__u32</entry>
+ <entry><structfield>frame_sequence</structfield></entry>
+ <entry>
+ The sequence number of the frame being received.
+ </entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
</refsect1>
<refsect1>
&return-value;
diff --git a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
index 5c70b61..406e5e0 100644
--- a/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-subscribe-event.xml
@@ -150,7 +150,23 @@
buffer underrun it might not be able to generate this event.
In such cases the <structfield>frame_sequence</structfield>
field in &v4l2-event-frame-sync; will not be incremented. This
- causes two consecutive frame sequence numbers to have n times
+ causes two consecutive frame sync events to have n times
+ frame interval in between them.</para>
+ </entry>
+ </row>
+ <row>
+ <entry><constant>V4L2_EVENT_FRAME_END</constant></entry>
+ <entry>5</entry>
+ <entry>
+ <para>Triggered immediately when the reception of a
+ frame has ended. This event has a
+ &v4l2-event-frame-sync; associated with it.</para>
+
+ <para>If the hardware needs to be stopped in the case of a
+ buffer underrun it might not be able to generate this event.
+ In such cases the <structfield>frame_sequence</structfield>
+ field in &v4l2-event-frame-sync; will not be incremented. This
+ causes two consecutive frame end events to have n times
frame interval in between them.</para>
</entry>
</row>
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 437f1b0..2307e1a 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1733,6 +1733,7 @@ struct v4l2_streamparm {
#define V4L2_EVENT_EOS 2
#define V4L2_EVENT_CTRL 3
#define V4L2_EVENT_FRAME_SYNC 4
+#define V4L2_EVENT_FRAME_END 5
#define V4L2_EVENT_PRIVATE_START 0x08000000
/* Payload for V4L2_EVENT_VSYNC */
@@ -1760,6 +1761,7 @@ struct v4l2_event_ctrl {
__s32 default_value;
};
+/* V4L2_EVENT_FRAME_SYNC or V4L2_EVENT_FRAME_END */
struct v4l2_event_frame_sync {
__u32 frame_sequence;
};
--
1.8.3.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-11-12 13:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-01 13:10 [PATCH 1/1] v4l: Add frame end event Sakari Ailus
2013-11-11 12:16 ` Hans Verkuil
2013-11-11 17:27 ` Sakari Ailus
2013-11-12 13:47 ` [PATCH v2 " Sakari Ailus
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).