Hi Kamil, On Tuesday 22 January 2013 11:24:09 Kamil Debski wrote: > On Tuesday, January 22, 2013 11:03 AM Sakari Ailus wrote: > > On Mon, Jan 21, 2013 at 03:07:55PM +0100, Kamil Debski wrote: > > > On Saturday, January 19, 2013 6:43 PM Sakari Ailus wrote: > > > > On Mon, Jan 14, 2013 at 10:36:04AM +0100, Kamil Debski wrote: > > > > > Set proper timestamp type in drivers that I am sure that use > > > > > either MONOTONIC or COPY timestamps. Other drivers will correctly > > > > > report UNKNOWN timestamp type instead of assuming that all > > > > > drivers use monotonic timestamps. > > > > > > > > What other kind of timestamps there can be? All drivers (at least > > > > those not mem-to-mem) that do obtain timestamps using system clock use > > > > monotonic ones. > > > > > > Not set. It is not a COPY or MONOTONIC either. Any new or custom kind > > > of timestamp, maybe? > > > > Then new timestamp types should be defined for the purpose. Which is > > indeed what your patch is about. > > > > And about "COPY" timestamps: if an application wants to use timestamps, > > it probably need to know what kind of timestamps they are. "COPY" > > doesn't provide that information as such. Only the program that sets > > the timestamps for the OUTPUT buffers does. > > For me the COPY type is clear. If the app gets a COPY timestamp on a > buffer (CAPTURE) then it knows that it was copied from the OUTPUT buffer. > If the application did not set the timestamp for OUTPUT buffer, then > it has to cope with the consequences. > > > > > I'd think that there should no longer be any drivers using the > > > > UNKNOWN timestamp type: UNKNOWN is either from monotonic or realtime > > > > clock, and we just replaced all of them with the monotonic ones. No > > > > driver uses realtime timestamps anymore. > > > > > > Maybe there should be no drivers using UNKNOWN. But definitely there > > > should be no driver reporting MONOTONIC when the timestamp is not > > > monotonic. > > > > > > > How about making MONOTONIC timestamps default instead, or at least > > > > assigning all drivers something else than UNKNOWN? > > > > > > So why did you add the UNKNOWN flag? > > > > This is for API compatibility only. Applications running on kernels > > prior to the headers of which define timestamp types will not have > > timestamp type set (i.e. is zero, which equals to UNKNOWN). There was a > > lengthy discussion on the topic back then, and the conclusion was that > > the kernel version itself isn't enough to tell what kind of timestamps > > are provided to the user. > > > > Any new driver shouldn't use UNKNOWN timestamps since in this case the > > application would have to know what kind of timestamps the driver uses > > --- which is why we now specify it in the API. > > > > > The way I see it - UNKNOWN is the default and the one who coded the > > > driver will set it to either MONOTONIC or COPY if it is one of these > > > two. It won't be changed otherwise. There are drivers, which do not > > > fill the timestamp field at all: > > > - drivers/media/platform/coda.c > > > - drivers/media/platform/exynos-gsc/gsc-m2m.c > > > - drivers/media/platform/m2m-deinterlace.c > > > - drivers/media/platform/mx2_emmaprp.c > > > - drivers/media/platform/s5p-fimc/fimc-m2m.c > > > - drivers/media/platform/s5p-g2d.c > > > - drivers/media/platform/s5p-jpeg/jpeg-core.c > > > > Excellent point. > > > > But --- should these drivers then fill the timestamp field? Isn't it a > > bug in the driver not to do so? > > Could be. The only thing I am complaining about is that the type should not > be reported as monotonic when it is not. Of course. > (We can argue that constant is monotonic, but I think that it is not the > point :-) ). This is why I propose to leave UNKNOWN as the default choice > (which it is in case of non videobuf2 drivers). It is possible to eliminate > the UNKNOWN in my opinion, but it should be up to the programmer to set the > type. Another option is to eliminate the UNKNOWN flag in the next, or in two > kernel releases. After the drivers are changed. Still I prefer to leave the > driver's programmer in charge and leave the UNKNOWN type. I think that Sakari's point is that all non mem-to-mem drivers should use the MONOTONIC timestamps type. UNKNOWN, in the non mem-to-mem case, is only meant for compatibility with older kernels. We should thus default to MONOTONIC and let drivers select a different timestamp type when needed (for mem-to-mem drivers for instance). > > > The way you did it in your patches left no room for any kind of > > > choice. I did comment at least twice about mem-2-mem devices in your > > > RFCs, if I remember correctly. I think Sylwester was also writing > > > about this. Still everything got marked as MONOTONIC. > > > > I must have missed this in the discussion back then. > > > > > If we were to assume that there were no other timestamp types then > > > monotonic (which is not true, but this was your assumption), then > > > what was the reason to add this timestamp framework? > > > > For capture devices whose video source has no native timestamps the > > timestamps are MONOTONIC, at least until it is made selectable. Other > > examples could include video decoders or encoders, but these timestamps > > will be entirely different kind, and probably doesn't end up to the > > timestamp field. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
