Sunshine icon indicating copy to clipboard operation
Sunshine copied to clipboard

feat(nvprefs): add V-Sync and frame limiter options

Open Unarelith opened this issue 1 year ago • 10 comments

Description

Title.

A lot of users are already setting frame limiter / disabling V-Sync in Nvidia Control Panel to reduce stutter with Sunshine.

However it would require to switch back and forth between:

  • Default config (no frame limiter, vsync set to passive)
  • Sunshine config (fps limit to 60, vsync forced off)

This PR aims to make this automatic.

Screenshot

Will provide later

Issues Fixed or Closed

  • Fixes #3405

Type of Change

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] Dependency update (updates to dependencies)
  • [ ] Documentation update (changes to documentation)
  • [ ] Repository update (changes to repository files, e.g. .github/...)

Checklist

  • [x] My code follows the style guidelines of this project
  • [x] I have performed a self-review of my own code
  • [x] I have commented my code, particularly in hard-to-understand areas
  • [x] I have added or updated the in code docstring/documentation-blocks for new or existing methods/components

Unarelith avatar Oct 20 '24 13:10 Unarelith

Quality Gate Failed Quality Gate failed

Failed conditions
3 New issues
3 New Code Smells (required ≤ 0)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

sonarqubecloud[bot] avatar Oct 20 '24 15:10 sonarqubecloud[bot]

I managed to try this and it worked as expected: it changed the settings for vsync and fps limit

I'd need a bit more info on those things:

  • What would be the best place to enable the temporary settings? (should be when the stream starts, but where in the code would be the best?)
  • What would be the best place to reset the original settings?
  • It's already possible to get the display refresh rate, how to use that in my PR to replace the hardcoded "60"?

Unarelith avatar Oct 20 '24 15:10 Unarelith

* What would be the best place to enable the temporary settings? (should be when the stream starts, but where in the code would be the best?)
* What would be the best place to reset the original settings?

Take a look at my PR where the display_device::revert_configuration(); and display_device::configure_display(...) are used (process.cpp, stream.cpp, nvhttp.cpp). I think those should cover all of the cases of where and how. https://github.com/LizardByte/Sunshine/pull/3441

* It's already possible to get the display refresh rate, how to use that in my PR to replace the hardcoded "60"?

I would not recommend using refresh rate, but would take it from const auto launch_session = make_launch_session(host_audio, args); (refer to my previous comment). It provides FPS value from Moonlight. When using the actual display for streaming I usually leave it at 120Hz, but limit FPS to 60, because for whatever reason I perceive it as a smoother stream.

It might also be a good idea to provide an override value to be used instead of an automatic one for the limiter.

FrogTheFrog avatar Dec 11 '24 21:12 FrogTheFrog

I was planning on implementing this myself when I saw this PR. I can pick up where you left off, @Unarelith, if you are no longer working on it.

kirksaunders avatar Jan 01 '25 00:01 kirksaunders

I was planning on implementing this myself when I saw this PR. I can pick up where you left off, @Unarelith, if you are no longer working on it.

Sure, please do.

Unarelith avatar Jan 01 '25 03:01 Unarelith

I was planning on implementing this myself when I saw this PR. I can pick up where you left off, @Unarelith, if you are no longer working on it.

Hi, if you do it, maybe it could be interesting to do it for nvidia, amd and intel? 😁

moi952 avatar Jan 01 '25 13:01 moi952

Hi, if you do it, maybe it could be interesting to do it for nvidia, amd and intel?

We already have a framework in place to work with Nvidia preferences. Currently we don't have anything for AMD or Intel.

ReenigneArcher avatar Jan 01 '25 21:01 ReenigneArcher

Just throwing out a couple of ideas:

  1. Should this also disable G-sync/Freesync? I'm not sure if these are harmful to a stream when the framerate is kept below the host's max VRR range though.
  2. For the framerate limit, this looks like it uses the host's refresh rate but I think it should be based on the stream fps value. If you want to stream at 90fps for example, the 120fps host limit doesn't really help you. At the risk of making it more complicated, I like to limit refresh to -3 fps if the client has VRR so it won't exceed the VRR range. A text entry field with a good default is probably what you'd need here to make everyone happy.

andygrundman avatar Mar 25 '25 07:03 andygrundman

Not sure why/how the VRR situation the host has should reflect on the client (unless maybe you are v-syncing, but then that seems already the wrong setting to begin with isn't it?). As for framerate limits.. Wouldn't forcing a waitable objects swapchain be the best thing here?

mirh avatar Mar 27 '25 18:03 mirh

It looks like this PR has been idle for 90 days. If it's still something you're working on or would like to pursue, please leave a comment or update your branch. Otherwise, we'll be closing this PR in 10 days to reduce our backlog. Thanks!

LizardByte-bot avatar Jun 26 '25 10:06 LizardByte-bot

This PR was closed because it has been stalled for 10 days with no activity.

LizardByte-bot avatar Jul 07 '25 10:07 LizardByte-bot