improve timing in scrot --delay
#261
Conversation
guijan
force-pushed
the
use-clock_nanosleep
branch
2 times, most recently
from
1647a6d to
6310502
Compare
|
Turns out the other |
guijan
changed the title
scrot --delay $DELAY --countscrot --delay
guijan
force-pushed
the
use-clock_nanosleep
branch
from
6310502 to
7c43218
Compare
|
Now the PR is significantly different, I've done very basic testing on OpenBSD, but it still needs more testing. |
guijan
force-pushed
the
use-clock_nanosleep
branch
from
7c43218 to
83fc97d
Compare
guijan
force-pushed
the
use-clock_nanosleep
branch
from
83fc97d to
e5f4e08
Compare
guijan
force-pushed
the
use-clock_nanosleep
branch
from
e5f4e08 to
202c347
Compare
guijan
force-pushed
the
use-clock_nanosleep
branch
from
202c347 to
83b7d8c
Compare
guijan
force-pushed
the
use-clock_nanosleep
branch
from
83b7d8c to
2ba3d62
Compare
There was a problem hiding this comment.
Tested it out and it seems to work OK.
However, one weird thing I noticed while stepping through the new code under gdb was that it was taking the nanosleep path instead of the clock_nanosleep one. Which was weird since linux should support clock_nanosleep. Turned out, the reason was because I didn't re-run the configure scripts.
But this lead me to question how much value clock_nanosleep is providing anyways, is it worth the check and the ifdef ?
On my system at least, the answer seems to be no.
I'm fine merging this PR as-is, since not re-running configure was a user-error. But if you don't have any objection to it, I'd like to just use the nanosleep path unconditionally and remove the clock_nanosleep check.
Upgrade scrotSleep() to scrotWaitUntil() which sleeps until a given time on the clock rather than sleeping for a given period of time. In all cases, this improves the timing by compensating for the time passed between entering main() and taking the screenshot. When used with --count, it fixes the drift caused by calling nanosleep() in a loop. Also remove an incorrect comment and rename the delay_selection variable to delaySelection to match our style.
guijan
force-pushed
the
use-clock_nanosleep
branch
from
2ba3d62 to
a2ffc11
Compare
|
Yeah, in an algorithmic sense, both paths are the same: they both make the error relative to the sleep constant. It just annoys me when we don't use the good features of each platform. |
| tmp.tv_sec--; | ||
| tmp.tv_nsec += 1000000000L; | ||
| } | ||
| } while (nanosleep(&tmp, &tmp) == -1 && errno == EINTR); |
There was a problem hiding this comment.
Just noticed, I don't think the 2nd tmp argument is doing anything. It'll just overwritten by clock_gettime at the top of the loop body.
This will conflict with #257, has only been minimally tested, and there are other places in the code where it's useful but hasn't been put to use yet, so it remains a draft.
Old
nanosleep()code (master) on OpenBSD:New
clock_nanosleep()code (this PR), falling back to a localclock_nanosleep()implementation on systems lacking this POSIX function like OpenBSD and OS X, running on OpenBSD:The timing error is consistently smaller across several runs. If you subtract the user and system runtime and the 60 second sleep from the real runtime, you get the timing error. It has been reduced by an order of magnitude here.