fix: Resolve minimum_fps_target related issues on all platforms

[psyke83]

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): set correct default minimum_fps_target and max_frametime interval alignment

The current minimum_fps_target and max_frametime intervals are set/logged incorrectly
and break capture methods on Linux. Example with a 60fps stream:

• default minimum_fps_target is populated as config.framerate (60),
• minimum FPS target is logged as 30 and interval logged as 33.33ms, but
  pop() interval is actually 16.6ms.
• result: default value is 2x what documentation implies should be the case.
  Setting a manual value logs the target FPS and interval incorrectly but
  sets the correct target interval for the duplicate insertion logic via pop().

Issues observed at 60fps:

• kmsgrab: fps cap violations and stuttering during cursor shape changes
  due to increased host processing latency triggering extra duplicates.
• portalgrab: idle desktop settles 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:

• minimum_fps_target defaults to (config.framerate / 2), and max_frametime
  aligns with the actual minimum target FPS. This makes the logging accurate
  and the existing documention becomes sensible.
• kmsgrab: during expensive cursor updates, no more framerate violations
  or stuttering.
• portalgrab: desktop idles at 30fps instead of ~52fps.
• portalgrab: bursting is resolved. No more violations at 45fps, 60fps, etc.

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.

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

• feat: New feature (non-breaking change which adds functionality)
• fix: Bug fix (non-breaking change which fixes an issue)
• docs: Documentation only changes
• style: Changes that do not affect the meaning of the code (white-space, formatting, missing semicolons, etc.)
• refactor: Code change that neither fixes a bug nor adds a feature
• perf: Code change that improves performance
• test: Adding missing tests or correcting existing tests
• build: Changes that affect the build system or external dependencies
• ci: Changes to CI configuration files and scripts
• chore: Other changes that don't modify src or test files
• revert: Reverts a previous commit
• BREAKING CHANGE: Introduces a breaking change (can be combined with any type above)

Checklist

• Code follows the style guidelines of this project
• Code has been self-reviewed
• Code has been commented, particularly in hard-to-understand areas
• Code docstring/documentation-blocks for new or existing methods/components have been added or updated
• Unit tests have been added or updated for any new or modified functionality

AI Usage

• None: No AI tools were used in creating this PR
• Light: AI provided minor assistance (formatting, simple suggestions)
• Moderate: AI helped with code generation or debugging specific parts
• Heavy: AI generated most or all of the code changes

[psyke83]

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:

• Host running KDE, making sure that the hardware cursor plane is enabled (i.e., make sure KWIN_FORCE_SW_CURSOR=1 is not set in your environment)
• Keep minimum_fps_target as the default (0) or unset.
• Capture using kmsgrab + vaapi or vulkan.
• Stream the desktop with active content that is rendering at the full stream framerate; for example, open https://www.vsynctester.com/ and leave it running.
• Repeatedly hover the mouse over elements on the desktop that will rapidly trigger cursor shape changes, such as moving from the browser's address URL bar to the tab selection/window decorations so that the cursor toggles between the selection cursor and pointer. Avoid moving too fast, as you can trigger the "shake" cursor effect on KDE, which switches to a software cursor that is not affected by the issue.

Result on master:

• During cursor shape changes, the decoding framerate violates the stream limit (bursting to 70+ on a 60fps stream) and frame queue delay becomes very high. This results in noticeable stuttering.

Results with PR:

• The stream no longer violates the limit or has noticeable stuttering, and the minimum_fps_target still works as expected.

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.

[codecov]

Bundle Report

Bundle size has no change ✅

[codecov]

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 18.00%. Comparing base (7228c25) to head (3f71759).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/thread_safe.h	0.00%	1 Missing and 3 partials ⚠️
src/video.cpp	0.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4967      +/-   ##
==========================================
+ Coverage   17.36%   18.00%   +0.63%     
==========================================
  Files         108      109       +1     
  Lines       23220    23502     +282     
  Branches    10138    10374     +236     
==========================================
+ Hits         4033     4232     +199     
+ Misses      17075    16041    -1034     
- Partials     2112     3229    +1117     

Flag	Coverage Δ
Archlinux	11.59% <0.00%> (+<0.01%)	⬆️
FreeBSD-14.3-aarch64	?
FreeBSD-14.3-amd64	13.58% <0.00%> (+<0.01%)	⬆️
Homebrew-ubuntu-22.04	13.97% <0.00%> (+<0.01%)	⬆️
Linux-AppImage	12.26% <0.00%> (+<0.01%)	⬆️
Windows-AMD64	14.90% <0.00%> (?)
Windows-ARM64	13.23% <0.00%> (+<0.01%)	⬆️
macOS-arm64	19.02% <0.00%> (+<0.01%)	⬆️
macOS-x86_64	18.37% <0.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/platform/common.h	33.01% <ø> (+2.46%)	⬆️
src/platform/linux/pipewire.cpp	0.00% <ø> (ø)
src/video.cpp	32.37% <0.00%> (+0.13%)	⬆️
src/thread_safe.h	57.20% <0.00%> (+0.68%)	⬆️

... and 62 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7228c25...3f71759. Read the comment docs.

