[PATCH v4 0/3] Fix buffer timestamp documentation

[PATCH v4 0/3] Fix buffer timestamp documentation

[Sakari Ailus]

Hi,

This patchset fixes the documentation related to V4L2 buffer timestamps.
Timestamps in the vast majority of drivers is taken af the end of the frame
rather than at the start of it.

since v3:

Besides the first patch, as suggested, I've added two more to add a new
v4l2_buffer.flags flag (V4L2_BUF_FLAG_TIMESTAMP_SOF) to tell the timestamp
is taken at the start of the frame. What is also changed in the
documentation is that the timestamps are end-of-frame by default and
start-of-frame when the flag is set.

-- 
Kind regards,
Sakari

[PATCH v4 1/3] v4l: Document timestamp behaviour to correspond to reality

[Sakari Ailus]

Document that monotonic timestamps are taken after the corresponding frame
has been received, not when the reception has begun. This corresponds to the
reality of current drivers: the timestamp is naturally taken when the
hardware triggers an interrupt to tell the driver to handle the received
frame.

Remove the note on timestamp accurary as it is fairly subjective what is
actually an unstable timestamp.

Also remove explanation that output buffer timestamps can be used to delay
outputting a frame.

Remove the footnote saying we always use realtime clock.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 Documentation/DocBook/media/v4l/io.xml |   47 ++++++--------------------------
 1 file changed, 8 insertions(+), 39 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
index 2c4c068..cd5f9de 100644
--- a/Documentation/DocBook/media/v4l/io.xml
+++ b/Documentation/DocBook/media/v4l/io.xml
@@ -654,38 +654,11 @@ plane, are stored in struct <structname>v4l2_plane</structname> instead.
 In that case, struct <structname>v4l2_buffer</structname> contains an array of
 plane structures.</para>
 
-      <para>Nominally timestamps refer to the first data byte transmitted.
-In practice however the wide range of hardware covered by the V4L2 API
-limits timestamp accuracy. Often an interrupt routine will
-sample the system clock shortly after the field or frame was stored
-completely in memory. So applications must expect a constant
-difference up to one field or frame period plus a small (few scan
-lines) random error. The delay and error can be much
-larger due to compression or transmission over an external bus when
-the frames are not properly stamped by the sender. This is frequently
-the case with USB cameras. Here timestamps refer to the instant the
-field or frame was received by the driver, not the capture time. These
-devices identify by not enumerating any video standards, see <xref
-linkend="standard" />.</para>
-
-      <para>Similar limitations apply to output timestamps. Typically
-the video hardware locks to a clock controlling the video timing, the
-horizontal and vertical synchronization pulses. At some point in the
-line sequence, possibly the vertical blanking, an interrupt routine
-samples the system clock, compares against the timestamp and programs
-the hardware to repeat the previous field or frame, or to display the
-buffer contents.</para>
-
-      <para>Apart of limitations of the video device and natural
-inaccuracies of all clocks, it should be noted system time itself is
-not perfectly stable. It can be affected by power saving cycles,
-warped to insert leap seconds, or even turned back or forth by the
-system administrator affecting long term measurements. <footnote>
-	  <para>Since no other Linux multimedia
-API supports unadjusted time it would be foolish to introduce here. We
-must use a universally supported clock to synchronize different media,
-hence time of day.</para>
-	</footnote></para>
+      <para>On timestamp types that are sampled from the system clock
+(V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC) it is guaranteed that the timestamp is
+taken after the complete frame has been received (or transmitted in
+case of video output devices). For other kinds of
+timestamps this may vary depending on the driver.</para>
 
     <table frame="none" pgwide="1" id="v4l2-buffer">
       <title>struct <structname>v4l2_buffer</structname></title>
@@ -745,13 +718,9 @@ applications when an output stream.</entry>
 	    byte was captured, as returned by the
 	    <function>clock_gettime()</function> function for the relevant
 	    clock id; see <constant>V4L2_BUF_FLAG_TIMESTAMP_*</constant> in
-	    <xref linkend="buffer-flags" />. For output streams the data
-	    will not be displayed before this time, secondary to the nominal
-	    frame rate determined by the current video standard in enqueued
-	    order. Applications can for example zero this field to display
-	    frames as soon as possible. The driver stores the time at which
-	    the first data byte was actually sent out in the
-	    <structfield>timestamp</structfield> field. This permits
+	    <xref linkend="buffer-flags" />. For output streams he driver
+	    stores the time at which the first data byte was actually sent out
+	    in the  <structfield>timestamp</structfield> field. This permits
 	    applications to monitor the drift between the video and system
 	    clock.</para></entry>
 	  </row>
-- 
1.7.10.4

[PATCH v4 2/3] v4l: Use full 32 bits for buffer flags

[Sakari Ailus]

The buffer flags field is 32 bits but the defined only used 16. This is
fine, but as more than 16 bits will be used in the very near future, define
them as 32-bit numbers for consistency.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 Documentation/DocBook/media/v4l/io.xml |   30 ++++++++++++-------------
 include/uapi/linux/videodev2.h         |   38 +++++++++++++++++++-------------
 2 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
index cd5f9de..b9a83bc 100644
--- a/Documentation/DocBook/media/v4l/io.xml
+++ b/Documentation/DocBook/media/v4l/io.xml
@@ -985,7 +985,7 @@ should set this to 0.</entry>
 	<tbody valign="top">
 	  <row>
 	    <entry><constant>V4L2_BUF_FLAG_MAPPED</constant></entry>
-	    <entry>0x0001</entry>
+	    <entry>0x00000001</entry>
 	    <entry>The buffer resides in device memory and has been mapped
 into the application's address space, see <xref linkend="mmap" /> for details.
 Drivers set or clear this flag when the
@@ -995,7 +995,7 @@ Drivers set or clear this flag when the
 	  </row>
 	  <row>
 	    <entry><constant>V4L2_BUF_FLAG_QUEUED</constant></entry>
-	    <entry>0x0002</entry>
+	    <entry>0x00000002</entry>
 	  <entry>Internally drivers maintain two buffer queues, an
 incoming and outgoing queue. When this flag is set, the buffer is
 currently on the incoming queue. It automatically moves to the
@@ -1008,7 +1008,7 @@ cleared.</entry>
 	  </row>
 	  <row>
 	    <entry><constant>V4L2_BUF_FLAG_DONE</constant></entry>
-	    <entry>0x0004</entry>
+	    <entry>0x00000004</entry>
 	    <entry>When this flag is set, the buffer is currently on
 the outgoing queue, ready to be dequeued from the driver. Drivers set
 or clear this flag when the <constant>VIDIOC_QUERYBUF</constant> ioctl
@@ -1022,7 +1022,7 @@ state, in the application domain to say so.</entry>
 	  </row>
 	  <row>
 	    <entry><constant>V4L2_BUF_FLAG_ERROR</constant></entry>
-	    <entry>0x0040</entry>
+	    <entry>0x00000040</entry>
 	    <entry>When this flag is set, the buffer has been dequeued
 	    successfully, although the data might have been corrupted.
 	    This is recoverable, streaming may continue as normal and
@@ -1032,7 +1032,7 @@ state, in the application domain to say so.</entry>
 	  </row>
 	  <row>
 	    <entry><constant>V4L2_BUF_FLAG_KEYFRAME</constant></entry>
-	    <entry>0x0008</entry>
+	    <entry>0x00000008</entry>
 	  <entry>Drivers set or clear this flag when calling the
 <constant>VIDIOC_DQBUF</constant> ioctl. It may be set by video
 capture devices when the buffer contains a compressed image which is a
@@ -1040,27 +1040,27 @@ key frame (or field), &ie; can be decompressed on its own.</entry>
 	  </row>
 	  <row>
 	    <entry><constant>V4L2_BUF_FLAG_PFRAME</constant></entry>
-	    <entry>0x0010</entry>
+	    <entry>0x00000010</entry>
 	    <entry>Similar to <constant>V4L2_BUF_FLAG_KEYFRAME</constant>
 this flags predicted frames or fields which contain only differences to a
 previous key frame.</entry>
 	  </row>
 	  <row>
 	    <entry><constant>V4L2_BUF_FLAG_BFRAME</constant></entry>
-	    <entry>0x0020</entry>
+	    <entry>0x00000020</entry>
 	    <entry>Similar to <constant>V4L2_BUF_FLAG_PFRAME</constant>
 	this is a bidirectional predicted frame or field. [ooc tbd]</entry>
 	  </row>
 	  <row>
 	    <entry><constant>V4L2_BUF_FLAG_TIMECODE</constant></entry>
-	    <entry>0x0100</entry>
+	    <entry>0x00000100</entry>
 	    <entry>The <structfield>timecode</structfield> field is valid.
 Drivers set or clear this flag when the <constant>VIDIOC_DQBUF</constant>
 ioctl is called.</entry>
 	  </row>
 	  <row>
 	    <entry><constant>V4L2_BUF_FLAG_PREPARED</constant></entry>
-	    <entry>0x0400</entry>
+	    <entry>0x00000400</entry>
 	    <entry>The buffer has been prepared for I/O and can be queued by the
 application. Drivers set or clear this flag when the
 <link linkend="vidioc-querybuf">VIDIOC_QUERYBUF</link>, <link
@@ -1070,7 +1070,7 @@ application. Drivers set or clear this flag when the
 	  </row>
 	  <row>
 	    <entry><constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant></entry>
-	    <entry>0x0800</entry>
+	    <entry>0x00000800</entry>
 	    <entry>Caches do not have to be invalidated for this buffer.
 Typically applications shall use this flag if the data captured in the buffer
 is not going to be touched by the CPU, instead the buffer will, probably, be
@@ -1079,7 +1079,7 @@ passed on to a DMA-capable hardware unit for further processing or output.
 	  </row>
 	  <row>
 	    <entry><constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant></entry>
-	    <entry>0x1000</entry>
+	    <entry>0x00001000</entry>
 	    <entry>Caches do not have to be cleaned for this buffer.
 Typically applications shall use this flag for output buffers if the data
 in this buffer has not been created by the CPU but by some DMA-capable unit,
@@ -1087,7 +1087,7 @@ in which case caches have not been used.</entry>
 	  </row>
 	  <row>
 	    <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_MASK</constant></entry>
-	    <entry>0xe000</entry>
+	    <entry>0x0000e000</entry>
 	    <entry>Mask for timestamp types below. To test the
 	    timestamp type, mask out bits not belonging to timestamp
 	    type by performing a logical and operation with buffer
@@ -1095,7 +1095,7 @@ in which case caches have not been used.</entry>
 	  </row>
 	  <row>
 	    <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN</constant></entry>
-	    <entry>0x0000</entry>
+	    <entry>0x00000000</entry>
 	    <entry>Unknown timestamp type. This type is used by
 	    drivers before Linux 3.9 and may be either monotonic (see
 	    below) or realtime (wall clock). Monotonic clock has been
@@ -1108,7 +1108,7 @@ in which case caches have not been used.</entry>
 	  </row>
 	  <row>
 	    <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC</constant></entry>
-	    <entry>0x2000</entry>
+	    <entry>0x00002000</entry>
 	    <entry>The buffer timestamp has been taken from the
 	    <constant>CLOCK_MONOTONIC</constant> clock. To access the
 	    same clock outside V4L2, use
@@ -1116,7 +1116,7 @@ in which case caches have not been used.</entry>
 	  </row>
 	  <row>
 	    <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_COPY</constant></entry>
-	    <entry>0x4000</entry>
+	    <entry>0x00004000</entry>
 	    <entry>The CAPTURE buffer timestamp has been taken from the
 	    corresponding OUTPUT buffer. This flag applies only to mem2mem devices.</entry>
 	  </row>
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 437f1b0..691077d 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -669,24 +669,32 @@ struct v4l2_buffer {
 };
 
 /*  Flags for 'flags' field */
-#define V4L2_BUF_FLAG_MAPPED	0x0001  /* Buffer is mapped (flag) */
-#define V4L2_BUF_FLAG_QUEUED	0x0002	/* Buffer is queued for processing */
-#define V4L2_BUF_FLAG_DONE	0x0004	/* Buffer is ready */
-#define V4L2_BUF_FLAG_KEYFRAME	0x0008	/* Image is a keyframe (I-frame) */
-#define V4L2_BUF_FLAG_PFRAME	0x0010	/* Image is a P-frame */
-#define V4L2_BUF_FLAG_BFRAME	0x0020	/* Image is a B-frame */
+/* Buffer is mapped (flag) */
+#define V4L2_BUF_FLAG_MAPPED			0x00000001
+/* Buffer is queued for processing */
+#define V4L2_BUF_FLAG_QUEUED			0x00000002
+/* Buffer is ready */
+#define V4L2_BUF_FLAG_DONE			0x00000004
+/* Image is a keyframe (I-frame) */
+#define V4L2_BUF_FLAG_KEYFRAME			0x00000008
+/* Image is a P-frame */
+#define V4L2_BUF_FLAG_PFRAME			0x00000010
+/* Image is a B-frame */
+#define V4L2_BUF_FLAG_BFRAME			0x00000020
 /* Buffer is ready, but the data contained within is corrupted. */
-#define V4L2_BUF_FLAG_ERROR	0x0040
-#define V4L2_BUF_FLAG_TIMECODE	0x0100	/* timecode field is valid */
-#define V4L2_BUF_FLAG_PREPARED	0x0400	/* Buffer is prepared for queuing */
+#define V4L2_BUF_FLAG_ERROR			0x00000040
+/* timecode field is valid */
+#define V4L2_BUF_FLAG_TIMECODE			0x00000100
+/* Buffer is prepared for queuing */
+#define V4L2_BUF_FLAG_PREPARED			0x00000400
 /* Cache handling flags */
-#define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE	0x0800
-#define V4L2_BUF_FLAG_NO_CACHE_CLEAN		0x1000
+#define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE	0x00000800
+#define V4L2_BUF_FLAG_NO_CACHE_CLEAN		0x00001000
 /* Timestamp type */
