fictrac
fictrac copied to clipboard
Timestamp discontinuity with `CVSource` class
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
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.
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:
source_timestamps: return the timestamps from OpenCV, Basler or PGR as they come.system_timestamps: returns the system timestamps when the frames are grabed by the program. Just yourts_ms()function.
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.
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
?
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.
Btw, why is
ms_since_midnightdefined forPGR_USB3https://github.com/rjdmoore/fictrac/blob/9ac055e52d89f49f492a8eb4e1f7c5b8cd6df40a/src/PGRSource.cpp#L236-L244
but not for
PGR_USB2https://github.com/rjdmoore/fictrac/blob/9ac055e52d89f49f492a8eb4e1f7c5b8cd6df40a/src/PGRSource.cpp#L292-L300
?
This looks suspiciously like a bug. Will take a look!
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.