rjdmoore / fictrac

FicTrac: A webcam-based method for tracking spherical motion and generating fictive animal paths.
http://fictrac.rjdmoore.net/
Other
26 stars 28 forks source link

Timestamp discontinuity with `CVSource` class #29

Open h-mayorquin opened 1 year ago

h-mayorquin commented 1 year ago

The condition to fallback to system time in the CVSource class is the following:

https://github.com/rjdmoore/fictrac/blob/9ac055e52d89f49f492a8eb4e1f7c5b8cd6df40a/src/CVSource.cpp#L162-L165

But I think the condition should be strict, that is, I think the condition should be _timestamp < 0. Otherwise, timestamps which are 0 are ignored and the fallback of the system time is used instead which creates a time discontinuity. As an example, this is the data that I got by running the algorithm in the sample.mp4:

frame_counter sequence_counter movement_direction movement_speed timestamp alt_timestamp delta_timestamp
0 0 0 -0 0 1.69019e+12 4.14554e+07 0
1 1 1 5.79849 0.0246007 33.3333 4.14554e+07 -1.69019e+12
2 2 2 3.5273 0.0211707 66.6667 4.14554e+07 33.3333

See the large discontinuity caused by the first timestamp being 0.

To confirm that the first timestamp is indeed 0 we can run ffprobe:

ffprobe -i sample.mp4 -select_streams v:0 -show_entries frame=best_effort_timestamp_time,pkt_pts_time,pkt_dts_time,pkt_duration_time -of default=noprint_wrappers=1 -v quiet | head -n 20
pkt_pts_time=0.000000
pkt_dts_time=0.000000
best_effort_timestamp_time=0.000000
pkt_duration_time=0.033333
pkt_pts_time=0.033333
pkt_dts_time=0.033333
best_effort_timestamp_time=0.033333
pkt_duration_time=0.033333
pkt_pts_time=0.066667
pkt_dts_time=0.066667
best_effort_timestamp_time=0.066667
pkt_duration_time=0.033333
pkt_pts_time=0.100000
pkt_dts_time=0.100000
best_effort_timestamp_time=0.100000
pkt_duration_time=0.033333
pkt_pts_time=0.133333
pkt_dts_time=0.133333
best_effort_timestamp_time=0.133333
pkt_duration_time=0.033333
rjdmoore commented 1 year ago

The problem is that opencv get() returns 0 if the device doesn't support the property (POS_MSECS). See here.

So fictrac's _timestamp would just be zero for ever in that case. Hence if the get() function returns 0 I use backup time instead.

Agree that it's a bit annoying to have very large timestamp jumps at the beginning of output though. Perhaps I could change the code to only default to backup when retrieved timestamp is zero for two or more frames in a row - assuming video files will only ever have first frame zero.

h-mayorquin commented 1 year ago

Thanks for your quick response and thanks for all your work in this project.

What a terrible decision by open CV to use a valid value to mark a null:

https://github.com/opencv/opencv/blob/617d7ff575200c0a647cc615b86003f10b64587b/modules/videoio/src/cap_ffmpeg_impl.hpp#L1772-L1776

My two cents is that it would be easier to just have two columns in the output data that then users can use as they wish:

I think that it is easier if the fallback is transparent like that and way less maintenance burden for you. Trying to hammer these two concepts into the single timestamps concept seems difficult and specially so if you decide to add more sources in the future.

I think you are proviidng similar value with the ms since midnight but for some reason it is not available in some of the files that I have received.

h-mayorquin commented 1 year ago

Btw, why is ms_since_midnight defined for PGR_USB3

https://github.com/rjdmoore/fictrac/blob/9ac055e52d89f49f492a8eb4e1f7c5b8cd6df40a/src/PGRSource.cpp#L236-L244

but not for PGR_USB2

https://github.com/rjdmoore/fictrac/blob/9ac055e52d89f49f492a8eb4e1f7c5b8cd6df40a/src/PGRSource.cpp#L292-L300

?

rjdmoore commented 1 year ago

Thanks for your quick response and thanks for all your work in My two cents is that it would be easier to just have two columns in the output data that then users can use as they wish:

I also use a timestamp in the program and have to decide which source to use anyway. I will have a closer look at your suggestion though. At least printing some indication of source for tinestamp would be sensible.

rjdmoore commented 1 year ago

Btw, why is ms_since_midnight defined for PGR_USB3

https://github.com/rjdmoore/fictrac/blob/9ac055e52d89f49f492a8eb4e1f7c5b8cd6df40a/src/PGRSource.cpp#L236-L244

but not for PGR_USB2

https://github.com/rjdmoore/fictrac/blob/9ac055e52d89f49f492a8eb4e1f7c5b8cd6df40a/src/PGRSource.cpp#L292-L300

?

This looks suspiciously like a bug. Will take a look!

h-mayorquin commented 1 year ago

Thanks again.

Btw, I edited my comment above to:

What a terrible decision by open CV to use a valid value to mark a null:

Just in case you read it differently. That was my original intention. I think that your decision of not trying to second guess Open CV is very sensible.