fix(linux): Split service unit into privileged and unprivileged variants

[psyke83]

Description

Split service unit into privileged and unprivileged variants needed for XDG Portal. Technically a breaking change because the default "sunshine" service will now run unprivileged, so user action is needed to restore KMS capture functionality (switch to sunshine-kms service).

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]

Note that I'm not sure how this impacts flatpak functionality. I assume that CAP_SYS_ADMIN cannot be set anyway, so splitting the service makes no sense, but I changed the code (and AppImage too) just to keep it consistent with the traditional packaging formats.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[codecov]

Bundle Report

Bundle size has no change ✅

[psyke83]

It's no problem if you want to do this after merging the branch to master. This will transparently change the default capture method for Wayland users from KMS to XDG, and performance will be poor if/until frame pacing gets improved, so users might not be happy until they realise the service switch is necessary.

Apart from that, perhaps it's worth discussing if we want to reverse the logic and install a sunshine-xdg service instead -- but that's slightly confusing, as the unprivileged service is likely to fix the X11 capture backend as well. X11 users may also not realize that KMS capture is normally used even for their X11 sessions (and probably performs better than the real X11 capture backend).

[ReenigneArcher]

I think it makes more sense to merge this into the XDG pr branch, before merging it into master.

If you think it's better I can leave the PR open for a while longer and try to improve the frame pacing before it's merged

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 15.12%. Comparing base (850f2a6) to head (3e6a8d8).
⚠️ Report is 1 commits behind head on feat/linux/add-xdg-portal-grab.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@                        Coverage Diff                         @@
##           feat/linux/add-xdg-portal-grab    #4619      +/-   ##
==================================================================
+ Coverage                           15.10%   15.12%   +0.02%     
==================================================================
  Files                                  95       95              
  Lines                               19708    19708              
  Branches                             9067     9067              
==================================================================
+ Hits                                 2977     2981       +4     
- Misses                              14892    15422     +530     
+ Partials                             1839     1305     -534     

Flag	Coverage Δ
Archlinux	11.10% <ø> (ø)
FreeBSD-14.3-amd64	13.23% <ø> (-0.01%)	⬇️
Homebrew-ubuntu-22.04	13.34% <ø> (ø)
Linux-AppImage	11.50% <ø> (ø)
Windows-AMD64	13.44% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 41 files with indirect coverage changes

[psyke83]

Significant changes may be necessary to improve pacing. To sketch it out:

1. Existing "next_frame" pacing needs to be synced with the changes to x11/kms code.
2. The current framerate code doesn't work at least on KWin, as it's trying to capture at 180fps/uncapped. Looking at KWin's screengrabber code, this change is likely necessary, as the maxFramerate is used by KWin to manage pacing:

diff --git a/src/platform/linux/portalgrab.cpp b/src/platform/linux/portalgrab.cpp
index 5cb1270b..4bf394a4 100644
--- a/src/platform/linux/portalgrab.cpp
+++ b/src/platform/linux/portalgrab.cpp
@@ -776,16 +776,17 @@ namespace portal {
       sizes[1] = SPA_RECTANGLE(1, 1);
       sizes[2] = SPA_RECTANGLE(8192, 4096);
 
-      framerates[0] = SPA_FRACTION(refresh_rate, 1);  // Preferred
-      framerates[1] = SPA_FRACTION(0, 1);
-      framerates[2] = SPA_FRACTION(1000, 1);
+      framerates[0] = SPA_FRACTION(refresh_rate, 1);  // maxFramerate
+      framerates[1] = SPA_FRACTION(0, 1); // framerate
 
       spa_pod_builder_push_object(b, &object_frame, SPA_TYPE_OBJECT_Format, SPA_PARAM_EnumFormat);
       spa_pod_builder_add(b, SPA_FORMAT_mediaType, SPA_POD_Id(SPA_MEDIA_TYPE_video), 0);
       spa_pod_builder_add(b, SPA_FORMAT_mediaSubtype, SPA_POD_Id(SPA_MEDIA_SUBTYPE_raw), 0);
       spa_pod_builder_add(b, SPA_FORMAT_VIDEO_format, SPA_POD_Id(format), 0);
       spa_pod_builder_add(b, SPA_FORMAT_VIDEO_size, SPA_POD_CHOICE_RANGE_Rectangle(&sizes[0], &sizes[1], &sizes[2]), 0);
-      spa_pod_builder_add(b, SPA_FORMAT_VIDEO_framerate, SPA_POD_CHOICE_RANGE_Fraction(&framerates[0], &framerates[1], &framerates[2]), 0);
+      spa_pod_builder_add(b, SPA_FORMAT_VIDEO_framerate, SPA_POD_Fraction(&framerates[1]), 0);
+      spa_pod_builder_add(b, SPA_FORMAT_VIDEO_maxFramerate, SPA_POD_Fraction(&framerates[0]), 0);
+
 
       if (n_modifiers) {
         spa_pod_builder_prop(b, SPA_FORMAT_VIDEO_modifier, SPA_POD_PROP_FLAG_MANDATORY | SPA_POD_PROP_FLAG_DONT_FIXATE);

This helps, but still doesn't improve things substantially, as KWin's fps monitor still shows a 180fps render rate on a 60Hz screen.

3. We need to hook up host processing latency. This is non-trivial as we need to enable metadata reporting at minimum via:

       // Ack the buffer type
       std::array<uint8_t, SPA_POD_BUFFER_SIZE> buffer;
-      std::array<const struct spa_pod *, 1> params;
+      std::array<const struct spa_pod *, 2> params;
       int n_params = 0;
       struct spa_pod_builder pod_builder = SPA_POD_BUILDER_INIT(buffer.data(), buffer.size());
       params[n_params++] = (const struct spa_pod *) spa_pod_builder_add_object(&pod_builder, SPA_TYPE_OBJECT_ParamBuffers, SPA_PARAM_Buffers, SPA_PARAM_BUFFERS_dataType, SPA_POD_Int(buffer_types));
+      params[n_params++] = (const struct spa_pod *) spa_pod_builder_add_object(&pod_builder, SPA_TYPE_OBJECT_ParamMeta, SPA_PARAM_Meta, SPA_PARAM_META_type, SPA_POD_Id(SPA_META_Header), SPA_PARAM_META_size, SPA_POD_Int(sizeof(struct spa_meta_header)));
+
       pw_stream_update_params(d->stream, params.data(), n_params);
     }

Then we need to extract the pts timestamps from the pipewire metadata and convert to a monotonic time point that can be fed to img->frame_timestamp to be consumed by Moonlight for its pacing code.

[psyke83]

The squash merge seems to have folded some of your previous sunshine-kms changes into my commit that I reverted in separate commits? It probably would have been better to rebase by removing your two commits and my two reverts? But if the branch will ultimately be squashed into one commit whenever it's merged to master, I suppose it doesn't matter. Sorry if OT, I'm a bit rusty on how github resolves merges.

[ReenigneArcher]

I probably should have removed those commits in the other branch before merging.

I will squash when it goes to master, so it will just be one commit (co-authored by everyone who contributed to the PR branch).