diff mbox

[v5,7/7] v4l: Document timestamp buffer flag behaviour

Message ID 1392497585-5084-8-git-send-email-sakari.ailus@iki.fi (mailing list archive)
State New, archived
Headers show

Commit Message

Sakari Ailus Feb. 15, 2014, 8:53 p.m. UTC
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 |   10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Hans Verkuil Feb. 15, 2014, 9:03 p.m. UTC | #1
On 02/15/2014 09:53 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 |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
> index fbd0c6e..4f76565 100644
> --- a/Documentation/DocBook/media/v4l/io.xml
> +++ b/Documentation/DocBook/media/v4l/io.xml
> @@ -653,6 +653,16 @@ 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>Dequeued video buffers 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

s/part/parts/

But I think I would write it somewhat differently:

"The driver decides at which part of the frame and with which clock
the timestamp is taken."

> +    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 autonomously. Changes
> +    in these flags may take place due as a side effect of

s/due//

> +    &VIDIOC-S-INPUT; or &VIDIOC-S-OUTPUT; however.</para>
> +
>      <table frame="none" pgwide="1" id="v4l2-buffer">
>        <title>struct <structname>v4l2_buffer</structname></title>
>        <tgroup cols="4">
> 

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sakari Ailus Feb. 16, 2014, 5:50 p.m. UTC | #2
Hi Hans,

On Sat, Feb 15, 2014 at 10:03:07PM +0100, Hans Verkuil wrote:
> On 02/15/2014 09:53 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 |   10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
> > index fbd0c6e..4f76565 100644
> > --- a/Documentation/DocBook/media/v4l/io.xml
> > +++ b/Documentation/DocBook/media/v4l/io.xml
> > @@ -653,6 +653,16 @@ 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>Dequeued video buffers 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
> 
> s/part/parts/
> 
> But I think I would write it somewhat differently:
> 
> "The driver decides at which part of the frame and with which clock
> the timestamp is taken."

I'll fix both for the next version.

> > +    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 autonomously. Changes
> > +    in these flags may take place due as a side effect of
> 
> s/due//
Laurent Pinchart Feb. 17, 2014, 12:56 a.m. UTC | #3
Hello,

On Saturday 15 February 2014 22:03:07 Hans Verkuil wrote:
> On 02/15/2014 09:53 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 |   10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/Documentation/DocBook/media/v4l/io.xml
> > b/Documentation/DocBook/media/v4l/io.xml index fbd0c6e..4f76565 100644
> > --- a/Documentation/DocBook/media/v4l/io.xml
> > +++ b/Documentation/DocBook/media/v4l/io.xml
> > @@ -653,6 +653,16 @@ 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>Dequeued video buffers 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
> 
> s/part/parts/
> 
> But I think I would write it somewhat differently:
> 
> "The driver decides at which part of the frame and with which clock
> the timestamp is taken."
> 
> > +    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 autonomously.

This sentence sounds a bit confusing to me. What about

"These flags are always valid and are constant across all buffers during the 
whole video stream."

> > Changes
> > +    in these flags may take place due as a side effect of
> 
> s/due//
> 
> > +    &VIDIOC-S-INPUT; or &VIDIOC-S-OUTPUT; however.</para>
> > +
> >      <table frame="none" pgwide="1" id="v4l2-buffer">
> >        <title>struct <structname>v4l2_buffer</structname></title>
> >        <tgroup cols="4">
Hans Verkuil Feb. 17, 2014, 8:43 a.m. UTC | #4
On 02/17/2014 01:56 AM, Laurent Pinchart wrote:
> Hello,
> 
> On Saturday 15 February 2014 22:03:07 Hans Verkuil wrote:
>> On 02/15/2014 09:53 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 |   10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/Documentation/DocBook/media/v4l/io.xml
>>> b/Documentation/DocBook/media/v4l/io.xml index fbd0c6e..4f76565 100644
>>> --- a/Documentation/DocBook/media/v4l/io.xml
>>> +++ b/Documentation/DocBook/media/v4l/io.xml
>>> @@ -653,6 +653,16 @@ 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>Dequeued video buffers 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
>>
>> s/part/parts/
>>
>> But I think I would write it somewhat differently:
>>
>> "The driver decides at which part of the frame and with which clock
>> the timestamp is taken."
>>
>>> +    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 autonomously.
> 
> This sentence sounds a bit confusing to me. What about
> 
> "These flags are always valid and are constant across all buffers during the 
> whole video stream."

I like this.

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sakari Ailus Feb. 17, 2014, 11:32 p.m. UTC | #5
On Mon, Feb 17, 2014 at 09:43:22AM +0100, Hans Verkuil wrote:
> >>> +    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 autonomously.
> > 
> > This sentence sounds a bit confusing to me. What about
> > 
> > "These flags are always valid and are constant across all buffers during the 
> > whole video stream."
> 
> I like this.

