All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
To: linux-media@vger.kernel.org
Cc: Sakari Ailus <sakari.ailus@iki.fi>,
	Hans Verkuil <hverkuil@xs4all.nl>,
	remi@remlab.net, daniel-gl@gmx.net,
	laurent.pinchart@ideasonboard.com
Subject: Re: [RFC] Timestamps and V4L2
Date: Sat, 22 Sep 2012 19:12:52 +0200	[thread overview]
Message-ID: <505DF194.9030007@gmail.com> (raw)
In-Reply-To: <505DB12F.1090600@iki.fi>

On 09/22/2012 02:38 PM, Sakari Ailus wrote:
>> You are missing one other option:
>>
>> Using v4l2_buffer flags to report the clock
>> -------------------------------------------
>>
>> By defining flags like this:
>>
>> V4L2_BUF_FLAG_CLOCK_MASK 0x7000
>> /* Possible Clocks */
>> V4L2_BUF_FLAG_CLOCK_UNKNOWN 0x0000 /* system or monotonic, we don't 
>> know */
>> V4L2_BUF_FLAG_CLOCK_MONOTONIC 0x1000
>>
>> you could tell the application which clock is used.
>>
>> This does allow for more clocks to be added in the future and clock 
>> selection
>> would then be done by a control or possibly an ioctl. For now there 
>> are no
>> plans to do such things, so this flag should be sufficient. And it can be
>> implemented very efficiently. It works with existing drivers as well, 
>> since
>> they will report CLOCK_UNKNOWN.
>>
>> I am very much in favor of this approach.

+1

I think I like this idea best, it's relatively simple (even with adding
support for reporting flags in VIDIOC_QUERYBUF) for the purpose.

If we ever need the clock selection API I would vote for an IOCTL.
The controls API is a bad choice for something such fundamental as
type of clock for buffer timestamping IMHO. Let's stop making the
controls API a dumping ground for almost everything in V4L2! ;)

> Thanks for adding this. I knew I was forgetting something but didn't 
> remember what --- I swear it was unintentional! :-)
> 
> If we'd add more clocks without providing an ability to choose the clock 
> from the user space, how would the clock be selected? It certainly isn't 
> the driver's job, nor I think it should be system-specific either 
> (platform data on embedded systems).
> 
> It's up to the application and its needs. That would suggest we should 
> always provide monotonic timestamps to applications (besides a potential 
> driver-specific timestamp), and for that purpose the capability flag --- 
> I admit I disliked the idea at first --- is enough.
> 
> What comes to buffer flags, the application would also have to receive 
> the first buffer from the device to even know what kind of timestamps 
> the device uses, or at least call QUERYBUF. And in principle the flag 
> should be checked on every buffer, unless we also specify the flag is 
> the same for all buffers. And at certain point this will stop to make 
> any sense...

Good point. Perhaps VIDIOC_QUERYBUF and VIDIOC_DQBUF should be reporting
timestamps type only for the time they are being called. Not per buffer,
per device. And applications would be checking the flags any time they
want to find out what is the buffer timestamp type. Or every time if it
don't have full control over the device (S/G_PRIORITY).

> A capability flag is cleaner solution from this perspective, and it can 
> be amended by a control (or an ioctl) later on: the flag can be 
> disregarded by applications whenever the control is present. If the 
> application doesn't know about the control it can still rely on the 
> flag. (I think this would be less clean than to go for the control right 
> from the beginning, but better IMO.)

But with the capability flag we would only be able to report one type of
clock, right ?

>>> Device-dependent timestamp
>>> --------------------------
>>>
>>> Should we agree on selectable timestamps, the existing timestamp 
>>> field (or a
>>> union with another field of different type) could be used for the
>>> device-dependent timestamps.
>>
>> No. Device timestamps should get their own field. You want to be able 
>> to relate
>> device timestamps with the monotonic timestamps, so you need both.
>>
>>> Alternatively we can choose to re-use the
>>> existing timecode field.
>>>
>>> At the moment there's no known use case for passing device-dependent
>>> timestamps at the same time with monotonic timestamps.
>>
>> Well, the use case is there, but there is no driver support. The device
>> timestamps should be 64 bits to accomodate things like PTS and DTS from
>> MPEG streams. Since timecode is 128 bits we might want to use two u64 
>> fields
>> or perhaps 4 u32 fields.
> 
> That should be an union for different kinds (or rather types) of 
> device-dependent timestamps. On uvcvideo I think this is u32, not u64. 
> We should be also able to tell what kind device dependent timestamp 
> there is --- should buffer flags be used for that as well?

Timecode has 'type' and 'flags' fields, couldn't it be accommodated for 
reporting device-dependant timestamps as well ?

--

Regards,
Sylwester

  reply	other threads:[~2012-09-22 17:12 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-20 20:21 [RFC] Timestamps and V4L2 Sakari Ailus
2012-09-20 21:08 ` Rémi Denis-Courmont
2012-09-21  8:47 ` Christian Gmeiner
2012-09-21  9:33 ` Hans Verkuil
2012-09-22 12:38   ` Sakari Ailus
2012-09-22 17:12     ` Sylwester Nawrocki [this message]
2012-09-22 20:28       ` Daniel Glöckner
2012-09-23 18:40         ` Sylwester Nawrocki
2012-09-25  0:35           ` Laurent Pinchart
     [not found]             ` <5061DAE3.2080808@samsung.com>
2012-09-25 17:17               ` Kamil Debski
2012-09-26 22:30             ` Sylwester Nawrocki
2012-09-27 10:41               ` Laurent Pinchart
2012-09-23 11:43       ` Sakari Ailus
2012-09-24 20:11         ` Rémi Denis-Courmont
2012-09-25  6:50           ` Hans Verkuil
2012-09-25  0:34       ` Laurent Pinchart
2012-09-25 22:48         ` Sylwester Nawrocki
2012-09-23  9:18     ` Hans Verkuil
2012-09-23 13:07       ` Sakari Ailus
2012-09-24  8:30         ` Hans Verkuil
2012-09-25  0:21       ` Laurent Pinchart
2012-09-24 23:42   ` Laurent Pinchart
2012-09-25  0:00   ` Laurent Pinchart
2012-09-25  6:47     ` Hans Verkuil
2012-09-25 10:48       ` Laurent Pinchart
2012-09-25 10:54         ` Hans Verkuil
2012-09-25 11:09           ` Laurent Pinchart
2012-09-25 20:12           ` Sakari Ailus
2012-09-26  9:13             ` Laurent Pinchart
2012-09-26 19:17               ` Sakari Ailus
2012-09-27 10:55                 ` Laurent Pinchart
2012-09-25 20:05       ` Sakari Ailus
2012-10-15 16:05 ` Sakari Ailus
2012-10-15 18:45   ` Laurent Pinchart
2012-10-15 18:53     ` Chris MacGregor
2012-10-15 19:59       ` Sakari Ailus
2012-10-15 20:10         ` Rémi Denis-Courmont
2012-10-16  1:25         ` Chris MacGregor
2012-10-25  0:47           ` Laurent Pinchart
2012-10-16  6:13     ` Hans Verkuil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=505DF194.9030007@gmail.com \
    --to=sylvester.nawrocki@gmail.com \
    --cc=daniel-gl@gmx.net \
    --cc=hverkuil@xs4all.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=remi@remlab.net \
    --cc=sakari.ailus@iki.fi \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.