fix timestamps #2658
fix timestamps #2658
Conversation
|
|
||
| //convert to ns | ||
| uint64_t int_part_ns = static_cast<uint64_t>(int_part_ms) * 1000000; | ||
| uint64_t fract_part_ns = static_cast<uint64_t>(fract_part_ms * 10000000); |
There was a problem hiding this comment.
Too many zeros.
It would be better to use a constant static constexpr uint64_t milli_to_nano = 1000000.
Can also use C++14 digit separator 1'000'000 :-)
|
|
||
| // Convert fract_parts_ns into 6 digits and round it | ||
| // to be aligned with librealsense get_timestamp API | ||
| fract_part_ns = (fract_part_ns % 10 > 4) ? (fract_part_ns/10 + 1) : (fract_part_ns/10); |
There was a problem hiding this comment.
More efficient ( fract_part_ns + 5 ) / 10. No need to use ? :.
Or use round function while converting to uint64_t
| if (frame.get_frame_timestamp_domain() == RS2_TIMESTAMP_DOMAIN_HARDWARE_CLOCK) | ||
| { | ||
| double elapsed_camera_ns = (/*ms*/ frame.get_timestamp() - /*ms*/ _camera_time_base) * 1e6; | ||
| double elapsed_camera_ns = millisecondsToNanoseconds(timestamp_ms - _camera_time_base); |
There was a problem hiding this comment.
Converting uint64_t to double might loose precision.
double can exactly represent numbers up to 2^51 which is around 26 days in ns. Is it OK? If timestamp is since power-up then probably yes. If since epoch then surely not.
There was a problem hiding this comment.
I think this question in more related to Librealsense API, since frame.get_timestamp is returning a double timestamp.
For this PR, I think this discussion can be skipped, but it is really worth taking this issue into account.
| // for fract_part_ns, multiplie ns * 10, then divide by 10 while rounding | ||
| // to be aligned with librealsense get_timestamp API | ||
| uint64_t fract_part_ns = static_cast<uint64_t>(fract_part_ms * milli_to_nano * 10); | ||
| fract_part_ns = (fract_part_ns + 5)/10; |
There was a problem hiding this comment.
Looking at this now it seems like a redundant division.
Use static_cast<uint64_t>( ( fract_part_ms + 0.5 ) * milli_to_nano ) to eliminate it.
There was a problem hiding this comment.
Or just std::round that does the same. It is more readable.
There was a problem hiding this comment.
After consulting with StackOverflow I suggest you use round as the +0.5 method does not handle all corner cases (Inf, NaN and such).
Fix timestamp calculation metadata header to be aligned with metadata json timestamp
Tracked by [DSO-18855]