| Message ID | 573d42b4b775afd8beeadc7a903cc2190a6f430a.1347619766.git.hans.verkuil@cisco.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Hans, Thanks for the patch. Hans Verkuil wrote: ... > --- a/include/linux/videodev2.h > +++ b/include/linux/videodev2.h > @@ -290,6 +290,7 @@ struct v4l2_capability { > #define V4L2_CAP_ASYNCIO 0x02000000 /* async I/O */ > #define V4L2_CAP_STREAMING 0x04000000 /* streaming I/O ioctls */ > > +#define V4L2_CAP_MONOTONIC_TS 0x40000000 /* uses monotonic timestamps */ > #define V4L2_CAP_DEVICE_CAPS 0x80000000 /* sets device capabilities field */ I have to say I'm still not a big fan of this new capability flag. I had a quick discussion with Laurent, and what he suggested was to use the kernel version to figure out the type of the timestamp. The drivers that use the monotonic time right now wouldn't be affected by the new flag on older kernels. If we've decided we're going to switch to monotonic time anyway, why not just change all the drivers now and forget the capability flag. Instead of a capability flag, the applications would know the type of the timestamp based on the kernel version. In the end we wouldn't be left with a useless flag that every single driver would have to set. It's true that there are some 70 such drivers that need to be converted but it's still a trivial, mechanical task. It would be even easier to implement a helper function called e.g. v4l2_buffer_timestamp() so that every user wouldn't have to convert timespec to timeval. As a result we'd also have a very short transition period to the timestamps we prefer. Kind regards,
Le vendredi 14 septembre 2012 23:25:01, Sakari Ailus a écrit : > I had a quick discussion with Laurent, and what he suggested was to use > the kernel version to figure out the type of the timestamp. The drivers > that use the monotonic time right now wouldn't be affected by the new > flag on older kernels. If we've decided we're going to switch to > monotonic time anyway, why not just change all the drivers now and > forget the capability flag. That does not work In Real Life. People do port old drivers forward to new kernels. People do port new drivers back to old kernels User space needs a flag is needed. Full point.
Hi Rémi, Rémi Denis-Courmont wrote: > Le vendredi 14 septembre 2012 23:25:01, Sakari Ailus a écrit : >> I had a quick discussion with Laurent, and what he suggested was to use >> the kernel version to figure out the type of the timestamp. The drivers >> that use the monotonic time right now wouldn't be affected by the new >> flag on older kernels. If we've decided we're going to switch to >> monotonic time anyway, why not just change all the drivers now and >> forget the capability flag. > > That does not work In Real Life. > > People do port old drivers forward to new kernels. > People do port new drivers back to old kernels Why would you port a driver from an old kernel to a new kernel? Or are you talking about out-of-tree drivers? If you do port drivers across different kernel versions I guess you're supposed to use the appropriate interfaces for those kernels, too. Using a helper function helps here, so compiling a backported driver would fail w/o the helper function that produces the timestamp, forcing the backporter to notice the situation. Anyway, I don't have a very strict opinion on this, so I'm okay with the flag, too, but I personally simply don't think it's the best choice we can make now. Also, without converting the drivers now the user space must live with different kinds of timestamps much longer. Cc Laurent and Hans. Kind regards,
On Fri September 14 2012 23:05:45 Sakari Ailus wrote: > Hi Rémi, > > Rémi Denis-Courmont wrote: > > Le vendredi 14 septembre 2012 23:25:01, Sakari Ailus a écrit : > >> I had a quick discussion with Laurent, and what he suggested was to use > >> the kernel version to figure out the type of the timestamp. The drivers > >> that use the monotonic time right now wouldn't be affected by the new > >> flag on older kernels. If we've decided we're going to switch to > >> monotonic time anyway, why not just change all the drivers now and > >> forget the capability flag. > > > > That does not work In Real Life. > > > > People do port old drivers forward to new kernels. > > People do port new drivers back to old kernels > > Why would you port a driver from an old kernel to a new kernel? Or are > you talking about out-of-tree drivers? More likely the latter. > If you do port drivers across different kernel versions I guess you're > supposed to use the appropriate interfaces for those kernels, too. Using > a helper function helps here, so compiling a backported driver would > fail w/o the helper function that produces the timestamp, forcing the > backporter to notice the situation. > > Anyway, I don't have a very strict opinion on this, so I'm okay with the > flag, too, but I personally simply don't think it's the best choice we > can make now. Also, without converting the drivers now the user space > must live with different kinds of timestamps much longer. There are a number of reasons why I want to go with a flag: - Out-of-tree drivers which are unlikely to switch to monotonic in practice - Backporting drivers - It makes it easy to verify in v4l2-compliance and enforce the use of the monotonic clock. - It's easy for apps to check. The third reason is probably the most important one for me. There tends to be a great deal of inertia before changes like this are applied to new drivers, and without being able to (easily) check this in v4l2-compliance more drivers will be merged that keep using gettimeofday. It's all too easy to miss in a review. That doesn't mean that it isn't a good idea to convert existing drivers asap. But it's not something I'm likely to take up myself. Creating a small helper function as you suggested elsewhere is a good idea as well. I'll write something for that. 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
Hi Hans, On Saturday 15 September 2012 09:41:59 Hans Verkuil wrote: > On Fri September 14 2012 23:05:45 Sakari Ailus wrote: > > Rémi Denis-Courmont wrote: > > > Le vendredi 14 septembre 2012 23:25:01, Sakari Ailus a écrit : > > >> I had a quick discussion with Laurent, and what he suggested was to use > > >> the kernel version to figure out the type of the timestamp. The drivers > > >> that use the monotonic time right now wouldn't be affected by the new > > >> flag on older kernels. If we've decided we're going to switch to > > >> monotonic time anyway, why not just change all the drivers now and > > >> forget the capability flag. > > > > > > That does not work In Real Life. > > > > > > People do port old drivers forward to new kernels. > > > People do port new drivers back to old kernels > > > > Why would you port a driver from an old kernel to a new kernel? Or are > > you talking about out-of-tree drivers? > > More likely the latter. > > > If you do port drivers across different kernel versions I guess you're > > supposed to use the appropriate interfaces for those kernels, too. Using > > a helper function helps here, so compiling a backported driver would > > fail w/o the helper function that produces the timestamp, forcing the > > backporter to notice the situation. > > > > Anyway, I don't have a very strict opinion on this, so I'm okay with the > > flag, too, but I personally simply don't think it's the best choice we > > can make now. Also, without converting the drivers now the user space > > must live with different kinds of timestamps much longer. > > There are a number of reasons why I want to go with a flag: > > - Out-of-tree drivers which are unlikely to switch to monotonic in practice > - Backporting drivers > - It makes it easy to verify in v4l2-compliance and enforce the use of > the monotonic clock. > - It's easy for apps to check. > > The third reason is probably the most important one for me. There tends to > be a great deal of inertia before changes like this are applied to new > drivers, and without being able to (easily) check this in v4l2-compliance > more drivers will be merged that keep using gettimeofday. It's all too easy > to miss in a review. If we switch all existing drivers to monotonic timestamps in kernel release 3.x, v4l2-compliance can just use the version it gets from VIDIOC_QUERYCAP and enforce monotonic timestamps verification if the version is >= 3.x. This isn't more difficult for apps to check than a dedicated flag (although it's less explicit). My concern is identical to Sakari's, I'm not very keen on introducing a flag that all drivers will set in the very near future and that we will have to keep around forever. > That doesn't mean that it isn't a good idea to convert existing drivers > asap. But it's not something I'm likely to take up myself. Sakari, are you volunteering for that ? ;-) > Creating a small helper function as you suggested elsewhere is a good idea > as well. I'll write something for that.
On Sat September 15 2012 11:31:29 Laurent Pinchart wrote: > Hi Hans, > > On Saturday 15 September 2012 09:41:59 Hans Verkuil wrote: > > On Fri September 14 2012 23:05:45 Sakari Ailus wrote: > > > Rémi Denis-Courmont wrote: > > > > Le vendredi 14 septembre 2012 23:25:01, Sakari Ailus a écrit : > > > >> I had a quick discussion with Laurent, and what he suggested was to use > > > >> the kernel version to figure out the type of the timestamp. The drivers > > > >> that use the monotonic time right now wouldn't be affected by the new > > > >> flag on older kernels. If we've decided we're going to switch to > > > >> monotonic time anyway, why not just change all the drivers now and > > > >> forget the capability flag. > > > > > > > > That does not work In Real Life. > > > > > > > > People do port old drivers forward to new kernels. > > > > People do port new drivers back to old kernels > > > > > > Why would you port a driver from an old kernel to a new kernel? Or are > > > you talking about out-of-tree drivers? > > > > More likely the latter. > > > > > If you do port drivers across different kernel versions I guess you're > > > supposed to use the appropriate interfaces for those kernels, too. Using > > > a helper function helps here, so compiling a backported driver would > > > fail w/o the helper function that produces the timestamp, forcing the > > > backporter to notice the situation. > > > > > > Anyway, I don't have a very strict opinion on this, so I'm okay with the > > > flag, too, but I personally simply don't think it's the best choice we > > > can make now. Also, without converting the drivers now the user space > > > must live with different kinds of timestamps much longer. > > > > There are a number of reasons why I want to go with a flag: > > > > - Out-of-tree drivers which are unlikely to switch to monotonic in practice > > - Backporting drivers > > - It makes it easy to verify in v4l2-compliance and enforce the use of > > the monotonic clock. > > - It's easy for apps to check. > > > > The third reason is probably the most important one for me. There tends to > > be a great deal of inertia before changes like this are applied to new > > drivers, and without being able to (easily) check this in v4l2-compliance > > more drivers will be merged that keep using gettimeofday. It's all too easy > > to miss in a review. > > If we switch all existing drivers to monotonic timestamps in kernel release > 3.x, v4l2-compliance can just use the version it gets from VIDIOC_QUERYCAP and > enforce monotonic timestamps verification if the version is >= 3.x. This isn't > more difficult for apps to check than a dedicated flag (although it's less > explicit). I think that checking for the driver (kernel) version is a very poor substitute for testing against a proper flag. One alternative might be to use a v4l2_buffer flag instead. That does have the advantage that in the future we can add additional flags should we need to support different clocks. Should we ever add support to switch clocks dynamically, then a buffer flag is more suitable than a driver capability. In that scenario it does make real sense to have a flag (or really mask). Say something like this: /* Clock Mask */ V4L2_BUF_FLAG_CLOCK_MASK 0xf000 /* Possible Clocks */ V4L2_BUF_FLAG_CLOCK_SYSTEM 0x0000 V4L2_BUF_FLAG_CLOCK_MONOTONIC 0x1000 > My concern is identical to Sakari's, I'm not very keen on introducing a flag > that all drivers will set in the very near future and that we will have to > keep around forever. > > > That doesn't mean that it isn't a good idea to convert existing drivers > > asap. But it's not something I'm likely to take up myself. > > Sakari, are you volunteering for that ? ;-) > > > Creating a small helper function as you suggested elsewhere is a good idea > > as well. I'll write something for that. 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
Hi, On 09/15/2012 11:31 AM, Laurent Pinchart wrote: > Hi Hans, > > On Saturday 15 September 2012 09:41:59 Hans Verkuil wrote: >> On Fri September 14 2012 23:05:45 Sakari Ailus wrote: >>> Rémi Denis-Courmont wrote: >>>> Le vendredi 14 septembre 2012 23:25:01, Sakari Ailus a écrit : >>>>> I had a quick discussion with Laurent, and what he suggested was to use >>>>> the kernel version to figure out the type of the timestamp. The drivers >>>>> that use the monotonic time right now wouldn't be affected by the new >>>>> flag on older kernels. If we've decided we're going to switch to >>>>> monotonic time anyway, why not just change all the drivers now and >>>>> forget the capability flag. >>>> >>>> That does not work In Real Life. >>>> >>>> People do port old drivers forward to new kernels. >>>> People do port new drivers back to old kernels >>> >>> Why would you port a driver from an old kernel to a new kernel? Or are >>> you talking about out-of-tree drivers? >> >> More likely the latter. >> >>> If you do port drivers across different kernel versions I guess you're >>> supposed to use the appropriate interfaces for those kernels, too. Using >>> a helper function helps here, so compiling a backported driver would >>> fail w/o the helper function that produces the timestamp, forcing the >>> backporter to notice the situation. >>> >>> Anyway, I don't have a very strict opinion on this, so I'm okay with the >>> flag, too, but I personally simply don't think it's the best choice we >>> can make now. Also, without converting the drivers now the user space >>> must live with different kinds of timestamps much longer. >> >> There are a number of reasons why I want to go with a flag: >> >> - Out-of-tree drivers which are unlikely to switch to monotonic in practice >> - Backporting drivers >> - It makes it easy to verify in v4l2-compliance and enforce the use of >> the monotonic clock. >> - It's easy for apps to check. >> >> The third reason is probably the most important one for me. There tends to >> be a great deal of inertia before changes like this are applied to new >> drivers, and without being able to (easily) check this in v4l2-compliance >> more drivers will be merged that keep using gettimeofday. It's all too easy >> to miss in a review. > > If we switch all existing drivers to monotonic timestamps in kernel release > 3.x, v4l2-compliance can just use the version it gets from VIDIOC_QUERYCAP and > enforce monotonic timestamps verification if the version is>= 3.x. This isn't > more difficult for apps to check than a dedicated flag (although it's less > explicit). > > My concern is identical to Sakari's, I'm not very keen on introducing a flag > that all drivers will set in the very near future and that we will have to > keep around forever. > >> That doesn't mean that it isn't a good idea to convert existing drivers >> asap. But it's not something I'm likely to take up myself. > > Sakari, are you volunteering for that ? ;-) > >> Creating a small helper function as you suggested elsewhere is a good idea >> as well. I'll write something for that. IMHO, using a flag is going to more reliable, i.e. it is going to be reliable. It shouldn't be a big deal to set the flag, unless we're running out of free bits in the caps field. Once all drivers are converted it could be set in v4l2-core. And applications would always know what they get. -- Regards, Sylwester -- 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
Hi Hans and Laurent, Hans Verkuil wrote: > On Sat September 15 2012 11:31:29 Laurent Pinchart wrote: >> Hi Hans, >> >> On Saturday 15 September 2012 09:41:59 Hans Verkuil wrote: >>> On Fri September 14 2012 23:05:45 Sakari Ailus wrote: >>>> Rémi Denis-Courmont wrote: >>>>> Le vendredi 14 septembre 2012 23:25:01, Sakari Ailus a écrit : >>>>>> I had a quick discussion with Laurent, and what he suggested was to use >>>>>> the kernel version to figure out the type of the timestamp. The drivers >>>>>> that use the monotonic time right now wouldn't be affected by the new >>>>>> flag on older kernels. If we've decided we're going to switch to >>>>>> monotonic time anyway, why not just change all the drivers now and >>>>>> forget the capability flag. >>>>> >>>>> That does not work In Real Life. >>>>> >>>>> People do port old drivers forward to new kernels. >>>>> People do port new drivers back to old kernels >>>> >>>> Why would you port a driver from an old kernel to a new kernel? Or are >>>> you talking about out-of-tree drivers? >>> >>> More likely the latter. >>> >>>> If you do port drivers across different kernel versions I guess you're >>>> supposed to use the appropriate interfaces for those kernels, too. Using >>>> a helper function helps here, so compiling a backported driver would >>>> fail w/o the helper function that produces the timestamp, forcing the >>>> backporter to notice the situation. >>>> >>>> Anyway, I don't have a very strict opinion on this, so I'm okay with the >>>> flag, too, but I personally simply don't think it's the best choice we >>>> can make now. Also, without converting the drivers now the user space >>>> must live with different kinds of timestamps much longer. >>> >>> There are a number of reasons why I want to go with a flag: >>> >>> - Out-of-tree drivers which are unlikely to switch to monotonic in practice >>> - Backporting drivers >>> - It makes it easy to verify in v4l2-compliance and enforce the use of >>> the monotonic clock. >>> - It's easy for apps to check. >>> >>> The third reason is probably the most important one for me. There tends to >>> be a great deal of inertia before changes like this are applied to new >>> drivers, and without being able to (easily) check this in v4l2-compliance >>> more drivers will be merged that keep using gettimeofday. It's all too easy >>> to miss in a review. >> >> If we switch all existing drivers to monotonic timestamps in kernel release >> 3.x, v4l2-compliance can just use the version it gets from VIDIOC_QUERYCAP and >> enforce monotonic timestamps verification if the version is >= 3.x. This isn't >> more difficult for apps to check than a dedicated flag (although it's less >> explicit). > > I think that checking for the driver (kernel) version is a very poor substitute > for testing against a proper flag. That flag should be the default in this case. The flag should be set by the framework instead giving every driver the job of setting it. > One alternative might be to use a v4l2_buffer flag instead. That does have the > advantage that in the future we can add additional flags should we need to > support different clocks. Should we ever add support to switch clocks dynamically, > then a buffer flag is more suitable than a driver capability. In that scenario > it does make real sense to have a flag (or really mask). > > Say something like this: > > /* Clock Mask */ > V4L2_BUF_FLAG_CLOCK_MASK 0xf000 > /* Possible Clocks */ > V4L2_BUF_FLAG_CLOCK_SYSTEM 0x0000 > V4L2_BUF_FLAG_CLOCK_MONOTONIC 0x1000 There are three clocks defined in clock_gettime (2) man page. It'd indeed be good that the timestamp was selectable, but this really depends on the user rather than the driver. As you suggested earlier on, I agree that only monotonic timestamps are seen necessary right now. It might be that raw monotonic timestamps could have some potential use (albeit I don't know a use case right now) but I still wouldn't think users would change the type of the timestamp that often. So I don't see a need for the buffer flag, but I still think it's better than a device capability flag. If we gave the user the ability to pick the type of the timestamp we should move to use timespec at the same time. >> My concern is identical to Sakari's, I'm not very keen on introducing a flag >> that all drivers will set in the very near future and that we will have to >> keep around forever. >> >>> That doesn't mean that it isn't a good idea to convert existing drivers >>> asap. But it's not something I'm likely to take up myself. >> >> Sakari, are you volunteering for that ? ;-) I'd be happy to do that. As the changes are mostly mechanical, it won't really take much time to do that. What comes to new drivers --- I think it's wrong to assume every new driver would have been written wrong kind of timestamps in mind. I mean, what do you think would be the reasons why a driver writer would pick do_gettimeofday() instead of ktime_get_ts() if every driver in the tree already uses ktime_get_ts()? Kind regards.
On Sat September 15 2012 12:37:58 Sakari Ailus wrote: > Hi Hans and Laurent, > > Hans Verkuil wrote: > > On Sat September 15 2012 11:31:29 Laurent Pinchart wrote: > >> Hi Hans, > >> > >> On Saturday 15 September 2012 09:41:59 Hans Verkuil wrote: > >>> On Fri September 14 2012 23:05:45 Sakari Ailus wrote: > >>>> Rémi Denis-Courmont wrote: > >>>>> Le vendredi 14 septembre 2012 23:25:01, Sakari Ailus a écrit : > >>>>>> I had a quick discussion with Laurent, and what he suggested was to use > >>>>>> the kernel version to figure out the type of the timestamp. The drivers > >>>>>> that use the monotonic time right now wouldn't be affected by the new > >>>>>> flag on older kernels. If we've decided we're going to switch to > >>>>>> monotonic time anyway, why not just change all the drivers now and > >>>>>> forget the capability flag. > >>>>> > >>>>> That does not work In Real Life. > >>>>> > >>>>> People do port old drivers forward to new kernels. > >>>>> People do port new drivers back to old kernels > >>>> > >>>> Why would you port a driver from an old kernel to a new kernel? Or are > >>>> you talking about out-of-tree drivers? > >>> > >>> More likely the latter. > >>> > >>>> If you do port drivers across different kernel versions I guess you're > >>>> supposed to use the appropriate interfaces for those kernels, too. Using > >>>> a helper function helps here, so compiling a backported driver would > >>>> fail w/o the helper function that produces the timestamp, forcing the > >>>> backporter to notice the situation. > >>>> > >>>> Anyway, I don't have a very strict opinion on this, so I'm okay with the > >>>> flag, too, but I personally simply don't think it's the best choice we > >>>> can make now. Also, without converting the drivers now the user space > >>>> must live with different kinds of timestamps much longer. > >>> > >>> There are a number of reasons why I want to go with a flag: > >>> > >>> - Out-of-tree drivers which are unlikely to switch to monotonic in practice > >>> - Backporting drivers > >>> - It makes it easy to verify in v4l2-compliance and enforce the use of > >>> the monotonic clock. > >>> - It's easy for apps to check. > >>> > >>> The third reason is probably the most important one for me. There tends to > >>> be a great deal of inertia before changes like this are applied to new > >>> drivers, and without being able to (easily) check this in v4l2-compliance > >>> more drivers will be merged that keep using gettimeofday. It's all too easy > >>> to miss in a review. > >> > >> If we switch all existing drivers to monotonic timestamps in kernel release > >> 3.x, v4l2-compliance can just use the version it gets from VIDIOC_QUERYCAP and > >> enforce monotonic timestamps verification if the version is >= 3.x. This isn't > >> more difficult for apps to check than a dedicated flag (although it's less > >> explicit). > > > > I think that checking for the driver (kernel) version is a very poor substitute > > for testing against a proper flag. > > That flag should be the default in this case. The flag should be set by > the framework instead giving every driver the job of setting it. > > > One alternative might be to use a v4l2_buffer flag instead. That does have the > > advantage that in the future we can add additional flags should we need to > > support different clocks. Should we ever add support to switch clocks dynamically, > > then a buffer flag is more suitable than a driver capability. In that scenario > > it does make real sense to have a flag (or really mask). > > > > Say something like this: > > > > /* Clock Mask */ > > V4L2_BUF_FLAG_CLOCK_MASK 0xf000 > > /* Possible Clocks */ > > V4L2_BUF_FLAG_CLOCK_SYSTEM 0x0000 I realized that this should be called: V4L2_BUF_FLAG_CLOCK_UNKNOWN 0x0000 With a comment saying that is clock is either the system clock or a monotonic clock. That reflects the current situation correctly. > > V4L2_BUF_FLAG_CLOCK_MONOTONIC 0x1000 > > There are three clocks defined in clock_gettime (2) man page. It'd > indeed be good that the timestamp was selectable, but this really > depends on the user rather than the driver. As you suggested earlier on, > I agree that only monotonic timestamps are seen necessary right now. > > It might be that raw monotonic timestamps could have some potential use > (albeit I don't know a use case right now) but I still wouldn't think > users would change the type of the timestamp that often. So I don't see > a need for the buffer flag, but I still think it's better than a device > capability flag. > > If we gave the user the ability to pick the type of the timestamp we > should move to use timespec at the same time. > > >> My concern is identical to Sakari's, I'm not very keen on introducing a flag > >> that all drivers will set in the very near future and that we will have to > >> keep around forever. > >> > >>> That doesn't mean that it isn't a good idea to convert existing drivers > >>> asap. But it's not something I'm likely to take up myself. > >> > >> Sakari, are you volunteering for that ? ;-) > > I'd be happy to do that. As the changes are mostly mechanical, it won't > really take much time to do that. > > What comes to new drivers --- I think it's wrong to assume every new > driver would have been written wrong kind of timestamps in mind. I mean, > what do you think would be the reasons why a driver writer would pick > do_gettimeofday() instead of ktime_get_ts() if every driver in the tree > already uses ktime_get_ts()? Because I see time and again that driver developers develop on an old kernel instead of using the latest and greatest. Esp. in the embedded world where it is often difficult to move to a more recent kernel. Since you are willing to take on this job, I'll remove the MONOTONIC changes from my patch series and hand that part over to you, if that's OK with you. 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
On 09/15/2012 02:35 PM, Hans Verkuil wrote: >>>> If we switch all existing drivers to monotonic timestamps in kernel release >>>> 3.x, v4l2-compliance can just use the version it gets from VIDIOC_QUERYCAP and >>>> enforce monotonic timestamps verification if the version is>= 3.x. This isn't >>>> more difficult for apps to check than a dedicated flag (although it's less >>>> explicit). >>> >>> I think that checking for the driver (kernel) version is a very poor substitute >>> for testing against a proper flag. >> >> That flag should be the default in this case. The flag should be set by >> the framework instead giving every driver the job of setting it. >> >>> One alternative might be to use a v4l2_buffer flag instead. That does have the >>> advantage that in the future we can add additional flags should we need to >>> support different clocks. Should we ever add support to switch clocks dynamically, >>> then a buffer flag is more suitable than a driver capability. In that scenario >>> it does make real sense to have a flag (or really mask). >>> >>> Say something like this: >>> >>> /* Clock Mask */ >>> V4L2_BUF_FLAG_CLOCK_MASK 0xf000 >>> /* Possible Clocks */ >>> V4L2_BUF_FLAG_CLOCK_SYSTEM 0x0000 > > I realized that this should be called: > > V4L2_BUF_FLAG_CLOCK_UNKNOWN 0x0000 > > With a comment saying that is clock is either the system clock or a monotonic > clock. That reflects the current situation correctly. > >>> V4L2_BUF_FLAG_CLOCK_MONOTONIC 0x1000 There is already lots of overhead related to the buffers management, could we perhaps have the most common option defined in a way that drivers don't need to update each buffer's flags before dequeuing, only to indicate the timestamp type (other than flags being modified in videobuf) ? This buffer flags idea sounds to me worse than the capability flag. After all the drivers should use monotonic clock timestamps, shouldn't they ? Have anyone has ever come with a use case for switching timestamps clock type, can anyone give an example of it ? How likely is we will ever need that ? :) -- Regards, Sylwester -- 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
On Sat September 15 2012 22:16:24 Sylwester Nawrocki wrote: > On 09/15/2012 02:35 PM, Hans Verkuil wrote: > >>>> If we switch all existing drivers to monotonic timestamps in kernel release > >>>> 3.x, v4l2-compliance can just use the version it gets from VIDIOC_QUERYCAP and > >>>> enforce monotonic timestamps verification if the version is>= 3.x. This isn't > >>>> more difficult for apps to check than a dedicated flag (although it's less > >>>> explicit). > >>> > >>> I think that checking for the driver (kernel) version is a very poor substitute > >>> for testing against a proper flag. > >> > >> That flag should be the default in this case. The flag should be set by > >> the framework instead giving every driver the job of setting it. > >> > >>> One alternative might be to use a v4l2_buffer flag instead. That does have the > >>> advantage that in the future we can add additional flags should we need to > >>> support different clocks. Should we ever add support to switch clocks dynamically, > >>> then a buffer flag is more suitable than a driver capability. In that scenario > >>> it does make real sense to have a flag (or really mask). > >>> > >>> Say something like this: > >>> > >>> /* Clock Mask */ > >>> V4L2_BUF_FLAG_CLOCK_MASK 0xf000 > >>> /* Possible Clocks */ > >>> V4L2_BUF_FLAG_CLOCK_SYSTEM 0x0000 > > > > I realized that this should be called: > > > > V4L2_BUF_FLAG_CLOCK_UNKNOWN 0x0000 > > > > With a comment saying that is clock is either the system clock or a monotonic > > clock. That reflects the current situation correctly. > > > >>> V4L2_BUF_FLAG_CLOCK_MONOTONIC 0x1000 > > There is already lots of overhead related to the buffers management, could > we perhaps have the most common option defined in a way that drivers don't > need to update each buffer's flags before dequeuing, only to indicate the > timestamp type (other than flags being modified in videobuf) ? Well, if all vb2 drivers use the monotonic clock, then you could do it in __fill_v4l2_buffer: instead of clearing just the state flags you'd clear state + clock flags, and you OR in the monotonic flag in the case statement below (adding just a single b->flags |= line in the DEQUEUED case). So that wouldn't add any overhead. Not that I think setting a flag will add any measurable overhead in any case. > This buffer flags idea sounds to me worse than the capability flag. After > all the drivers should use monotonic clock timestamps, shouldn't they ? Yes. But you have monotonic and raw monotonic clocks at the moment, and perhaps others will be added in the future. You can't change clocks if you put this in the querycap capabilities. > Have anyone has ever come with a use case for switching timestamps clock > type, can anyone give an example of it ? How likely is we will ever need > that ? Well, ALSA allows you to switch between gettimeofday and monotonic. So in theory at least if an app selects gettimeofday for alsa, that app might also want to select gettimeofday for v4l2. I'd really like to keep this door open. My experience is that if something is possible, then someone somewhere will want to use it. 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
On Sunday 16 September 2012 15:57:14 Hans Verkuil wrote: > On Sat September 15 2012 22:16:24 Sylwester Nawrocki wrote: > > On 09/15/2012 02:35 PM, Hans Verkuil wrote: > > >>>> If we switch all existing drivers to monotonic timestamps in kernel > > >>>> release > > >>>> 3.x, v4l2-compliance can just use the version it gets from > > >>>> VIDIOC_QUERYCAP and enforce monotonic timestamps verification if the > > >>>> version is>= 3.x. This isn't more difficult for apps to check than a > > >>>> dedicated flag (although it's less explicit). > > >>> > > >>> I think that checking for the driver (kernel) version is a very poor > > >>> substitute for testing against a proper flag. > > >> > > >> That flag should be the default in this case. The flag should be set by > > >> the framework instead giving every driver the job of setting it. > > >> > > >>> One alternative might be to use a v4l2_buffer flag instead. That does > > >>> have the advantage that in the future we can add additional flags > > >>> should we need to support different clocks. Should we ever add > > >>> support to switch clocks dynamically, then a buffer flag is more > > >>> suitable than a driver capability. In that scenario it does make real > > >>> sense to have a flag (or really mask). > > >>> > > >>> Say something like this: > > >>> > > >>> /* Clock Mask */ > > >>> V4L2_BUF_FLAG_CLOCK_MASK 0xf000 > > >>> /* Possible Clocks */ > > >>> V4L2_BUF_FLAG_CLOCK_SYSTEM 0x0000 > > > > > > I realized that this should be called: > > > > > > V4L2_BUF_FLAG_CLOCK_UNKNOWN 0x0000 > > > > > > With a comment saying that is clock is either the system clock or a > > > monotonic clock. That reflects the current situation correctly. > > > > > >>> V4L2_BUF_FLAG_CLOCK_MONOTONIC 0x1000 > > > > There is already lots of overhead related to the buffers management, could > > we perhaps have the most common option defined in a way that drivers don't > > need to update each buffer's flags before dequeuing, only to indicate the > > timestamp type (other than flags being modified in videobuf) ? > > Well, if all vb2 drivers use the monotonic clock, then you could do it in > __fill_v4l2_buffer: instead of clearing just the state flags you'd clear > state + clock flags, and you OR in the monotonic flag in the case statement > below (adding just a single b->flags |= line in the DEQUEUED case). > > So that wouldn't add any overhead. Not that I think setting a flag will add > any measurable overhead in any case. > > > This buffer flags idea sounds to me worse than the capability flag. After > > all the drivers should use monotonic clock timestamps, shouldn't they ? > > Yes. But you have monotonic and raw monotonic clocks at the moment, and > perhaps others will be added in the future. You can't change clocks if you > put this in the querycap capabilities. > > > Have anyone has ever come with a use case for switching timestamps clock > > type, can anyone give an example of it ? How likely is we will ever need > > that ? > > Well, ALSA allows you to switch between gettimeofday and monotonic. So in > theory at least if an app selects gettimeofday for alsa, that app might also > want to select gettimeofday for v4l2. > > I'd really like to keep this door open. My experience is that if something > is possible, then someone somewhere will want to use it. As far as system timestamps are concerned I think the monotonic clock should be enough, at least for now. Raw monotonic could possibly be useful later. Another important use case I have in mind is to provide raw device timestamps. For instance UVC devices send a device clock timestamp along with video frames. That timestamp can be useful to userspace applications.
On 09/16/2012 05:33 PM, Laurent Pinchart wrote: > On Sunday 16 September 2012 15:57:14 Hans Verkuil wrote: >> On Sat September 15 2012 22:16:24 Sylwester Nawrocki wrote: >>> On 09/15/2012 02:35 PM, Hans Verkuil wrote: >>>>>>> If we switch all existing drivers to monotonic timestamps in kernel >>>>>>> release >>>>>>> 3.x, v4l2-compliance can just use the version it gets from >>>>>>> VIDIOC_QUERYCAP and enforce monotonic timestamps verification if the >>>>>>> version is>= 3.x. This isn't more difficult for apps to check than a >>>>>>> dedicated flag (although it's less explicit). >>>>>> >>>>>> I think that checking for the driver (kernel) version is a very poor >>>>>> substitute for testing against a proper flag. >>>>> >>>>> That flag should be the default in this case. The flag should be set by >>>>> the framework instead giving every driver the job of setting it. >>>>> >>>>>> One alternative might be to use a v4l2_buffer flag instead. That does >>>>>> have the advantage that in the future we can add additional flags >>>>>> should we need to support different clocks. Should we ever add >>>>>> support to switch clocks dynamically, then a buffer flag is more >>>>>> suitable than a driver capability. In that scenario it does make real >>>>>> sense to have a flag (or really mask). >>>>>> >>>>>> Say something like this: >>>>>> >>>>>> /* Clock Mask */ >>>>>> V4L2_BUF_FLAG_CLOCK_MASK 0xf000 >>>>>> /* Possible Clocks */ >>>>>> V4L2_BUF_FLAG_CLOCK_SYSTEM 0x0000 >>>> >>>> I realized that this should be called: >>>> >>>> V4L2_BUF_FLAG_CLOCK_UNKNOWN 0x0000 >>>> >>>> With a comment saying that is clock is either the system clock or a >>>> monotonic clock. That reflects the current situation correctly. >>>> >>>>>> V4L2_BUF_FLAG_CLOCK_MONOTONIC 0x1000 >>> >>> There is already lots of overhead related to the buffers management, could >>> we perhaps have the most common option defined in a way that drivers don't >>> need to update each buffer's flags before dequeuing, only to indicate the >>> timestamp type (other than flags being modified in videobuf) ? >> >> Well, if all vb2 drivers use the monotonic clock, then you could do it in >> __fill_v4l2_buffer: instead of clearing just the state flags you'd clear >> state + clock flags, and you OR in the monotonic flag in the case statement >> below (adding just a single b->flags |= line in the DEQUEUED case). >> >> So that wouldn't add any overhead. Not that I think setting a flag will add >> any measurable overhead in any case. Yes, that might be indeed negligible overhead, especially if it's done well. User space logic usually adds much more to complexity. Might be good idea to add some helpers to videobuf2, so handling timestamps is as simple as possible in drivers. >>> This buffer flags idea sounds to me worse than the capability flag. After >>> all the drivers should use monotonic clock timestamps, shouldn't they ? >> >> Yes. But you have monotonic and raw monotonic clocks at the moment, and >> perhaps others will be added in the future. You can't change clocks if you >> put this in the querycap capabilities. Fair enough. BTW, CLOCK_MONOTONIC_RAW is not defined in any POSIX standard, is it ? >>> Have anyone has ever come with a use case for switching timestamps clock >>> type, can anyone give an example of it ? How likely is we will ever need >>> that ? >> >> Well, ALSA allows you to switch between gettimeofday and monotonic. So in >> theory at least if an app selects gettimeofday for alsa, that app might also >> want to select gettimeofday for v4l2. OK, I'm not complaining any more. :) >> I'd really like to keep this door open. My experience is that if something >> is possible, then someone somewhere will want to use it. Indeed, caps flags approach might be too limited anyway. And a v4l2 control might be not good for reporting things like these. > As far as system timestamps are concerned I think the monotonic clock should > be enough, at least for now. Raw monotonic could possibly be useful later. > > Another important use case I have in mind is to provide raw device timestamps. > For instance UVC devices send a device clock timestamp along with video > frames. That timestamp can be useful to userspace applications. Could be interesting to add support for something like this. Of what format are then such device timestamps ? -- Regards, Sylwester -- 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
On Sun, Sep 16, 2012 at 11:59:42PM +0200, Sylwester Nawrocki wrote: > On 09/16/2012 05:33 PM, Laurent Pinchart wrote: > > On Sunday 16 September 2012 15:57:14 Hans Verkuil wrote: > >> On Sat September 15 2012 22:16:24 Sylwester Nawrocki wrote: > >>> There is already lots of overhead related to the buffers management, could > >>> we perhaps have the most common option defined in a way that drivers don't > >>> need to update each buffer's flags before dequeuing, only to indicate the > >>> timestamp type (other than flags being modified in videobuf) ? > >> > >> Well, if all vb2 drivers use the monotonic clock, then you could do it in > >> __fill_v4l2_buffer: instead of clearing just the state flags you'd clear > >> state + clock flags, and you OR in the monotonic flag in the case statement > >> below (adding just a single b->flags |= line in the DEQUEUED case). > >> > >> So that wouldn't add any overhead. Not that I think setting a flag will add > >> any measurable overhead in any case. > > Yes, that might be indeed negligible overhead, especially if it's done well. > User space logic usually adds much more to complexity. > > Might be good idea to add some helpers to videobuf2, so handling timestamps > is as simple as possible in drivers. The kernel keeps the time of the last timer interrupt in timekeeper.xtime_sec and timekeeper.xtime_nsec in CLOCK_REALTIME, so it just has to add the nanoseconds since then. Getting CLOCK_MONOTONIC always involves adding timekeeper.wall_to_monotonic, so it is a few cycles less overhead to get CLOCK_REALTIME. How about storing both values and deferring the addition to __fill_v4l2_buffer just for applications that want monotonic time? I wouldn't justify this with the reduced overhead but with backward compatibility. Daniel -- 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
Hi Sylwester, On Sunday 16 September 2012 23:59:42 Sylwester Nawrocki wrote: > On 09/16/2012 05:33 PM, Laurent Pinchart wrote: > > On Sunday 16 September 2012 15:57:14 Hans Verkuil wrote: > >> On Sat September 15 2012 22:16:24 Sylwester Nawrocki wrote: > >>> On 09/15/2012 02:35 PM, Hans Verkuil wrote: > >>> Have anyone has ever come with a use case for switching timestamps clock > >>> type, can anyone give an example of it ? How likely is we will ever need > >>> that ? > >> > >> Well, ALSA allows you to switch between gettimeofday and monotonic. So in > >> theory at least if an app selects gettimeofday for alsa, that app might > >> also want to select gettimeofday for v4l2. Does it, in its kernel API ? The userspace ALSA library (or possibly PulseAudio, I'd need to check) allows converting between clock sources, but I don't think the kernel API supports several clock sources. > OK, I'm not complaining any more. :) > > >> I'd really like to keep this door open. My experience is that if > >> something is possible, then someone somewhere will want to use it. > > Indeed, caps flags approach might be too limited anyway. And a v4l2 control > might be not good for reporting things like these. > > > As far as system timestamps are concerned I think the monotonic clock > > should be enough, at least for now. Raw monotonic could possibly be > > useful later. > > > > Another important use case I have in mind is to provide raw device > > timestamps. For instance UVC devices send a device clock timestamp along > > with video frames. That timestamp can be useful to userspace > > applications. > > Could be interesting to add support for something like this. Of what format > are then such device timestamps ? They're device-dependent :-) In the UVC case they're 32-bit integers.
On Mon September 17 2012 11:18:58 Laurent Pinchart wrote: > Hi Sylwester, > > On Sunday 16 September 2012 23:59:42 Sylwester Nawrocki wrote: > > On 09/16/2012 05:33 PM, Laurent Pinchart wrote: > > > On Sunday 16 September 2012 15:57:14 Hans Verkuil wrote: > > >> On Sat September 15 2012 22:16:24 Sylwester Nawrocki wrote: > > >>> On 09/15/2012 02:35 PM, Hans Verkuil wrote: > > > >>> Have anyone has ever come with a use case for switching timestamps clock > > >>> type, can anyone give an example of it ? How likely is we will ever need > > >>> that ? > > >> > > >> Well, ALSA allows you to switch between gettimeofday and monotonic. So in > > >> theory at least if an app selects gettimeofday for alsa, that app might > > >> also want to select gettimeofday for v4l2. > > Does it, in its kernel API ? The userspace ALSA library (or possibly > PulseAudio, I'd need to check) allows converting between clock sources, but I > don't think the kernel API supports several clock sources. Through the SNDRV_PCM_IOCTL_TTSTAMP ioctl AFAICT. Regards, Hans > > > OK, I'm not complaining any more. :) > > > > >> I'd really like to keep this door open. My experience is that if > > >> something is possible, then someone somewhere will want to use it. > > > > Indeed, caps flags approach might be too limited anyway. And a v4l2 control > > might be not good for reporting things like these. > > > > > As far as system timestamps are concerned I think the monotonic clock > > > should be enough, at least for now. Raw monotonic could possibly be > > > useful later. > > > > > > Another important use case I have in mind is to provide raw device > > > timestamps. For instance UVC devices send a device clock timestamp along > > > with video frames. That timestamp can be useful to userspace > > > applications. > > > > Could be interesting to add support for something like this. Of what format > > are then such device timestamps ? > > They're device-dependent :-) In the UVC case they're 32-bit integers. > > -- 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
On Mon, Sep 17, 2012 at 11:18:58AM +0200, Laurent Pinchart wrote: > > >> Well, ALSA allows you to switch between gettimeofday and monotonic. So in > > >> theory at least if an app selects gettimeofday for alsa, that app might > > >> also want to select gettimeofday for v4l2. > > Does it, in its kernel API ? The userspace ALSA library (or possibly > PulseAudio, I'd need to check) allows converting between clock sources, but I > don't think the kernel API supports several clock sources. See snd_pcm_gettime in <sound/pcm.h>. This header is not exported to userspace. Daniel -- 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
Hi Sylwester, Sylwester Nawrocki wrote: > On 09/16/2012 05:33 PM, Laurent Pinchart wrote: >> On Sunday 16 September 2012 15:57:14 Hans Verkuil wrote: >>> On Sat September 15 2012 22:16:24 Sylwester Nawrocki wrote: >>>> On 09/15/2012 02:35 PM, Hans Verkuil wrote: >>>>>>>> If we switch all existing drivers to monotonic timestamps in kernel >>>>>>>> release >>>>>>>> 3.x, v4l2-compliance can just use the version it gets from >>>>>>>> VIDIOC_QUERYCAP and enforce monotonic timestamps verification if the >>>>>>>> version is>= 3.x. This isn't more difficult for apps to check than a >>>>>>>> dedicated flag (although it's less explicit). >>>>>>> >>>>>>> I think that checking for the driver (kernel) version is a very poor >>>>>>> substitute for testing against a proper flag. >>>>>> >>>>>> That flag should be the default in this case. The flag should be set by >>>>>> the framework instead giving every driver the job of setting it. >>>>>> >>>>>>> One alternative might be to use a v4l2_buffer flag instead. That does >>>>>>> have the advantage that in the future we can add additional flags >>>>>>> should we need to support different clocks. Should we ever add >>>>>>> support to switch clocks dynamically, then a buffer flag is more >>>>>>> suitable than a driver capability. In that scenario it does make real >>>>>>> sense to have a flag (or really mask). >>>>>>> >>>>>>> Say something like this: >>>>>>> >>>>>>> /* Clock Mask */ >>>>>>> V4L2_BUF_FLAG_CLOCK_MASK 0xf000 >>>>>>> /* Possible Clocks */ >>>>>>> V4L2_BUF_FLAG_CLOCK_SYSTEM 0x0000 >>>>> >>>>> I realized that this should be called: >>>>> >>>>> V4L2_BUF_FLAG_CLOCK_UNKNOWN 0x0000 >>>>> >>>>> With a comment saying that is clock is either the system clock or a >>>>> monotonic clock. That reflects the current situation correctly. >>>>> >>>>>>> V4L2_BUF_FLAG_CLOCK_MONOTONIC 0x1000 >>>> >>>> There is already lots of overhead related to the buffers management, could >>>> we perhaps have the most common option defined in a way that drivers don't >>>> need to update each buffer's flags before dequeuing, only to indicate the >>>> timestamp type (other than flags being modified in videobuf) ? >>> >>> Well, if all vb2 drivers use the monotonic clock, then you could do it in >>> __fill_v4l2_buffer: instead of clearing just the state flags you'd clear >>> state + clock flags, and you OR in the monotonic flag in the case statement >>> below (adding just a single b->flags |= line in the DEQUEUED case). >>> >>> So that wouldn't add any overhead. Not that I think setting a flag will add >>> any measurable overhead in any case. > > Yes, that might be indeed negligible overhead, especially if it's done well. > User space logic usually adds much more to complexity. > > Might be good idea to add some helpers to videobuf2, so handling timestamps > is as simple as possible in drivers. Of the V4L2 core. Taking the timestamp has to be done usually at a very precise point in the code, and that's a decision I think is better done in the driver. Timestamps are also independent of the videobuf2. >>>> This buffer flags idea sounds to me worse than the capability flag. After >>>> all the drivers should use monotonic clock timestamps, shouldn't they ? >>> >>> Yes. But you have monotonic and raw monotonic clocks at the moment, and >>> perhaps others will be added in the future. You can't change clocks if you >>> put this in the querycap capabilities. > > Fair enough. BTW, CLOCK_MONOTONIC_RAW is not defined in any POSIX standard, > is it ? It's Linux-specific. Perhaps it's worth noting that both V4L2 and ALSA are Linux-specific, too. :-) Raw wonotonic time could be better in some use cases as it's not NTP-adjusted. Which one is better for the purpose might be system-specific, albeit I'm leaning on the side of the monotonic in a general case. ... >>> I'd really like to keep this door open. My experience is that if something >>> is possible, then someone somewhere will want to use it. > > Indeed, caps flags approach might be too limited anyway. And a v4l2 control > might be not good for reporting things like these. Why not? Are there other mechanisms that are suitable for this than controls? If we end up using controls for this, then we should make it as easy as possible for the drivers. Kind regards,
Hi Sakari, On 09/17/2012 07:19 PM, Sakari Ailus wrote: > Sylwester Nawrocki wrote: >> On 09/16/2012 05:33 PM, Laurent Pinchart wrote: >>> On Sunday 16 September 2012 15:57:14 Hans Verkuil wrote: >>>> On Sat September 15 2012 22:16:24 Sylwester Nawrocki wrote: >>>>> On 09/15/2012 02:35 PM, Hans Verkuil wrote: >>>>>>>> One alternative might be to use a v4l2_buffer flag instead. That >>>>>>>> does >>>>>>>> have the advantage that in the future we can add additional flags >>>>>>>> should we need to support different clocks. Should we ever add >>>>>>>> support to switch clocks dynamically, then a buffer flag is more >>>>>>>> suitable than a driver capability. In that scenario it does make >>>>>>>> real >>>>>>>> sense to have a flag (or really mask). >>>>>>>> >>>>>>>> Say something like this: >>>>>>>> >>>>>>>> /* Clock Mask */ >>>>>>>> V4L2_BUF_FLAG_CLOCK_MASK 0xf000 >>>>>>>> /* Possible Clocks */ >>>>>>>> V4L2_BUF_FLAG_CLOCK_SYSTEM 0x0000 >>>>>> >>>>>> I realized that this should be called: >>>>>> >>>>>> V4L2_BUF_FLAG_CLOCK_UNKNOWN 0x0000 >>>>>> >>>>>> With a comment saying that is clock is either the system clock or a >>>>>> monotonic clock. That reflects the current situation correctly. >>>>>> >>>>>>>> V4L2_BUF_FLAG_CLOCK_MONOTONIC 0x1000 >>>>> >>>>> There is already lots of overhead related to the buffers >>>>> management, could >>>>> we perhaps have the most common option defined in a way that >>>>> drivers don't >>>>> need to update each buffer's flags before dequeuing, only to >>>>> indicate the >>>>> timestamp type (other than flags being modified in videobuf) ? >>>> >>>> Well, if all vb2 drivers use the monotonic clock, then you could do >>>> it in >>>> __fill_v4l2_buffer: instead of clearing just the state flags you'd >>>> clear >>>> state + clock flags, and you OR in the monotonic flag in the case >>>> statement >>>> below (adding just a single b->flags |= line in the DEQUEUED case). >>>> >>>> So that wouldn't add any overhead. Not that I think setting a flag >>>> will add >>>> any measurable overhead in any case. >> >> Yes, that might be indeed negligible overhead, especially if it's done >> well. >> User space logic usually adds much more to complexity. >> >> Might be good idea to add some helpers to videobuf2, so handling >> timestamps >> is as simple as possible in drivers. > > Of the V4L2 core. Taking the timestamp has to be done usually at a very > precise point in the code, and that's a decision I think is better done > in the driver. Timestamps are also independent of the videobuf2. Yes, good point. All in all videobuf2 belongs to v4l2-core, doesn't it ? ;) Taking a timestamp indeed needs some care and precision, but setting a flag could be considered a sort of separate issue - it's more relaxed and videobuf2 already handles the buffer flags. >>>>> This buffer flags idea sounds to me worse than the capability flag. >>>>> After >>>>> all the drivers should use monotonic clock timestamps, shouldn't >>>>> they ? >>>> >>>> Yes. But you have monotonic and raw monotonic clocks at the moment, and >>>> perhaps others will be added in the future. You can't change clocks >>>> if you >>>> put this in the querycap capabilities. >> >> Fair enough. BTW, CLOCK_MONOTONIC_RAW is not defined in any POSIX >> standard, >> is it ? > > It's Linux-specific. Perhaps it's worth noting that both V4L2 and ALSA > are Linux-specific, too. :-) OK. I don't mind V4L2 and ALSA being Linux-specific... :) > Raw wonotonic time could be better in some use cases as it's not > NTP-adjusted. Which one is better for the purpose might be > system-specific, albeit I'm leaning on the side of the monotonic in a > general case. Yeah, I guess it's all determined by streams from what subsystems we're trying to synchronize and what clocks are used there. If there is a possibility to select from various clocks in at least one of the subsystems then we're all set. The main issue here is that we already have plenty of different clocks and there is a need on the video side for at least: 1. reporting to user space what clock is used by a driver, and optionally 2. selecting clock type on user request. >>>> I'd really like to keep this door open. My experience is that if >>>> something >>>> is possible, then someone somewhere will want to use it. >> >> Indeed, caps flags approach might be too limited anyway. And a v4l2 >> control >> might be not good for reporting things like these. > > Why not? Are there other mechanisms that are suitable for this than > controls? If we end up using controls for this, then we should make it > as easy as possible for the drivers. Sorry, my concern here was that timestamps are needed by all video devices and I wasn't sure if there are any video nodes that don't implement the v4l2 control ioctls. I.e. we might be enforcing adding controls support only for the purpose of being able to query the timestamps type. That was my concern here about using controls. However if all video devices implement the controls API then it's negligible. Moreover some parts of such control implementation could likely be a part v4l2-core. I'm just wondering why we need a flag when a control is going to be used anyway. It sounds like per-buffer controls/status but that's an issue that was previously discussed and is still not really addressed in V4L2 AFAICT. Flags + a control is likely going to fulfil all (most of) possible app requirements. Not sure if the applications really need to get timestamp type from each v4l2 buffer and the drivers need to be setting it. Rather than just using a control before starting/after stopping streaming to select, and at any time to query, the clock type. -- Regards, Sylwester -- 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
Hi Sylwester, Sylwester Nawrocki wrote: > On 09/17/2012 07:19 PM, Sakari Ailus wrote: >> Sylwester Nawrocki wrote: >>> On 09/16/2012 05:33 PM, Laurent Pinchart wrote: >>>> On Sunday 16 September 2012 15:57:14 Hans Verkuil wrote: >>>>> On Sat September 15 2012 22:16:24 Sylwester Nawrocki wrote: >>>>>> On 09/15/2012 02:35 PM, Hans Verkuil wrote: >>>>>>>>> One alternative might be to use a v4l2_buffer flag instead. That >>>>>>>>> does >>>>>>>>> have the advantage that in the future we can add additional flags >>>>>>>>> should we need to support different clocks. Should we ever add >>>>>>>>> support to switch clocks dynamically, then a buffer flag is more >>>>>>>>> suitable than a driver capability. In that scenario it does make >>>>>>>>> real >>>>>>>>> sense to have a flag (or really mask). >>>>>>>>> >>>>>>>>> Say something like this: >>>>>>>>> >>>>>>>>> /* Clock Mask */ >>>>>>>>> V4L2_BUF_FLAG_CLOCK_MASK 0xf000 >>>>>>>>> /* Possible Clocks */ >>>>>>>>> V4L2_BUF_FLAG_CLOCK_SYSTEM 0x0000 >>>>>>> >>>>>>> I realized that this should be called: >>>>>>> >>>>>>> V4L2_BUF_FLAG_CLOCK_UNKNOWN 0x0000 >>>>>>> >>>>>>> With a comment saying that is clock is either the system clock or a >>>>>>> monotonic clock. That reflects the current situation correctly. >>>>>>> >>>>>>>>> V4L2_BUF_FLAG_CLOCK_MONOTONIC 0x1000 >>>>>> >>>>>> There is already lots of overhead related to the buffers >>>>>> management, could >>>>>> we perhaps have the most common option defined in a way that >>>>>> drivers don't >>>>>> need to update each buffer's flags before dequeuing, only to >>>>>> indicate the >>>>>> timestamp type (other than flags being modified in videobuf) ? >>>>> >>>>> Well, if all vb2 drivers use the monotonic clock, then you could do >>>>> it in >>>>> __fill_v4l2_buffer: instead of clearing just the state flags you'd >>>>> clear >>>>> state + clock flags, and you OR in the monotonic flag in the case >>>>> statement >>>>> below (adding just a single b->flags |= line in the DEQUEUED case). >>>>> >>>>> So that wouldn't add any overhead. Not that I think setting a flag >>>>> will add >>>>> any measurable overhead in any case. >>> >>> Yes, that might be indeed negligible overhead, especially if it's done >>> well. >>> User space logic usually adds much more to complexity. >>> >>> Might be good idea to add some helpers to videobuf2, so handling >>> timestamps >>> is as simple as possible in drivers. >> >> Of the V4L2 core. Taking the timestamp has to be done usually at a very >> precise point in the code, and that's a decision I think is better done >> in the driver. Timestamps are also independent of the videobuf2. > > Yes, good point. All in all videobuf2 belongs to v4l2-core, doesn't > it ? ;) You're correct. I meant to say that it could (or should) be separate from handling the buffers themselves. > Taking a timestamp indeed needs some care and precision, but setting > a flag could be considered a sort of separate issue - it's more relaxed > and videobuf2 already handles the buffer flags. True. >>>>>> This buffer flags idea sounds to me worse than the capability flag. >>>>>> After >>>>>> all the drivers should use monotonic clock timestamps, shouldn't >>>>>> they ? >>>>> >>>>> Yes. But you have monotonic and raw monotonic clocks at the moment, and >>>>> perhaps others will be added in the future. You can't change clocks >>>>> if you >>>>> put this in the querycap capabilities. >>> >>> Fair enough. BTW, CLOCK_MONOTONIC_RAW is not defined in any POSIX >>> standard, >>> is it ? >> >> It's Linux-specific. Perhaps it's worth noting that both V4L2 and ALSA >> are Linux-specific, too. :-) > > OK. I don't mind V4L2 and ALSA being Linux-specific... > > :) >> Raw wonotonic time could be better in some use cases as it's not >> NTP-adjusted. Which one is better for the purpose might be >> system-specific, albeit I'm leaning on the side of the monotonic in a >> general case. > > Yeah, I guess it's all determined by streams from what subsystems > we're trying to synchronize and what clocks are used there. If there > is a possibility to select from various clocks in at least one of > the subsystems then we're all set. It's not only that, it's also that the clock has to be suitable for the synchronisation problem at hand. Currently realtime timestamps could be used by ALSA and V4L2 but I could hardly recommend using them for audio/video synchronisation. > The main issue here is that we already have plenty of different > clocks and there is a need on the video side for at least: > 1. reporting to user space what clock is used by a driver, > and optionally > 2. selecting clock type on user request. I think the solution for 1 should be such it makes easy and clean to do 2. >>>>> I'd really like to keep this door open. My experience is that if >>>>> something >>>>> is possible, then someone somewhere will want to use it. >>> >>> Indeed, caps flags approach might be too limited anyway. And a v4l2 >>> control >>> might be not good for reporting things like these. >> >> Why not? Are there other mechanisms that are suitable for this than >> controls? If we end up using controls for this, then we should make it >> as easy as possible for the drivers. > > Sorry, my concern here was that timestamps are needed by all video > devices and I wasn't sure if there are any video nodes that don't > implement the v4l2 control ioctls. I.e. we might be enforcing adding > controls support only for the purpose of being able to query the > timestamps type. That was my concern here about using controls. However > if all video devices implement the controls API then it's negligible. > Moreover some parts of such control implementation could likely be > a part v4l2-core. > > I'm just wondering why we need a flag when a control is going to be > used anyway. It sounds like per-buffer controls/status but that's an > issue that was previously discussed and is still not really addressed > in V4L2 AFAICT. > > Flags + a control is likely going to fulfil all (most of) possible > app requirements. Not sure if the applications really need to get > timestamp type from each v4l2 buffer and the drivers need to be setting > it. Rather than just using a control before starting/after stopping > streaming to select, and at any time to query, the clock type. I think so, too. It's unlikely that the user would want to change the value of the timestamp type control while streaming. Those flags could well be added later on if the need to do so arises. Kind regards,
diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml index 2dc39d8..b680d66 100644 --- a/Documentation/DocBook/media/v4l/io.xml +++ b/Documentation/DocBook/media/v4l/io.xml @@ -582,10 +582,14 @@ applications when an output stream.</entry> <entry>struct timeval</entry> <entry><structfield>timestamp</structfield></entry> <entry></entry> - <entry><para>For input streams this is the + <entry><para>This is either the system time (as returned by the <function>gettimeofday()</function> -function) when the first data byte was captured. For output streams -the data will not be displayed before this time, secondary to the +function) or a monotonic timestamp (as returned by the +<function>clock_gettime(CLOCK_MONOTONIC, &ts)</function> function). +A monotonic timestamp is used if the <constant>V4L2_CAP_MONOTONIC_TS</constant> +capability is set, otherwise the system time is used. +For input streams this is the timestamp when the first data byte was captured. +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 diff --git a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml index 98a856f..00757d4 100644 --- a/Documentation/DocBook/media/v4l/vidioc-dqevent.xml +++ b/Documentation/DocBook/media/v4l/vidioc-dqevent.xml @@ -120,7 +120,8 @@ <entry>struct timespec</entry> <entry><structfield>timestamp</structfield></entry> <entry></entry> - <entry>Event timestamp.</entry> + <entry>Event timestamp using the monotonic clock as returned by the + <function>clock_gettime(CLOCK_MONOTONIC, &ts)</function> function.</entry> </row> <row> <entry>u32</entry> diff --git a/Documentation/DocBook/media/v4l/vidioc-querycap.xml b/Documentation/DocBook/media/v4l/vidioc-querycap.xml index 4c70215..fae2036 100644 --- a/Documentation/DocBook/media/v4l/vidioc-querycap.xml +++ b/Documentation/DocBook/media/v4l/vidioc-querycap.xml @@ -315,6 +315,13 @@ linkend="async">asynchronous</link> I/O methods.</entry> linkend="mmap">streaming</link> I/O method.</entry> </row> <row> + <entry><constant>V4L2_CAP_MONOTONIC_TS</constant></entry> + <entry>0x40000000</entry> + <entry>The driver uses a monotonic timestamp instead of wallclock time for the + &v4l2-buffer; <structfield>timestamp</structfield> field. + </entry> + </row> + <row> <entry><constant>V4L2_CAP_DEVICE_CAPS</constant></entry> <entry>0x80000000</entry> <entry>The driver fills the <structfield>device_caps</structfield> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index 292a2ef..3aad418 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -290,6 +290,7 @@ struct v4l2_capability { #define V4L2_CAP_ASYNCIO 0x02000000 /* async I/O */ #define V4L2_CAP_STREAMING 0x04000000 /* streaming I/O ioctls */ +#define V4L2_CAP_MONOTONIC_TS 0x40000000 /* uses monotonic timestamps */ #define V4L2_CAP_DEVICE_CAPS 0x80000000 /* sets device capabilities field */ /*