fix: Resolve minimum_fps_target related issues on all platforms#4967
fix: Resolve minimum_fps_target related issues on all platforms#4967psyke83 wants to merge 8 commits intoLizardByte:masterfrom
Conversation
psyke83
force-pushed
the
min_fps_target_fixes
branch
from
3a84519 to
3bac8e3
Compare
psyke83
changed the title
|
The basic issue that this PR aims to solve is framerate violations that occur when frame processing latency is a bit higher than usual. This can happen with any capture method, but a solid Linux testcase that can be replicated for the issue against master is the following:
Result on master:
Results with PR:
Requesting a test from @andygrundman and @oryschakj-personal if it's not too much trouble. @ReenigneArcher I've also added two additional commits. One implements snapshot timeout + duplicate filtering on kmsgrab, and the other reverts the portalgrab workaround related to the same issue that this PR should hopefully solve. If you want we can split these into separate PRs, but I think it's helpful to test all three changes together as a first step. |
Bundle ReportChanges will decrease total bundle size by 7.43kB (-0.47%) ⬇️. This is within the configured threshold ✅ Detailed changes
ℹ️ *Bundle size includes cached data from a previous commit Affected Assets, Files, and Routes:view changes for bundle: sunshine-esmAssets Changed:
view changes for bundle: rapideye-esmAssets Changed:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4967 +/- ##
==========================================
- Coverage 17.97% 17.96% -0.02%
==========================================
Files 108 108
Lines 23424 23433 +9
Branches 10340 10345 +5
==========================================
- Hits 4211 4209 -2
- Misses 14227 14235 +8
- Partials 4986 4989 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
psyke83
force-pushed
the
min_fps_target_fixes
branch
from
3bac8e3 to
48e6847
Compare
psyke83
changed the title
|
I've backed out the kmsgrab duplicate detection change as it was triggering some false timeouts. I'll check if that can be fixed later and send in a separate PR if so. The testcase and fix I described for kmsgrab still applies to this PR in its current state, however. I'm still testing this PR with portalgrab to make 100% sure that it solves the issue of runaway framerate bursting; it's more difficult to reproduce consistently, so it may take some time to see if there's a regression. |
|
I got the build installed (48e6847) but can't get either VA-API or Vulkan running on my 5090. Is the fix still applicable to nvenc? |
Yes. You can still try the test case as long as you test with the kmsgrab capture method, the encoder shouldn't matter. This PR should resolve your issue reported in #4906 (but make sure the hardware cursor is enabled in kwin and set minimum_fps_target back to the default if you want to trigger the stutter issue properly). |
Excellent, thank you! I've been running this for a couple hours with the appropriate Sunshine config settings, vsync tester, stream stats, and top up and am not seeing any of the problems I reported for that bug. |
|
Actually - maybe a false alarm. I can recreate the laggy/stuttering mouse behavior with high Sunshine CPU usage and 50ms+ host processing times by mousing around an open Konsole window. Nothing needs to be running in the console session. |
|
It would really help if you could confirm: on master/stable builds, were you able to reproduce the specific symptoms of the framerate cap being violated during cursor shape changes using the testcase I described? Keep in mind that the high CPU usage in Sunshine that's specific to kmsgrab and cursor shape changes is not being solved by the PR, but the visible stuttering and framerate violations during this mouse movement is what I'm attempting to address. With this PR, there are no stutters visible on the stream on my system when the cursor shape changes, but the maximum host processing latency does still increase from ~3-4ms to ~11-15ms when the mouse issue is triggered, and Sunshine's CPU usage still increases. I'm only testing at 60fps @ 1080P, but I did try a 120fps stream and it seems roughly the same (but my client has a 60Hz screen, so naturally I'm not seeing all frames). I'll mark this as draft, as I've discovered some other issues related to minimum_fps_target that requires more investigation. |
|
Do you have problems if you set your min_fps to a lower value like 10 or 20? Does this affect Windows? I am not sure we should change this part of the code for everyone, if it's just a Linux issue. I would really like to have a nice Linux test environment, if you guys have any tips let me know. I still get overwhelmed thinking about the matrix of backends and encoders and all the stuff I don't know how to run or test. |
|
I believe that the logic needs to be changed in The kmsgrab backend has no duplicate filtering on frames, so it always captures at full framerate, and capture cadence is usually consistent, meaning that Portalgrab is receiving frames from the compositor via pipewire, and these buffers arrive at irregular intervals compared to a traditional capture method like DXGI or kmsgrab. If you recall, I effectively disabled duplicate frame insertion to work around the issue, but I had a lot of trouble reproducing a testcase to justify the change. Now I can reproduce it clearly:
This consistently results in the stream violating the framerate cap, running at 90fps for a 60fps stream due to duplicate frames being inserted and bunched together with the actual 45fps worth of frames. I'm trying to find a better resolution than this PR as it stands currently. Some WIP code: diff --git a/src/video.cpp b/src/video.cpp
index 09645818..346e7fb7 100644
--- a/src/video.cpp
+++ b/src/video.cpp
@@ -2069,6 +2069,9 @@ namespace video {
}
}
+ // 1. Initialize the tracker before the while(true) loop
+ auto last_encode_time = std::chrono::steady_clock::now();
+ const auto slack_factor = 2.5;
while (true) {
// Break out of the encoding loop if any of the following are true:
// a) The stream is ending
@@ -2102,7 +2105,17 @@ namespace video {
// Encode at a minimum FPS to avoid image quality issues with static content
if (!requested_idr_frame || images->peek()) {
- if (auto img = images->pop(max_frametime)) {
+ // 2. Calculate how much 'patience' we have left
+ auto now = std::chrono::steady_clock::now();
+ auto elapsed = now - last_encode_time;
+
+ // We want to wait until at least (max_frametime * slack_factor)
+ // has passed since the last encode.
+ auto wait_limit = std::chrono::duration_cast<std::chrono::milliseconds>(max_frametime * slack_factor);
+ auto remaining_wait = (wait_limit > elapsed) ? (wait_limit - elapsed) : 0ms;
+
+ if (auto img = images->pop(remaining_wait)) {
+ // We got a real frame!
frame_timestamp = img->frame_timestamp;
if (session->convert(*img)) {
BOOST_LOG(error) << "Could not convert image"sv;
@@ -2110,6 +2123,10 @@ namespace video {
}
} else if (!images->running()) {
break;
+ } else {
+ // 3. Timeout reached: We haven't seen a new frame in ~1.5x intervals.
+ // We will fall through and encode() the last known image as a duplicate.
+ BOOST_LOG(debug) << "Minimum FPS heartbeat triggered (Duplicate sent)"sv;
}
}
@@ -2117,6 +2134,7 @@ namespace video {
BOOST_LOG(error) << "Could not encode video packet"sv;
return;
}
+ last_encode_time = std::chrono::steady_clock::now();
session->request_normal_frame();
This resolves the 45 -> 90fps issue on portalgrab, but the slack factor needs consideration. If set to 1.5x, it doesn't fix the kmsgrab cursor stuttering. if set to 2.0x exactly, it resolves kmsgrab cursor stutter, but we may still see duplicates inserted for 30fps content on a 60fps stream for portalgrab. I believe this issue can also potentially manifest on Windows, but I've not been testing Sunshine on that platform lately myself. There may be some issues tracked here (90fps FPS on a 60fps stream) that could point the finger to |
OT, but I have a SSD connected via a USB adapter plugged into my host PC, and I use ventoy + vtoyboot that allows me to native boot into multiple VDI images of Fedora, Arch etc., and the images are stored on an exfat partition. I believe it also supports booting from an ntfs partition (even your Windows partition), but I wouldn't risk it myself (hence the external storage). |
|
I’d like to be able to test more methodically but I suspect if I do a Spectacle screen capture that itself will mess with whatever I’m trying to identify. When I get some time to dig into it I’ll try to get some comparative tests done between builds. On Apr 11, 2026, at 2:55 PM, psyke83 ***@***.***> wrote:psyke83 left a comment (LizardByte/Sunshine#4967)
Do you have problems if you set your min_fps to a lower value like 10 or 20? Does this affect Windows? I am not sure we should change this part of the code for everyone, if it's just a Linux issue.
I would really like to have a nice Linux test environment, if you guys have any tips let me know. I still get overwhelmed thinking about the matrix of backends and encoders and all the stuff I don't know how to run or test.
OT, but I have a SSD connected via a USB adapter plugged into my host PC, and I use ventoy + vtoyboot that allows me to native boot into multiple VDI images of Fedora, Arch etc., and the images are stored on an exfat partition. I believe it also supports booting from an ntfs partition (even your Windows partition), but I wouldn't risk it myself (hence the external storage).
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
psyke83
force-pushed
the
min_fps_target_fixes
branch
2 times, most recently
from
d3912a4 to
d2cb654
Compare
…Byte#4839)" This reverts commit 99d4e05.
…tering This allows kmsgrab to only send frames when updates truly occur rather than sending maximum (potential duplicate) frames unconditionally. * snapshot: Implement deadline-based timeout detection. * refresh: Filter duplicate frames by comparing fb_id of current vs previous frame and cursor updates. * update_cursor: Return status on any cursor change to override fb_id duplicate filtering.
The previous implementation called wait_for(delay) inside a manual loop, causing the full timeout to reset on each spurious wakeup. Replace with a single predicate-based wait_for call, which handles spurious wakeups internally while correctly honoring the original deadline.
Previously, the correct target frame time was being logged, but the actual pop() interval delay is half the expected rate. Issues observed at 60fps, default minimum_fps_target: * Logged: Info: Minimum FPS target set to ~30fps (33.3333ms), but max_frametime is actually set to 16.6ms. * kmsgrab: minor fps cap violations during mouse movement (cursor shape changes) due to increased host processing latency triggering duplicates. * kmsgrab & portalgrab: idle desktop settling on ~52fps instead of the correct 30fps default target. * portalgrab: massive framerate bursting due to irregular frame arrival that manifests at specific framerates; example: "strangle 45 glxgears" produces sustained 90fps on a 60fps stream. Can also manifest when framerate = stream framerate, but is harder to reproduce. New behaviour: * max_frametime is properly set to the value that is logged that aligns with the actual minimum_fps_target (33.3333ms for 60fps). * kmsgrab: during expensive cursor updates, no more framerate violations or stuttering. * kmsgrab & portalgrab: desktop idles at 30fps. * portalgrab: bursting is resolved. No more violations at 45fps, 60fps, etc. * portalgrab: sub-30fps content is correctly duplicated in even intervals. ex: 12fps -> 36fps, 24fps -> 48fps.
Keep a running tally of time credit to smoothen capture jitter, so slow frames can pay for fast frames (and vice-versa) whilst maintaining the minimum FPS target. Especially useful for the knife-edge case where the on-screen content matches the minimum FPS target. Example for 60fps stream with 30fps target: * If a frame arrives at 30ms, bank 3.33ms credit on next frame budget * If a frame arrives at 34ms, bank 1.33ms debit on next frame budget This strategy allows capture methods prone to jitter (such as portalgrab) to maintain a 30fps average without inserting duplicates when jitter is present between frames.
psyke83
force-pushed
the
min_fps_target_fixes
branch
from
d2cb654 to
7330c1f
Compare
|
I believe these changes are worth checking. I haven't yet verified if Windows capture is affected negatively, but if I can test later if the build artifacts succeed. Two commits are not directly related or needed to fix the bursting, but benefit from testing with these changes: I understand that you don't have a Linux test system up and running, but what are your thoughts on the main fix? Was this an oversight in the logging vs actual wait period to be used by the pop() timeout, or am I completely misunderstanding the intended logic of the minimum_fps_target? This change alone resolves all the bursting issues I was seeing with both portalgrab and kmsgrab, and fixes idle desktop (previously, 60fps at idle was settling on ~52fps, now idles at ~28-30fps). |
|
I'm going to give this a try after having a few issues with one game (Blue Prince) that is stuttering when streaming using moonlight-android (on a CCwGTV). I'm also getting Bandwidth warnings (and black screens on connecting until seeing activity) with moonlight-android on idle desktop (but I'm not sure if are related to Vulkan encoding with VBR yet). I'll report my findings once I've had some time to give this an extended try. First thing I've noted with the patch is from having a minimum_fps_target=30 set explicitly in config (as a workaround for those moonlight-android issues) that the Desktop will idle at around ~15fps. Setting it to 40 will make the desktop idle at ~20fps. Removing the minimum_fps_target (or setting it to 0) and setting 60fps as the stream framerate in Moonlight will make the Desktop idle at ~28-30fps. Looks like the real minimum fps now is 1/2*minimum_fps_target if that is set with the current state of the patch. EDIT: All using portalgrab with vulkan encoder on CachyOS latest with an RX9070XT |
If you could turn down the client bitrate to eliminate transient network issues that would help make sure it's not an issue with the PR. I don't believe any of my changes should cause bitrate bursting.
With the changes to this PR, that's the expected result; unset will select the stream framerate, and the value will be halved to give the actual target FPS which you can check from your log: The difference related to this logging is that master branch is lying about the frame time target (it was using a 16ms interval in my case). That's the main change that fixes the bursting issues on portalgrab and kmsgrab. |
Sorry, I misunderstood what you meant here. The previous |
I see but then the config setting should probably be renamed shouldn't it? As it's more like the intended fps target and the minimum is half of that value or am I misunderstanding something here?
That is what it looks like as setting a minium_fps_target of half the framerate set on moonlight was also helping with the issue. The patch is helping for that issue as expected. |
or just use 2*minimum_fps_target to get the intended effect. |
|
Well, it would help to understand the reason why it was changed. I believe the previous setting was causing issues on Windows, but I wasn't actively using Sunshine on that OS at the time. The documentation is half-correct; the value 0 is "treated" as half the stream framerate, but the effective value would be 60 for 60fps. Additionally, you would want to set 40 instead of 20 to avoid duplicating 24fps content. |
It seems linear on the lower end, but it's not linear as you go up.
This goes way back to #754 which set the floor to ~10fps. But this caused other issues, so it became configurable. I really just wish we could stream the exact rate that's being requested. We shouldn't really worry about tiny bandwidth saving in my opinion, since live gamestreaming (or any kind of video streaming) is going to consume a ton of bandwidth anyway. |
|
If you look at the original implementation, the 10fps floor was correctly using a 100ms pop interval. Some time from then till now, the pop timeout interval became half the stated target floor (that was configured & logged), thus the effective frametime became the same as the stream's frametime (60fps -> 16ms), which was the main culprit for all the bursting issues. |
|
I'm also happy that this is configurable (even though it's not working on master as expected), as the difference between idle and 30fps is 10W+ in measured GPU load. Electricity is not cheap in Europe, and I don't have decoding issues at low framerate. Either way, the implementation needs to be fixed to resolve the bursting. |
Adjust logic so that the variable and max_frametime are aligned, and ensure that the default case assigns the value of half of the stream framerate to make the functionality consistent with the existing documentation.
|
OK, so I've added a commit to align the variable with the actual desired FPS value, which is what the documentation already implies is the case. minimum_fps_target = 0 or unset -> real value is assigned to (config.framerate / 2), so 30 for a 60fps stream. The PR is messy right now, but I'm trying to avoid squashing or rebasing commits so that my recent changes can be more easily tracked. |
|
|
If you have an Xbox try the Display-locked frame pacer, I think it’s the only actual solution to the variable framerate situation. The client takes over all responsibility for duping frames, and simply renders 1 frame every frame at the refresh rate, choosing between a new or repeat frame based on interpolating the incoming framerate. It works well, there is a bit more resource use on the client but no bandwidth is wasted. It would also let you stream at 1fps from the server if you wanted. I am almost done porting this frame pacer option to Mac and then the rest of Qt (plus lots more improvements like the ImGui graphs, a dev UI panel for viewing data and tweaking everything, etc. |
5 participants
Description
Multiples changes that affects all platforms, aimed at resolving issues related to the minimum_fps_target setting. During testing of both portalgrab and kmsgrab, certain streaming conditions can be observed to cause the stream framerate cap to be violated, which is resolved by these changes.
fix(video): use honest minimum_fps_target value
Adjust logic so that the variable and max_frametime are aligned,
and ensure that the default case assigns the value of half of the
stream framerate to make the functionality consistent with the
existing documentation.
feat(video): use time credit slots for duplicate frame insertion
Keep a running tally of time credit to smoothen capture jitter, so
slow frames can pay for fast frames (and vice-versa) whilst maintaining
the minimum FPS target. Especially useful for the knife-edge case where
the on-screen content matches the minimum FPS target.
Example for 60fps stream with 30fps target:
This strategy allows capture methods prone to jitter (such as portalgrab)
to maintain a 30fps average without inserting duplicates when jitter is
present between frames.
fix(video): set correct max_frametime period
Previously, the correct target frame time was being logged, but the
actual pop() interval delay is half the expected rate.
Issues observed at 60fps, default minimum_fps_target:
is actually set to 16.6ms.
due to increased host processing latency triggering duplicates.
default target.
that manifests at specific framerates; example: "strangle 45 glxgears"
produces sustained 90fps on a 60fps stream. Can also manifest when framerate =
stream framerate, but is harder to reproduce.
New behaviour:
actual minimum_fps_target (33.3333ms for 60fps).
or stuttering.
fix: resolve timed pop() resetting timeout on spurious wakeups
The previous implementation called wait_for(delay) inside a manual loop,
causing the full timeout to reset on each spurious wakeup.
Replace with a single predicate-based wait_for call, which handles
spurious wakeups internally while correctly honoring the original deadline.
feat(linux/kmsgrab): implement snapshot timeout & duplicate frame filtering
This allows kmsgrab to only send frames when updates truly occur rather
than sending maximum (potential duplicate) frames unconditionally.
previous frame and cursor updates.
duplicate filtering.
Revert "fix(linux/xdgportal): avoid duplicate frame insertion (#4839)"
This reverts commit 99d4e05.
Screenshot
Issues Fixed or Closed
Fixes #4906
Roadmap Issues
Type of Change
Checklist
AI Usage