I'll put that to the next version.
Sakari Ailus Feb. 17, 2014, 11:33 p.m. UTC | #6
On Tue, Feb 18, 2014 at 01:32:03AM +0200, Sakari Ailus wrote:
> On Mon, Feb 17, 2014 at 09:43:22AM +0100, Hans Verkuil wrote:
> > >>> +    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 autonomously.
> > > 
> > > This sentence sounds a bit confusing to me. What about
> > > 
> > > "These flags are always valid and are constant across all buffers during the 
> > > whole video stream."
> > 
> > I like this.
> 
> I'll put that to the next version.

Well, after removing the second "are ". :-)
Hans Verkuil Feb. 23, 2014, 11:45 a.m. UTC | #7
On 02/15/2014 09:53 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 |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
> index fbd0c6e..4f76565 100644
> --- a/Documentation/DocBook/media/v4l/io.xml
> +++ b/Documentation/DocBook/media/v4l/io.xml
> @@ -653,6 +653,16 @@ 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>Dequeued video buffers 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 autonomously. Changes
> +    in these flags may take place due as a side effect of
> +    &VIDIOC-S-INPUT; or &VIDIOC-S-OUTPUT; however.</para>

There is one exception to this: if the timestamps are copied from the output
buffer to the capture buffer (TIMESTAMP_COPY), then it can change theoretically
for every buffer since it entirely depends on what is being sent to it. The
value comes from userspace and you simply don't have any control over that.

I'm stress testing vb2 in lots of different ways, including timestamp handling.
It's not a pretty sight, I'm afraid. Expect a looong list of patches in the
coming week.

Regards,

	Hans

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

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sakari Ailus Feb. 25, 2014, 5:08 p.m. UTC | #8
Hi Hans,

On Sun, Feb 23, 2014 at 12:45:28PM +0100, Hans Verkuil wrote:
> On 02/15/2014 09:53 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 |   10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
> > index fbd0c6e..4f76565 100644
> > --- a/Documentation/DocBook/media/v4l/io.xml
> > +++ b/Documentation/DocBook/media/v4l/io.xml
> > @@ -653,6 +653,16 @@ 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>Dequeued video buffers 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 autonomously. Changes
> > +    in these flags may take place due as a side effect of
> > +    &VIDIOC-S-INPUT; or &VIDIOC-S-OUTPUT; however.</para>
> 
> There is one exception to this: if the timestamps are copied from the output
> buffer to the capture buffer (TIMESTAMP_COPY), then it can change theoretically
> for every buffer since it entirely depends on what is being sent to it. The
> value comes from userspace and you simply don't have any control over that.

Yes; I agree.

And a good point as well --- the timestamp source flags currently come from
__fill_v4l2_buffer() which takes them from q->timestamp. This isn't right
for m2m devices.

I'll fix and resend (3rd patch most likely).

> I'm stress testing vb2 in lots of different ways, including timestamp handling.
> It's not a pretty sight, I'm afraid. Expect a looong list of patches in the
> coming week.

:-)
Hans Verkuil Feb. 25, 2014, 5:28 p.m. UTC | #9
On 02/25/2014 06:08 PM, Sakari Ailus wrote:
> Hi Hans,
> 
> On Sun, Feb 23, 2014 at 12:45:28PM +0100, Hans Verkuil wrote:
>> On 02/15/2014 09:53 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 |   10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
>>> index fbd0c6e..4f76565 100644
>>> --- a/Documentation/DocBook/media/v4l/io.xml
>>> +++ b/Documentation/DocBook/media/v4l/io.xml
>>> @@ -653,6 +653,16 @@ 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>Dequeued video buffers 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 autonomously. Changes
>>> +    in these flags may take place due as a side effect of
>>> +    &VIDIOC-S-INPUT; or &VIDIOC-S-OUTPUT; however.</para>
>>
>> There is one exception to this: if the timestamps are copied from the output
>> buffer to the capture buffer (TIMESTAMP_COPY), then it can change theoretically
>> for every buffer since it entirely depends on what is being sent to it. The
>> value comes from userspace and you simply don't have any control over that.
> 
> Yes; I agree.
> 
> And a good point as well --- the timestamp source flags currently come from
> __fill_v4l2_buffer() which takes them from q->timestamp. This isn't right
> for m2m devices.
> 
> I'll fix and resend (3rd patch most likely).

You'll want to reference this patch I posted today:

[RFCv1 PATCH 16/20] vb2: fix timecode and flags handling for output buffers

Also available in this git repo:

http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/vb2-part4

