fix(linux/xdgportal): avoid duplicate frame insertion#4839
fix(linux/xdgportal): avoid duplicate frame insertion#4839psyke83 wants to merge 1 commit intoLizardByte:masterfrom
Conversation
|
Without this PR, I need to set Pinging @andygrundman - let me know if you object to this change (as I believe you last touched the fake frame logic). |
7edf090 to
ec11388
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
|
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 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. |
|
apollo also does not have xdg portal capture |
ec34205 to
d59f2a5
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
Bundle ReportBundle size has no change ✅ |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4839 +/- ##
=========================================
Coverage ? 17.52%
=========================================
Files ? 106
Lines ? 21847
Branches ? 9788
=========================================
Hits ? 3828
Misses ? 16641
Partials ? 1378
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
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.
|



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
Checklist
AI Usage