[psyke83]

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.

[oryschakj-personal]

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?

[psyke83]

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

[oryschakj-personal]

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.

[oryschakj-personal]

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.

[psyke83]

@oryschakj-personal

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.

[andygrundman]

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.

[psyke83]

@andygrundman

I believe that the logic needs to be changed in encode_run() as this logic is potentially exposing issues for all capture methods in different ways. Let me document what I know:

The kmsgrab backend has no duplicate filtering on frames, so it always captures at full framerate, and capture cadence is usually consistent, meaning that minimum_fps_target setting should not affect this capture method at all... in theory. There's one exception: the update_cursor() function has expensive mmap/memcpy calls that only happens when the cursor shape changes, causing quite large CPU usage and host latency spikes for individual frames (the testcase in comment 2 of this PR). During this time, multiple frames arrive at irregular intervals, but duplicate frames are also inserted which violates the framerate cap.

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:

• Revert fix(linux/xdgportal): avoid duplicate frame insertion #4839 OR explicitly set minimum_fps_target = 60 to override the workaround. On a 60fps stream, this results in:

Apr 11 19:27:58 archlinux sunshine[141610]: [2026-04-11 19:27:58.092]: Info: Minimum FPS target set to ~30fps (33.3333ms)

• Using libstrangle, test content running at 45fps on a 60fps stream:

strangle 45 glxgears

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 minimum_fps_target that can happen if the frame receive interval is erratic.

[psyke83]

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

[oryschakj-personal]

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]

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:

• 7330c1f
• e1e5d05

@andygrundman

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

[Kishi85]

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

[psyke83]

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.

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.

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

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:

Apr 14 17:06:00 archlinux sunshine[54921]: [2026-04-14 17:06:00.514]: Info: Minimum FPS target set to ~30fps (33.3333ms)

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.

[psyke83]

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.

Sorry, I misunderstood what you meant here. The previous is_event_driven() workaround was setting the target fps to 0.5fps and your Android client is having decoding issues due to the low framerate, right? If so, the PR will definitely help for that specific issue, as the workaround is reverted and portalgrab will use the same default FPS target as other capture methods.

[Kishi85]

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:

Apr 14 17:06:00 archlinux sunshine[54921]: [2026-04-14 17:06:00.514]: Info: Minimum FPS target set to ~30fps (33.3333ms)

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.

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?

The previous is_event_driven() workaround was setting the target fps to 0.5fps and your Android client is having decoding issues due to the low framerate, right? If so, the PR will definitely help for that specific issue, as the workaround is reverted and portalgrab will use the same default FPS target as other capture methods.

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.

[Kishi85]

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?

or just use 2*minimum_fps_target to get the intended effect.

[psyke83]

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.

[ReenigneArcher]

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?

or just use 2*minimum_fps_target to get the intended effect.

It seems linear on the lower end, but it's not linear as you go up.

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.

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.

[psyke83]

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.

[psyke83]

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.

[psyke83]

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.
minimum_fps_target = 30 -> (keeping in mind that both the fps and frametime target are now used for the base duplication logic):

Apr 14 21:41:41 archlinux sunshine[81074]: [2026-04-14 21:41:41.419]: Info: Minimum FPS target set to ~30fps (33.3333ms)

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.

[andygrundman]

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.

[psyke83]

I've rebased the PR with just the minimal fixes; kmsgrab and pacing changes that will improve kmsgrab and 30fps pacing with portalgrab may be submitted later when the PR load reduces.

@ReenigneArcher I believe these changes are ready for merge, but I'd like to test a Windows build first. I'd appreciate if you could kick off a build when possible so I can test the Windows artifact, as my dev environment on my old Windows install was wiped. Thanks.

[ReenigneArcher]

No worries. Your PRs are always welcomed and valuable. I really appreciate you testing and reviewing other PRs as well. It helps a lot!

[Kishi85]

This looks good on my end as well. Have been using it for a while now and haven't had any issues when using kmsgrab, portalgrab or kwingrab (#5009).

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[psyke83]

@ReenigneArcher

I checked the Windows build and it looks OK to me. It may idle a bit lower than before (~22fps on a 60fps stream), but the previous behavior was logging an invalid interval and causing havoc with other capture methods, and the documentation was not making sense, so this fix should be committed IMO.

If the lower effective floor on Windows introduces an issue with poor client decoders, the proper fix would be to target a different default value of the minimum_fps_target variable, or add an artificial multiplication factor for known problematic capture method such as Windows instead of using the wrong pop() duplicate pacing interval. For my example, adding a 1.35 (or rather, 0.75 if the capture method really needs a shorter pop interval) factor for Windows only would bring it closer to 30, but of course it would need more testing and some clarity to make sure that the real frame interval is logged properly with the factor applied.

Using a factor to bring the real FPS to slightly above 50% of config.framerate would also indirectly benefit Linux capture, as it would help avoid unnecessary dropped frames caused by pipewire capture jitter for the exact half framerate case such as 30fps active rendering on a 60fps stream - but my duplicate pacing changes would be a better solution. We can revisit the pacing changes or evaluate a different default value later, if needed.