feat(linux/xdgportal): use accurate frame timestamps from PipeWire metadata#4780
feat(linux/xdgportal): use accurate frame timestamps from PipeWire metadata#4780psyke83 wants to merge 3 commits intoLizardByte:masterfrom
Conversation
|
Pinging @andygrundman in case you want to try it out. This PR temporarily includes the same commits in #4768 as a prerequisite. When in-game, the real timestamps are being processed consecutively without any noticeable breaks, and generally they look a bit more inflated than the current (less accurate) steady_clock::now() sampling. Unless I made a mistake, however, these larger values should be more reflective of the actual compositor overhead rather than simply sampling the time when the buffer is received on Sunshine's side. When the desktop is idle, it falls back to the inaccurate sampling due to the "pw now - pts" delta being too large, but reverts back to the hardware timestamps when fresh data is sampled that meets the age check thresholds. Unfortunately, this current logic doesn't seem to work well on mutter, as it reports very low min/avg/max host latency values (sub 0.3ms, for ex), so it either needs work, or perhaps pw timestamps are unworkable due to compositor variance in pts calculations. |
Summary of changes:
* Synchronized capture:
- Move frame_cv wait to beginning of loop as a more efficient way to
wait for a new buffer whilst ensuring that unintentional buffer
reuse doesn't occur.
* Improved duplicate detection:
- Transition to duplicate detection logic using a combination
of SPA_META_Header (PTS) and SPA_META_VideoDamage metadata.
- Drop frames when PTS delta is zero and no damage is reported;
this helps to smoothen out stuttering unique to KWin.
* Destroying the core and context during a reinit can cause a deadlock which can be reproduced when PW_STREAM_FLAG_RT_PROCESS is set. Fix by ensuring that full teardown is only done in pipewire destructor. * Add a delay before sending the interrupt signal, as the GNOME XDG portal needs time to detect changes, otherwise Sunshine will crash when the users manually stops the stream on mutter.
…tadata On new frame: * Sample Sunshine's steady_clock and PipeWire clock. * Measure frame age using difference between PW clock "now" and pts. * Sanity check age (limit of 100ms or 5 frames in past, half a frame delay in future). This is needed to filter large time differences in the last received pts when the desktop goes idle, clock is reset, etc. * If age check passes, use hardware timestamp, otherwise use steady clock.
psyke83
force-pushed
the
xdg_pw-timestamps
branch
from
1978bcc to
e2781dc
Compare
|
| last_seq = img_egl->seq.value(); | ||
| last_pts = img_egl->pts.value(); | ||
|
|
||
| // Snapshot both clock domains simultaneously |
There was a problem hiding this comment.
What is all this time code doing? If the frame is a duplicate, frame_timestamp should be left null. It gets filled in by code around stream.cpp:1471.
There was a problem hiding this comment.
update_frame_metadata() is only called on frames that pass a duplicate and snapshot timeout check. Previously, portalgrab's snapshot timeout was a no-op, but reactive capture (and the updates in this PR) make it actually block for new data, so it was only timing out for extreme cases where no data was being received.
The timeout is 1000ms, so only frames that are extremely late will hit the timeout.
| img_egl->frame_timestamp = steady_now; | ||
|
|
||
| if (age_ns < future_tolerance_ns) { | ||
| BOOST_LOG(debug) << "Frame age was future dated: " << (age_ns / 1000) << "us"; |
There was a problem hiding this comment.
I know this code will be removed but just FYI, use verbose log level for anything that logs very frequently or in a loop, etc.
There was a problem hiding this comment.
Good advice, but I suspect any logging level might be too chatty to be included in a final PR state. Right now, the change is not appropriate as there is just way too much compositor variance to be confident that this is working as expected, so either it needs some changes to the logic, or might just be better to drop it entirely. More eyes on the code might help figure out if the former is possible.
In the case of KWin (6.6.1), stale PTS data explodes the host latency when returning from idle - that necessitates the positive (past) age check. In the case of mutter, I don't think that frames returning from idle have stale pts timestamps, but it has the opposite issue where timestamps end up consistently future dated sometimes by as high as -2ms for an active stream running at 60fps.
There was a problem hiding this comment.
OK, if we can't trust pipewire it's probably fine to just use now().
3 participants
Description
feat(linux/xdgportal): use accurate frame timestamps from PipeWire metadata
On new frame:
This is needed to filter large time differences in the last received pts
when the desktop goes idle, clock is reset, etc.
(Note: this PR temporarily includes the event driven capture changes of #4768 as the are a prerequisite).
Screenshot
Issues Fixed or Closed
Roadmap Issues
Type of Change
Checklist
AI Usage