-#define V4L2_BUF_FLAG_TIMESTAMP_MASK		0xe000
-#define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN		0x0000
-#define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC	0x2000
-#define V4L2_BUF_FLAG_TIMESTAMP_COPY		0x4000
+#define V4L2_BUF_FLAG_TIMESTAMP_MASK		0x0000e000
+#define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN		0x00000000
+#define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC	0x00002000
+#define V4L2_BUF_FLAG_TIMESTAMP_COPY		0x00004000
 
 /**
  * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor
-- 
1.7.10.4

[PATCH v4 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Sakari Ailus]

Some devices such as the uvc produce timestamps at the beginning of the
frame rather than at the end of it. Add a buffer flag
(V4L2_BUF_FLAG_TIMESTAMP_SOF) to tell about this.

Also document timestamp_type in struct vb2_queue.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 Documentation/DocBook/media/v4l/io.xml |   17 ++++++++++++-----
 drivers/media/usb/uvc/uvc_queue.c      |    3 ++-
 include/media/videobuf2-core.h         |    1 +
 include/uapi/linux/videodev2.h         |   10 ++++++++++
 4 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
index b9a83bc..d3a725c 100644
--- a/Documentation/DocBook/media/v4l/io.xml
+++ b/Documentation/DocBook/media/v4l/io.xml
@@ -654,11 +654,11 @@ plane, are stored in struct <structname>v4l2_plane</structname> instead.
 In that case, struct <structname>v4l2_buffer</structname> contains an array of
 plane structures.</para>
 
-      <para>On timestamp types that are sampled from the system clock
-(V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC) it is guaranteed that the timestamp is
-taken after the complete frame has been received (or transmitted in
-case of video output devices). For other kinds of
-timestamps this may vary depending on the driver.</para>
+      <para>The timestamp is taken once the complete frame has been
+received unless <constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant>
+buffer flag is set. If <constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant>
+is set, the timestamp is taken when the first pixel of the frame is
+received.</para>
 
     <table frame="none" pgwide="1" id="v4l2-buffer">
       <title>struct <structname>v4l2_buffer</structname></title>
@@ -1120,6 +1120,13 @@ in which case caches have not been used.</entry>
 	    <entry>The CAPTURE buffer timestamp has been taken from the
 	    corresponding OUTPUT buffer. This flag applies only to mem2mem devices.</entry>
 	  </row>
+	  <row>
+	    <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant></entry>
+	    <entry>0x00010000</entry>
+	    <entry>The buffer timestamp has been taken when the first
+	    pixel is received. If this flag is not set, the timestamp
+	    is taken when the entire frame has been received.</entry>
+	  </row>
 	</tbody>
       </tgroup>
     </table>
diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
index cd962be..0d80512 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -149,7 +149,8 @@ int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type,
 	queue->queue.buf_struct_size = sizeof(struct uvc_buffer);
 	queue->queue.ops = &uvc_queue_qops;
 	queue->queue.mem_ops = &vb2_vmalloc_memops;
-	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC
+		| V4L2_BUF_FLAG_TIMESTAMP_SOF;
 	ret = vb2_queue_init(&queue->queue);
 	if (ret)
 		return ret;
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 6781258..6eb2d59 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -307,6 +307,7 @@ struct v4l2_fh;
  * @buf_struct_size: size of the driver-specific buffer structure;
  *		"0" indicates the driver doesn't want to use a custom buffer
  *		structure type, so sizeof(struct vb2_buffer) will is used
+ * @timestamp_type: Type of the timestamp; V4L2_BUF_FLAGS_TIMESTAMP_*
  * @gfp_flags:	additional gfp flags used when allocating the buffers.
  *		Typically this is 0, but it may be e.g. GFP_DMA or __GFP_DMA32
  *		to force the buffer allocation to a specific memory zone.
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 691077d..ca2b4fc 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -695,6 +695,16 @@ struct v4l2_buffer {
 #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN		0x00000000
 #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC	0x00002000
 #define V4L2_BUF_FLAG_TIMESTAMP_COPY		0x00004000
+/*
+ * Timestamp taken once the first pixel is received. If the flag is
+ * not set the buffer timestamp is taken at the end of the frame. This
+ * is not a timestamp type.
+ *
+ * In general drivers should not use this flag if the end-of-frame
+ * timestamps is as good quality as the start-of-frame one; the
+ * V4L2_EVENT_FRAME_SYNC event should be used in that case instead.
+ */
+#define V4L2_BUF_FLAG_TIMESTAMP_SOF		0x00010000
 
 /**
  * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor
-- 
1.7.10.4

Re: [PATCH v4 1/3] v4l: Document timestamp behaviour to correspond to reality

[Hans Verkuil]

On Mon 26 August 2013 01:02:01 Sakari Ailus wrote:
> Document that monotonic timestamps are taken after the corresponding frame
> has been received, not when the reception has begun. This corresponds to the
> reality of current drivers: the timestamp is naturally taken when the
> hardware triggers an interrupt to tell the driver to handle the received
> frame.
> 
> Remove the note on timestamp accurary as it is fairly subjective what is

accurary -> accuracy

> actually an unstable timestamp.
> 
> Also remove explanation that output buffer timestamps can be used to delay
> outputting a frame.
> 
> Remove the footnote saying we always use realtime clock.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> ---
>  Documentation/DocBook/media/v4l/io.xml |   47 ++++++--------------------------
>  1 file changed, 8 insertions(+), 39 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
> index 2c4c068..cd5f9de 100644
> --- a/Documentation/DocBook/media/v4l/io.xml
> +++ b/Documentation/DocBook/media/v4l/io.xml
> @@ -654,38 +654,11 @@ plane, are stored in struct <structname>v4l2_plane</structname> instead.
>  In that case, struct <structname>v4l2_buffer</structname> contains an array of
>  plane structures.</para>
>  
> -      <para>Nominally timestamps refer to the first data byte transmitted.
> -In practice however the wide range of hardware covered by the V4L2 API
> -limits timestamp accuracy. Often an interrupt routine will
> -sample the system clock shortly after the field or frame was stored
> -completely in memory. So applications must expect a constant
> -difference up to one field or frame period plus a small (few scan
> -lines) random error. The delay and error can be much
> -larger due to compression or transmission over an external bus when
> -the frames are not properly stamped by the sender. This is frequently
> -the case with USB cameras. Here timestamps refer to the instant the
> -field or frame was received by the driver, not the capture time. These
> -devices identify by not enumerating any video standards, see <xref
> -linkend="standard" />.</para>
> -
> -      <para>Similar limitations apply to output timestamps. Typically
> -the video hardware locks to a clock controlling the video timing, the
> -horizontal and vertical synchronization pulses. At some point in the
> -line sequence, possibly the vertical blanking, an interrupt routine
> -samples the system clock, compares against the timestamp and programs
> -the hardware to repeat the previous field or frame, or to display the
> -buffer contents.</para>
> -
> -      <para>Apart of limitations of the video device and natural
> -inaccuracies of all clocks, it should be noted system time itself is
> -not perfectly stable. It can be affected by power saving cycles,
> -warped to insert leap seconds, or even turned back or forth by the
> -system administrator affecting long term measurements. <footnote>
> -	  <para>Since no other Linux multimedia
> -API supports unadjusted time it would be foolish to introduce here. We
> -must use a universally supported clock to synchronize different media,
> -hence time of day.</para>
> -	</footnote></para>
> +      <para>On timestamp types that are sampled from the system clock

On -> For

> +(V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC) it is guaranteed that the timestamp is
> +taken after the complete frame has been received (or transmitted in
> +case of video output devices). For other kinds of
> +timestamps this may vary depending on the driver.</para>
>  
>      <table frame="none" pgwide="1" id="v4l2-buffer">
>        <title>struct <structname>v4l2_buffer</structname></title>
> @@ -745,13 +718,9 @@ applications when an output stream.</entry>
>  	    byte was captured, as returned by the
>  	    <function>clock_gettime()</function> function for the relevant
>  	    clock id; see <constant>V4L2_BUF_FLAG_TIMESTAMP_*</constant> in
> -	    <xref linkend="buffer-flags" />. For output streams the data
> -	    will not be displayed before this time, secondary to the nominal
> -	    frame rate determined by the current video standard in enqueued
> -	    order. Applications can for example zero this field to display
> -	    frames as soon as possible. The driver stores the time at which
> -	    the first data byte was actually sent out in the
> -	    <structfield>timestamp</structfield> field. This permits
> +	    <xref linkend="buffer-flags" />. For output streams he driver

he -> the

> +	    stores the time at which the first data byte was actually sent out

first -> last

Otherwise it would be inconsistent with what you say above (i.e. timestamp is
set after the complete frame has been transmitted).

> +	    in the  <structfield>timestamp</structfield> field. This permits
>  	    applications to monitor the drift between the video and system
>  	    clock.</para></entry>
>  	  </row>
> 

Regards,

	Hans

Re: [PATCH v4 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Hans Verkuil]

On Mon 26 August 2013 01:02:03 Sakari Ailus wrote:
> Some devices such as the uvc produce timestamps at the beginning of the
> frame rather than at the end of it. Add a buffer flag
> (V4L2_BUF_FLAG_TIMESTAMP_SOF) to tell about this.
> 
> Also document timestamp_type in struct vb2_queue.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> ---
>  Documentation/DocBook/media/v4l/io.xml |   17 ++++++++++++-----
>  drivers/media/usb/uvc/uvc_queue.c      |    3 ++-
>  include/media/videobuf2-core.h         |    1 +
>  include/uapi/linux/videodev2.h         |   10 ++++++++++
>  4 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
> index b9a83bc..d3a725c 100644
> --- a/Documentation/DocBook/media/v4l/io.xml
> +++ b/Documentation/DocBook/media/v4l/io.xml
> @@ -654,11 +654,11 @@ plane, are stored in struct <structname>v4l2_plane</structname> instead.
>  In that case, struct <structname>v4l2_buffer</structname> contains an array of
>  plane structures.</para>
>  
> -      <para>On timestamp types that are sampled from the system clock
> -(V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC) it is guaranteed that the timestamp is
> -taken after the complete frame has been received (or transmitted in
> -case of video output devices). For other kinds of
> -timestamps this may vary depending on the driver.</para>
> +      <para>The timestamp is taken once the complete frame has been
> +received unless <constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant>

received -> received (or transmitted for output devices)

> +buffer flag is set. If <constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant>
> +is set, the timestamp is taken when the first pixel of the frame is
> +received.</para>

received -> received (or transmitted)

>  
>      <table frame="none" pgwide="1" id="v4l2-buffer">
>        <title>struct <structname>v4l2_buffer</structname></title>
> @@ -1120,6 +1120,13 @@ in which case caches have not been used.</entry>
>  	    <entry>The CAPTURE buffer timestamp has been taken from the
>  	    corresponding OUTPUT buffer. This flag applies only to mem2mem devices.</entry>
>  	  </row>
> +	  <row>
> +	    <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant></entry>
> +	    <entry>0x00010000</entry>
> +	    <entry>The buffer timestamp has been taken when the first
> +	    pixel is received. If this flag is not set, the timestamp
> +	    is taken when the entire frame has been received.</entry>

Ditto: received -> received (or transmitted for output devices)

> +	  </row>
>  	</tbody>
>        </tgroup>
>      </table>
> diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
> index cd962be..0d80512 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -149,7 +149,8 @@ int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type,
>  	queue->queue.buf_struct_size = sizeof(struct uvc_buffer);
>  	queue->queue.ops = &uvc_queue_qops;
>  	queue->queue.mem_ops = &vb2_vmalloc_memops;
> -	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> +	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC
> +		| V4L2_BUF_FLAG_TIMESTAMP_SOF;
>  	ret = vb2_queue_init(&queue->queue);
>  	if (ret)
>  		return ret;
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 6781258..6eb2d59 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -307,6 +307,7 @@ struct v4l2_fh;
>   * @buf_struct_size: size of the driver-specific buffer structure;
>   *		"0" indicates the driver doesn't want to use a custom buffer
>   *		structure type, so sizeof(struct vb2_buffer) will is used
> + * @timestamp_type: Type of the timestamp; V4L2_BUF_FLAGS_TIMESTAMP_*
>   * @gfp_flags:	additional gfp flags used when allocating the buffers.
>   *		Typically this is 0, but it may be e.g. GFP_DMA or __GFP_DMA32
>   *		to force the buffer allocation to a specific memory zone.
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 691077d..ca2b4fc 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -695,6 +695,16 @@ struct v4l2_buffer {
>  #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN		0x00000000
>  #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC	0x00002000
>  #define V4L2_BUF_FLAG_TIMESTAMP_COPY		0x00004000
> +/*
> + * Timestamp taken once the first pixel is received. If the flag is

received -> received (or transmitted for output devices)

> + * not set the buffer timestamp is taken at the end of the frame. This
> + * is not a timestamp type.
> + *
> + * In general drivers should not use this flag if the end-of-frame
> + * timestamps is as good quality as the start-of-frame one; the
> + * V4L2_EVENT_FRAME_SYNC event should be used in that case instead.
> + */
> +#define V4L2_BUF_FLAG_TIMESTAMP_SOF		0x00010000
>  
>  /**
>   * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor
> 

Regards,

	Hans

Re: [PATCH v4 1/3] v4l: Document timestamp behaviour to correspond to reality

[Sakari Ailus]

Hi Hans,

Thanks for the comments.

On Wed, Aug 28, 2013 at 02:13:31PM +0200, Hans Verkuil wrote:
> On Mon 26 August 2013 01:02:01 Sakari Ailus wrote:
> > Document that monotonic timestamps are taken after the corresponding frame
> > has been received, not when the reception has begun. This corresponds to the
> > reality of current drivers: the timestamp is naturally taken when the
> > hardware triggers an interrupt to tell the driver to handle the received
> > frame.
> > 
> > Remove the note on timestamp accurary as it is fairly subjective what is
> 
> accurary -> accuracy

Fixed.

> > actually an unstable timestamp.
> > 
> > Also remove explanation that output buffer timestamps can be used to delay
> > outputting a frame.
> > 
> > Remove the footnote saying we always use realtime clock.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > ---
> >  Documentation/DocBook/media/v4l/io.xml |   47 ++++++--------------------------
> >  1 file changed, 8 insertions(+), 39 deletions(-)
> > 
> > diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
> > index 2c4c068..cd5f9de 100644
> > --- a/Documentation/DocBook/media/v4l/io.xml
> > +++ b/Documentation/DocBook/media/v4l/io.xml
> > @@ -654,38 +654,11 @@ plane, are stored in struct <structname>v4l2_plane</structname> instead.
> >  In that case, struct <structname>v4l2_buffer</structname> contains an array of
> >  plane structures.</para>
> >  
> > -      <para>Nominally timestamps refer to the first data byte transmitted.
> > -In practice however the wide range of hardware covered by the V4L2 API
> > -limits timestamp accuracy. Often an interrupt routine will
> > -sample the system clock shortly after the field or frame was stored
> > -completely in memory. So applications must expect a constant
> > -difference up to one field or frame period plus a small (few scan
> > -lines) random error. The delay and error can be much
> > -larger due to compression or transmission over an external bus when
> > -the frames are not properly stamped by the sender. This is frequently
> > -the case with USB cameras. Here timestamps refer to the instant the
> > -field or frame was received by the driver, not the capture time. These
> > -devices identify by not enumerating any video standards, see <xref
> > -linkend="standard" />.</para>
> > -
> > -      <para>Similar limitations apply to output timestamps. Typically
> > -the video hardware locks to a clock controlling the video timing, the
> > -horizontal and vertical synchronization pulses. At some point in the
> > -line sequence, possibly the vertical blanking, an interrupt routine
> > -samples the system clock, compares against the timestamp and programs
> > -the hardware to repeat the previous field or frame, or to display the
> > -buffer contents.</para>
> > -
> > -      <para>Apart of limitations of the video device and natural
> > -inaccuracies of all clocks, it should be noted system time itself is
> > -not perfectly stable. It can be affected by power saving cycles,
> > -warped to insert leap seconds, or even turned back or forth by the
> > -system administrator affecting long term measurements. <footnote>
> > -	  <para>Since no other Linux multimedia
> > -API supports unadjusted time it would be foolish to introduce here. We
> > -must use a universally supported clock to synchronize different media,
> > -hence time of day.</para>
> > -	</footnote></para>
> > +      <para>On timestamp types that are sampled from the system clock
> 
> On -> For

Fixed.

> > +(V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC) it is guaranteed that the timestamp is
> > +taken after the complete frame has been received (or transmitted in
> > +case of video output devices). For other kinds of
> > +timestamps this may vary depending on the driver.</para>
> >  
> >      <table frame="none" pgwide="1" id="v4l2-buffer">
> >        <title>struct <structname>v4l2_buffer</structname></title>
> > @@ -745,13 +718,9 @@ applications when an output stream.</entry>
> >  	    byte was captured, as returned by the
> >  	    <function>clock_gettime()</function> function for the relevant
> >  	    clock id; see <constant>V4L2_BUF_FLAG_TIMESTAMP_*</constant> in
> > -	    <xref linkend="buffer-flags" />. For output streams the data
> > -	    will not be displayed before this time, secondary to the nominal
> > -	    frame rate determined by the current video standard in enqueued
> > -	    order. Applications can for example zero this field to display
> > -	    frames as soon as possible. The driver stores the time at which
> > -	    the first data byte was actually sent out in the
> > -	    <structfield>timestamp</structfield> field. This permits
> > +	    <xref linkend="buffer-flags" />. For output streams he driver
> 
> he -> the
> 
> > +	    stores the time at which the first data byte was actually sent out
> 
> first -> last
> 
> Otherwise it would be inconsistent with what you say above (i.e. timestamp is
> set after the complete frame has been transmitted).

Same for both. Silly mistakes. :-P

> > +	    in the  <structfield>timestamp</structfield> field. This permits
> >  	    applications to monitor the drift between the video and system
> >  	    clock.</para></entry>
> >  	  </row>
> > 

-- 
Cheers,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [PATCH v4.1 1/3] v4l: Document timestamp behaviour to correspond to reality

[Hans Verkuil]

On 08/28/2013 05:23 PM, Sakari Ailus wrote:
> Document that monotonic timestamps are taken after the corresponding frame
> has been received, not when the reception has begun. This corresponds to the
> reality of current drivers: the timestamp is naturally taken when the
> hardware triggers an interrupt to tell the driver to handle the received
> frame.
> 
> Remove the note on timestamp accuracy as it is fairly subjective what is
> actually an unstable timestamp.
> 
> Also remove explanation that output buffer timestamps can be used to delay
> outputting a frame.
> 
> Remove the footnote saying we always use realtime clock.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

> ---
> since v4:
> - Fixes according to Hans's comments.
> 
>  Documentation/DocBook/media/v4l/io.xml |   47 ++++++--------------------------
>  1 file changed, 8 insertions(+), 39 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
> index 2c4c068..9720606 100644
> --- a/Documentation/DocBook/media/v4l/io.xml
> +++ b/Documentation/DocBook/media/v4l/io.xml
> @@ -654,38 +654,11 @@ plane, are stored in struct <structname>v4l2_plane</structname> instead.
>  In that case, struct <structname>v4l2_buffer</structname> contains an array of
>  plane structures.</para>
>  
> -      <para>Nominally timestamps refer to the first data byte transmitted.
> -In practice however the wide range of hardware covered by the V4L2 API
> -limits timestamp accuracy. Often an interrupt routine will
> -sample the system clock shortly after the field or frame was stored
> -completely in memory. So applications must expect a constant
> -difference up to one field or frame period plus a small (few scan
> -lines) random error. The delay and error can be much
> -larger due to compression or transmission over an external bus when
> -the frames are not properly stamped by the sender. This is frequently
> -the case with USB cameras. Here timestamps refer to the instant the
> -field or frame was received by the driver, not the capture time. These
> -devices identify by not enumerating any video standards, see <xref
> -linkend="standard" />.</para>
> -
> -      <para>Similar limitations apply to output timestamps. Typically
> -the video hardware locks to a clock controlling the video timing, the
> -horizontal and vertical synchronization pulses. At some point in the
> -line sequence, possibly the vertical blanking, an interrupt routine
> -samples the system clock, compares against the timestamp and programs
> -the hardware to repeat the previous field or frame, or to display the
> -buffer contents.</para>
> -
> -      <para>Apart of limitations of the video device and natural
> -inaccuracies of all clocks, it should be noted system time itself is
> -not perfectly stable. It can be affected by power saving cycles,
> -warped to insert leap seconds, or even turned back or forth by the
> -system administrator affecting long term measurements. <footnote>
> -	  <para>Since no other Linux multimedia
> -API supports unadjusted time it would be foolish to introduce here. We
> -must use a universally supported clock to synchronize different media,
> -hence time of day.</para>
> -	</footnote></para>
> +      <para>For timestamp types that are sampled from the system clock
> +(V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC) it is guaranteed that the timestamp is
> +taken after the complete frame has been received (or transmitted in
> +case of video output devices). For other kinds of
> +timestamps this may vary depending on the driver.</para>
>  
>      <table frame="none" pgwide="1" id="v4l2-buffer">
>        <title>struct <structname>v4l2_buffer</structname></title>
> @@ -745,13 +718,9 @@ applications when an output stream.</entry>
>  	    byte was captured, as returned by the
>  	    <function>clock_gettime()</function> function for the relevant
>  	    clock id; see <constant>V4L2_BUF_FLAG_TIMESTAMP_*</constant> in
> -	    <xref linkend="buffer-flags" />. For output streams the data
> -	    will not be displayed before this time, secondary to the nominal
> -	    frame rate determined by the current video standard in enqueued
> -	    order. Applications can for example zero this field to display
> -	    frames as soon as possible. The driver stores the time at which
> -	    the first data byte was actually sent out in the
> -	    <structfield>timestamp</structfield> field. This permits
> +	    <xref linkend="buffer-flags" />. For output streams the driver
> +	    stores the time at which the last data byte was actually sent out
> +	    in the  <structfield>timestamp</structfield> field. This permits
>  	    applications to monitor the drift between the video and system
>  	    clock.</para></entry>
>  	  </row>
> 

[PATCH v4.1 1/3] v4l: Document timestamp behaviour to correspond to reality

[Sakari Ailus]

Document that monotonic timestamps are taken after the corresponding frame
has been received, not when the reception has begun. This corresponds to the
reality of current drivers: the timestamp is naturally taken when the
hardware triggers an interrupt to tell the driver to handle the received
frame.

Remove the note on timestamp accuracy as it is fairly subjective what is
actually an unstable timestamp.

Also remove explanation that output buffer timestamps can be used to delay
outputting a frame.

Remove the footnote saying we always use realtime clock.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
since v4:
- Fixes according to Hans's comments.

 Documentation/DocBook/media/v4l/io.xml |   47 ++++++--------------------------
 1 file changed, 8 insertions(+), 39 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
index 2c4c068..9720606 100644
--- a/Documentation/DocBook/media/v4l/io.xml
+++ b/Documentation/DocBook/media/v4l/io.xml
@@ -654,38 +654,11 @@ plane, are stored in struct <structname>v4l2_plane</structname> instead.
 In that case, struct <structname>v4l2_buffer</structname> contains an array of
 plane structures.</para>
 
-      <para>Nominally timestamps refer to the first data byte transmitted.
-In practice however the wide range of hardware covered by the V4L2 API
-limits timestamp accuracy. Often an interrupt routine will
-sample the system clock shortly after the field or frame was stored
-completely in memory. So applications must expect a constant
-difference up to one field or frame period plus a small (few scan
-lines) random error. The delay and error can be much
-larger due to compression or transmission over an external bus when
-the frames are not properly stamped by the sender. This is frequently
-the case with USB cameras. Here timestamps refer to the instant the
-field or frame was received by the driver, not the capture time. These
-devices identify by not enumerating any video standards, see <xref
-linkend="standard" />.</para>
-
-      <para>Similar limitations apply to output timestamps. Typically
-the video hardware locks to a clock controlling the video timing, the
-horizontal and vertical synchronization pulses. At some point in the
-line sequence, possibly the vertical blanking, an interrupt routine
-samples the system clock, compares against the timestamp and programs
-the hardware to repeat the previous field or frame, or to display the
-buffer contents.</para>
-
-      <para>Apart of limitations of the video device and natural
-inaccuracies of all clocks, it should be noted system time itself is
-not perfectly stable. It can be affected by power saving cycles,
-warped to insert leap seconds, or even turned back or forth by the
-system administrator affecting long term measurements. <footnote>
-	  <para>Since no other Linux multimedia
-API supports unadjusted time it would be foolish to introduce here. We
-must use a universally supported clock to synchronize different media,
-hence time of day.</para>
-	</footnote></para>
+      <para>For timestamp types that are sampled from the system clock
+(V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC) it is guaranteed that the timestamp is
+taken after the complete frame has been received (or transmitted in
+case of video output devices). For other kinds of
+timestamps this may vary depending on the driver.</para>
 
     <table frame="none" pgwide="1" id="v4l2-buffer">
       <title>struct <structname>v4l2_buffer</structname></title>
@@ -745,13 +718,9 @@ applications when an output stream.</entry>
 	    byte was captured, as returned by the
 	    <function>clock_gettime()</function> function for the relevant
 	    clock id; see <constant>V4L2_BUF_FLAG_TIMESTAMP_*</constant> in
-	    <xref linkend="buffer-flags" />. For output streams the data
-	    will not be displayed before this time, secondary to the nominal
-	    frame rate determined by the current video standard in enqueued
-	    order. Applications can for example zero this field to display
-	    frames as soon as possible. The driver stores the time at which
-	    the first data byte was actually sent out in the
-	    <structfield>timestamp</structfield> field. This permits
+	    <xref linkend="buffer-flags" />. For output streams the driver
+	    stores the time at which the last data byte was actually sent out
+	    in the  <structfield>timestamp</structfield> field. This permits
 	    applications to monitor the drift between the video and system
 	    clock.</para></entry>
 	  </row>
-- 
1.7.10.4

[PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Sakari Ailus]

Some devices such as the uvc produce timestamps at the beginning of the
frame rather than at the end of it. Add a buffer flag
(V4L2_BUF_FLAG_TIMESTAMP_SOF) to tell about this.

Also document timestamp_type in struct vb2_queue, and make the uvc set the
buffer flag.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
since v4:
- Fixes according to Hans's comments.

- Note in comment the uvc driver will set the SOF flag from now on.

- Change comment of vb2_queue timestamp_type field: this is timestamp flags
  rather than just type. I stopped short of renaming the field.

 Documentation/DocBook/media/v4l/io.xml |   19 ++++++++++++++-----
 drivers/media/usb/uvc/uvc_queue.c      |    3 ++-
 include/media/videobuf2-core.h         |    1 +
 include/uapi/linux/videodev2.h         |   10 ++++++++++
 4 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
index 2c155cc..3aee210 100644
--- a/Documentation/DocBook/media/v4l/io.xml
+++ b/Documentation/DocBook/media/v4l/io.xml
@@ -654,11 +654,12 @@ plane, are stored in struct <structname>v4l2_plane</structname> instead.
 In that case, struct <structname>v4l2_buffer</structname> contains an array of
 plane structures.</para>
 
-      <para>For timestamp types that are sampled from the system clock
-(V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC) it is guaranteed that the timestamp is
-taken after the complete frame has been received (or transmitted in
-case of video output devices). For other kinds of
-timestamps this may vary depending on the driver.</para>
+      <para>The timestamp is taken once the complete frame has been
+received (or transmitted for output devices) unless
+<constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant> buffer flag is set.
+If <constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant> is set, the
+timestamp is taken when the first pixel of the frame is received
+(or transmitted).</para>
 
     <table frame="none" pgwide="1" id="v4l2-buffer">
       <title>struct <structname>v4l2_buffer</structname></title>
@@ -1120,6 +1121,14 @@ in which case caches have not been used.</entry>
 	    <entry>The CAPTURE buffer timestamp has been taken from the
 	    corresponding OUTPUT buffer. This flag applies only to mem2mem devices.</entry>
 	  </row>
+	  <row>
+	    <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant></entry>
+	    <entry>0x00010000</entry>
+	    <entry>The buffer timestamp has been taken when the first
+	    pixel is received (or transmitted for output devices). If
+	    this flag is not set, the timestamp is taken when the
+	    entire frame has been received (or transmitted).</entry>
+	  </row>
 	</tbody>
       </tgroup>
     </table>
diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
index cd962be..0d80512 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -149,7 +149,8 @@ int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type,
 	queue->queue.buf_struct_size = sizeof(struct uvc_buffer);
 	queue->queue.ops = &uvc_queue_qops;
 	queue->queue.mem_ops = &vb2_vmalloc_memops;
-	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC
+		| V4L2_BUF_FLAG_TIMESTAMP_SOF;
 	ret = vb2_queue_init(&queue->queue);
 	if (ret)
 		return ret;
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 6781258..033efc7 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -307,6 +307,7 @@ struct v4l2_fh;
  * @buf_struct_size: size of the driver-specific buffer structure;
  *		"0" indicates the driver doesn't want to use a custom buffer
  *		structure type, so sizeof(struct vb2_buffer) will is used
+ * @timestamp_type: Timestamp flags; V4L2_BUF_FLAGS_TIMESTAMP_*
  * @gfp_flags:	additional gfp flags used when allocating the buffers.
  *		Typically this is 0, but it may be e.g. GFP_DMA or __GFP_DMA32
  *		to force the buffer allocation to a specific memory zone.
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 691077d..c57765e 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -695,6 +695,16 @@ struct v4l2_buffer {
 #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN		0x00000000
 #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC	0x00002000
 #define V4L2_BUF_FLAG_TIMESTAMP_COPY		0x00004000
+/*
+ * Timestamp taken once the first pixel is received (or transmitted). 
+ * If the flag is not set the buffer timestamp is taken at the end of
+ * the frame. This is not a timestamp type.
+ *
+ * In general drivers should not use this flag if the end-of-frame
+ * timestamps is as good quality as the start-of-frame one; the
+ * V4L2_EVENT_FRAME_SYNC event should be used in that case instead.
+ */
+#define V4L2_BUF_FLAG_TIMESTAMP_SOF		0x00010000
 
 /**
  * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor
-- 
1.7.10.4

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Hans Verkuil]

On 08/28/2013 05:24 PM, Sakari Ailus wrote:
> Some devices such as the uvc produce timestamps at the beginning of the
> frame rather than at the end of it. Add a buffer flag
> (V4L2_BUF_FLAG_TIMESTAMP_SOF) to tell about this.
> 
> Also document timestamp_type in struct vb2_queue, and make the uvc set the
> buffer flag.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> ---
> since v4:
> - Fixes according to Hans's comments.
> 
> - Note in comment the uvc driver will set the SOF flag from now on.
> 
> - Change comment of vb2_queue timestamp_type field: this is timestamp flags
>   rather than just type. I stopped short of renaming the field.
> 
>  Documentation/DocBook/media/v4l/io.xml |   19 ++++++++++++++-----
>  drivers/media/usb/uvc/uvc_queue.c      |    3 ++-
>  include/media/videobuf2-core.h         |    1 +
>  include/uapi/linux/videodev2.h         |   10 ++++++++++
>  4 files changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
> index 2c155cc..3aee210 100644
> --- a/Documentation/DocBook/media/v4l/io.xml
> +++ b/Documentation/DocBook/media/v4l/io.xml
> @@ -654,11 +654,12 @@ plane, are stored in struct <structname>v4l2_plane</structname> instead.
>  In that case, struct <structname>v4l2_buffer</structname> contains an array of
>  plane structures.</para>
>  
> -      <para>For timestamp types that are sampled from the system clock
> -(V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC) it is guaranteed that the timestamp is
> -taken after the complete frame has been received (or transmitted in
> -case of video output devices). For other kinds of
> -timestamps this may vary depending on the driver.</para>
> +      <para>The timestamp is taken once the complete frame has been
> +received (or transmitted for output devices) unless

unless -> unless the

> +<constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant> buffer flag is set.
> +If <constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant> is set, the

the -> then the

> +timestamp is taken when the first pixel of the frame is received
> +(or transmitted).</para>
>  
>      <table frame="none" pgwide="1" id="v4l2-buffer">
>        <title>struct <structname>v4l2_buffer</structname></title>
> @@ -1120,6 +1121,14 @@ in which case caches have not been used.</entry>
>  	    <entry>The CAPTURE buffer timestamp has been taken from the
>  	    corresponding OUTPUT buffer. This flag applies only to mem2mem devices.</entry>
>  	  </row>
> +	  <row>
> +	    <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant></entry>
> +	    <entry>0x00010000</entry>
> +	    <entry>The buffer timestamp has been taken when the first

I think 'has been' should be 'was' in this context.

> +	    pixel is received (or transmitted for output devices). If
> +	    this flag is not set, the timestamp is taken when the
> +	    entire frame has been received (or transmitted).</entry>

Ditto.

> +	  </row>
>  	</tbody>
>        </tgroup>
>      </table>
> diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
> index cd962be..0d80512 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -149,7 +149,8 @@ int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type,
>  	queue->queue.buf_struct_size = sizeof(struct uvc_buffer);
>  	queue->queue.ops = &uvc_queue_qops;
>  	queue->queue.mem_ops = &vb2_vmalloc_memops;
> -	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> +	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC
> +		| V4L2_BUF_FLAG_TIMESTAMP_SOF;
>  	ret = vb2_queue_init(&queue->queue);
>  	if (ret)
>  		return ret;
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 6781258..033efc7 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -307,6 +307,7 @@ struct v4l2_fh;
>   * @buf_struct_size: size of the driver-specific buffer structure;
>   *		"0" indicates the driver doesn't want to use a custom buffer
>   *		structure type, so sizeof(struct vb2_buffer) will is used
> + * @timestamp_type: Timestamp flags; V4L2_BUF_FLAGS_TIMESTAMP_*
>   * @gfp_flags:	additional gfp flags used when allocating the buffers.
>   *		Typically this is 0, but it may be e.g. GFP_DMA or __GFP_DMA32
>   *		to force the buffer allocation to a specific memory zone.
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 691077d..c57765e 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -695,6 +695,16 @@ struct v4l2_buffer {
>  #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN		0x00000000
>  #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC	0x00002000
>  #define V4L2_BUF_FLAG_TIMESTAMP_COPY		0x00004000
> +/*
> + * Timestamp taken once the first pixel is received (or transmitted). 
> + * If the flag is not set the buffer timestamp is taken at the end of
> + * the frame. This is not a timestamp type.
> + *
> + * In general drivers should not use this flag if the end-of-frame
> + * timestamps is as good quality as the start-of-frame one; the
> + * V4L2_EVENT_FRAME_SYNC event should be used in that case instead.
> + */
> +#define V4L2_BUF_FLAG_TIMESTAMP_SOF		0x00010000
>  
>  /**
>   * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor
> 

After making the changes suggested above:

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Laurent Pinchart]

Hi Sakari,

Thank you for the patches.

On Wednesday 28 August 2013 18:24:55 Sakari Ailus wrote:
> Some devices such as the uvc produce timestamps at the beginning of the
> frame rather than at the end of it. Add a buffer flag
> (V4L2_BUF_FLAG_TIMESTAMP_SOF) to tell about this.
> 
> Also document timestamp_type in struct vb2_queue, and make the uvc set the
> buffer flag.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> ---
> since v4:
> - Fixes according to Hans's comments.
> 
> - Note in comment the uvc driver will set the SOF flag from now on.
> 
> - Change comment of vb2_queue timestamp_type field: this is timestamp flags
>   rather than just type. I stopped short of renaming the field.
> 
>  Documentation/DocBook/media/v4l/io.xml |   19 ++++++++++++++-----
>  drivers/media/usb/uvc/uvc_queue.c      |    3 ++-
>  include/media/videobuf2-core.h         |    1 +
>  include/uapi/linux/videodev2.h         |   10 ++++++++++
>  4 files changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/io.xml
> b/Documentation/DocBook/media/v4l/io.xml index 2c155cc..3aee210 100644
> --- a/Documentation/DocBook/media/v4l/io.xml
> +++ b/Documentation/DocBook/media/v4l/io.xml
> @@ -654,11 +654,12 @@ plane, are stored in struct
> <structname>v4l2_plane</structname> instead. In that case, struct
> <structname>v4l2_buffer</structname> contains an array of plane
> structures.</para>
> 
> -      <para>For timestamp types that are sampled from the system clock
> -(V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC) it is guaranteed that the timestamp is
> -taken after the complete frame has been received (or transmitted in
> -case of video output devices). For other kinds of
> -timestamps this may vary depending on the driver.</para>
> +      <para>The timestamp is taken once the complete frame has been
> +received (or transmitted for output devices) unless
> +<constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant> buffer flag is set.
> +If <constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant> is set, the
> +timestamp is taken when the first pixel of the frame is received
> +(or transmitted).</para>
> 
>      <table frame="none" pgwide="1" id="v4l2-buffer">
>        <title>struct <structname>v4l2_buffer</structname></title>
> @@ -1120,6 +1121,14 @@ in which case caches have not been used.</entry>
>  	    <entry>The CAPTURE buffer timestamp has been taken from the
>  	    corresponding OUTPUT buffer. This flag applies only to mem2mem
> devices.</entry> </row>
> +	  <row>
> +	    <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant></entry>
> +	    <entry>0x00010000</entry>
> +	    <entry>The buffer timestamp has been taken when the first
> +	    pixel is received (or transmitted for output devices). If
> +	    this flag is not set, the timestamp is taken when the
> +	    entire frame has been received (or transmitted).</entry>
> +	  </row>
>  	</tbody>
>        </tgroup>
>      </table>
> diff --git a/drivers/media/usb/uvc/uvc_queue.c
> b/drivers/media/usb/uvc/uvc_queue.c index cd962be..0d80512 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -149,7 +149,8 @@ int uvc_queue_init(struct uvc_video_queue *queue, enum
> v4l2_buf_type type, queue->queue.buf_struct_size = sizeof(struct
> uvc_buffer);
>  	queue->queue.ops = &uvc_queue_qops;
>  	queue->queue.mem_ops = &vb2_vmalloc_memops;
> -	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> +	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC
> +		| V4L2_BUF_FLAG_TIMESTAMP_SOF;
>  	ret = vb2_queue_init(&queue->queue);
>  	if (ret)
>  		return ret;
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 6781258..033efc7 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -307,6 +307,7 @@ struct v4l2_fh;
>   * @buf_struct_size: size of the driver-specific buffer structure;
>   *		"0" indicates the driver doesn't want to use a custom buffer
>   *		structure type, so sizeof(struct vb2_buffer) will is used
> + * @timestamp_type: Timestamp flags; V4L2_BUF_FLAGS_TIMESTAMP_*
>   * @gfp_flags:	additional gfp flags used when allocating the buffers.
>   *		Typically this is 0, but it may be e.g. GFP_DMA or __GFP_DMA32
>   *		to force the buffer allocation to a specific memory zone.
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 691077d..c57765e 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -695,6 +695,16 @@ struct v4l2_buffer {
>  #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN		0x00000000
>  #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC	0x00002000
>  #define V4L2_BUF_FLAG_TIMESTAMP_COPY		0x00004000
> +/*
> + * Timestamp taken once the first pixel is received (or transmitted).
> + * If the flag is not set the buffer timestamp is taken at the end of
> + * the frame. This is not a timestamp type.

UVC devices timestamp frames when the frame is captured, not when the first 
pixel is transmitted.

For the other two patches,

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> + * In general drivers should not use this flag if the end-of-frame
> + * timestamps is as good quality as the start-of-frame one; the
> + * V4L2_EVENT_FRAME_SYNC event should be used in that case instead.
> + */
> +#define V4L2_BUF_FLAG_TIMESTAMP_SOF		0x00010000
> 
>  /**
>   * struct v4l2_exportbuffer - export of video buffer as DMABUF file
> descriptor
-- 
Regards,

Laurent Pinchart

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Sakari Ailus]

Hi Hans,

Thanks for your prompt comments.

On Wed, Aug 28, 2013 at 05:30:01PM +0200, Hans Verkuil wrote:
> On 08/28/2013 05:24 PM, Sakari Ailus wrote:
> > Some devices such as the uvc produce timestamps at the beginning of the
> > frame rather than at the end of it. Add a buffer flag
> > (V4L2_BUF_FLAG_TIMESTAMP_SOF) to tell about this.
> > 
> > Also document timestamp_type in struct vb2_queue, and make the uvc set the
> > buffer flag.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > ---
> > since v4:
> > - Fixes according to Hans's comments.
> > 
> > - Note in comment the uvc driver will set the SOF flag from now on.
> > 
> > - Change comment of vb2_queue timestamp_type field: this is timestamp flags
> >   rather than just type. I stopped short of renaming the field.
> > 
> >  Documentation/DocBook/media/v4l/io.xml |   19 ++++++++++++++-----
> >  drivers/media/usb/uvc/uvc_queue.c      |    3 ++-
> >  include/media/videobuf2-core.h         |    1 +
> >  include/uapi/linux/videodev2.h         |   10 ++++++++++
> >  4 files changed, 27 insertions(+), 6 deletions(-)
> > 
> > diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
> > index 2c155cc..3aee210 100644
> > --- a/Documentation/DocBook/media/v4l/io.xml
> > +++ b/Documentation/DocBook/media/v4l/io.xml
> > @@ -654,11 +654,12 @@ plane, are stored in struct <structname>v4l2_plane</structname> instead.
> >  In that case, struct <structname>v4l2_buffer</structname> contains an array of
> >  plane structures.</para>
> >  
> > -      <para>For timestamp types that are sampled from the system clock
> > -(V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC) it is guaranteed that the timestamp is
> > -taken after the complete frame has been received (or transmitted in
> > -case of video output devices). For other kinds of
> > -timestamps this may vary depending on the driver.</para>
> > +      <para>The timestamp is taken once the complete frame has been
> > +received (or transmitted for output devices) unless
> 
> unless -> unless the
> 
> > +<constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant> buffer flag is set.
> > +If <constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant> is set, the
> 
> the -> then the

Fixed both.

> > +timestamp is taken when the first pixel of the frame is received
> > +(or transmitted).</para>
> >  
> >      <table frame="none" pgwide="1" id="v4l2-buffer">
> >        <title>struct <structname>v4l2_buffer</structname></title>
> > @@ -1120,6 +1121,14 @@ in which case caches have not been used.</entry>
> >  	    <entry>The CAPTURE buffer timestamp has been taken from the
> >  	    corresponding OUTPUT buffer. This flag applies only to mem2mem devices.</entry>
> >  	  </row>
> > +	  <row>
> > +	    <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant></entry>
> > +	    <entry>0x00010000</entry>
> > +	    <entry>The buffer timestamp has been taken when the first
> 
> I think 'has been' should be 'was' in this context.

Then I wonder if I should change all the other flags, too. :-) "Has been" is
consistent with the documentation of other flags.

-- 
Cheers,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Sakari Ailus]

Hi Laurent,

Thanks for the comments!

On Wed, Aug 28, 2013 at 06:03:20PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patches.
> 
> On Wednesday 28 August 2013 18:24:55 Sakari Ailus wrote:
> > Some devices such as the uvc produce timestamps at the beginning of the
> > frame rather than at the end of it. Add a buffer flag
> > (V4L2_BUF_FLAG_TIMESTAMP_SOF) to tell about this.
> > 
> > Also document timestamp_type in struct vb2_queue, and make the uvc set the
> > buffer flag.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > ---
> > since v4:
> > - Fixes according to Hans's comments.
> > 
> > - Note in comment the uvc driver will set the SOF flag from now on.
> > 
> > - Change comment of vb2_queue timestamp_type field: this is timestamp flags
> >   rather than just type. I stopped short of renaming the field.
> > 
> >  Documentation/DocBook/media/v4l/io.xml |   19 ++++++++++++++-----
> >  drivers/media/usb/uvc/uvc_queue.c      |    3 ++-
> >  include/media/videobuf2-core.h         |    1 +
> >  include/uapi/linux/videodev2.h         |   10 ++++++++++
> >  4 files changed, 27 insertions(+), 6 deletions(-)
> > 
> > diff --git a/Documentation/DocBook/media/v4l/io.xml
> > b/Documentation/DocBook/media/v4l/io.xml index 2c155cc..3aee210 100644
> > --- a/Documentation/DocBook/media/v4l/io.xml
> > +++ b/Documentation/DocBook/media/v4l/io.xml
> > @@ -654,11 +654,12 @@ plane, are stored in struct
> > <structname>v4l2_plane</structname> instead. In that case, struct
> > <structname>v4l2_buffer</structname> contains an array of plane
> > structures.</para>
> > 
> > -      <para>For timestamp types that are sampled from the system clock
> > -(V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC) it is guaranteed that the timestamp is
> > -taken after the complete frame has been received (or transmitted in
> > -case of video output devices). For other kinds of
> > -timestamps this may vary depending on the driver.</para>
> > +      <para>The timestamp is taken once the complete frame has been
> > +received (or transmitted for output devices) unless
> > +<constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant> buffer flag is set.
> > +If <constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant> is set, the
> > +timestamp is taken when the first pixel of the frame is received
> > +(or transmitted).</para>
> > 
> >      <table frame="none" pgwide="1" id="v4l2-buffer">
> >        <title>struct <structname>v4l2_buffer</structname></title>
> > @@ -1120,6 +1121,14 @@ in which case caches have not been used.</entry>
> >  	    <entry>The CAPTURE buffer timestamp has been taken from the
> >  	    corresponding OUTPUT buffer. This flag applies only to mem2mem
> > devices.</entry> </row>
> > +	  <row>
> > +	    <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant></entry>
> > +	    <entry>0x00010000</entry>
> > +	    <entry>The buffer timestamp has been taken when the first
> > +	    pixel is received (or transmitted for output devices). If
> > +	    this flag is not set, the timestamp is taken when the
> > +	    entire frame has been received (or transmitted).</entry>
> > +	  </row>
> >  	</tbody>
> >        </tgroup>
> >      </table>
> > diff --git a/drivers/media/usb/uvc/uvc_queue.c
> > b/drivers/media/usb/uvc/uvc_queue.c index cd962be..0d80512 100644
> > --- a/drivers/media/usb/uvc/uvc_queue.c
> > +++ b/drivers/media/usb/uvc/uvc_queue.c
> > @@ -149,7 +149,8 @@ int uvc_queue_init(struct uvc_video_queue *queue, enum
> > v4l2_buf_type type, queue->queue.buf_struct_size = sizeof(struct
> > uvc_buffer);
> >  	queue->queue.ops = &uvc_queue_qops;
> >  	queue->queue.mem_ops = &vb2_vmalloc_memops;
> > -	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> > +	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC
> > +		| V4L2_BUF_FLAG_TIMESTAMP_SOF;
> >  	ret = vb2_queue_init(&queue->queue);
> >  	if (ret)
> >  		return ret;
> > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> > index 6781258..033efc7 100644
> > --- a/include/media/videobuf2-core.h
> > +++ b/include/media/videobuf2-core.h
> > @@ -307,6 +307,7 @@ struct v4l2_fh;
> >   * @buf_struct_size: size of the driver-specific buffer structure;
> >   *		"0" indicates the driver doesn't want to use a custom buffer
> >   *		structure type, so sizeof(struct vb2_buffer) will is used
> > + * @timestamp_type: Timestamp flags; V4L2_BUF_FLAGS_TIMESTAMP_*
> >   * @gfp_flags:	additional gfp flags used when allocating the buffers.
> >   *		Typically this is 0, but it may be e.g. GFP_DMA or __GFP_DMA32
> >   *		to force the buffer allocation to a specific memory zone.
> > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > index 691077d..c57765e 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -695,6 +695,16 @@ struct v4l2_buffer {
> >  #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN		0x00000000
> >  #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC	0x00002000
> >  #define V4L2_BUF_FLAG_TIMESTAMP_COPY		0x00004000
> > +/*
> > + * Timestamp taken once the first pixel is received (or transmitted).
> > + * If the flag is not set the buffer timestamp is taken at the end of
> > + * the frame. This is not a timestamp type.
> 
> UVC devices timestamp frames when the frame is captured, not when the first 
> pixel is transmitted.

I.e. we shouldn't set the SOF flag? "When the frame is captured" doesn't say
much, or almost anything in terms of *when*. The frames have exposure time
and rolling shutter makes a difference, too.

> For the other two patches,
> 
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks! :-)

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Laurent Pinchart]

On Wednesday 28 August 2013 19:09:23 Sakari Ailus wrote:
> Hi Laurent,
> 
> Thanks for the comments!
> 
> On Wed, Aug 28, 2013 at 06:03:20PM +0200, Laurent Pinchart wrote:
> > Hi Sakari,
> > 
> > Thank you for the patches.
> > 
> > On Wednesday 28 August 2013 18:24:55 Sakari Ailus wrote:
> > > Some devices such as the uvc produce timestamps at the beginning of the
> > > frame rather than at the end of it. Add a buffer flag
> > > (V4L2_BUF_FLAG_TIMESTAMP_SOF) to tell about this.
> > > 
> > > Also document timestamp_type in struct vb2_queue, and make the uvc set
> > > the
> > > buffer flag.
> > > 
> > > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > > ---
> > > since v4:
> > > - Fixes according to Hans's comments.
> > > 
> > > - Note in comment the uvc driver will set the SOF flag from now on.
> > > 
> > > - Change comment of vb2_queue timestamp_type field: this is timestamp
> > > flags
> > > 
> > >   rather than just type. I stopped short of renaming the field.
> > >  
> > >  Documentation/DocBook/media/v4l/io.xml |   19 ++++++++++++++-----
> > >  drivers/media/usb/uvc/uvc_queue.c      |    3 ++-
> > >  include/media/videobuf2-core.h         |    1 +
> > >  include/uapi/linux/videodev2.h         |   10 ++++++++++
> > >  4 files changed, 27 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/Documentation/DocBook/media/v4l/io.xml
> > > b/Documentation/DocBook/media/v4l/io.xml index 2c155cc..3aee210 100644
> > > --- a/Documentation/DocBook/media/v4l/io.xml
> > > +++ b/Documentation/DocBook/media/v4l/io.xml
> > > @@ -654,11 +654,12 @@ plane, are stored in struct
> > > <structname>v4l2_plane</structname> instead. In that case, struct
> > > <structname>v4l2_buffer</structname> contains an array of plane
> > > structures.</para>
> > > 
> > > -      <para>For timestamp types that are sampled from the system clock
> > > -(V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC) it is guaranteed that the timestamp
> > > is
> > > -taken after the complete frame has been received (or transmitted in
> > > -case of video output devices). For other kinds of
> > > -timestamps this may vary depending on the driver.</para>
> > > +      <para>The timestamp is taken once the complete frame has been
> > > +received (or transmitted for output devices) unless
> > > +<constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant> buffer flag is set.
> > > +If <constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant> is set, the
> > > +timestamp is taken when the first pixel of the frame is received
> > > +(or transmitted).</para>
> > > 
> > >      <table frame="none" pgwide="1" id="v4l2-buffer">
> > >      
> > >        <title>struct <structname>v4l2_buffer</structname></title>
> > > 
> > > @@ -1120,6 +1121,14 @@ in which case caches have not been used.</entry>
> > > 
> > >  	    <entry>The CAPTURE buffer timestamp has been taken from the
> > >  	    corresponding OUTPUT buffer. This flag applies only to mem2mem
> > > 
> > > devices.</entry> </row>
> > > +	  <row>
> > > +	    <entry><constant>V4L2_BUF_FLAG_TIMESTAMP_SOF</constant></entry>
> > > +	    <entry>0x00010000</entry>
> > > +	    <entry>The buffer timestamp has been taken when the first
> > > +	    pixel is received (or transmitted for output devices). If
> > > +	    this flag is not set, the timestamp is taken when the
> > > +	    entire frame has been received (or transmitted).</entry>
> > > +	  </row>
> > > 
> > >  	</tbody>
> > >  	
> > >        </tgroup>
> > >      
> > >      </table>
> > > 
> > > diff --git a/drivers/media/usb/uvc/uvc_queue.c
> > > b/drivers/media/usb/uvc/uvc_queue.c index cd962be..0d80512 100644
> > > --- a/drivers/media/usb/uvc/uvc_queue.c
> > > +++ b/drivers/media/usb/uvc/uvc_queue.c
> > > @@ -149,7 +149,8 @@ int uvc_queue_init(struct uvc_video_queue *queue,
> > > enum
> > > v4l2_buf_type type, queue->queue.buf_struct_size = sizeof(struct
> > > uvc_buffer);
> > > 
> > >  	queue->queue.ops = &uvc_queue_qops;
> > >  	queue->queue.mem_ops = &vb2_vmalloc_memops;
> > > 
> > > -	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> > > +	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC
> > > +		| V4L2_BUF_FLAG_TIMESTAMP_SOF;
> > > 
> > >  	ret = vb2_queue_init(&queue->queue);
> > >  	if (ret)
> > >  	
> > >  		return ret;
> > > 
> > > diff --git a/include/media/videobuf2-core.h
> > > b/include/media/videobuf2-core.h index 6781258..033efc7 100644
> > > --- a/include/media/videobuf2-core.h
> > > +++ b/include/media/videobuf2-core.h
> > > @@ -307,6 +307,7 @@ struct v4l2_fh;
> > > 
> > >   * @buf_struct_size: size of the driver-specific buffer structure;
> > >   *		"0" indicates the driver doesn't want to use a custom buffer
> > >   *		structure type, so sizeof(struct vb2_buffer) will is used
> > > 
> > > + * @timestamp_type: Timestamp flags; V4L2_BUF_FLAGS_TIMESTAMP_*
> > > 
> > >   * @gfp_flags:	additional gfp flags used when allocating the buffers.
> > >   *		Typically this is 0, but it may be e.g. GFP_DMA or __GFP_DMA32
> > >   *		to force the buffer allocation to a specific memory zone.
> > > 
> > > diff --git a/include/uapi/linux/videodev2.h
> > > b/include/uapi/linux/videodev2.h index 691077d..c57765e 100644
> > > --- a/include/uapi/linux/videodev2.h
> > > +++ b/include/uapi/linux/videodev2.h
> > > @@ -695,6 +695,16 @@ struct v4l2_buffer {
> > > 
> > >  #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN		0x00000000
> > >  #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC	0x00002000
> > >  #define V4L2_BUF_FLAG_TIMESTAMP_COPY		0x00004000
> > > 
> > > +/*
> > > + * Timestamp taken once the first pixel is received (or transmitted).
> > > + * If the flag is not set the buffer timestamp is taken at the end of
> > > + * the frame. This is not a timestamp type.
> > 
> > UVC devices timestamp frames when the frame is captured, not when the
> > first
> > pixel is transmitted.
> 
> I.e. we shouldn't set the SOF flag? "When the frame is captured" doesn't say
> much, or almost anything in terms of *when*. The frames have exposure time
> and rolling shutter makes a difference, too.

The UVC 1.1 specification defines the timestamp as

"The source clock time in native deviceclock units when the raw frame capture 
begins."

What devices do in practice may differ :-)

> > For the other two patches,
> > 
> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Thanks! :-)
-- 
Regards,

Laurent Pinchart

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Sakari Ailus]

Hi Laurent,

On Wed, Aug 28, 2013 at 06:14:44PM +0200, Laurent Pinchart wrote:
...
> > > > diff --git a/drivers/media/usb/uvc/uvc_queue.c
> > > > b/drivers/media/usb/uvc/uvc_queue.c index cd962be..0d80512 100644
> > > > --- a/drivers/media/usb/uvc/uvc_queue.c
> > > > +++ b/drivers/media/usb/uvc/uvc_queue.c
> > > > @@ -149,7 +149,8 @@ int uvc_queue_init(struct uvc_video_queue *queue,
> > > > enum
> > > > v4l2_buf_type type, queue->queue.buf_struct_size = sizeof(struct
> > > > uvc_buffer);
> > > > 
> > > >  	queue->queue.ops = &uvc_queue_qops;
> > > >  	queue->queue.mem_ops = &vb2_vmalloc_memops;
> > > > 
> > > > -	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> > > > +	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC
> > > > +		| V4L2_BUF_FLAG_TIMESTAMP_SOF;
> > > > 
> > > >  	ret = vb2_queue_init(&queue->queue);
> > > >  	if (ret)
> > > >  	
> > > >  		return ret;
> > > > 
> > > > diff --git a/include/media/videobuf2-core.h
> > > > b/include/media/videobuf2-core.h index 6781258..033efc7 100644
> > > > --- a/include/media/videobuf2-core.h
> > > > +++ b/include/media/videobuf2-core.h
> > > > @@ -307,6 +307,7 @@ struct v4l2_fh;
> > > > 
> > > >   * @buf_struct_size: size of the driver-specific buffer structure;
> > > >   *		"0" indicates the driver doesn't want to use a custom buffer
> > > >   *		structure type, so sizeof(struct vb2_buffer) will is used
> > > > 
> > > > + * @timestamp_type: Timestamp flags; V4L2_BUF_FLAGS_TIMESTAMP_*
> > > > 
> > > >   * @gfp_flags:	additional gfp flags used when allocating the buffers.
> > > >   *		Typically this is 0, but it may be e.g. GFP_DMA or __GFP_DMA32
> > > >   *		to force the buffer allocation to a specific memory zone.
> > > > 
> > > > diff --git a/include/uapi/linux/videodev2.h
> > > > b/include/uapi/linux/videodev2.h index 691077d..c57765e 100644
> > > > --- a/include/uapi/linux/videodev2.h
> > > > +++ b/include/uapi/linux/videodev2.h
> > > > @@ -695,6 +695,16 @@ struct v4l2_buffer {
> > > > 
> > > >  #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN		0x00000000
> > > >  #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC	0x00002000
> > > >  #define V4L2_BUF_FLAG_TIMESTAMP_COPY		0x00004000
> > > > 
> > > > +/*
> > > > + * Timestamp taken once the first pixel is received (or transmitted).
> > > > + * If the flag is not set the buffer timestamp is taken at the end of
> > > > + * the frame. This is not a timestamp type.
> > > 
> > > UVC devices timestamp frames when the frame is captured, not when the
> > > first
> > > pixel is transmitted.
> > 
> > I.e. we shouldn't set the SOF flag? "When the frame is captured" doesn't say
> > much, or almost anything in terms of *when*. The frames have exposure time
> > and rolling shutter makes a difference, too.
> 
> The UVC 1.1 specification defines the timestamp as
> 
> "The source clock time in native deviceclock units when the raw frame capture 
> begins."
> 
> What devices do in practice may differ :-)

I think that this should mean start-of-frame - exposure time. I'd really
wonder if any practical implementation does that however.

What's your suggestion; should we use the SOF flag for this or do you prefer
the end-of-frame timestamp instead? I think it'd be quite nice for drivers
to know which one is which without having to guess, and based on the above
start-of-frame comes as close to that definition as is meaningful.

> > > For the other two patches,
> > > 
> > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > Thanks! :-)

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Laurent Pinchart]

Hi Sakari,

On Wednesday 28 August 2013 19:39:19 Sakari Ailus wrote:
> On Wed, Aug 28, 2013 at 06:14:44PM +0200, Laurent Pinchart wrote:
> ...
> 
> > > > > diff --git a/drivers/media/usb/uvc/uvc_queue.c
> > > > > b/drivers/media/usb/uvc/uvc_queue.c index cd962be..0d80512 100644
> > > > > --- a/drivers/media/usb/uvc/uvc_queue.c
> > > > > +++ b/drivers/media/usb/uvc/uvc_queue.c
> > > > > @@ -149,7 +149,8 @@ int uvc_queue_init(struct uvc_video_queue
> > > > > *queue, enum v4l2_buf_type type,
> > > > >  	queue->queue.buf_struct_size = sizeof(struct uvc_buffer);
> > > > >  	queue->queue.ops = &uvc_queue_qops;
> > > > >  	queue->queue.mem_ops = &vb2_vmalloc_memops;
> > > > > -	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> > > > > +	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC
> > > > > +		| V4L2_BUF_FLAG_TIMESTAMP_SOF;
> > > > >  	ret = vb2_queue_init(&queue->queue);
> > > > >  	if (ret)
> > > > >  		return ret;
> > > > > diff --git a/include/media/videobuf2-core.h
> > > > > b/include/media/videobuf2-core.h index 6781258..033efc7 100644
> > > > > --- a/include/media/videobuf2-core.h
> > > > > +++ b/include/media/videobuf2-core.h
> > > > > @@ -307,6 +307,7 @@ struct v4l2_fh;
> > > > > 
> > > > >   * @buf_struct_size: size of the driver-specific buffer structure;
> > > > >   *		"0" indicates the driver doesn't want to use a custom buffer
> > > > >   *		structure type, so sizeof(struct vb2_buffer) will is used
> > > > > 
> > > > > + * @timestamp_type: Timestamp flags; V4L2_BUF_FLAGS_TIMESTAMP_*
> > > > > 
> > > > >   * @gfp_flags:	additional gfp flags used when allocating the
> > > > >   buffers.
> > > > >   *		Typically this is 0, but it may be e.g. GFP_DMA or 
__GFP_DMA32
> > > > >   *		to force the buffer allocation to a specific memory zone.
> > > > > 
> > > > > diff --git a/include/uapi/linux/videodev2.h
> > > > > b/include/uapi/linux/videodev2.h index 691077d..c57765e 100644
> > > > > --- a/include/uapi/linux/videodev2.h
> > > > > +++ b/include/uapi/linux/videodev2.h
> > > > > @@ -695,6 +695,16 @@ struct v4l2_buffer {
> > > > > 
> > > > >  #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN		0x00000000
> > > > >  #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC	0x00002000
> > > > >  #define V4L2_BUF_FLAG_TIMESTAMP_COPY		0x00004000
> > > > > 
> > > > > +/*
> > > > > + * Timestamp taken once the first pixel is received (or
> > > > > transmitted).
> > > > > + * If the flag is not set the buffer timestamp is taken at the end
> > > > > of
> > > > > + * the frame. This is not a timestamp type.
> > > > 
> > > > UVC devices timestamp frames when the frame is captured, not when the
> > > > first pixel is transmitted.
> > > 
> > > I.e. we shouldn't set the SOF flag? "When the frame is captured" doesn't
> > > say much, or almost anything in terms of *when*. The frames have
> > > exposure time and rolling shutter makes a difference, too.
> > 
> > The UVC 1.1 specification defines the timestamp as
> > 
> > "The source clock time in native deviceclock units when the raw frame
> > capture begins."
> > 
> > What devices do in practice may differ :-)
> 
> I think that this should mean start-of-frame - exposure time. I'd really
> wonder if any practical implementation does that however.

It's start-of-frame - exposure time - internal delays (UVC webcams are 
supposed to report their internal delay value as well).

> What's your suggestion; should we use the SOF flag for this or do you prefer
> the end-of-frame timestamp instead? I think it'd be quite nice for drivers
> to know which one is which without having to guess, and based on the above
> start-of-frame comes as close to that definition as is meaningful.

SOF is better than EOF. Do we need a start-of-capture flag, or could we 
document SOF as meaning start-of-capture or start-of-reception depending on 
what the device can do ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Sakari Ailus]

Hi Laurent,

On Thu, Aug 29, 2013 at 01:25:05AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Wednesday 28 August 2013 19:39:19 Sakari Ailus wrote:
> > On Wed, Aug 28, 2013 at 06:14:44PM +0200, Laurent Pinchart wrote:
> > ...
> > 
> > > > > > diff --git a/drivers/media/usb/uvc/uvc_queue.c
> > > > > > b/drivers/media/usb/uvc/uvc_queue.c index cd962be..0d80512 100644
> > > > > > --- a/drivers/media/usb/uvc/uvc_queue.c
> > > > > > +++ b/drivers/media/usb/uvc/uvc_queue.c
> > > > > > @@ -149,7 +149,8 @@ int uvc_queue_init(struct uvc_video_queue
> > > > > > *queue, enum v4l2_buf_type type,
> > > > > >  	queue->queue.buf_struct_size = sizeof(struct uvc_buffer);
> > > > > >  	queue->queue.ops = &uvc_queue_qops;
> > > > > >  	queue->queue.mem_ops = &vb2_vmalloc_memops;
> > > > > > -	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> > > > > > +	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC
> > > > > > +		| V4L2_BUF_FLAG_TIMESTAMP_SOF;
> > > > > >  	ret = vb2_queue_init(&queue->queue);
> > > > > >  	if (ret)
> > > > > >  		return ret;
> > > > > > diff --git a/include/media/videobuf2-core.h
> > > > > > b/include/media/videobuf2-core.h index 6781258..033efc7 100644
> > > > > > --- a/include/media/videobuf2-core.h
> > > > > > +++ b/include/media/videobuf2-core.h
> > > > > > @@ -307,6 +307,7 @@ struct v4l2_fh;
> > > > > > 
> > > > > >   * @buf_struct_size: size of the driver-specific buffer structure;
> > > > > >   *		"0" indicates the driver doesn't want to use a custom buffer
> > > > > >   *		structure type, so sizeof(struct vb2_buffer) will is used
> > > > > > 
> > > > > > + * @timestamp_type: Timestamp flags; V4L2_BUF_FLAGS_TIMESTAMP_*
> > > > > > 
> > > > > >   * @gfp_flags:	additional gfp flags used when allocating the
> > > > > >   buffers.
> > > > > >   *		Typically this is 0, but it may be e.g. GFP_DMA or 
> __GFP_DMA32
> > > > > >   *		to force the buffer allocation to a specific memory zone.
> > > > > > 
> > > > > > diff --git a/include/uapi/linux/videodev2.h
> > > > > > b/include/uapi/linux/videodev2.h index 691077d..c57765e 100644
> > > > > > --- a/include/uapi/linux/videodev2.h
> > > > > > +++ b/include/uapi/linux/videodev2.h
> > > > > > @@ -695,6 +695,16 @@ struct v4l2_buffer {
> > > > > > 
> > > > > >  #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN		0x00000000
> > > > > >  #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC	0x00002000
> > > > > >  #define V4L2_BUF_FLAG_TIMESTAMP_COPY		0x00004000
> > > > > > 
> > > > > > +/*
> > > > > > + * Timestamp taken once the first pixel is received (or
> > > > > > transmitted).
> > > > > > + * If the flag is not set the buffer timestamp is taken at the end
> > > > > > of
> > > > > > + * the frame. This is not a timestamp type.
> > > > > 
> > > > > UVC devices timestamp frames when the frame is captured, not when the
> > > > > first pixel is transmitted.
> > > > 
> > > > I.e. we shouldn't set the SOF flag? "When the frame is captured" doesn't
> > > > say much, or almost anything in terms of *when*. The frames have
> > > > exposure time and rolling shutter makes a difference, too.
> > > 
> > > The UVC 1.1 specification defines the timestamp as
> > > 
> > > "The source clock time in native deviceclock units when the raw frame
> > > capture begins."
> > > 
> > > What devices do in practice may differ :-)
> > 
> > I think that this should mean start-of-frame - exposure time. I'd really
> > wonder if any practical implementation does that however.
> 
> It's start-of-frame - exposure time - internal delays (UVC webcams are 
> supposed to report their internal delay value as well).

Do they report it? How about the exposure time?

If you know them all you can calculate the SOF timestamp. The fewer
timestamps are available for user programs the better.

It's another matter then if there are webcams that report these values
wrong. Then you could get timestamps that are complete garbage. But I guess
you could compare them to the current monotonic timestamp and detect such
cases.

> > What's your suggestion; should we use the SOF flag for this or do you prefer
> > the end-of-frame timestamp instead? I think it'd be quite nice for drivers
> > to know which one is which without having to guess, and based on the above
> > start-of-frame comes as close to that definition as is meaningful.
> 
> SOF is better than EOF. Do we need a start-of-capture flag, or could we 
> document SOF as meaning start-of-capture or start-of-reception depending on 
> what the device can do ?

One possibility is to dedicate a few flags for this; by using three bits
we'd get eight different timestamps already. But I have to say that fewer is
better. :-)

-- 
Cheers,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Laurent Pinchart]

Hi Sakari,

On Thursday 29 August 2013 14:33:39 Sakari Ailus wrote:
> On Thu, Aug 29, 2013 at 01:25:05AM +0200, Laurent Pinchart wrote:
> > On Wednesday 28 August 2013 19:39:19 Sakari Ailus wrote:
> > > On Wed, Aug 28, 2013 at 06:14:44PM +0200, Laurent Pinchart wrote:
> > > ...
> > > 
> > > > > > UVC devices timestamp frames when the frame is captured, not when
> > > > > > the first pixel is transmitted.
> > > > > 
> > > > > I.e. we shouldn't set the SOF flag? "When the frame is captured"
> > > > > doesn't say much, or almost anything in terms of *when*. The frames
> > > > > have exposure time and rolling shutter makes a difference, too.
> > > > 
> > > > The UVC 1.1 specification defines the timestamp as
> > > > 
> > > > "The source clock time in native deviceclock units when the raw frame
> > > > capture begins."
> > > > 
> > > > What devices do in practice may differ :-)
> > > 
> > > I think that this should mean start-of-frame - exposure time. I'd really
> > > wonder if any practical implementation does that however.
> > 
> > It's start-of-frame - exposure time - internal delays (UVC webcams are
> > supposed to report their internal delay value as well).
> 
> Do they report it? How about the exposure time?

It's supposed to be configurable.

> If you know them all you can calculate the SOF timestamp. The fewer
> timestamps are available for user programs the better.
> 
> It's another matter then if there are webcams that report these values
> wrong.

There most probably are :-)

> Then you could get timestamps that are complete garbage. But I guess you
> could compare them to the current monotonic timestamp and detect such cases.
> 
> > > What's your suggestion; should we use the SOF flag for this or do you
> > > prefer the end-of-frame timestamp instead? I think it'd be quite nice
> > > for drivers to know which one is which without having to guess, and
> > > based on the above start-of-frame comes as close to that definition as
> > > is meaningful.
> > 
> > SOF is better than EOF. Do we need a start-of-capture flag, or could we
> > document SOF as meaning start-of-capture or start-of-reception depending
> > on what the device can do ?
> 
> One possibility is to dedicate a few flags for this; by using three bits
> we'd get eight different timestamps already. But I have to say that fewer is
> better. :-)

Does it really need to be a per-buffer flag ? This seems to be a driver-wide 
(or at least device-wide) behaviour to me.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Sakari Ailus]

Hi Laurent,

On Fri, Aug 30, 2013 at 01:31:44PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Thursday 29 August 2013 14:33:39 Sakari Ailus wrote:
> > On Thu, Aug 29, 2013 at 01:25:05AM +0200, Laurent Pinchart wrote:
> > > On Wednesday 28 August 2013 19:39:19 Sakari Ailus wrote:
> > > > On Wed, Aug 28, 2013 at 06:14:44PM +0200, Laurent Pinchart wrote:
> > > > ...
> > > > 
> > > > > > > UVC devices timestamp frames when the frame is captured, not when
> > > > > > > the first pixel is transmitted.
> > > > > > 
> > > > > > I.e. we shouldn't set the SOF flag? "When the frame is captured"
> > > > > > doesn't say much, or almost anything in terms of *when*. The frames
> > > > > > have exposure time and rolling shutter makes a difference, too.
> > > > > 
> > > > > The UVC 1.1 specification defines the timestamp as
> > > > > 
> > > > > "The source clock time in native deviceclock units when the raw frame
> > > > > capture begins."
> > > > > 
> > > > > What devices do in practice may differ :-)
> > > > 
> > > > I think that this should mean start-of-frame - exposure time. I'd really
> > > > wonder if any practical implementation does that however.
> > > 
> > > It's start-of-frame - exposure time - internal delays (UVC webcams are
> > > supposed to report their internal delay value as well).
> > 
> > Do they report it? How about the exposure time?
> 
> It's supposed to be configurable.

Is the exposure reported with the frame so it could be used to construct the
per-frame SOF timestamp?

> > If you know them all you can calculate the SOF timestamp. The fewer
> > timestamps are available for user programs the better.
> > 
> > It's another matter then if there are webcams that report these values
> > wrong.
> 
> There most probably are :-)
> 
> > Then you could get timestamps that are complete garbage. But I guess you
> > could compare them to the current monotonic timestamp and detect such cases.
> > 
> > > > What's your suggestion; should we use the SOF flag for this or do you
> > > > prefer the end-of-frame timestamp instead? I think it'd be quite nice
> > > > for drivers to know which one is which without having to guess, and
> > > > based on the above start-of-frame comes as close to that definition as
> > > > is meaningful.
> > > 
> > > SOF is better than EOF. Do we need a start-of-capture flag, or could we
> > > document SOF as meaning start-of-capture or start-of-reception depending
> > > on what the device can do ?
> > 
> > One possibility is to dedicate a few flags for this; by using three bits
> > we'd get eight different timestamps already. But I have to say that fewer is
> > better. :-)
> 
> Does it really need to be a per-buffer flag ? This seems to be a driver-wide 
> (or at least device-wide) behaviour to me.

Same goes for timestamp clock sources. It was concluded to use buffer flags
for those as well.

Using a control for the purpose would however require quite non-zero amount
of initialisation code from each driver so that would probably need to be
sorted out first.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Laurent Pinchart]

Hi Sakari,

On Friday 30 August 2013 19:08:48 Sakari Ailus wrote:
> On Fri, Aug 30, 2013 at 01:31:44PM +0200, Laurent Pinchart wrote:
> > On Thursday 29 August 2013 14:33:39 Sakari Ailus wrote:
> > > On Thu, Aug 29, 2013 at 01:25:05AM +0200, Laurent Pinchart wrote:
> > > > On Wednesday 28 August 2013 19:39:19 Sakari Ailus wrote:
> > > > > On Wed, Aug 28, 2013 at 06:14:44PM +0200, Laurent Pinchart wrote:
> > > > > ...
> > > > > 
> > > > > > > > UVC devices timestamp frames when the frame is captured, not
> > > > > > > > when the first pixel is transmitted.
> > > > > > > 
> > > > > > > I.e. we shouldn't set the SOF flag? "When the frame is captured"
> > > > > > > doesn't say much, or almost anything in terms of *when*. The
> > > > > > > frames have exposure time and rolling shutter makes a
> > > > > > > difference, too.
> > > > > > 
> > > > > > The UVC 1.1 specification defines the timestamp as
> > > > > > 
> > > > > > "The source clock time in native deviceclock units when the raw
> > > > > > frame capture begins."
> > > > > > 
> > > > > > What devices do in practice may differ :-)
> > > > > 
> > > > > I think that this should mean start-of-frame - exposure time. I'd
> > > > > really wonder if any practical implementation does that however.
> > > > 
> > > > It's start-of-frame - exposure time - internal delays (UVC webcams are
> > > > supposed to report their internal delay value as well).
> > > 
> > > Do they report it? How about the exposure time?
> > 
> > It's supposed to be configurable.
> 
> Is the exposure reported with the frame so it could be used to construct the
> per-frame SOF timestamp?

Not when auto-exposure is turned on I'm afraid :-S

I believe that the capture timestamp makes more sense than the SOF timestamp 
for applications. SOF/EOF are more of a poor man's timestamp in case nothing 
else is available, but when you want to synchronize multiple audio and/or 
video streams the capture timestamp is what you're interested in. I don't 
think converting a capture timestamp to an SOF would be a good idea.

> > > If you know them all you can calculate the SOF timestamp. The fewer
> > > timestamps are available for user programs the better.
> > > 
> > > It's another matter then if there are webcams that report these values
> > > wrong.
> > 
> > There most probably are :-)
> > 
> > > Then you could get timestamps that are complete garbage. But I guess you
> > > could compare them to the current monotonic timestamp and detect such
> > > cases.
> > >
> > > > > What's your suggestion; should we use the SOF flag for this or do
> > > > > you prefer the end-of-frame timestamp instead? I think it'd be quite
> > > > > nice for drivers to know which one is which without having to guess,
> > > > > and based on the above start-of-frame comes as close to that
> > > > > definition as is meaningful.
> > > > 
> > > > SOF is better than EOF. Do we need a start-of-capture flag, or could
> > > > we document SOF as meaning start-of-capture or start-of-reception
> > > > depending on what the device can do ?
> > > 
> > > One possibility is to dedicate a few flags for this; by using three bits
> > > we'd get eight different timestamps already. But I have to say that
> > > fewer is better. :-)
> > 
> > Does it really need to be a per-buffer flag ? This seems to be a
> > driver-wide (or at least device-wide) behaviour to me.
> 
> Same goes for timestamp clock sources. It was concluded to use buffer flags
> for those as well.

Yes, and I don't think I was convinced, so I'm not convinced here either :-)

> Using a control for the purpose would however require quite non-zero amount
> of initialisation code from each driver so that would probably need to be
> sorted out first.

We could also use a capabilities flag.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Sakari Ailus]

Hi Laurent,

On Sat, Aug 31, 2013 at 11:43:18PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Friday 30 August 2013 19:08:48 Sakari Ailus wrote:
> > On Fri, Aug 30, 2013 at 01:31:44PM +0200, Laurent Pinchart wrote:
> > > On Thursday 29 August 2013 14:33:39 Sakari Ailus wrote:
> > > > On Thu, Aug 29, 2013 at 01:25:05AM +0200, Laurent Pinchart wrote:
> > > > > On Wednesday 28 August 2013 19:39:19 Sakari Ailus wrote:
> > > > > > On Wed, Aug 28, 2013 at 06:14:44PM +0200, Laurent Pinchart wrote:
> > > > > > ...
> > > > > > 
> > > > > > > > > UVC devices timestamp frames when the frame is captured, not
> > > > > > > > > when the first pixel is transmitted.
> > > > > > > > 
> > > > > > > > I.e. we shouldn't set the SOF flag? "When the frame is captured"
> > > > > > > > doesn't say much, or almost anything in terms of *when*. The
> > > > > > > > frames have exposure time and rolling shutter makes a
> > > > > > > > difference, too.
> > > > > > > 
> > > > > > > The UVC 1.1 specification defines the timestamp as
> > > > > > > 
> > > > > > > "The source clock time in native deviceclock units when the raw
> > > > > > > frame capture begins."
> > > > > > > 
> > > > > > > What devices do in practice may differ :-)
> > > > > > 
> > > > > > I think that this should mean start-of-frame - exposure time. I'd
> > > > > > really wonder if any practical implementation does that however.
> > > > > 
> > > > > It's start-of-frame - exposure time - internal delays (UVC webcams are
> > > > > supposed to report their internal delay value as well).
> > > > 
> > > > Do they report it? How about the exposure time?
> > > 
> > > It's supposed to be configurable.
> > 
> > Is the exposure reported with the frame so it could be used to construct the
> > per-frame SOF timestamp?
> 
> Not when auto-exposure is turned on I'm afraid :-S
> 
> I believe that the capture timestamp makes more sense than the SOF timestamp 
> for applications. SOF/EOF are more of a poor man's timestamp in case nothing 
> else is available, but when you want to synchronize multiple audio and/or 
> video streams the capture timestamp is what you're interested in. I don't 
> think converting a capture timestamp to an SOF would be a good idea.

I'm not quite sure of that --- I think the SOF/EOF will be more stable than
the exposure start which depends on the exposure time. If you're recording a
video you may want to keep the time between the frames constant.

Nevertheless --- if we don't get such a timestamp from the device this will
only remain speculation. Applications might be best using e.g. half the
frame period to get a guesstimate of the differences between the two
timestamps.

> > > > If you know them all you can calculate the SOF timestamp. The fewer
> > > > timestamps are available for user programs the better.
> > > > 
> > > > It's another matter then if there are webcams that report these values
> > > > wrong.
> > > 
> > > There most probably are :-)
> > > 
> > > > Then you could get timestamps that are complete garbage. But I guess you
> > > > could compare them to the current monotonic timestamp and detect such
> > > > cases.
> > > >
> > > > > > What's your suggestion; should we use the SOF flag for this or do
> > > > > > you prefer the end-of-frame timestamp instead? I think it'd be quite
> > > > > > nice for drivers to know which one is which without having to guess,
> > > > > > and based on the above start-of-frame comes as close to that
> > > > > > definition as is meaningful.
> > > > > 
> > > > > SOF is better than EOF. Do we need a start-of-capture flag, or could
> > > > > we document SOF as meaning start-of-capture or start-of-reception
> > > > > depending on what the device can do ?
> > > > 
> > > > One possibility is to dedicate a few flags for this; by using three bits
> > > > we'd get eight different timestamps already. But I have to say that
> > > > fewer is better. :-)
> > > 
> > > Does it really need to be a per-buffer flag ? This seems to be a
> > > driver-wide (or at least device-wide) behaviour to me.
> > 
> > Same goes for timestamp clock sources. It was concluded to use buffer flags
> > for those as well.
> 
> Yes, and I don't think I was convinced, so I'm not convinced here either :-)
> 
> > Using a control for the purpose would however require quite non-zero amount
> > of initialisation code from each driver so that would probably need to be
> > sorted out first.
> 
> We could also use a capabilities flag.

Interesting idea. I'm fine that as well. Hans?

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Laurent Pinchart]

Hi Sakari,

On Thursday 05 September 2013 19:31:30 Sakari Ailus wrote:
> On Sat, Aug 31, 2013 at 11:43:18PM +0200, Laurent Pinchart wrote:
> > On Friday 30 August 2013 19:08:48 Sakari Ailus wrote:
> > > On Fri, Aug 30, 2013 at 01:31:44PM +0200, Laurent Pinchart wrote:
> > > > On Thursday 29 August 2013 14:33:39 Sakari Ailus wrote:
> > > > > On Thu, Aug 29, 2013 at 01:25:05AM +0200, Laurent Pinchart wrote:
> > > > > > On Wednesday 28 August 2013 19:39:19 Sakari Ailus wrote:
> > > > > > > On Wed, Aug 28, 2013 at 06:14:44PM +0200, Laurent Pinchart
> > > > > > > wrote:
> > > > > > > ...
> > > > > > > 
> > > > > > > > > > UVC devices timestamp frames when the frame is captured,
> > > > > > > > > > not when the first pixel is transmitted.
> > > > > > > > > 
> > > > > > > > > I.e. we shouldn't set the SOF flag? "When the frame is
> > > > > > > > > captured" doesn't say much, or almost anything in terms of
> > > > > > > > > *when*. The frames have exposure time and rolling shutter
> > > > > > > > > makes a difference, too.
> > > > > > > > 
> > > > > > > > The UVC 1.1 specification defines the timestamp as
> > > > > > > > 
> > > > > > > > "The source clock time in native deviceclock units when the
> > > > > > > > raw frame capture begins."
> > > > > > > > 
> > > > > > > > What devices do in practice may differ :-)
> > > > > > > 
> > > > > > > I think that this should mean start-of-frame - exposure time.
> > > > > > > I'd really wonder if any practical implementation does that
> > > > > > > however.
> > > > > > 
> > > > > > It's start-of-frame - exposure time - internal delays (UVC webcams
> > > > > > are supposed to report their internal delay value as well).
> > > > > 
> > > > > Do they report it? How about the exposure time?
> > > > 
> > > > It's supposed to be configurable.
> > > 
> > > Is the exposure reported with the frame so it could be used to construct
> > > the per-frame SOF timestamp?
> > 
> > Not when auto-exposure is turned on I'm afraid :-S
> > 
> > I believe that the capture timestamp makes more sense than the SOF
> > timestamp for applications. SOF/EOF are more of a poor man's timestamp in
> > case nothing else is available, but when you want to synchronize multiple
> > audio and/or video streams the capture timestamp is what you're
> > interested in. I don't think converting a capture timestamp to an SOF
> > would be a good idea.
>
> I'm not quite sure of that --- I think the SOF/EOF will be more stable than
> the exposure start which depends on the exposure time. If you're recording a
> video you may want to keep the time between the frames constant.

I can see two main use cases for timestamps. The first one is multi-stream 
synchronization (audio and video, stereo video, ...), the second one is 
playback rate control.

To synchronize media streams you need to timestamp samples with a common 
clock. Timestamps must be correlated to the time at which the sound and/or 
image events occur. If we consider the speed of sound and speed of light as 
negligible (the former could be compensated for if needed, but that's out of 
scope), the time at which the sound or image is produced can be considered as 
equal to the time at which they're captured. Given that we only need to 
synchronize streams here, an offset wouldn't matter, so any clock that is 
synchronized to the capture clock with a fixed offset would do. The SOF event, 
in particular, will do if the capture time and device processing time is 
constant, and if interrupt latencies are kept small enough.. So will the EOF 
event if the transmission time is also constant.

Granted, frames are not captured at a precise point of time, as the sensor 
needs to be exposed for a certain duration. There is thus no such thing as a 
capture time, we instead have a capture interval. However, that's irrelevant 
for multi-video synchronization purposes. It could matter for audio+video 
synchronization though.

Regarding playback rate control, the goal is to render frames at the same rate 
they are captured. If the frame rate isn't constant (for instance because of a 
variable exposure time), then a time stamp is required for every frame. Here 
we care about the difference between timestamps for two consecutive frames, 
and the start of capture timestamp is what will give best results.

Let's consider three frames, A, B and C, captured as follows.


00000000001111111111222222222233333333334444444444555555555566666666667777
01234567890123456789012345678901234567890123456789012345678901234567890123

| --------- A ------------ |      | ----- B ----- |      | ----- C ----- |

On the playback side, we want to display A for a duration of 34. If we 
timestamp the frames with the start of capture time, we will have the 
following timestamps.

A  0
B  34
C  57

B-A = 34, which is the time during which A needs to be displayed.

If we use the end of capture time, we will get

A  27
B  50
C  73

B-A = 23, which is too short.

> Nevertheless --- if we don't get such a timestamp from the device this will
> only remain speculation. Applications might be best using e.g. half the
> frame period to get a guesstimate of the differences between the two
> timestamps.

Obviously if the device can't provide the start of capture timestamp we will 
need to use any source of timestamps, but I believe we should aim for start of 
capture as a first class citizen.

> > > > > If you know them all you can calculate the SOF timestamp. The fewer
> > > > > timestamps are available for user programs the better.
> > > > > 
> > > > > It's another matter then if there are webcams that report these
> > > > > values wrong.
> > > > 
> > > > There most probably are :-)
> > > > 
> > > > > Then you could get timestamps that are complete garbage. But I guess
> > > > > you could compare them to the current monotonic timestamp and detect
> > > > > such cases.
> > > > > 
> > > > > > > What's your suggestion; should we use the SOF flag for this or
> > > > > > > do you prefer the end-of-frame timestamp instead? I think it'd
> > > > > > > be quite nice for drivers to know which one is which without
> > > > > > > having to guess, and based on the above start-of-frame comes as
> > > > > > > close to that definition as is meaningful.
> > > > > > 
> > > > > > SOF is better than EOF. Do we need a start-of-capture flag, or
> > > > > > could we document SOF as meaning start-of-capture or start-of-
> > > > > > reception depending on what the device can do ?
> > > > > 
> > > > > One possibility is to dedicate a few flags for this; by using three
> > > > > bits we'd get eight different timestamps already. But I have to say
> > > > > that fewer is better. :-)
> > > > 
> > > > Does it really need to be a per-buffer flag ? This seems to be a
> > > > driver-wide (or at least device-wide) behaviour to me.
> > > 
> > > Same goes for timestamp clock sources. It was concluded to use buffer
> > > flags for those as well.
> > 
> > Yes, and I don't think I was convinced, so I'm not convinced here either
> > :-)
> > 
> > > Using a control for the purpose would however require quite non-zero
> > > amount of initialisation code from each driver so that would probably
> > > need to be sorted out first.
> > 
> > We could also use a capabilities flag.
> 
> Interesting idea. I'm fine that as well. Hans?
-- 
Regards,

Laurent Pinchart

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Hans Verkuil]

Sakari asked me to reply to this old thread...

On 09/06/13 13:05, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Thursday 05 September 2013 19:31:30 Sakari Ailus wrote:
>> On Sat, Aug 31, 2013 at 11:43:18PM +0200, Laurent Pinchart wrote:
>>> On Friday 30 August 2013 19:08:48 Sakari Ailus wrote:
>>>> On Fri, Aug 30, 2013 at 01:31:44PM +0200, Laurent Pinchart wrote:
>>>>> On Thursday 29 August 2013 14:33:39 Sakari Ailus wrote:
>>>>>> On Thu, Aug 29, 2013 at 01:25:05AM +0200, Laurent Pinchart wrote:
>>>>>>> On Wednesday 28 August 2013 19:39:19 Sakari Ailus wrote:
>>>>>>>> On Wed, Aug 28, 2013 at 06:14:44PM +0200, Laurent Pinchart
>>>>>>>> wrote:
>>>>>>>> ...
>>>>>>>>
>>>>>>>>>>> UVC devices timestamp frames when the frame is captured,
>>>>>>>>>>> not when the first pixel is transmitted.
>>>>>>>>>>
>>>>>>>>>> I.e. we shouldn't set the SOF flag? "When the frame is
>>>>>>>>>> captured" doesn't say much, or almost anything in terms of
>>>>>>>>>> *when*. The frames have exposure time and rolling shutter
>>>>>>>>>> makes a difference, too.
>>>>>>>>>
>>>>>>>>> The UVC 1.1 specification defines the timestamp as
>>>>>>>>>
>>>>>>>>> "The source clock time in native deviceclock units when the
>>>>>>>>> raw frame capture begins."
>>>>>>>>>
>>>>>>>>> What devices do in practice may differ :-)
>>>>>>>>
>>>>>>>> I think that this should mean start-of-frame - exposure time.
>>>>>>>> I'd really wonder if any practical implementation does that
>>>>>>>> however.
>>>>>>>
>>>>>>> It's start-of-frame - exposure time - internal delays (UVC webcams
>>>>>>> are supposed to report their internal delay value as well).
>>>>>>
>>>>>> Do they report it? How about the exposure time?
>>>>>
>>>>> It's supposed to be configurable.
>>>>
>>>> Is the exposure reported with the frame so it could be used to construct
>>>> the per-frame SOF timestamp?
>>>
>>> Not when auto-exposure is turned on I'm afraid :-S
>>>
>>> I believe that the capture timestamp makes more sense than the SOF
>>> timestamp for applications. SOF/EOF are more of a poor man's timestamp in
>>> case nothing else is available, but when you want to synchronize multiple
>>> audio and/or video streams the capture timestamp is what you're
>>> interested in. I don't think converting a capture timestamp to an SOF
>>> would be a good idea.
>>
>> I'm not quite sure of that --- I think the SOF/EOF will be more stable than
>> the exposure start which depends on the exposure time. If you're recording a
>> video you may want to keep the time between the frames constant.
> 
> I can see two main use cases for timestamps. The first one is multi-stream 
> synchronization (audio and video, stereo video, ...), the second one is 
> playback rate control.
> 
> To synchronize media streams you need to timestamp samples with a common 
> clock. Timestamps must be correlated to the time at which the sound and/or 
> image events occur. If we consider the speed of sound and speed of light as 
> negligible (the former could be compensated for if needed, but that's out of 
> scope), the time at which the sound or image is produced can be considered as 
> equal to the time at which they're captured. Given that we only need to 
> synchronize streams here, an offset wouldn't matter, so any clock that is 
> synchronized to the capture clock with a fixed offset would do. The SOF event, 
> in particular, will do if the capture time and device processing time is 
> constant, and if interrupt latencies are kept small enough.. So will the EOF 
> event if the transmission time is also constant.
> 
> Granted, frames are not captured at a precise point of time, as the sensor 
> needs to be exposed for a certain duration. There is thus no such thing as a 
> capture time, we instead have a capture interval. However, that's irrelevant 
> for multi-video synchronization purposes. It could matter for audio+video 
> synchronization though.
> 
> Regarding playback rate control, the goal is to render frames at the same rate 
> they are captured. If the frame rate isn't constant (for instance because of a 
> variable exposure time), then a time stamp is required for every frame. Here 
> we care about the difference between timestamps for two consecutive frames, 
> and the start of capture timestamp is what will give best results.
> 
> Let's consider three frames, A, B and C, captured as follows.
> 
> 
> 00000000001111111111222222222233333333334444444444555555555566666666667777
> 01234567890123456789012345678901234567890123456789012345678901234567890123
> 
> | --------- A ------------ |      | ----- B ----- |      | ----- C ----- |
> 
> On the playback side, we want to display A for a duration of 34. If we 
> timestamp the frames with the start of capture time, we will have the 
> following timestamps.
> 
> A  0
> B  34
> C  57
> 
> B-A = 34, which is the time during which A needs to be displayed.
> 
> If we use the end of capture time, we will get
> 
> A  27
> B  50
> C  73
> 
> B-A = 23, which is too short.
> 
>> Nevertheless --- if we don't get such a timestamp from the device this will
>> only remain speculation. Applications might be best using e.g. half the
>> frame period to get a guesstimate of the differences between the two
>> timestamps.
> 
> Obviously if the device can't provide the start of capture timestamp we will 
> need to use any source of timestamps, but I believe we should aim for start of 
> capture as a first class citizen.
> 
>>>>>> If you know them all you can calculate the SOF timestamp. The fewer
>>>>>> timestamps are available for user programs the better.
>>>>>>
>>>>>> It's another matter then if there are webcams that report these
>>>>>> values wrong.
>>>>>
>>>>> There most probably are :-)
>>>>>
>>>>>> Then you could get timestamps that are complete garbage. But I guess
>>>>>> you could compare them to the current monotonic timestamp and detect
>>>>>> such cases.
>>>>>>
>>>>>>>> What's your suggestion; should we use the SOF flag for this or
>>>>>>>> do you prefer the end-of-frame timestamp instead? I think it'd
>>>>>>>> be quite nice for drivers to know which one is which without
>>>>>>>> having to guess, and based on the above start-of-frame comes as
>>>>>>>> close to that definition as is meaningful.
>>>>>>>
>>>>>>> SOF is better than EOF. Do we need a start-of-capture flag, or
>>>>>>> could we document SOF as meaning start-of-capture or start-of-
>>>>>>> reception depending on what the device can do ?
>>>>>>
>>>>>> One possibility is to dedicate a few flags for this; by using three
>>>>>> bits we'd get eight different timestamps already. But I have to say
>>>>>> that fewer is better. :-)
>>>>>
>>>>> Does it really need to be a per-buffer flag ? This seems to be a
>>>>> driver-wide (or at least device-wide) behaviour to me.
>>>>
>>>> Same goes for timestamp clock sources. It was concluded to use buffer
>>>> flags for those as well.
>>>
>>> Yes, and I don't think I was convinced, so I'm not convinced here either
>>> :-)
>>>
>>>> Using a control for the purpose would however require quite non-zero
>>>> amount of initialisation code from each driver so that would probably
>>>> need to be sorted out first.
>>>
>>> We could also use a capabilities flag.
>>
>> Interesting idea. I'm fine that as well. Hans?

That would work for uvc, but not in the general case. Depending on the video
routing you might have either SOF or EOF timestamps. Unlikely, I admit, but
I feel keeping this flag in v4l2_buffers is the most generic solution.

Regards,

	Hans

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Laurent Pinchart]

Hi Hans,

On Thursday 12 December 2013 13:37:10 Hans Verkuil wrote:
> Sakari asked me to reply to this old thread...

He asked me to reply as well :-)

> On 09/06/13 13:05, Laurent Pinchart wrote:
> > On Thursday 05 September 2013 19:31:30 Sakari Ailus wrote:
> >> On Sat, Aug 31, 2013 at 11:43:18PM +0200, Laurent Pinchart wrote:
> >>> On Friday 30 August 2013 19:08:48 Sakari Ailus wrote:
> >>>> On Fri, Aug 30, 2013 at 01:31:44PM +0200, Laurent Pinchart wrote:
> >>>>> On Thursday 29 August 2013 14:33:39 Sakari Ailus wrote:
> >>>>>> On Thu, Aug 29, 2013 at 01:25:05AM +0200, Laurent Pinchart wrote:
> >>>>>>> On Wednesday 28 August 2013 19:39:19 Sakari Ailus wrote:
> >>>>>>>> On Wed, Aug 28, 2013 at 06:14:44PM +0200, Laurent Pinchart
> >>>>>>>> wrote:
> >>>>>>>> ...
> >>>>>>>> 
> >>>>>>>>>>> UVC devices timestamp frames when the frame is captured,
> >>>>>>>>>>> not when the first pixel is transmitted.
> >>>>>>>>>> 
> >>>>>>>>>> I.e. we shouldn't set the SOF flag? "When the frame is
> >>>>>>>>>> captured" doesn't say much, or almost anything in terms of
> >>>>>>>>>> *when*. The frames have exposure time and rolling shutter
> >>>>>>>>>> makes a difference, too.
> >>>>>>>>> 
> >>>>>>>>> The UVC 1.1 specification defines the timestamp as
> >>>>>>>>> 
> >>>>>>>>> "The source clock time in native deviceclock units when the
> >>>>>>>>> raw frame capture begins."
> >>>>>>>>> 
> >>>>>>>>> What devices do in practice may differ :-)
> >>>>>>>> 
> >>>>>>>> I think that this should mean start-of-frame - exposure time.
> >>>>>>>> I'd really wonder if any practical implementation does that
> >>>>>>>> however.
> >>>>>>> 
> >>>>>>> It's start-of-frame - exposure time - internal delays (UVC webcams
> >>>>>>> are supposed to report their internal delay value as well).
> >>>>>> 
> >>>>>> Do they report it? How about the exposure time?
> >>>>> 
> >>>>> It's supposed to be configurable.
> >>>> 
> >>>> Is the exposure reported with the frame so it could be used to
> >>>> construct
> >>>> the per-frame SOF timestamp?
> >>> 
> >>> Not when auto-exposure is turned on I'm afraid :-S
> >>> 
> >>> I believe that the capture timestamp makes more sense than the SOF
> >>> timestamp for applications. SOF/EOF are more of a poor man's timestamp
> >>> in case nothing else is available, but when you want to synchronize
> >>> multiple audio and/or video streams the capture timestamp is what you're
> >>> interested in. I don't think converting a capture timestamp to an SOF
> >>> would be a good idea.
> >> 
> >> I'm not quite sure of that --- I think the SOF/EOF will be more stable
> >> than the exposure start which depends on the exposure time. If you're
> >> recording a video you may want to keep the time between the frames
> >> constant.
> > 
> > I can see two main use cases for timestamps. The first one is multi-stream
> > synchronization (audio and video, stereo video, ...), the second one is
> > playback rate control.
> > 
> > To synchronize media streams you need to timestamp samples with a common
> > clock. Timestamps must be correlated to the time at which the sound and/or
> > image events occur. If we consider the speed of sound and speed of light
> > as negligible (the former could be compensated for if needed, but that's
> > out of scope), the time at which the sound or image is produced can be
> > considered as equal to the time at which they're captured. Given that we
> > only need to synchronize streams here, an offset wouldn't matter, so any
> > clock that is synchronized to the capture clock with a fixed offset would
> > do. The SOF event, in particular, will do if the capture time and device
> > processing time is constant, and if interrupt latencies are kept small
> > enough.. So will the EOF event if the transmission time is also constant.
> > 
> > Granted, frames are not captured at a precise point of time, as the sensor
> > needs to be exposed for a certain duration. There is thus no such thing as
> > a capture time, we instead have a capture interval. However, that's
> > irrelevant for multi-video synchronization purposes. It could matter for
> > audio+video synchronization though.
> > 
> > Regarding playback rate control, the goal is to render frames at the same
> > rate they are captured. If the frame rate isn't constant (for instance
> > because of a variable exposure time), then a time stamp is required for
> > every frame. Here we care about the difference between timestamps for two
> > consecutive frames, and the start of capture timestamp is what will give
> > best results.
> > 
> > Let's consider three frames, A, B and C, captured as follows.
> > 
> > 
> > 00000000001111111111222222222233333333334444444444555555555566666666667777
> > 01234567890123456789012345678901234567890123456789012345678901234567890123
> > 
> > | --------- A ------------ |      | ----- B ----- |      | ----- C ----- |
> > 
> > On the playback side, we want to display A for a duration of 34. If we
> > timestamp the frames with the start of capture time, we will have the
> > following timestamps.
> > 
> > A  0
> > B  34
> > C  57
> > 
> > B-A = 34, which is the time during which A needs to be displayed.
> > 
> > If we use the end of capture time, we will get
> > 
> > A  27
> > B  50
> > C  73
> > 
> > B-A = 23, which is too short.
> > 
> >> Nevertheless --- if we don't get such a timestamp from the device this
> >> will only remain speculation. Applications might be best using e.g. half
> >> the frame period to get a guesstimate of the differences between the two
> >> timestamps.
> > 
> > Obviously if the device can't provide the start of capture timestamp we
> > will need to use any source of timestamps, but I believe we should aim
> > for start of capture as a first class citizen.
> > 
> >>>>>> If you know them all you can calculate the SOF timestamp. The fewer
> >>>>>> timestamps are available for user programs the better.
> >>>>>> 
> >>>>>> It's another matter then if there are webcams that report these
> >>>>>> values wrong.
> >>>>> 
> >>>>> There most probably are :-)
> >>>>> 
> >>>>>> Then you could get timestamps that are complete garbage. But I guess
> >>>>>> you could compare them to the current monotonic timestamp and detect
> >>>>>> such cases.
> >>>>>> 
> >>>>>>>> What's your suggestion; should we use the SOF flag for this or
> >>>>>>>> do you prefer the end-of-frame timestamp instead? I think it'd
> >>>>>>>> be quite nice for drivers to know which one is which without
> >>>>>>>> having to guess, and based on the above start-of-frame comes as
> >>>>>>>> close to that definition as is meaningful.
> >>>>>>> 
> >>>>>>> SOF is better than EOF. Do we need a start-of-capture flag, or
> >>>>>>> could we document SOF as meaning start-of-capture or start-of-
> >>>>>>> reception depending on what the device can do ?
> >>>>>> 
> >>>>>> One possibility is to dedicate a few flags for this; by using three
> >>>>>> bits we'd get eight different timestamps already. But I have to say
> >>>>>> that fewer is better. :-)
> >>>>> 
> >>>>> Does it really need to be a per-buffer flag ? This seems to be a
> >>>>> driver-wide (or at least device-wide) behaviour to me.
> >>>> 
> >>>> Same goes for timestamp clock sources. It was concluded to use buffer
> >>>> flags for those as well.
> >>> 
> >>> Yes, and I don't think I was convinced, so I'm not convinced here either
> >>> 
> >>> :-)
> >>>
> >>>> Using a control for the purpose would however require quite non-zero
> >>>> amount of initialisation code from each driver so that would probably
> >>>> need to be sorted out first.
> >>> 
> >>> We could also use a capabilities flag.
> >> 
> >> Interesting idea. I'm fine that as well. Hans?
> 
> That would work for uvc, but not in the general case. Depending on the video
> routing you might have either SOF or EOF timestamps. Unlikely, I admit, but
> I feel keeping this flag in v4l2_buffers is the most generic solution.

My main concern about this (beside using an extra buffer flags bit, which is a 
scarce resource - but OK, that's not really a big concern) is complexity for 
userspace. Correctly handling buffer timestamps when the timestamp type can 
vary per buffer isn't easy, and I most applications will likely implement it 
wrong. I expect most applications to look at the timestamp type of the first 
buffer and use that information for all subsequent buffers. This would defeat 
the point of having per-buffer timestamp types.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Hans Verkuil]

Hi Laurent,

On 01/31/2014 04:39 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Thursday 12 December 2013 13:37:10 Hans Verkuil wrote:
>> Sakari asked me to reply to this old thread...
> 
> He asked me to reply as well :-)
> 
>> On 09/06/13 13:05, Laurent Pinchart wrote:
>>> On Thursday 05 September 2013 19:31:30 Sakari Ailus wrote:
>>>> On Sat, Aug 31, 2013 at 11:43:18PM +0200, Laurent Pinchart wrote:
>>>>> On Friday 30 August 2013 19:08:48 Sakari Ailus wrote:
>>>>>> On Fri, Aug 30, 2013 at 01:31:44PM +0200, Laurent Pinchart wrote:
>>>>>>> On Thursday 29 August 2013 14:33:39 Sakari Ailus wrote:
>>>>>>>> On Thu, Aug 29, 2013 at 01:25:05AM +0200, Laurent Pinchart wrote:
>>>>>>>>> On Wednesday 28 August 2013 19:39:19 Sakari Ailus wrote:
>>>>>>>>>> On Wed, Aug 28, 2013 at 06:14:44PM +0200, Laurent Pinchart
>>>>>>>>>> wrote:
>>>>>>>>>> ...
>>>>>>>>>>
>>>>>>>>>>>>> UVC devices timestamp frames when the frame is captured,
>>>>>>>>>>>>> not when the first pixel is transmitted.
>>>>>>>>>>>>
>>>>>>>>>>>> I.e. we shouldn't set the SOF flag? "When the frame is
>>>>>>>>>>>> captured" doesn't say much, or almost anything in terms of
>>>>>>>>>>>> *when*. The frames have exposure time and rolling shutter
>>>>>>>>>>>> makes a difference, too.
>>>>>>>>>>>
>>>>>>>>>>> The UVC 1.1 specification defines the timestamp as
>>>>>>>>>>>
>>>>>>>>>>> "The source clock time in native deviceclock units when the
>>>>>>>>>>> raw frame capture begins."
>>>>>>>>>>>
>>>>>>>>>>> What devices do in practice may differ :-)
>>>>>>>>>>
>>>>>>>>>> I think that this should mean start-of-frame - exposure time.
>>>>>>>>>> I'd really wonder if any practical implementation does that
>>>>>>>>>> however.
>>>>>>>>>
>>>>>>>>> It's start-of-frame - exposure time - internal delays (UVC webcams
>>>>>>>>> are supposed to report their internal delay value as well).
>>>>>>>>
>>>>>>>> Do they report it? How about the exposure time?
>>>>>>>
>>>>>>> It's supposed to be configurable.
>>>>>>
>>>>>> Is the exposure reported with the frame so it could be used to
>>>>>> construct
>>>>>> the per-frame SOF timestamp?
>>>>>
>>>>> Not when auto-exposure is turned on I'm afraid :-S
>>>>>
>>>>> I believe that the capture timestamp makes more sense than the SOF
>>>>> timestamp for applications. SOF/EOF are more of a poor man's timestamp
>>>>> in case nothing else is available, but when you want to synchronize
>>>>> multiple audio and/or video streams the capture timestamp is what you're
>>>>> interested in. I don't think converting a capture timestamp to an SOF
>>>>> would be a good idea.
>>>>
>>>> I'm not quite sure of that --- I think the SOF/EOF will be more stable
>>>> than the exposure start which depends on the exposure time. If you're
>>>> recording a video you may want to keep the time between the frames
>>>> constant.
>>>
>>> I can see two main use cases for timestamps. The first one is multi-stream
>>> synchronization (audio and video, stereo video, ...), the second one is
>>> playback rate control.
>>>
>>> To synchronize media streams you need to timestamp samples with a common
>>> clock. Timestamps must be correlated to the time at which the sound and/or
>>> image events occur. If we consider the speed of sound and speed of light
>>> as negligible (the former could be compensated for if needed, but that's
>>> out of scope), the time at which the sound or image is produced can be
>>> considered as equal to the time at which they're captured. Given that we
>>> only need to synchronize streams here, an offset wouldn't matter, so any
>>> clock that is synchronized to the capture clock with a fixed offset would
>>> do. The SOF event, in particular, will do if the capture time and device
>>> processing time is constant, and if interrupt latencies are kept small
>>> enough.. So will the EOF event if the transmission time is also constant.
>>>
>>> Granted, frames are not captured at a precise point of time, as the sensor
>>> needs to be exposed for a certain duration. There is thus no such thing as
>>> a capture time, we instead have a capture interval. However, that's
>>> irrelevant for multi-video synchronization purposes. It could matter for
>>> audio+video synchronization though.
>>>
>>> Regarding playback rate control, the goal is to render frames at the same
>>> rate they are captured. If the frame rate isn't constant (for instance
>>> because of a variable exposure time), then a time stamp is required for
>>> every frame. Here we care about the difference between timestamps for two
>>> consecutive frames, and the start of capture timestamp is what will give
>>> best results.
>>>
>>> Let's consider three frames, A, B and C, captured as follows.
>>>
>>>
>>> 00000000001111111111222222222233333333334444444444555555555566666666667777
>>> 01234567890123456789012345678901234567890123456789012345678901234567890123
>>>
>>> | --------- A ------------ |      | ----- B ----- |      | ----- C ----- |
>>>
>>> On the playback side, we want to display A for a duration of 34. If we
>>> timestamp the frames with the start of capture time, we will have the
>>> following timestamps.
>>>
>>> A  0
>>> B  34
>>> C  57
>>>
>>> B-A = 34, which is the time during which A needs to be displayed.
>>>
>>> If we use the end of capture time, we will get
>>>
>>> A  27
>>> B  50
>>> C  73
>>>
>>> B-A = 23, which is too short.
>>>
>>>> Nevertheless --- if we don't get such a timestamp from the device this
>>>> will only remain speculation. Applications might be best using e.g. half
>>>> the frame period to get a guesstimate of the differences between the two
>>>> timestamps.
>>>
>>> Obviously if the device can't provide the start of capture timestamp we
>>> will need to use any source of timestamps, but I believe we should aim
>>> for start of capture as a first class citizen.
>>>
>>>>>>>> If you know them all you can calculate the SOF timestamp. The fewer
>>>>>>>> timestamps are available for user programs the better.
>>>>>>>>
>>>>>>>> It's another matter then if there are webcams that report these
>>>>>>>> values wrong.
>>>>>>>
>>>>>>> There most probably are :-)
>>>>>>>
>>>>>>>> Then you could get timestamps that are complete garbage. But I guess
>>>>>>>> you could compare them to the current monotonic timestamp and detect
>>>>>>>> such cases.
>>>>>>>>
>>>>>>>>>> What's your suggestion; should we use the SOF flag for this or
>>>>>>>>>> do you prefer the end-of-frame timestamp instead? I think it'd
>>>>>>>>>> be quite nice for drivers to know which one is which without
>>>>>>>>>> having to guess, and based on the above start-of-frame comes as
>>>>>>>>>> close to that definition as is meaningful.
>>>>>>>>>
>>>>>>>>> SOF is better than EOF. Do we need a start-of-capture flag, or
>>>>>>>>> could we document SOF as meaning start-of-capture or start-of-
>>>>>>>>> reception depending on what the device can do ?
>>>>>>>>
>>>>>>>> One possibility is to dedicate a few flags for this; by using three
>>>>>>>> bits we'd get eight different timestamps already. But I have to say
>>>>>>>> that fewer is better. :-)
>>>>>>>
>>>>>>> Does it really need to be a per-buffer flag ? This seems to be a
>>>>>>> driver-wide (or at least device-wide) behaviour to me.
>>>>>>
>>>>>> Same goes for timestamp clock sources. It was concluded to use buffer
>>>>>> flags for those as well.
>>>>>
>>>>> Yes, and I don't think I was convinced, so I'm not convinced here either
>>>>>
>>>>> :-)
>>>>>
>>>>>> Using a control for the purpose would however require quite non-zero
>>>>>> amount of initialisation code from each driver so that would probably
>>>>>> need to be sorted out first.
>>>>>
>>>>> We could also use a capabilities flag.
>>>>
>>>> Interesting idea. I'm fine that as well. Hans?
>>
>> That would work for uvc, but not in the general case. Depending on the video
>> routing you might have either SOF or EOF timestamps. Unlikely, I admit, but
>> I feel keeping this flag in v4l2_buffers is the most generic solution.
> 
> My main concern about this (beside using an extra buffer flags bit, which is a 
> scarce resource - but OK, that's not really a big concern) is complexity for 
> userspace. Correctly handling buffer timestamps when the timestamp type can 
> vary per buffer isn't easy, and I most applications will likely implement it 
> wrong. I expect most applications to look at the timestamp type of the first 
> buffer and use that information for all subsequent buffers. This would defeat 
> the point of having per-buffer timestamp types.

How about defining a capability for use with ENUMINPUT/OUTPUT? I agree that this
won't change between buffers, but it is a property of a specific input or output.

There are more than enough bits available in v4l2_input/output to add one for
SOF timestamps.

Regards,

	Hans

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Sakari Ailus]

Hi Hans and Laurent,

On Fri, Jan 31, 2014 at 04:45:56PM +0100, Hans Verkuil wrote:
> How about defining a capability for use with ENUMINPUT/OUTPUT? I agree that this
> won't change between buffers, but it is a property of a specific input or output.

Over 80 characters per line. :-P

> There are more than enough bits available in v4l2_input/output to add one for
> SOF timestamps.

In complex devices with a non-linear media graph inputs and outputs are not
very relevant, and for that reason many drivers do not even implement them.
I'd rather not bind video buffer queues to inputs or outputs.

My personal favourite is still to use controls for the purpose but the
buffer flags come close, too, especially now that we're using them for
timestamp sources.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Hans Verkuil]

Hi Sakari,

On 01/31/2014 05:42 PM, Sakari Ailus wrote:
> Hi Hans and Laurent,
> 
> On Fri, Jan 31, 2014 at 04:45:56PM +0100, Hans Verkuil wrote:
>> How about defining a capability for use with ENUMINPUT/OUTPUT? I agree that this
>> won't change between buffers, but it is a property of a specific input or output.
> 
> Over 80 characters per line. :-P

Stupid thunderbird doesn't show the column, and I can't enable
automatic word-wrap because that plays hell with patches. Solutions
welcome!

> 
>> There are more than enough bits available in v4l2_input/output to add one for
>> SOF timestamps.
> 
> In complex devices with a non-linear media graph inputs and outputs are not
> very relevant, and for that reason many drivers do not even implement them.
> I'd rather not bind video buffer queues to inputs or outputs.

Then we end up again with buffer flags. It's a property of the selected input
or output, so if you can't/don't want to use that, then it's buffer flags.

Which I like as well, but it's probably useful that the documentation states
that it can only change if the input or output changes as well.

> My personal favourite is still to use controls for the purpose but the
> buffer flags come close, too, especially now that we're using them for
> timestamp sources.

Laurent, can we please end this discussion? It makes perfect sense to store
this information as a BUF_FLAG IMHO. You can just do a QUERYBUF once after
you called REQBUFS and you know what you have to deal with.

Regards,

	Hans

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Sakari Ailus]

Hi Hans,

Hans Verkuil wrote:
> Hi Sakari,
> 
> On 01/31/2014 05:42 PM, Sakari Ailus wrote:
>> Hi Hans and Laurent,
>>
>> On Fri, Jan 31, 2014 at 04:45:56PM +0100, Hans Verkuil wrote:
>>> How about defining a capability for use with ENUMINPUT/OUTPUT? I agree that this
>>> won't change between buffers, but it is a property of a specific input or output.
>>
>> Over 80 characters per line. :-P
> 
> Stupid thunderbird doesn't show the column, and I can't enable
> automatic word-wrap because that plays hell with patches. Solutions
> welcome!

Does it? I've used Thunderbird (well, mostly just tried) but I've
thought it behaves the same way as Seamonkey: wrapping only applies to
newly written lines. My $EDITOR does the same when I use Mutt.

-- 
Regards,

Sakari Ailus
sakari.ailus@iki.fi

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Laurent Pinchart]

Hi Hans,

On Friday 31 January 2014 18:21:15 Hans Verkuil wrote:
> On 01/31/2014 05:42 PM, Sakari Ailus wrote:
> > On Fri, Jan 31, 2014 at 04:45:56PM +0100, Hans Verkuil wrote:
> >>
> >> How about defining a capability for use with ENUMINPUT/OUTPUT? I agree
> >> that this won't change between buffers, but it is a property of a
> >> specific input or output.
> >
> > Over 80 characters per line. :-P
> 
> Stupid thunderbird doesn't show the column, and I can't enable
> automatic word-wrap because that plays hell with patches. Solutions
> welcome!
> 
> >> There are more than enough bits available in v4l2_input/output to add one
> >> for SOF timestamps.
> > 
> > In complex devices with a non-linear media graph inputs and outputs are
> > not very relevant, and for that reason many drivers do not even implement
> > them. I'd rather not bind video buffer queues to inputs or outputs.
> 
> Then we end up again with buffer flags. It's a property of the selected
> input or output, so if you can't/don't want to use that, then it's buffer
> flags.
> 
> Which I like as well, but it's probably useful that the documentation states
> that it can only change if the input or output changes as well.
> 
> > My personal favourite is still to use controls for the purpose but the
> > buffer flags come close, too, especially now that we're using them for
> > timestamp sources.
> 
> Laurent, can we please end this discussion? It makes perfect sense to store
> this information as a BUF_FLAG IMHO. You can just do a QUERYBUF once after
> you called REQBUFS and you know what you have to deal with.

I'm OK with a buffer flag. Can we state in the documentation that the same 
timestamp flag will be used for all buffers and that QUERYBUF can be used to 
query it before the first buffer gets dequeued ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4.1 3/3] v4l: Add V4L2_BUF_FLAG_TIMESTAMP_SOF and use it

[Sakari Ailus]

On Sun, Feb 02, 2014 at 10:27:49AM +0100, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Friday 31 January 2014 18:21:15 Hans Verkuil wrote:
> > On 01/31/2014 05:42 PM, Sakari Ailus wrote:
> > > On Fri, Jan 31, 2014 at 04:45:56PM +0100, Hans Verkuil wrote:
> > >>
> > >> How about defining a capability for use with ENUMINPUT/OUTPUT? I agree
> > >> that this won't change between buffers, but it is a property of a
> > >> specific input or output.
> > >
> > > Over 80 characters per line. :-P
> > 
> > Stupid thunderbird doesn't show the column, and I can't enable
> > automatic word-wrap because that plays hell with patches. Solutions
> > welcome!
> > 
> > >> There are more than enough bits available in v4l2_input/output to add one
> > >> for SOF timestamps.
> > > 
> > > In complex devices with a non-linear media graph inputs and outputs are
> > > not very relevant, and for that reason many drivers do not even implement
> > > them. I'd rather not bind video buffer queues to inputs or outputs.
> > 
> > Then we end up again with buffer flags. It's a property of the selected
> > input or output, so if you can't/don't want to use that, then it's buffer
> > flags.
> > 
> > Which I like as well, but it's probably useful that the documentation states
> > that it can only change if the input or output changes as well.
> > 
> > > My personal favourite is still to use controls for the purpose but the
> > > buffer flags come close, too, especially now that we're using them for
> > > timestamp sources.
> > 
> > Laurent, can we please end this discussion? It makes perfect sense to store
> > this information as a BUF_FLAG IMHO. You can just do a QUERYBUF once after
> > you called REQBUFS and you know what you have to deal with.
> 
> I'm OK with a buffer flag. Can we state in the documentation that the same 
> timestamp flag will be used for all buffers and that QUERYBUF can be used to 
> query it before the first buffer gets dequeued ?

I think that'd be reasonable. Otherwise it'd be quite painful to use. I'll
resend.

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[PATCH v4.2 3/4] v4l: Add timestamp source flags, mask and document them

[Sakari Ailus]

Some devices do not produce timestamps that correspond to the end of the
frame. The user space should be informed on the matter. This patch achieves
that by adding buffer flags (and a mask) for timestamp sources since more
possible timestamping points are expected than just two.

A three-bit mask is defined (V4L2_BUF_FLAG_TSTAMP_SRC_MASK) and two of the
eight possible values is are defined V4L2_BUF_FLAG_TSTAMP_SRC_EOF for end of
frame (value zero) V4L2_BUF_FLAG_TSTAMP_SRC_SOE for start of exposure (next
value).

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
since v4.1:
- Replace SOF flag by SOE flag
- Add mask for timestamp sources

 Documentation/DocBook/media/v4l/io.xml |   28 ++++++++++++++++++++++------
 drivers/media/usb/uvc/uvc_queue.c      |    3 ++-
 include/media/videobuf2-core.h         |    2 ++
 include/uapi/linux/videodev2.h         |    4 ++++
 4 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
index 2c155cc..451626f 100644
--- a/Documentation/DocBook/media/v4l/io.xml
+++ b/Documentation/DocBook/media/v4l/io.xml
@@ -654,12 +654,6 @@ plane, are stored in struct <structname>v4l2_plane</structname> instead.
 In that case, struct <structname>v4l2_buffer</structname> contains an array of
 plane structures.</para>
 
-      <para>For timestamp types that are sampled from the system clock
-(V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC) it is guaranteed that the timestamp is
-taken after the complete frame has been received (or transmitted in
-case of video output devices). For other kinds of
-timestamps this may vary depending on the driver.</para>
-
     <table frame="none" pgwide="1" id="v4l2-buffer">
       <title>struct <structname>v4l2_buffer</structname></title>
       <tgroup cols="4">
@@ -1120,6 +1114,28 @@ in which case caches have not been used.</entry>
 	    <entry>The CAPTURE buffer timestamp has been taken from the
 	    corresponding OUTPUT buffer. This flag applies only to mem2mem devices.</entry>
 	  </row>
+	  <row>
+	    <entry><constant>V4L2_BUF_FLAG_TSTAMP_SRC_MASK</constant></entry>
+	    <entry>0x00070000</entry>
+	    <entry>Mask for timestamp sources below. The timestamp source
+	    defines the point of time the timestamp is taken in relation to
+	    the frame. Logical and operation between the
+	    <structfield>flags</structfield> field and
+	    <constant>V4L2_BUF_FLAG_TSTAMP_SRC_MASK</constant> produces the
+	    value of the timestamp source.</entry>
+	  </row>
+	  <row>
+	    <entry><constant>V4L2_BUF_FLAG_TSTAMP_SRC_EOF</constant></entry>
+	    <entry>0x00000000</entry>
+	    <entry>"End of frame." The buffer timestamp has been taken when
+	    the last pixel of the frame has been received.</entry>
+	  </row>
+	  <row>
+	    <entry><constant>V4L2_BUF_FLAG_TSTAMP_SRC_SOE</constant></entry>
+	    <entry>0x00010000</entry>
+	    <entry>"Start of exposure." The buffer timestamp has been taken
+	    when the exposure of the frame has begun.</entry>
+	  </row>
 	</tbody>
       </tgroup>
     </table>
diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
index cd962be..a9292d2 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -149,7 +149,8 @@ int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type,
 	queue->queue.buf_struct_size = sizeof(struct uvc_buffer);
 	queue->queue.ops = &uvc_queue_qops;
 	queue->queue.mem_ops = &vb2_vmalloc_memops;
-	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC
+		| V4L2_BUF_FLAG_TSTAMP_SRC_SOE;
 	ret = vb2_queue_init(&queue->queue);
 	if (ret)
 		return ret;
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index bef53ce..b6b992d 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -312,6 +312,8 @@ struct v4l2_fh;
  * @buf_struct_size: size of the driver-specific buffer structure;
  *		"0" indicates the driver doesn't want to use a custom buffer
  *		structure type, so sizeof(struct vb2_buffer) will is used
+ * @timestamp_type: Timestamp flags; V4L2_BUF_FLAGS_TIMESTAMP_* and
+ *		V4L2_BUF_FLAGS_TSTAMP_SRC_*
  * @gfp_flags:	additional gfp flags used when allocating the buffers.
  *		Typically this is 0, but it may be e.g. GFP_DMA or __GFP_DMA32
  *		to force the buffer allocation to a specific memory zone.
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index e9ee444..82e8661 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -695,6 +695,10 @@ struct v4l2_buffer {
 #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN		0x00000000
 #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC	0x00002000
 #define V4L2_BUF_FLAG_TIMESTAMP_COPY		0x00004000
+/* Timestamp sources. */
+#define V4L2_BUF_FLAG_TSTAMP_SRC_MASK		0x00070000
+#define V4L2_BUF_FLAG_TSTAMP_SRC_EOF		0x00000000
+#define V4L2_BUF_FLAG_TSTAMP_SRC_SOE		0x00010000
 
 /**
  * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor
-- 
1.7.10.4

[PATCH v4.2 4/4] v4l: Document timestamp buffer flag behaviour

[Sakari Ailus]

Timestamp buffer flags are constant at the moment. Document them so that 1)
they're always valid and 2) not changed by the drivers. This leaves room to
extend the functionality later on if needed.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 Documentation/DocBook/media/v4l/io.xml |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
index 451626f..f523725 100644
--- a/Documentation/DocBook/media/v4l/io.xml
+++ b/Documentation/DocBook/media/v4l/io.xml
@@ -654,6 +654,14 @@ plane, are stored in struct <structname>v4l2_plane</structname> instead.
 In that case, struct <structname>v4l2_buffer</structname> contains an array of
 plane structures.</para>
 
+    <para>Buffers that have been dequeued come with timestamps. These
+    timestamps can be taken from different clocks and at different part of
+    the frame, depending on the driver. Please see flags in the masks
+    <constant>V4L2_BUF_FLAG_TIMESTAMP_MASK</constant> and
+    <constant>V4L2_BUF_FLAG_TSTAMP_SRC_MASK</constant> in <xref
+    linkend="buffer-flags">. These flags are guaranteed to be always valid
+    and will not be changed by the driver.</para>
+
     <table frame="none" pgwide="1" id="v4l2-buffer">
       <title>struct <structname>v4l2_buffer</structname></title>
       <tgroup cols="4">
-- 
1.7.10.4

Re: [PATCH v4.2 4/4] v4l: Document timestamp buffer flag behaviour

[Hans Verkuil]

On 02/07/2014 11:52 PM, Sakari Ailus wrote:
> Timestamp buffer flags are constant at the moment. Document them so that 1)
> they're always valid and 2) not changed by the drivers. This leaves room to
> extend the functionality later on if needed.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> ---
>  Documentation/DocBook/media/v4l/io.xml |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
> index 451626f..f523725 100644
> --- a/Documentation/DocBook/media/v4l/io.xml
> +++ b/Documentation/DocBook/media/v4l/io.xml
> @@ -654,6 +654,14 @@ plane, are stored in struct <structname>v4l2_plane</structname> instead.
>  In that case, struct <structname>v4l2_buffer</structname> contains an array of
>  plane structures.</para>
>  
> +    <para>Buffers that have been dequeued come with timestamps. These
> +    timestamps can be taken from different clocks and at different part of
> +    the frame, depending on the driver. Please see flags in the masks
> +    <constant>V4L2_BUF_FLAG_TIMESTAMP_MASK</constant> and
> +    <constant>V4L2_BUF_FLAG_TSTAMP_SRC_MASK</constant> in <xref
> +    linkend="buffer-flags">. These flags are guaranteed to be always valid
> +    and will not be changed by the driver.</para>

That's a bit too strong. Different inputs or outputs may have different timestamp
sources. Also add a note that the SOE does not apply to outputs (there is no
exposure there after all). For EOF the formulation for outputs should be:
"..last pixel of the frame has been transmitted."

For the COPY mode I think the SRC_MASK bits should be copied as well. That should
be stated in the documentation.

Regards,

	Hans

> +
>      <table frame="none" pgwide="1" id="v4l2-buffer">
>        <title>struct <structname>v4l2_buffer</structname></title>
>        <tgroup cols="4">
> 

Re: [PATCH v4.2 4/4] v4l: Document timestamp buffer flag behaviour

[Sakari Ailus]

Hi Hans,

Thanks for the comments.

Hans Verkuil wrote:
> On 02/07/2014 11:52 PM, Sakari Ailus wrote:
>> Timestamp buffer flags are constant at the moment. Document them so that 1)
>> they're always valid and 2) not changed by the drivers. This leaves room to
>> extend the functionality later on if needed.
>>
>> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
>> ---
>>  Documentation/DocBook/media/v4l/io.xml |    8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
>> index 451626f..f523725 100644
>> --- a/Documentation/DocBook/media/v4l/io.xml
>> +++ b/Documentation/DocBook/media/v4l/io.xml
>> @@ -654,6 +654,14 @@ plane, are stored in struct <structname>v4l2_plane</structname> instead.
>>  In that case, struct <structname>v4l2_buffer</structname> contains an array of
>>  plane structures.</para>
>>  
>> +    <para>Buffers that have been dequeued come with timestamps. These
>> +    timestamps can be taken from different clocks and at different part of
>> +    the frame, depending on the driver. Please see flags in the masks
>> +    <constant>V4L2_BUF_FLAG_TIMESTAMP_MASK</constant> and
>> +    <constant>V4L2_BUF_FLAG_TSTAMP_SRC_MASK</constant> in <xref
>> +    linkend="buffer-flags">. These flags are guaranteed to be always valid
>> +    and will not be changed by the driver.</para>
> 
> That's a bit too strong. Different inputs or outputs may have different timestamp

Fixed.

> sources. Also add a note that the SOE does not apply to outputs (there is no

The flags are already documented separately, but I can add that to make
it explicit.

> exposure there after all). For EOF the formulation for outputs should be:
> "..last pixel of the frame has been transmitted."

Added.

> For the COPY mode I think the SRC_MASK bits should be copied as well. That should
> be stated in the documentation.

Good point. I'll make that a separate patch as it changes a number of
drivers. Perhaps this could be a good occasion to clean up some timecode
field usage from those drivers, too.

-- 
Kind regards,

Sakari Ailus
sakari.ailus@iki.fi

Re: [PATCH v4.2 3/4] v4l: Add timestamp source flags, mask and document them

[Hans Verkuil]

On 02/07/2014 11:52 PM, Sakari Ailus wrote:
> Some devices do not produce timestamps that correspond to the end of the
> frame. The user space should be informed on the matter. This patch achieves
> that by adding buffer flags (and a mask) for timestamp sources since more
> possible timestamping points are expected than just two.
> 
> A three-bit mask is defined (V4L2_BUF_FLAG_TSTAMP_SRC_MASK) and two of the
> eight possible values is are defined V4L2_BUF_FLAG_TSTAMP_SRC_EOF for end of
> frame (value zero) V4L2_BUF_FLAG_TSTAMP_SRC_SOE for start of exposure (next
> value).
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>

I would prefer to split the uvc change into a separate patch. It doesn't really
belong here.

Regards,

	Hans

> ---
> since v4.1:
> - Replace SOF flag by SOE flag
> - Add mask for timestamp sources
> 
>  Documentation/DocBook/media/v4l/io.xml |   28 ++++++++++++++++++++++------
>  drivers/media/usb/uvc/uvc_queue.c      |    3 ++-
>  include/media/videobuf2-core.h         |    2 ++
>  include/uapi/linux/videodev2.h         |    4 ++++
>  4 files changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
> index 2c155cc..451626f 100644
> --- a/Documentation/DocBook/media/v4l/io.xml
> +++ b/Documentation/DocBook/media/v4l/io.xml
> @@ -654,12 +654,6 @@ plane, are stored in struct <structname>v4l2_plane</structname> instead.
>  In that case, struct <structname>v4l2_buffer</structname> contains an array of
>  plane structures.</para>
>  
> -      <para>For timestamp types that are sampled from the system clock
> -(V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC) it is guaranteed that the timestamp is
> -taken after the complete frame has been received (or transmitted in
> -case of video output devices). For other kinds of
> -timestamps this may vary depending on the driver.</para>
> -
>      <table frame="none" pgwide="1" id="v4l2-buffer">
>        <title>struct <structname>v4l2_buffer</structname></title>
>        <tgroup cols="4">
> @@ -1120,6 +1114,28 @@ in which case caches have not been used.</entry>
>  	    <entry>The CAPTURE buffer timestamp has been taken from the
>  	    corresponding OUTPUT buffer. This flag applies only to mem2mem devices.</entry>
>  	  </row>
> +	  <row>
> +	    <entry><constant>V4L2_BUF_FLAG_TSTAMP_SRC_MASK</constant></entry>
> +	    <entry>0x00070000</entry>
> +	    <entry>Mask for timestamp sources below. The timestamp source
> +	    defines the point of time the timestamp is taken in relation to
> +	    the frame. Logical and operation between the
> +	    <structfield>flags</structfield> field and
> +	    <constant>V4L2_BUF_FLAG_TSTAMP_SRC_MASK</constant> produces the
> +	    value of the timestamp source.</entry>
> +	  </row>
> +	  <row>
> +	    <entry><constant>V4L2_BUF_FLAG_TSTAMP_SRC_EOF</constant></entry>
> +	    <entry>0x00000000</entry>
> +	    <entry>"End of frame." The buffer timestamp has been taken when
> +	    the last pixel of the frame has been received.</entry>
> +	  </row>
> +	  <row>
> +	    <entry><constant>V4L2_BUF_FLAG_TSTAMP_SRC_SOE</constant></entry>
> +	    <entry>0x00010000</entry>
> +	    <entry>"Start of exposure." The buffer timestamp has been taken
> +	    when the exposure of the frame has begun.</entry>
> +	  </row>
>  	</tbody>
>        </tgroup>
>      </table>
> diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
> index cd962be..a9292d2 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -149,7 +149,8 @@ int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type,
>  	queue->queue.buf_struct_size = sizeof(struct uvc_buffer);
>  	queue->queue.ops = &uvc_queue_qops;
>  	queue->queue.mem_ops = &vb2_vmalloc_memops;
> -	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> +	queue->queue.timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC
> +		| V4L2_BUF_FLAG_TSTAMP_SRC_SOE;
>  	ret = vb2_queue_init(&queue->queue);
>  	if (ret)
>  		return ret;
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index bef53ce..b6b992d 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -312,6 +312,8 @@ struct v4l2_fh;
>   * @buf_struct_size: size of the driver-specific buffer structure;
>   *		"0" indicates the driver doesn't want to use a custom buffer
>   *		structure type, so sizeof(struct vb2_buffer) will is used
> + * @timestamp_type: Timestamp flags; V4L2_BUF_FLAGS_TIMESTAMP_* and
> + *		V4L2_BUF_FLAGS_TSTAMP_SRC_*
>   * @gfp_flags:	additional gfp flags used when allocating the buffers.
>   *		Typically this is 0, but it may be e.g. GFP_DMA or __GFP_DMA32
>   *		to force the buffer allocation to a specific memory zone.
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index e9ee444..82e8661 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -695,6 +695,10 @@ struct v4l2_buffer {
>  #define V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN		0x00000000
>  #define V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC	0x00002000
>  #define V4L2_BUF_FLAG_TIMESTAMP_COPY		0x00004000
> +/* Timestamp sources. */
> +#define V4L2_BUF_FLAG_TSTAMP_SRC_MASK		0x00070000
> +#define V4L2_BUF_FLAG_TSTAMP_SRC_EOF		0x00000000
> +#define V4L2_BUF_FLAG_TSTAMP_SRC_SOE		0x00010000
>  
>  /**
>   * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor
> 

Re: [PATCH v4.2 3/4] v4l: Add timestamp source flags, mask and document them

[Sakari Ailus]

Hans Verkuil wrote:
> On 02/07/2014 11:52 PM, Sakari Ailus wrote:
>> Some devices do not produce timestamps that correspond to the end of the
>> frame. The user space should be informed on the matter. This patch achieves
>> that by adding buffer flags (and a mask) for timestamp sources since more
>> possible timestamping points are expected than just two.
>>
>> A three-bit mask is defined (V4L2_BUF_FLAG_TSTAMP_SRC_MASK) and two of the
>> eight possible values is are defined V4L2_BUF_FLAG_TSTAMP_SRC_EOF for end of
>> frame (value zero) V4L2_BUF_FLAG_TSTAMP_SRC_SOE for start of exposure (next
>> value).
>>
>> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> 
> I would prefer to split the uvc change into a separate patch. It doesn't really
> belong here.

Fine for me. I'll take that into account in the next version.

-- 
Sakari Ailus
sakari.ailus@iki.fi