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

[Unarelith]

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

• FixesStill jittery after all these years when Vsync on, Client and Server set to same refresh rate. #3405

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• 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

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

[ReenigneArcher]

Thanks for the PR! this is a preliminary review.

[ReenigneArcher]

This one is fine, but I'm not sure why the one above is missing the conditional for the windows platform... or if any of them are even needed as the entire nvenc encoder is Windows only.

[ReenigneArcher]

I suppose you can get this from #2894 after it's merged.

[sonarqubecloud]

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

[Unarelith]

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"?

[FrogTheFrog]

* 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.
#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.

[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.

[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.

Sure, please do.

[moi952]

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? 😁

[ReenigneArcher]

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.

[andygrundman]

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.

[mirh]

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?

[LizardByte-bot]

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]

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

[Unarelith]

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

• FixesStill jittery after all these years when Vsync on, Client and Server set to same refresh rate. #3405

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• 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

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

[ReenigneArcher]

Thanks for the PR! this is a preliminary review.

[ReenigneArcher]

This one is fine, but I'm not sure why the one above is missing the conditional for the windows platform... or if any of them are even needed as the entire nvenc encoder is Windows only.

[ReenigneArcher]

I suppose you can get this from #2894 after it's merged.

[sonarqubecloud]

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

[Unarelith]

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"?

[FrogTheFrog]

* 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.
#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.

[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.

[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.

Sure, please do.

[moi952]

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? 😁

[ReenigneArcher]

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.

[andygrundman]

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.

[mirh]

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?

[LizardByte-bot]

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]

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

[Unarelith]

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

• FixesStill jittery after all these years when Vsync on, Client and Server set to same refresh rate. #3405

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• 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

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

[ReenigneArcher]

Thanks for the PR! this is a preliminary review.

[ReenigneArcher]

This one is fine, but I'm not sure why the one above is missing the conditional for the windows platform... or if any of them are even needed as the entire nvenc encoder is Windows only.

[ReenigneArcher]

I suppose you can get this from #2894 after it's merged.

[sonarqubecloud]

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

[Unarelith]

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"?

[FrogTheFrog]

* 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.
#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.

[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.

[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.

Sure, please do.

[moi952]

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? 😁

[ReenigneArcher]

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.

[andygrundman]

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.

[mirh]

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?

[LizardByte-bot]

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]

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

[Unarelith]

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

• FixesStill jittery after all these years when Vsync on, Client and Server set to same refresh rate. #3405

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• 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

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

[ReenigneArcher]

Thanks for the PR! this is a preliminary review.

[ReenigneArcher]

This one is fine, but I'm not sure why the one above is missing the conditional for the windows platform... or if any of them are even needed as the entire nvenc encoder is Windows only.

[ReenigneArcher]

I suppose you can get this from #2894 after it's merged.

[sonarqubecloud]

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

[Unarelith]

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"?

[FrogTheFrog]

* 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.
#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.

[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.

[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.

Sure, please do.

[moi952]

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? 😁

[ReenigneArcher]

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.

[andygrundman]

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.

[mirh]

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?

[LizardByte-bot]

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]

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

[Unarelith]

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

• FixesStill jittery after all these years when Vsync on, Client and Server set to same refresh rate. #3405

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• 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

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

[ReenigneArcher]

Thanks for the PR! this is a preliminary review.

[ReenigneArcher]

This one is fine, but I'm not sure why the one above is missing the conditional for the windows platform... or if any of them are even needed as the entire nvenc encoder is Windows only.

[ReenigneArcher]

I suppose you can get this from #2894 after it's merged.

[sonarqubecloud]

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

[Unarelith]

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"?

[FrogTheFrog]

* 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.
#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.

[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.

[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.

Sure, please do.

[moi952]

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? 😁

[ReenigneArcher]

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.

[andygrundman]

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.

[mirh]

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?

[LizardByte-bot]

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]

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

[Unarelith]

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

• FixesStill jittery after all these years when Vsync on, Client and Server set to same refresh rate. #3405

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• 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

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

[ReenigneArcher]

Thanks for the PR! this is a preliminary review.

[ReenigneArcher]

This one is fine, but I'm not sure why the one above is missing the conditional for the windows platform... or if any of them are even needed as the entire nvenc encoder is Windows only.

[ReenigneArcher]

I suppose you can get this from #2894 after it's merged.

[sonarqubecloud]

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

[Unarelith]

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"?

[FrogTheFrog]

* 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.
#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.

[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.

[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.

Sure, please do.

[moi952]

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? 😁

[ReenigneArcher]

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.

[andygrundman]

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.

[mirh]

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?

[LizardByte-bot]

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]

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

[Unarelith]

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

• FixesStill jittery after all these years when Vsync on, Client and Server set to same refresh rate. #3405

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• 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

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

[ReenigneArcher]

Thanks for the PR! this is a preliminary review.

[ReenigneArcher]

This one is fine, but I'm not sure why the one above is missing the conditional for the windows platform... or if any of them are even needed as the entire nvenc encoder is Windows only.

[ReenigneArcher]

I suppose you can get this from #2894 after it's merged.

[sonarqubecloud]

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

[Unarelith]

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"?

[FrogTheFrog]

* 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.
#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.

[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.

[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.

Sure, please do.

[moi952]

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? 😁

[ReenigneArcher]

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.

[andygrundman]

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.

[mirh]

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?

[LizardByte-bot]

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]

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

[Unarelith]

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

• FixesStill jittery after all these years when Vsync on, Client and Server set to same refresh rate. #3405

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• 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

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

[ReenigneArcher]

Thanks for the PR! this is a preliminary review.

[ReenigneArcher]

This one is fine, but I'm not sure why the one above is missing the conditional for the windows platform... or if any of them are even needed as the entire nvenc encoder is Windows only.

[ReenigneArcher]

I suppose you can get this from #2894 after it's merged.

[sonarqubecloud]

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

[Unarelith]

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"?

[FrogTheFrog]

* 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.
#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.

[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.

[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.

Sure, please do.

[moi952]

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? 😁

[ReenigneArcher]

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.

[andygrundman]

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.

[mirh]

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?

[LizardByte-bot]

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]

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