Hi, > From: Laurent Pinchart [mailto:laurent.pinchart@xxxxxxxxxxxxxxxx] > Sent: Wednesday, January 23, 2013 1:26 AM > > 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. Good point. I didn't say they should not. It's all ok with me. > 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). I can agree with that if the MONOTONIC is a real default. Not videobuf2 exclusive default. In case of non vb2 drivers the default (driver does not set the timestamp type flag at all) is UNKNOWN. I would expect that if no action on the programmer side is taken then videobuf2 and non videobuf2 drivers would act in a similar way. Laurent, I really think that it may confuse someone. I think the MONOTONIC timestamp type should indicate a driver that supplies a monotonic timestamp. Not a newer kernel that sets it as default (and it does only in case of vb2 drivers, not others). > > > > > 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 Best wishes, -- Kamil Debski Linux Platform Group Samsung Poland R&D Center -- 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
