SDL_CondWaitTimeout makes an incorrect timespec

[GoogleCodeExporter]

LWP_CondTimedWait's timespec parameter does not work like pthread. Instead of 
passing in an end time, you simply pass in the timeout duration. The current 
implementation will make the timeout impossibly high every time, and will 
likely overflow the timeout.

Original issue reported on code.google.com by baby.lueshi@gmail.com on 8 Nov 2013 at 3:14

[GoogleCodeExporter]

I believe there is a mistake in the conversion formula of the 
SDL_CondWaitTimeout implementation:

clock_gettime(&now);

abstime.tv_sec = now.tv_sec + (ms / 1000);
abstime.tv_nsec = (now.tv_nsec + (ms % 1000) * 1000) * 1000;

The last line code should be:
abstime.tv_nsec = now.tv_nsec + ((ms % 1000) * 1000) * 1000;

Original comment by olimpier...@gmail.com on 5 Nov 2014 at 4:32

[GoogleCodeExporter]

Moreover clock_gettime should return the elapsed time from 1970.

Whilst LWP_CondTimedWait expects the time elapsed from the CPU bootstrap (i.e. 
clicks).

So clock_gettime is the wrong function to use in this case. 

gettime() should be used instead along with ticks_to_secs and tick_nanosecs 
functions of lwp_watchdog.h. 

Anyhow also tick_nanosecs of libogc is buggy.

Moreover also clock_gettime is buggy since in the current wrong implementation 
the tv_sec is calculated as the seconds elapsed from 1970 and tv_nsec (which 
should be the number of nanoseconds expired in the current second) is the 
nanoseconds elapsed from the CPU bootstrap.

See my post here:

http://devkitpro.org/viewtopic.php?f=3&t=3056

Original comment by olimpier...@gmail.com on 14 Dec 2014 at 12:50

[GoogleCodeExporter]

I believe that the correct implementation should be

#include <ogc/lwp_watchdog.h>

int SDL_CondWaitTimeout(SDL_cond *cond, SDL_mutex *mutex, Uint32 ms)
{
    struct timespec now;
    struct timespec abstime;
    u64 ticks; 

    if (!cond)
    {
        SDL_SetError("Passed a NULL condition variable");
        return -1;
    }

    ticks=gettime();

    now.tv_sec = ticks_to_secs(ticks);
#define tick_nanosecs(ticks) 
((((u64)(ticks)*8000)/(u64)(TB_TIMER_CLOCK/125))%1000000000) //To remove when 
it will be fixed il libogc
    now.tv_nsec = tick_nanosecs(ticks);

    abstime.tv_sec = now.tv_sec + (ms / 1000);
    abstime.tv_nsec = now.tv_nsec + ((ms % 1000) * 1000) * 1000;
    if (abstime.tv_nsec > 1000000000)
    {
        abstime.tv_sec += 1;
        abstime.tv_nsec -= 1000000000;
    }

    return LWP_CondTimedWait(cond->cond, mutex->id, &abstime);
}

Original comment by olimpier...@gmail.com on 14 Dec 2014 at 1:14

[GoogleCodeExporter]

Indeed I checked that WP_CondTimedWait's timespec parameter passes relative 
time instead of absolute time as specified in the documentation. So the 
implementation should be:

int SDL_CondWaitTimeout(SDL_cond *cond, SDL_mutex *mutex, Uint32 ms)
{
    struct timespec time; 

    if (!cond)
    {
        SDL_SetError("Passed a NULL condition variable");
        return -1;
    }
    time.tv_sec = (ms / 1000);
    time.tv_nsec = (ms % 1000) * 1000000;

    return LWP_CondTimedWait(cond->cond, mutex->id, &time);
}

Original comment by olimpier...@gmail.com on 21 Dec 2014 at 5:38

[GoogleCodeExporter]

Committed the changes in the SVN

Original comment by olimpier...@gmail.com on 21 Dec 2014 at 5:59

• Changed state: Fixed