AprilRobotics / apriltag

AprilTag is a visual fiducial system popular for robotics research.
https://april.eecs.umich.edu/software/apriltag
Other
1.56k stars 535 forks source link

Windows gettimeofday implementation is not correct. #259

Open JohnGaby opened 2 years ago

JohnGaby commented 2 years ago

Describe the bug I have built apriltag for windows using Visual Studio. When I run your apriltag_demo.c it shows the times for the various operations but they all show up as zero.

To Reproduce Steps to reproduce the behavior: Run apriltag_demo with pretty much any image.

Expected behavior It should show the number of ms for each operation but they all show as zero

Operating Sytem Windows

Installation Method Built using Visual Studio 2022

Code version 3.3.0

Additional context The problem appears to be that your Windows implementation of gettimeofday is not correct. You are calling time() which returns the time in seconds and does not contain any microsecond information.

The following is a Windows version of gettimeofday that I found on StackOverflow which seems to work:

inline int gettimeofday(struct timeval* tp, struct timezone* tzp)
{
    // Note: some broken versions only have 8 trailing zero's, the correct epoch has 9 trailing zero's
    // This magic number is the number of 100 nanosecond intervals since January 1, 1601 (UTC)
    // until 00:00:00 January 1, 1970 
    static const uint64_t EPOCH = ((uint64_t)116444736000000000ULL);

    SYSTEMTIME  system_time;
    FILETIME    file_time;
    uint64_t    time;

    GetSystemTime(&system_time);
    SystemTimeToFileTime(&system_time, &file_time);
    time = ((uint64_t)file_time.dwLowDateTime);
    time += ((uint64_t)file_time.dwHighDateTime) << 32;

    tp->tv_sec = (long)((time - EPOCH) / 10000000L);
    tp->tv_usec = (long)(system_time.wMilliseconds * 1000);
    return 0;
}
christian-rauch commented 2 years ago

You are right about time(). But instead of reimplementing gettimeofday, it would make more sense to provide a Windows implementation of utime_now, as this is the only place where gettimeofday is used. This would also have the benefit that we do not need more ifdefs in the header time_util.h but would just do this at compile time in time_util.c. Can you provide a PR for this?