osc.lua: add option to make rendering smoother

[na-na-hi]

OSC rendering used to be smooth (up to OSD rendering fps) before 48f906249ef359c66fc6c5bf4cc5885c8293bf8e, but after that commit the frame duration is hardcoded to 30 ms. This is too high and results in choppy OSC rendering, which is very noticable with the progress bar while moving mouse over it or playing a short 60 fps video.

This makes the duration an option so that it can be decreased to make OSC rendering smoother.

[github-actions[bot]]

Download the artifacts for this pull request:

Windows

* [mpv-i686-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/1581690700.zip) * [mpv-x86_64-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/1581692151.zip) * [mpv-x86_64-windows-msvc](https://nightly.link/mpv-player/mpv/actions/artifacts/1581698156.zip)

macOS

* [mpv-macos-12-intel](https://nightly.link/mpv-player/mpv/actions/artifacts/1581691401.zip) * [mpv-macos-13-intel](https://nightly.link/mpv-player/mpv/actions/artifacts/1581691992.zip) * [mpv-macos-14-arm](https://nightly.link/mpv-player/mpv/actions/artifacts/1581692114.zip)

[verygoodlee]

10ms means 100fps, which may not be needed on a 60Hz screen. is that better to observe the screen refresh rate change to update tick_delay?

mp.observe_property('display-fps', 'number', 
    function(_, val)
        if not val then return end -- may be nil
        tick_delay = math.max(1 / val, 0.01)
        request_tick()
    end
)

[na-na-hi]

is that better to observe the screen refresh rate change to update tick_delay?

A default value is still needed. Some VOs don't support that property.

[kasper93]

use display fps for tick delay

I don't think it makes sense. OSD (and OSC) is never drawn faster than display fps. Our ticks are tied to video fps or display fps if in display resample mode. Maybe if someone is spamming keys/commands it can request faster, but still redraw will not happen untill next vsync.

This OSC rate limiting as I guess is to workaround performance drawing osc.lua. It seems to be decided that 30 fps for UI is enough.

[na-na-hi]

I don't think it makes sense. OSD (and OSC) is never drawn faster than display fps.

Not all video rendering methods are vsynced, so this is not true. But I agree that a sufficient limit is enough so it doesn't make much sense to use display fps for this. If you think it's a bad idea I can drop that commit.

This OSC rate limiting as I guess is to workaround performance drawing osc.lua. It seems to be decided that 30 fps for UI is enough.

That commit mentioned the case for high fps videos, so I assume that it was implemented for that case. But 30 fps is too low to me, and the OSC progress bar visually stutters when playing 60 fps videos. Meanwhile, OSD bar doesn't have this problem since it's updated the same rate as the video. I would like the UI to be refreshed at least 60 fps to accommodate this common use case. With the current 100 fps limit I'm not seeing any performance problems even with Core 2 era CPUs.

[kasper93]

That commit mentioned the case for high fps videos, so I assume that it was implemented for that case. But 30 fps is too low to me, and the OSC progress bar visually stutters when playing 60 fps videos. Meanwhile, OSD bar doesn't have this problem since it's updated the same rate as the video. I would like the UI to be refreshed at least 60 fps to accommodate this common use case. With the current 100 fps limit I'm not seeing any performance problems even with Core 2 era CPUs.

I'm thinking if this rate limit is really needed. Added here https://github.com/mpv-player/mpv/commit/48f906249ef359c66fc6c5bf4cc5885c8293bf8e

What do you think we just remove this rate limit? I don't think other even more complex OSC scripts are doing things like that. UOSC even requires higher vsync rate for smooth operation.

[na-na-hi]

What do you think we just remove this rate limit?

This essentially takes back to the state before https://github.com/mpv-player/mpv/commit/48f906249ef359c66fc6c5bf4cc5885c8293bf8e, just without the idle state CPU waste. It mentioned some other cases where this causes problems like event queue overflow. High polling rate mouse input (can be thousands of Hz) might be a problem, and even though the OSC isn't rendered at that rate, the script tick handler is still called very often which just wastes CPU if the result isn't rendered.

I just think that even if we remove the limit, it will eventually come back to solve a few corner cases.

[avih]

What kinds of OSC updates requite smoother rendering? The only thing I can think of is dragging or moving the mouse pointer over the seek slider, and 30 fps doesn't sound bad in these cases for me personally.

I don't think we should remove the throttling. I think it's OK to reduce it a bit, but not less than 10ms. Also, IIRC without throttling it can add noticeable lag on slower systems, because most of the time at the OSC is spent on rendering (and in rendering, most of the time is spent waiting for the many mp.get_property(...) calls), so in "hard" cases where the rendering takes longer than the throttling duration, it can enter a 100% cpu scenario, which causes lag in the response to mouse moves.

Another big source of lag in the OSC is buffered frames by the VO, which IIRC can reach even 8 in some cases, and results in very noticeable lag of mouse-event-to-screen-update.

[na-na-hi]

What kinds of OSC updates requite smoother rendering?

See commit message. There is also fade in/out.

30 fps doesn't sound bad in these cases for me personally

It's very noticeable for me, and for many others. The fact that "soap opera effect" is a thing shows that the difference between 24/30 fps and 60 fps is very noticeable to the general public.

I don't think we should remove the throttling. I think it's OK to reduce it a bit, but not less than 10ms.

This is what this PR does exactly. The frame duration limit is lowered to 10 ms, which is a visual improvement over 30 ms. I have mentioned why the throttling shouldn't be removed. By lowering the refresh time it also reduces the lag of mouse-event-to-screen-update.

[kasper93]

What kinds of OSC updates requite smoother rendering?

Any kind. UI responsiveness and smoothness is one of the biggest selling points, that's why they put 120 Hz screens into mobile phones of all things.

most of the time is spent waiting for the many mp.get_property(...) calls), so in "hard" cases where the rendering takes longer than the throttling duration

Which ones are problematic? Recently we measured retrieving all of them and there wasn't any problem as far as I remember. Also if get_property is a problem it should be replaced with observe and cached inside a script for next update.

High polling rate mouse input (can be thousands of Hz) might be a problem, and even though the OSC isn't rendered at that rate, the script tick handler is still called very often which just wastes CPU if the result isn't rendered.

Maybe it is core issue to resolve and run scripts at resonable rate.

This is what this PR does exactly. The frame duration limit is lowered to 10 ms, which is a visual improvement over 30 ms.

I just want to understand what this changes, why 10 ms is ok, while 6.4 ms is not ok. What is the bottleneck, essentially why this rate limit inside a script is needed. Are other scripts not affected? Rate limiting to arbitrary fps is generally not the best practice, maybe it should be an option, but first I would like to understand why it is needed in the first place.

[na-na-hi]

I just want to understand what this changes, why 10 ms is ok, while 6.4 ms is not ok.

10 ms is enough for the vast majority of monitors which have a refresh rate of 60 Hz. Lower time might not be a performance problem in practice, in fact I tried 0 ms which seems not causing issues for me with normal usage, but it still wastes CPU for little benefit.

Are other scripts not affected?

Other built-in scripts which draw GUI all have throttling in place: stats.lua redraws once per second, and console.lua has a 20 fps limit for log printing. They're not throttled for key inputs, but the default key repeat rate is only 40 per second, and these scripts don't respond to mouse inputs, and have no elements which require smooth rendering.

but first I would like to understand why it is needed in the first place.

Because the alternative (to throttle the script event rate) causes more problems. It can cause missed events for all scripts and libmpv clients.

[kasper93]

10 ms is enough for the vast majority of monitors which have a refresh rate of 60 Hz.

I don't have stats, but many TV/Projectors are not 60 Hz. They are 120 Hz or other multiple of 24. Rarely you want to use 60 Hz for HTPC. If you are saying about low end PC monitors, than sure.

I don't disagree 10 ms is better. But what is the upper limit, like you said OSC needs smooth rendering, so why rate limit? To put in other words, what is so much CPU intensive in OSC that is not negligible when rendering faster? I'm not arguing with the change, I'm just asking, why arbitrary 10 ms is ok. It is higher than mentioned 60fps, less than 120fps. Is it good number, because it is 10?

and have no elements which require smooth rendering.

Exactly. They are not really rate limited, they just have fixed update rate that suites certain script.

It can cause missed events for all scripts and libmpv clients.

Wholly depends on implementation.

[na-na-hi]

But what is the upper limit, like you said OSC needs smooth rendering, so why rate limit? I'm just asking, why arbitrary 10 ms is ok. It is higher than mentioned 60fps, less than 120fps. Is it good number, because it is 10?

I already have a commit here which uses the display fps instead. Hopefully that value is "non-arbitrary" enough. As mentioned, updating at a rate above the display fps has no benefit since the maximum smoothness is already achieved. I can remove the 100 fps upper limit so that there will be no arbitrary limits left. Is this good enough, or does the limit have to eliminated completely?

[na-na-hi]

Decided to make it an option instead. Default to 0. From my testing this increases CPU usage but only by a small amount (about 2% of a single core). If someone wants lower power consumption, the duration can be increased by the user.

[DanOscarsson]

I use mpv for looking at movies and TV series on my TV. And as those are normally about 24 Hz I will run my TV at 24 Hz as that gets best results as each video frame will match a vsync. I think you should not update more the once per vsync (display fps) as a default. There could be an option for other preferences. Also osc should not update itself unless there is a visible change on what it wants to display.

[na-na-hi]

I agree with you that using the display fps is the best choice, but the idea was rejected. I made it an option this time just to make sure that it's still possible to disable the throttling completely.

[kasper93]

I agree with you that using the display fps is the best choice, but the idea was rejected.

Where? Give me one example when OSC/OSD or video is drawn faster than display fps, I will fix that.

[na-na-hi]

The concern was never about that. Property retrieval and computation in the script still use CPU resources and these are not limited by the display fps.

[Jules-A]

Might cause some issues for people on HFR displays running with display-resample, I really think there needs to be a limit in place for the default.

[na-na-hi]

No more default changes, it's too controversial. The options are still added so they can be set if smoother rendering is wanted.

[kasper93]

Needs rebase.