feat(linux): pace capture at exact fractional NTSC framerates#5282
Conversation
|
Would be possible to implement it to the new pipewire based capture (xdg portal, kwin grab) too? Or is it enough there that the compositor sets the max fps? |
Double-checked: both PipeWire paths (xdg-portal Compositor pacing isn't involved there: the format is deliberately negotiated as |
ReenigneArcher
left a comment
ReenigneArcher
left a comment
There was a problem hiding this comment.
Let's make a reusable function instead of copying the same code to so many different places.
ReenigneArcher
left a comment
ReenigneArcher
left a comment
There was a problem hiding this comment.
Perhaps a few other areas to update?
|
Thanks, I am glad someone besides me cares about NTSC. :) |
ReenigneArcher
force-pushed
the
feat/linux-fractional-pacing
branch
from
99f06f4 to
dd923f4
Compare
Bundle ReportBundle size has no change ✅ |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #5282 +/- ##
==========================================
- Coverage 27.51% 25.53% -1.99%
==========================================
Files 113 112 -1
Lines 25593 25202 -391
Branches 11237 10922 -315
==========================================
- Hits 7043 6435 -608
- Misses 15560 16043 +483
+ Partials 2990 2724 -266
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 71 files with indirect coverage changes Continue to review full report in Codecov by Harness.
|
|
@ReenigneArcher, should I add anything to get this PR merged ? |
djadjka
force-pushed
the
feat/linux-fractional-pacing
branch
from
fe85434 to
00acec7
Compare
ReenigneArcher
left a comment
ReenigneArcher
left a comment
There was a problem hiding this comment.
New requirements since you created the PR: need more complete code documentation. https://app.readthedocs.org/projects/sunshinestream/builds/33342103/ (search for error:)
@andygrundman could you review since you're the only contributor I know using the NTSC framerates?
|
This looks pretty good to me! |
When a client requests a fractional refresh rate via x-nv-video[0].clientRefreshRateX100 (e.g. 11988 for an Xbox whose display pipeline runs at 120/1.001 Hz), the encoder time base is already set to the exact rational since LizardByte#4019, but the Linux capture loops still paced at the integer maxFPS. The resulting 0.1% delivery surplus accumulates one extra frame every ~8 s, which the client pacer keeps correcting (periodic frame queue oscillation / micro-judder). Apply the same exact-rational handling to the kmsgrab, wlgrab, x11grab and CUDA capture pacing, mirroring the Windows implementation from LizardByte#4019. Tested on an AMD (RDNA4) + KDE Wayland host with KMS capture against a moonlight-xbox client at 4K 119.88: the standing frame-queue oscillation on the client disappears.
Extract video::capture_frame_interval() and use it in all Linux capture pacers, including the PipeWire path which carried its own copy of the same math.
Address review feedback: extract video::framerate_to_rational() as the
single place implementing the "exact rational when framerateX100 is set,
integer framerate otherwise" pattern, and rebuild capture_frame_interval()
on top of it. Migrate the call sites that re-implemented the pattern:
avcodec and NVENC encoder setup become branchless, the PipeWire and
wlroots capture pacers no longer re-derive the rational for logging, and
the Windows strict frame rate reuses the helper inside its sentinel
branch (the {0,0} sentinel must stay to keep the refresh-rate matching
heuristic). Add unit tests for the new helper and the capture frame
interval, including the integer fallback.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Doxygen runs with WARN_NO_PARAMDOC + WARN_AS_ERROR in the Read the Docs build, which failed because framerate_to_rational and capture_frame_interval carried a @brief-only block without documenting their config parameter or return value. Add @param/@return to match framerateX100_to_rational. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ReenigneArcher
force-pushed
the
feat/linux-fractional-pacing
branch
from
7919ddd to
eabb2f2
Compare
|
4 participants
Description
#4019 added support for the
x-nv-video[0].clientRefreshRateX100RTSP parameter, butimplemented the capture-side handling only for the Windows backend
(
platform/windows/display_base.cpp); on Linux only the encoder time base got the exactrational. The Linux capture loops still pace at the integer
maxFPS, so for a client thatrequests e.g. 11988 (Xbox, whose display pipeline runs at 120/1.001 Hz) the host delivers
120.00 fps while the client drains 119.88 fps. The 0.1% surplus accumulates one extra frame
every ~8 s, which the client-side pacer keeps correcting — visible as a periodic frame-queue
oscillation / micro-judder that no client setting can remove.
This PR mirrors the Windows handling in the four Linux capture pacers (kmsgrab, wlgrab,
x11grab, CUDA): when
framerateX100is present, the frame interval is derived from theexact rational returned by
video::framerateX100_to_rational().Tested on an AMD RDNA4 + KDE Wayland host (KMS capture, Vulkan encoder) against a
moonlight-xbox client at 4K 119.88 Hz: the standing frame-queue oscillation on
the client disappears (queue settles flat instead of breathing by one frame every ~8 s).
Issues Fixed or Closed
Completes the Linux side of #4019.
Type of Change
Checklist
AI Usage