[FFmpeg-devel] [PATCH 2/2] v4l2: allow to convert monotonic timestamps.

[Nicolas George]

L'octidi 8 germinal, an CCXX, Luca Abeni a écrit :
> Ok, this is part of my misunderstanding. I assumed that "user"
> timestamps are wall-clock (what libavdevice always used), and that

"User" timestamps should be wall-clock always, but if the kernel does not
provide them, the conversion is somewhat fragile, and since it is not always
necessary, making it an option seems better.

> Ok, I see I am not the one being confused :) It seems to me that there
> is no V4L_TS_RAW value (I think you mean V4L_TS_DEFAULT, right?

Right.

>								  BTW. I
> think _RAW is more understandable)

For people who know the inners of the device, indeed. But for a simple user,
I prefer "default".

> One of the things that initially confused me is that convert_timestamp()
> is called also in the V4L_TS_DEFAULT case... This is of course ok, it
> just confused me when reading the patch (I think it would have been
> simpler to call convert_timestamp() only if ts_mode !=
> V4L_TS_DEFAULT/RAW)

I wanted to keep changes grouped.

> Yes, this originally confused me too... Maybe a comment in the code
> would help.

Added.

> Just provide a V4L_TS_* value which always asks for a conversion
> (without detecting if the kernel timestamps are monotonic or something
> else... When I specify this value, I say "I know kernel timestamps are
> monotonic, please convert them to wall-clock").

I added "mono2abs". Note that specifying it when timestamps are not from the
monotonic clock result in ffmpeg freezing (duplicating frames endlessly,
actually); but of course, that is to be expected.

> Well, it could just fail,

Not very practical from the application point of view.

>			    or return the wall clock...

Could be a concern if the code that uses it relies on actually monotonic
values.

>							Anyway, this can
> be done later.

Agree.

> I think the patch looks ok, and IMHO it can be applied (if you can
> address my small doubts highlighted in these emails, that would be good.
> But these doubts are not stoppers).

I send an updated patch immediately. Thanks for your review.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120401/b2a32994/attachment.asc>