Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vo: add --display-fps-limiter option #14625

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Dudemanguy
Copy link
Member

On my end, it seems to be about the same as using --opengl-swapinterval=2 + --display-fps-override=30. But people with better monitors should test to make sure it actually works. Also, I'm pretty sure using this breaks interpolation so that's a gap item as well.

This is questionable but something like this has been commonly requested
over the years. Using any display-* mode will make mpv render at the
monitor's maximum refresh rate. This is correct given what the modes are
supposed to do, but nowadays people have super high refresh rate
monitors which means mpv can be rendering at 240fps or something crazy
high. One could make a custom modeline or just use the default audio
sync mode, but it might be nice to add in the ability to have an
artificial limiter on the frame rate. The --display-fps-override option
already exists to set a specific fps. All that is missing is a flag to
use that number as a hard cap. Fixes mpv-player#11122.
Copy link

github-actions bot commented Aug 2, 2024

Download the artifacts for this pull request:

Windows
macOS

@llyyr
Copy link
Contributor

llyyr commented Aug 2, 2024

When using a --video-sync=display-*, limit mpv's framerate to the detected display FPS. This can be combined with the --display-fps-override

This is already the case though? I'm not sure what this PR is doing. You can already use the display-fps-override option to achieve this

@Dudemanguy
Copy link
Member Author

I guess I should be more clear with the wording/naming here. --display-fps-override will indeed limit the fps, but in a vacuum it will probably not accomplish the desired result since it will stutter and look terrible. It's because the internal renderloop still requires that whatever fps you specify is actually accurate. You can combine it with something like --opengl-swapinterval=2 to trick mpv and smooth out playback.

This is like a generic --opengl-swapinterval except it's not limited to integers and you don't have to do the math/multiplications to get the desired result. i.e. --display-fps-override=30 + --display-fps-limiter will force the render loop to wait at a rate of at least 30hz.

@llyyr
Copy link
Contributor

llyyr commented Aug 2, 2024

I get the intention but I don't think this works as intended. With --video-sync=display-resample --display-fps-limiter=yes --display-fps-override=48, I get more judder on a VRR display that supports a lower-end of 48Hz than any other combination of options you could possibly use in mpv.

int64_t forced_wait = 0;
if (vo->opts->display_fps_limiter && VS_IS_DISP(vo->opts->video_sync)) {
forced_wait = now + in->nominal_vsync_interval;
wait_until(vo, forced_wait);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is useful. Aside from the inaccuracy of timing with system clock (which display-* modes try to avoid), nominal vsync interval rarely matches the real vsync interval in my experience. Combining these two means that this will pretty much cause stuttering, negating the major benefit of display-* modes.

@llyyr
Copy link
Contributor

llyyr commented Aug 2, 2024

I think for this to work, we'll need to make a lot of assumptions about the display and the media.

I think we'll need an option to let the users specify the lower end of the VRR range of their displays, or detect it somehow. Then automatically set display-fps-override to media_fps * ceil(lower_vrr_end / media_fps).

edit: I implemented that locally and I still can't say it's better than just video-sync=audio + VRR

int64_t forced_wait = 0;
if (vo->opts->display_fps_limiter && VS_IS_DISP(vo->opts->video_sync)) {
forced_wait = now + in->nominal_vsync_interval;
wait_until(vo, forced_wait);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you wanted to sleep before flip page. Also this will not be synchronized with vsync, so we will miss vsync points, causing stuttering, not sure how it solves things.

@Dudemanguy
Copy link
Member Author

Dudemanguy commented Aug 2, 2024

In theory this should be at least equivalent to audio mode with VRR in terms of presentation (e.g. it should be a consistent targets fps that is constantly rendered). If it's not, I'm not sure what can be done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants