convert more things to nanoseconds #12419

Dudemanguy · 2023-09-18T04:04:45Z

Depends on #12371. Merely using ppoll wrecks havoc on wayland's event loop for some reason so I omitted that. No idea why that happens (epoll weirdness?). Maybe that can be revisited later.

Dudemanguy · 2023-09-18T04:24:28Z

Very cool, freebsd fails building for some reason but I can't see anything in the CI logs. In theory, it's supposed to support ppoll like linux so not sure what the issue would be...

NiLuJe · 2023-09-18T06:07:51Z

In file included from ../osdep/poll_wrapper.c:26:
../osdep/poll_wrapper.h:11:43: error: unknown type name 'int64_t'; did you mean '__int64_t'?

Missing <stdint.h> include? (Might be pulled in implicitly by another include on Linux and/or GCC?)

kasper93 · 2023-09-18T09:01:20Z

Very cool, freebsd fails building for some reason but I can't see anything in the CI logs. In theory, it's supposed to support ppoll like linux so not sure what the issue would be...

Need to expand the run section to see the logs.

Dudemanguy · 2023-09-18T13:38:47Z

Need to expand the run section to see the logs.

Oh I feel stupid. I was blocking the javascript that is apparently now required to see that...

video/out/wayland_common.c

kasper93 · 2023-09-29T21:20:45Z

LGTM

github-actions · 2023-09-29T21:25:25Z

Download the artifacts for this pull request:

Windows

Dudemanguy · 2023-09-29T22:48:14Z

Also changed mp_sleep_us to mp_sleep_ns since that was trivial.

kasper93 · 2023-09-30T14:07:54Z

Also changed mp_sleep_us to mp_sleep_ns since that was trivial.

At this point you might also cherry-pick (and change to ns) this commit for maximum memes 6883d1e

Allows to request sleep with 100ns resolution. Although Windows kernel doesn't go below 0.5ms in practice, but we can defer this limitation to kernel and request high resolution from API.

I know @avih didn't like the additional code, but we are already bikeshedding the timer/timestamp code so might as well.

Dudemanguy · 2023-09-30T14:29:17Z

At this point you might also cherry-pick (and change to ns) this commit for maximum memes 6883d1e

Sure why not. I mean linux and macos are already using nanosecond sleeps under the hood so.

osdep/timer-win2.c

kasper93 · 2023-09-30T14:54:12Z

audio/out/ao_wasapi_utils.c