The current implementation in vb2 is actually broken (which is one of the
things fixed by this patch): if you prepare a buffer (VIDIOC_PREPARE_BUF)
and only then call VIDIOC_QBUF with a timestamp, that timestamp will be
lost since it will use the one set by PREPARE_BUF (either that or it is
zeroed, I've forgotten which of the two it was).

If you want to take that patch and add your own changes to it, then that's
fine by me. It should be pretty much standalone.

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sakari Ailus Feb. 26, 2014, 12:04 a.m. UTC | #10
Hi Hans,

On Tue, Feb 25, 2014 at 06:28:06PM +0100, Hans Verkuil wrote:
> On 02/25/2014 06:08 PM, Sakari Ailus wrote:
> > Hi Hans,
> > 
> > On Sun, Feb 23, 2014 at 12:45:28PM +0100, Hans Verkuil wrote:
> >> On 02/15/2014 09:53 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 |   10 ++++++++++
> >>>  1 file changed, 10 insertions(+)
> >>>
> >>> diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
> >>> index fbd0c6e..4f76565 100644
> >>> --- a/Documentation/DocBook/media/v4l/io.xml
> >>> +++ b/Documentation/DocBook/media/v4l/io.xml
> >>> @@ -653,6 +653,16 @@ 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>Dequeued video buffers 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 autonomously. Changes
> >>> +    in these flags may take place due as a side effect of
> >>> +    &VIDIOC-S-INPUT; or &VIDIOC-S-OUTPUT; however.</para>
> >>
> >> There is one exception to this: if the timestamps are copied from the output
> >> buffer to the capture buffer (TIMESTAMP_COPY), then it can change theoretically
> >> for every buffer since it entirely depends on what is being sent to it. The
> >> value comes from userspace and you simply don't have any control over that.
> > 
> > Yes; I agree.
> > 
> > And a good point as well --- the timestamp source flags currently come from
> > __fill_v4l2_buffer() which takes them from q->timestamp. This isn't right
> > for m2m devices.
> > 
> > I'll fix and resend (3rd patch most likely).
> 
> You'll want to reference this patch I posted today:
> 
> [RFCv1 PATCH 16/20] vb2: fix timecode and flags handling for output buffers
> 
> Also available in this git repo:
> 
> http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/vb2-part4
> 
> The current implementation in vb2 is actually broken (which is one of the
> things fixed by this patch): if you prepare a buffer (VIDIOC_PREPARE_BUF)
> and only then call VIDIOC_QBUF with a timestamp, that timestamp will be
> lost since it will use the one set by PREPARE_BUF (either that or it is
> zeroed, I've forgotten which of the two it was).
> 
> If you want to take that patch and add your own changes to it, then that's
> fine by me. It should be pretty much standalone.

I'll keep that as-is and write another to pass the timestamp source flags
when needed. Would it be ok if I prepend the patch to the set?
Hans Verkuil Feb. 26, 2014, 12:07 a.m. UTC | #11
On 02/26/2014 01:04 AM, Sakari Ailus wrote:
> Hi Hans,
> 
> On Tue, Feb 25, 2014 at 06:28:06PM +0100, Hans Verkuil wrote:
>> On 02/25/2014 06:08 PM, Sakari Ailus wrote:
>>> Hi Hans,
>>>
>>> On Sun, Feb 23, 2014 at 12:45:28PM +0100, Hans Verkuil wrote:
>>>> On 02/15/2014 09:53 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 |   10 ++++++++++
>>>>>  1 file changed, 10 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
>>>>> index fbd0c6e..4f76565 100644
>>>>> --- a/Documentation/DocBook/media/v4l/io.xml
>>>>> +++ b/Documentation/DocBook/media/v4l/io.xml
>>>>> @@ -653,6 +653,16 @@ 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>Dequeued video buffers 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 autonomously. Changes
>>>>> +    in these flags may take place due as a side effect of
>>>>> +    &VIDIOC-S-INPUT; or &VIDIOC-S-OUTPUT; however.</para>
>>>>
>>>> There is one exception to this: if the timestamps are copied from the output
>>>> buffer to the capture buffer (TIMESTAMP_COPY), then it can change theoretically
>>>> for every buffer since it entirely depends on what is being sent to it. The
>>>> value comes from userspace and you simply don't have any control over that.
>>>
>>> Yes; I agree.
>>>
>>> And a good point as well --- the timestamp source flags currently come from
>>> __fill_v4l2_buffer() which takes them from q->timestamp. This isn't right
>>> for m2m devices.
>>>
>>> I'll fix and resend (3rd patch most likely).
>>
>> You'll want to reference this patch I posted today:
>>
>> [RFCv1 PATCH 16/20] vb2: fix timecode and flags handling for output buffers
>>
>> Also available in this git repo:
>>
>> http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/vb2-part4
>>
>> The current implementation in vb2 is actually broken (which is one of the
>> things fixed by this patch): if you prepare a buffer (VIDIOC_PREPARE_BUF)
>> and only then call VIDIOC_QBUF with a timestamp, that timestamp will be
>> lost since it will use the one set by PREPARE_BUF (either that or it is
>> zeroed, I've forgotten which of the two it was).
>>
>> If you want to take that patch and add your own changes to it, then that's
>> fine by me. It should be pretty much standalone.
> 
> I'll keep that as-is and write another to pass the timestamp source flags
> when needed. Would it be ok if I prepend the patch to the set?
> 

Sure, no problem.

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
index fbd0c6e..4f76565 100644
--- a/Documentation/DocBook/media/v4l/io.xml
+++ b/Documentation/DocBook/media/v4l/io.xml
@@ -653,6 +653,16 @@  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>Dequeued video buffers 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 autonomously. Changes
+    in these flags may take place due as a side effect of
+    &VIDIOC-S-INPUT; or &VIDIOC-S-OUTPUT; however.</para>
+
     <table frame="none" pgwide="1" id="v4l2-buffer">
       <title>struct <structname>v4l2_buffer</structname></title>
       <tgroup cols="4">