We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 2764 - Timer is not rescheduled with the returned interval
Summary: Timer is not rescheduled with the returned interval
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: timer (show other bugs)
Version: 2.0.3
Hardware: x86_64 Linux
: P2 normal
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-23 13:51 UTC by afwlehmann
Modified: 2017-08-14 04:49 UTC (History)
2 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description afwlehmann 2014-10-23 13:51:44 UTC 
According to the API documentation, a timer callback is always called with the current interval which should correspond to the return value from the previous call. Instead, the implementation is always using the same interval:

            /* We're going to do something with this timer */
            data->timers = current->next;

            if (current->canceled) {
                interval = 0;
            } else {
                interval = current->callback(current->interval, current->param);
            }

            if (interval > 0) {
                /* Reschedule this timer */
                current->scheduled = tick + interval;
                SDL_AddTimerInternal(data, current);
                ...
Comment 1 afwlehmann 2014-10-23 13:55:11 UTC 
Err, seems I made a mistake. It actually gets updated. Please close this bug.
Comment 2 Philipp Wiesemann 2014-12-12 19:31:04 UTC 
Resolved as invalid because it is no bug. See previous comments.
Comment 3 afwlehmann 2015-03-19 16:36:46 UTC 
Sorry for re-opening, but it turns out that the current interval is indeed not updated. I've just checked the source code of the 2.0.3 release again:

   163	    if (current->canceled) {
   164	        interval = 0;
   165	    } else {
   166	        interval = current->callback(current->interval, current->param);
   167	    }
   168	
   169	    if (interval > 0) {
   170	        /* Reschedule this timer */
   171	        current->interval = interval; // <-- this line is missing
   172	        current->scheduled = tick + interval;
   173	        SDL_AddTimerInternal(data, current);
   174	    } else {

According to the documentation: "The callback function is passed the current timer interval and the user supplied parameter from the SDL_AddTimer() call and returns the next timer interval. If the returned value from the callback is 0, the timer is canceled." 

If I understand the text correctly, then the current interval should in fact be updated according to the returned value. Otherwise there would be a discrepancy between the next time for which the timer is actually re-scheduled and the value that's passed to the callback once the timer fires again.

This could be fixed by adding line #171.
Comment 4 Sam Lantinga 2017-08-14 04:49:02 UTC 
Good point. This is fixed, thanks!
https://hg.libsdl.org/SDL/rev/37f4d29e155d