@@ -945,7 +945,7 @@ bool wasapi_thread_init(struct ao *ao)
 {
    struct wasapi_state *state = ao->priv;
    MP_DBG(ao, "Init wasapi thread\n");
-    int64_t retry_wait = 1;
+    int64_t retry_wait = 1000;


Not really important for this PR. But maybe we could have macros for defining wait times in more readable way

like

#define MP_TIME_S(s) ((s) * 1000000000) #define MP_TIME_MS(ms) ((ms) * 1000000) #define MP_TIME_US(ms) ((ms) * 1000) #define MP_TIME_NS(ns) ((ns))

then we can abstract all zeros and multiplications. and just do MP_TIME_MS(1), especially we do a lot MP_TIME_S(1) conversions.

Amusingly, MP_SECOND_US already exists apparently. What an awful name. But yes, having such macros sounds like a good idea.

This is weird. The caller should be responsible for converting the value if desired. Move the conversion to player/command.c instead.

In many cases, this is purely cosmetic because poll still only accepts microseconds. There's still a gain here however since pthread_cond_timedwait can take a realtime ts now. Additionally, 37d6604 changed the value added to timeout_ms in X11 and Wayland to ensure that it would never be 0 and rounded up. This was both incomplete, several other parts of the player have this same problem like drm, and not really needed. Instead the MPCLAMP is just adjusted to have a min of 1.

Originally, this was added as purely a shim for macOS. However since we want to do high resolution polling which is not neccesarily available on all platforms, making this a generic wrapper for poll functions is useful so rename it.

On linux, several platforms poll for events over a fd. This has ms accuracy, but mpv's timer is in ns now so lots of precision is lost. We can use an mp_poll wrapper to use ppoll instead which takes a timespec directly with nanosecond precision. On systems without ppoll this falls back to old poll behavior. On wayland, we don't actually use this because ppoll completely messes up the event loop for some unknown reason.

9606c3f added mp_time_ns(). Since we apparently expose the mp_time_us() to clients already, there's no reason to not also expose the new nanosecond one.

Linux and macOS already use nanosecond resolution for their sleep functions. It was just being converted from microseconds before. Since we have mp_time_ns now, go ahead and bump the precision here. The timer for windows uses some timeBeginPeriod thing which I'm not sure what it does really but whatever just convert the units to ms like they were doing before. There's really no reason to keep the mp_sleep_us helper around. A multiplication by 1000 is trivial and underlying OS clocks have nanosecond precision.

Allows higher resolution sleeps than Sleep which has milliseconds resolution. In practice Windows kernel does not really go below 0.5ms, but we don't have to limit ourselves on API side of things and do the best we can.

Dudemanguy · 2023-10-10T19:09:17Z

Considerably more risky than #12371, so I apologize in advance (it works for me though...)

Dudemanguy changed the title ~~convert more things to nanoseconds.~~ convert more things to nanoseconds Sep 18, 2023

Dudemanguy force-pushed the nanoseconds-everywhere branch 2 times, most recently from 49caae1 to 686e2ff Compare September 18, 2023 13:44

Dudemanguy force-pushed the nanoseconds-everywhere branch from 686e2ff to f09acbc Compare September 29, 2023 20:59

kasper93 reviewed Sep 29, 2023

View reviewed changes

video/out/wayland_common.c Outdated Show resolved Hide resolved

Dudemanguy force-pushed the nanoseconds-everywhere branch from 9ada013 to af68bb2 Compare September 29, 2023 22:44

Dudemanguy marked this pull request as ready for review September 29, 2023 22:47

Dudemanguy force-pushed the nanoseconds-everywhere branch 3 times, most recently from 16a91f1 to 0290f23 Compare September 29, 2023 23:38

Dudemanguy commented Sep 30, 2023

View reviewed changes

osdep/timer-win2.c Outdated Show resolved Hide resolved

Dudemanguy force-pushed the nanoseconds-everywhere branch from 88415d3 to 434c6ca Compare September 30, 2023 14:36

kasper93 reviewed Sep 30, 2023

View reviewed changes

Dudemanguy added 8 commits October 7, 2023 20:49

vo: remove vo_get_estimated_vsync_interval conversion to seconds

03b9bbc

This is weird. The caller should be responsible for converting the value if desired. Move the conversion to player/command.c instead.

player: store last_time timestamp in nanoseconds

e9d48fd

vo: use nanoseconds for frame duration and pts

4b7d100

osdep: rename polldev to poll_wrapper

7515f20

Originally, this was added as purely a shim for macOS. However since we want to do high resolution polling which is not neccesarily available on all platforms, making this a generic wrapper for poll functions is useful so rename it.

libmpv: add mpv_time_ns()

760c1a4

9606c3f added mp_time_ns(). Since we apparently expose the mp_time_us() to clients already, there's no reason to not also expose the new nanosecond one.

osdep/timer-win2: use Waitable Timer for sleeping

a016025

Allows higher resolution sleeps than Sleep which has milliseconds resolution. In practice Windows kernel does not really go below 0.5ms, but we don't have to limit ourselves on API side of things and do the best we can.

Dudemanguy force-pushed the nanoseconds-everywhere branch from 434c6ca to a016025 Compare October 8, 2023 01:51

Dudemanguy merged commit a27d402 into mpv-player:master Oct 10, 2023
15 checks passed

Dudemanguy deleted the nanoseconds-everywhere branch October 10, 2023 19:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

convert more things to nanoseconds #12419

convert more things to nanoseconds #12419

Dudemanguy commented Sep 18, 2023

Dudemanguy commented Sep 18, 2023

NiLuJe commented Sep 18, 2023 •

edited

Loading

kasper93 commented Sep 18, 2023

Dudemanguy commented Sep 18, 2023

kasper93 commented Sep 29, 2023

github-actions bot commented Sep 29, 2023 •

edited

Loading

Dudemanguy commented Sep 29, 2023

kasper93 commented Sep 30, 2023 •

edited

Loading

Dudemanguy commented Sep 30, 2023 •

edited

Loading

kasper93 Sep 30, 2023

Dudemanguy Sep 30, 2023 •

edited

Loading

Dudemanguy commented Oct 10, 2023 •

edited

Loading

convert more things to nanoseconds #12419

convert more things to nanoseconds #12419

Conversation

Dudemanguy commented Sep 18, 2023

Dudemanguy commented Sep 18, 2023

NiLuJe commented Sep 18, 2023 • edited Loading

kasper93 commented Sep 18, 2023

Dudemanguy commented Sep 18, 2023

kasper93 commented Sep 29, 2023

github-actions bot commented Sep 29, 2023 • edited Loading

Dudemanguy commented Sep 29, 2023

kasper93 commented Sep 30, 2023 • edited Loading

Dudemanguy commented Sep 30, 2023 • edited Loading

kasper93 Sep 30, 2023

Choose a reason for hiding this comment

Dudemanguy Sep 30, 2023 • edited Loading

Choose a reason for hiding this comment

Dudemanguy commented Oct 10, 2023 • edited Loading

NiLuJe commented Sep 18, 2023 •

edited

Loading

github-actions bot commented Sep 29, 2023 •

edited

Loading

kasper93 commented Sep 30, 2023 •

edited

Loading

Dudemanguy commented Sep 30, 2023 •

edited

Loading

Dudemanguy Sep 30, 2023 •

edited

Loading

Dudemanguy commented Oct 10, 2023 •

edited

Loading