GNOME Bugzilla – Bug 758012
systemclock: Use mach_time on Apple platforms
Last modified: 2015-12-07 12:22:03 UTC
Under iOS and OSX, posix timers are not available, and system clock currently falls back to using g_get_current_time, which ultimately uses gettimeofday. This is not ideal, rather it is recommended to use mach_absolute_time for high-resolution timers on these platforms. Attached is a patch that provides this implementation. A new test for clock_get_resolution of the system clock implementation is also added. I have tested this on OSX 10.11, but I haven't had the chance to test this on iOS. All changes should be compatible with iOS, but it would be great if someone could test this. Also, mach_absolute_time is likely more accurate on iOS than g_get_current_time, but it could come at a price of using more CPU cycles, i.e. use more power, which might be relevant on mobile devices. Apple's own documentation, though, suggests using mach_absolute_time on iOS, too: https://developer.apple.com/library/ios/qa/qa1643/_index.html Also related: https://developer.apple.com/library/ios/technotes/tn2169/_index.html
Created attachment 315347 [details] [review] systemclock: Use mach_time on Apple platforms On iOS/OSX g_get_current_time was used by default. However, mach_time is the preferred high-resolution monotonic clock to be used on Apple platforms.
Created attachment 315348 [details] [review] systemclock: Add test for gst_clock_get_resolution In a series of time measurements, the diff between now and previous timestamps is either 0 or at least as long as get_resolution returned.
Review of attachment 315347 [details] [review]: Looks good and makes sense to do, just minor style issue ::: gst/gstsystemclock.c @@ +567,1 @@ #ifdef G_OS_WIN32 #elif defined(G_OS_WIN32) We don't have to write that many #endifs in the end then :) @@ +613,1 @@ #ifdef G_OS_WIN32 same here
Merging line 566+567 to a single #elif is not the same logically, though, is it? I know it looks weird, but it's clearer if you would indent the preprocessor macros after the first #else (which I did, but was rewritten by gst-indent). In pseudo code, it currently looks like this: #if defined __APPLE__ // USE mach_time (always available on Apple platforms) #else #ifdef G_OS_WIN32 // if win32 pfc is good use it, ELSE ... #endif /* G_OS_WIN32 */ #if !defined HAVE_POSIX_TIMERS || !defined HAVE_CLOCK_GETTIME // use g_get_current_time #else // use posix timer #endif #endif /* __APPLE__ */ ... so basically, on Apple platform we don't need a fallback to something else. Apparently, on Windows we'd like to have a fallback (at least the current code does that). If I applied your suggestion, it would look like this: #if defined __APPLE__ // USE mach_time (always available on Apple platform) #elif defined (G_OS_WIN32) // if win32 pfc is good use it, else ... #endif #if !defined HAVE_POSIX_TIMERS || !defined HAVE_CLOCK_GETTIME // use g_get_current_time #else // use posix timer #endif that wouldn't make sense, because we don't need the fallback to using posix-time/g_get_current_time on __APPLE__, and with the current code that also wouldn't compile I think (at least have a warning about unreachable code). Does that make sense? Or did I miss something?
No, you're right. That code is indeed confusing :) Will double check tomorrow and then merge
Attachment 315347 [details] pushed as d59022f - systemclock: Use mach_time on Apple platforms Attachment 315348 [details] pushed as 87691d0 - systemclock: Add test for gst_clock_get_resolution
*** Bug 667303 has been marked as a duplicate of this bug. ***
*** Bug 748893 has been marked as a duplicate of this bug. ***