Timestamp discontinuity with `CVSource` class

[h-mayorquin]

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

fictrac/src/CVSource.cpp

Lines 162 to 165 in 9ac055e

	LOG_DBG("Frame captured %dx%d%d @ %f (t_sys: %f ms, t_day: %f ms)", _frame_cap.cols, _frame_cap.rows, _frame_cap.channels(), _timestamp, ts, _ms_since_midnight);
	if (_timestamp <= 0) {
	_timestamp = ts;
	}

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]

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]

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 your ts_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.

[h-mayorquin]

Btw, why is ms_since_midnight defined for PGR_USB3

fictrac/src/PGRSource.cpp

Lines 236 to 244 in 9ac055e

	#if defined(PGR_USB3)
	ImagePtr pgr_image = NULL;
	try {
	// Retrieve next received image
	long int timeout = _fps > 0 ? std::max(static_cast<long int>(1000), static_cast<long int>(1000. / _fps)) : 1000; // set capture timeout to at least 1000 ms
	pgr_image = _cam->GetNextImage(timeout);
	double ts = ts_ms(); // backup, in case the device timestamp is junk
	_ms_since_midnight = ms_since_midnight();

but not for PGR_USB2

fictrac/src/PGRSource.cpp

Lines 292 to 300 in 9ac055e

	#elif defined(PGR_USB2)
	Image frame_raw;
	Error error = _cam->RetrieveBuffer(&frame_raw);
	double ts = ts_ms(); // backup, in case the device timestamp is junk
	//LOG_DBG("Frame captured %dx%d%d @ %f (%f)", pgr_image->GetWidth(), pgr_image->GetHeight(), pgr_image->GetNumChannels(), _timestamp, ts);
	if (error != PGRERROR_OK) {
	LOG_ERR("Error grabbing image frame!");
	return false;
	}

?

[rjdmoore]

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]

Btw, why is ms_since_midnight defined for PGR_USB3

fictrac/src/PGRSource.cpp

Lines 236 to 244 in 9ac055e

	#if defined(PGR_USB3)
	ImagePtr pgr_image = NULL;
	try {
	// Retrieve next received image
	long int timeout = _fps > 0 ? std::max(static_cast<long int>(1000), static_cast<long int>(1000. / _fps)) : 1000; // set capture timeout to at least 1000 ms
	pgr_image = _cam->GetNextImage(timeout);
	double ts = ts_ms(); // backup, in case the device timestamp is junk
	_ms_since_midnight = ms_since_midnight();

but not for PGR_USB2

fictrac/src/PGRSource.cpp

Lines 292 to 300 in 9ac055e

	#elif defined(PGR_USB2)
	Image frame_raw;
	Error error = _cam->RetrieveBuffer(&frame_raw);
	double ts = ts_ms(); // backup, in case the device timestamp is junk
	//LOG_DBG("Frame captured %dx%d%d @ %f (%f)", pgr_image->GetWidth(), pgr_image->GetHeight(), pgr_image->GetNumChannels(), _timestamp, ts);
	if (error != PGRERROR_OK) {
	LOG_ERR("Error grabbing image frame!");
	return false;
	}

?

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

[h-mayorquin]

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.

[shivChitinous]

Looks like PGR_USB2 reports timestamps in milliseconds, while PGR_USB3 reports timestamps in nanoseconds?