fix(pipewire): gate variable rate capture and fix fractional framerates - #5538
Conversation
|
Technically I'm happy with the change as-is, but I'm keeping it in draft as it's a quasi-breaking change until Plasma 6.8 releases in October, or if the related fix for improved pacing gets backported to older branches. Summary of effects:
If anyone is interested in testing the improved pacing, it's trivial to manually backport the single commit to KWin 6.7. |
Not sure if holding this back until 6.8 will help as much because only rolling release distributions will get the fix within a reasonable time frame. Most regular distributions will stick to the KDE (therefore KWin) version that it shipped on release and therefore will lack behind until a new release is available (and upgraded to by the user). One way to handle this would be to detect the running KWin version and change the logic accordingly based on that. IMHO we should avoid such workarounds if possible (keep code simple for future maintenance) but atm it's the only thing I could come up that would solve the issues above for users stuck on older versions of KWin. |
|
We should probably keep in mind that Plasma 6.6 is the currently assigned LTS release, and that version currently has no downsides with variable rate pacing; only 6.7 onwards exhibits the half-rate desktop animation bug. Merging this PR as-is would not be good for distros adhering to the 6.6 LTS release or earlier. Other non-rolling release distros that use Plasma 6.7 (such as Fedora 43 onwards) will also be impacted, as they have good general pacing but very poor desktop responsiveness due to the animation bug caused by variable pacing. Maybe a version check to determine whether to set up variable rate is warranted, and we can justify the effort by also using it to determine if the compositor is not KWin at all, just avoid variable rate entirely and thus avoid mutter having to renegotiate. Let's first see if the pacing improvement commit is cherry-picked to the 6.7 branch at least... but I doubt it. |
|
Here's a simple check for KWin including the currently running version using it's DBus supportInformation() method I've come up using only GLib which is already required for pipewire and also used by portalgrab's dbus stuff (should you use it for this PR a Co-Authored-By on the commit adding it would be nice): /**
* Fetch the currently running KWin version (if applicable from it's DBUS support information method)
*
* @return A vector with 3 elements containing KWin's major.minor.micro version or an empty vector if KWin's version could not be determined
*/
static std::vector<int> get_running_kwin_version() {
auto conn = g_bus_get_sync(G_BUS_TYPE_SESSION, nullptr, nullptr);
std::vector<int> result;
if (conn) {
if (auto reply = g_dbus_connection_call_sync(conn, "org.kde.KWin", "/KWin", "org.kde.KWin", "supportInformation", nullptr, G_VARIANT_TYPE("(s)"), G_DBUS_CALL_FLAGS_NONE, -1, nullptr, nullptr); reply) {
gchar *support_info = nullptr;
g_variant_get(reply, "(s)", &support_info);
auto *regex = g_regex_new("KWin version: ([0-9]+)\\.([0-9]+)\\.([0-9]+)", G_REGEX_DEFAULT, G_REGEX_MATCH_DEFAULT, nullptr);
GMatchInfo *match_info = nullptr;
g_regex_match(regex, support_info, G_REGEX_MATCH_DEFAULT, &match_info);
if (g_match_info_matches(match_info)) {
gchar *major = g_match_info_fetch(match_info, 1);
result.emplace_back(atoi(major));
g_free(major);
gchar *minor = g_match_info_fetch(match_info, 2);
result.emplace_back(atoi(minor));
g_free(minor);
gchar *micro = g_match_info_fetch(match_info, 3);
result.emplace_back(atoi(micro));
g_free(micro);
}
g_match_info_free(match_info);
g_regex_unref(regex);
g_free(support_info);
g_variant_unref(reply);
}
g_clear_object(&conn);
}
return result;
}Example usage: if (auto kwin_version = get_running_kwin_version(); !kwin_version.empty()) {
BOOST_LOG(debug) << "[pipewire] KWin version: "sv << kwin_version[0] << "."sv << kwin_version[1] << "."sv << kwin_version[2];
if (kwin_version[0] == 5 || (kwin_version[0] == 6 && kwin_version[1] < 8)) {
BOOST_LOG(info) << "[pipewire] KWin variable rate capture";
} else {
BOOST_LOG(info) << "[pipewire] KWin compositor-side pacing";
}
} else {
BOOST_LOG(info) << "[pipewire] No KWin support information available"sv;
}Depending on how often this is called it might be a good idea to cache the function result as it shouldn't change during Sunshine's process lifetime. |
90f2b2e to
c3ac121
Compare
|
@Kishi85 appreciated. I've added some extra fixes related to framerate negotiation, but decided to overwrite the original commit to make absolutely sure that you get the co-author credit if/when the branch gets squashed for merge to master. The new behaviour is:
|
psyke83
changed the title
97cec93 to
a760b0c
Compare
psyke83
changed the title
a760b0c to
14418f6
Compare
Just, FYI. When I squash and merge, I keep the co-authors. |
Bundle ReportBundle size has no change ✅ |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5538 +/- ##
==========================================
- Coverage 33.84% 33.78% -0.07%
==========================================
Files 104 104
Lines 25089 25135 +46
Branches 11098 11126 +28
==========================================
Hits 8492 8492
+ Misses 15610 12982 -2628
- Partials 987 3661 +2674
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 57 files with indirect coverage changes Continue to review full report in Codecov by Harness.
|
Thanks. This is technically ready for review, but if the improved pacing is backported to KWin 6.7.x we could adjust the version gating logic before merging. Otherwise, there's no hurry to merge; aside from the NTSC fractional framerate fix that also affects mutter, this will only have major impact when KWin 6.8 is released in October. |
|
@psyke83 Could you change the e-mail on the co-author to |
e2fe3fc to
d1784a9
Compare
|
Done. I squashed the commit while updating so that your real email is not visible on my branch either. Hopefully that's ok with you. |
Targeted fix for half-speed desktop animations that will apply to KWin 6.8+, but also improves compatibility with mutter, and ensures that NTSC fractional framerates are requested properly. Unfortunately, KWin 6.7 will continue exhibiting half-speed desktop animations unless the related fix is backported to this branch. * Isolate variable rate capture to KWin 5.x-6.7.x. Other compositors and KWin 6.8+ will request the actual framerate. Ref: https://bugs.kde.org/show_bug.cgi?id=524129 * Since we now rely on compositor pacing, ensure that the pipewire session negotiates using the fractional rate instead of the coarse framerate value. * Nit: rename refresh_rate to more descriptive target_framerate * Nit: ensure target_framerate is populated during initial format negotiation * Nit: log host-side delay interval when variable rate is negotiated Co-authored-by: Kishi85 <41839133+Kishi85@users.noreply.github.com>
d1784a9 to
9e0362d
Compare
All good! Thanks again. |
|
Screenshot ComparisonPR #5538 screenshots vs Matrix:
|
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
Matrix: Windows-AMD64
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
Matrix: Windows-ARM64
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
Matrix: macOS-arm64
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
Matrix: macOS-x86_64
| Image | Baseline | PR |
|---|---|---|
sunshine_tray_initial.png |
 |
 |
sunshine_tray_menu.png |
 |
 |
sunshine_tray_pairing_request.png |
 |
 |
sunshine_tray_paused.png |
 |
 |
sunshine_tray_stopped.png |
 |
 |
sunshine_tray_streaming.png |
 |
 |
sunshine_tray_tooltip.png |
 |
 |
3 participants
Description
fix(pipewire): gate variable rate capture and fix fractional framerates
Targeted fix for half-speed desktop animations that will apply to KWin 6.8+, but also improves compatibility with
mutter, and ensures that NTSC fractional framerates are requested properly. Unfortunately, KWin 6.7 will
continue exhibiting half-speed desktop animations unless the related fix is backported to this branch.
and KWin 6.8+ will request the actual framerate.
Ref: https://bugs.kde.org/show_bug.cgi?id=524129
session negotiates using the fractional rate instead of the coarse
framerate value.
Co-authored-by: Kishi85 41839133+Kishi85@users.noreply.github.com
Screenshot
Issues Fixed or Closed
Roadmap Issues
Type of Change
Checklist
AI Usage
See our AI usage policy.