Skip to content

fix(linux/xdgportal): avoid duplicate frame insertion#4839

Open
psyke83 wants to merge 1 commit intoLizardByte:masterfrom
psyke83:xdg_nominfps
Open

fix(linux/xdgportal): avoid duplicate frame insertion#4839
psyke83 wants to merge 1 commit intoLizardByte:masterfrom
psyke83:xdg_nominfps

Conversation

@psyke83
Copy link
Contributor

@psyke83 psyke83 commented Mar 11, 2026

Portal capture is event driven/blocking, so duplicate frames can compete
with pacing due to irregular buffer arrival.

Resolve by ensuring that event-driven capture methods default to 0.5fps/
2000ms instead of half the host framerate. This will not impact other
capture methods, but if future event-driven capture methods are added, they
can also utilise the is_event_driven() override.

Description

Screenshot

Issues Fixed or Closed

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 psyke83 marked this pull request as ready for review March 11, 2026 03:32
@psyke83
Copy link
Contributor Author

psyke83 commented Mar 11, 2026

Without this PR, I need to set min_fps_target = 1 to avoid random small FPS bursts that causes intermittent stuttering, which in certain circumstances can degrade to runaway bursting (constant 90fps on a 60fps stream while in a fullscreen game that can only be temporarily fixed by toggling vsync, alt-tabbing or reconnecting the stream). With the PR, the min_fps_target defaults to 1 for event-driven capture methods (currently only portalgrab) so no user intervention will be necessary, but custom config will be respected.

Pinging @andygrundman - let me know if you object to this change (as I believe you last touched the fake frame logic).
Also pinging @shelterx due to your comment in the vulkan PR. If you see stuttering with VAAPI + portalgrab, then you can try this PR and see if it helps (this PR will also work with Vulkan encoding when the other PR is merged).

@psyke83 psyke83 force-pushed the xdg_nominfps branch 2 times, most recently from 7edf090 to ec11388 Compare March 11, 2026 05:07
@shelterx

This comment was marked as off-topic.

@psyke83
Copy link
Contributor Author

psyke83 commented Mar 11, 2026

Just a note: the original version of this PR was disabling fake frame insertion completely, but I didn't realize that images->pop() could timeout indefinitely when a reinit was called (which happens on disconnect or mode change), so I changed it so that portalgrab defaults to the lowest setting when no user config is present.

I tried various things KMS/XDG/Vaapi, disabling VRR in KDE, setting monitor to 60Hz in KDE, enabling vsync in game but there's always this slight hitching every 2 second or so.

Fwiw, Apollo (the sunshine fork) does not have this issue.

I assumed from your comment it was an issue exclusively with portal capture. If you're seeing the issue across different capture methods and encoders, this PR is not relevant to you.

@ReenigneArcher
Copy link
Member

apollo also does not have xdg portal capture

@shelterx

This comment was marked as off-topic.

@codecov
Copy link

codecov bot commented Mar 11, 2026

Bundle Report

Bundle size has no change ✅

@codecov
Copy link

codecov bot commented Mar 11, 2026

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@b3f0e23). Learn more about missing BASE report.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/platform/common.h 0.00% 2 Missing ⚠️
src/platform/linux/portalgrab.cpp 0.00% 2 Missing ⚠️
src/video.cpp 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master    #4839   +/-   ##
=========================================
  Coverage          ?   17.52%           
=========================================
  Files             ?      106           
  Lines             ?    21847           
  Branches          ?     9788           
=========================================
  Hits              ?     3828           
  Misses            ?    16641           
  Partials          ?     1378           
Flag Coverage Δ
Archlinux 12.02% <0.00%> (?)
FreeBSD-14.3-amd64 13.84% <0.00%> (?)
Homebrew-ubuntu-22.04 14.33% <0.00%> (?)
Linux-AppImage 12.52% <0.00%> (?)
Windows-AMD64 14.48% <0.00%> (?)
Windows-ARM64 13.10% <0.00%> (?)
macOS-arm64 18.21% <0.00%> (?)
macOS-x86_64 16.69% <0.00%> (?)

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

Files with missing lines Coverage Δ
src/platform/common.h 32.40% <0.00%> (ø)
src/platform/linux/portalgrab.cpp 3.61% <0.00%> (ø)
src/video.cpp 33.20% <0.00%> (ø)

@andygrundman
Copy link
Contributor

Let's not use the term "fake" for these frames, they are duplicate frames. I think we both want the same thing, which is to change the default, but I haven't tested such a low value to see how clients react to it. Do you have problems if you use 20, or even 10? I wonder if your 90fps issue might be something else. I have never seen Sunshine send more than the requested fps on Windows.

@psyke83
Copy link
Contributor Author

psyke83 commented Mar 12, 2026

Let's not use the term "fake" for these frames, they are duplicate frames. I think we both want the same thing, which is to change the default, but I haven't tested such a low value to see how clients react to it. Do you have problems if you use 20, or even 10? I wonder if your 90fps issue might be something else. I have never seen Sunshine send more than the requested fps on Windows.

I'll change the naming to duplicate frames on the next refresh. The 90fps (and more frequent random bursts of ~63fps) is definitely related to this setting, as I no longer have any issues when it's set to 1. I spent a lot of time trying to troubleshoot the issue via changes to portalgrab's code (in vain) before I noticed the generic duplicate insertion logic, and changing this setting immediately fixed the issue. The smoking gun that it's responsible is that the issues only started after snapshot was changed to a blocking call, as the original portalgrab was happily grabbing duplicate buffers and sending frames (some of which were recaptured duplicates) to the encoder at predictable intervals just like kmsgrab, etc.

This PR is not aimed at changing the default value for any other capture method besides portalgrab; I never really noticed any issue with kmsgrab or Windows related to this, so I can't judge as to whether changing the overall default to something else is desirable.

I do believe that for portalgrab, the setting should be as low as possible (effectively disabled), as the timing of snapshot is dictated by on_process callbacks whenever pipewire sends buffers. This behaviour varies by compositor, and can't be compared to a traditional snapshot function that's capture driven with a predictable capture timing for each frame (like kmsgrab or dxgi), as the buffers can arrive in bursts or at irregular intervals which causes extra duplicate frames to be randomly inserted and violate the cap.

If I understand correctly, the original purpose of duplicate frame insertion was to ameliorate input latency or visual lag caused by encoder framerate ramping in Windows, but with this PR (and a RX 6600, testing against libx264, VAAPI and Vulkan), the desktop is always fluid when ramping from idle and I see no input lag when typing. The PR only changes the default and user config is still respected, if needed.

Portal capture is event driven/blocking, so duplicate frames can compete
with pacing due to irregular buffer arrival.

Resolve by ensuring that event-driven capture methods default to 0.5fps/
2000ms instead of half the host framerate. This will not impact other
capture methods, but if future event-driven capture methods are added, they
can also utilise the is_event_driven() override.
@psyke83 psyke83 changed the title fix(linux/xdgportal): avoid fake frame insertion fix(linux/xdgportal): avoid duplicate frame insertion Mar 12, 2026
@sonarqubecloud
Copy link

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants