Skip to content

nixos/jellyfin: fix hardware acceleration - #108224

Closed
minijackson wants to merge 1 commit into
NixOS:masterfrom
minijackson:jellyfin-fix-hwaccel
Closed

nixos/jellyfin: fix hardware acceleration#108224
minijackson wants to merge 1 commit into
NixOS:masterfrom
minijackson:jellyfin-fix-hwaccel

Conversation

@minijackson

Copy link
Copy Markdown
Member

Issue mentioned in #98176

Motivation for this change

Hardware acceleration on jellyfin broke due to added systemd security options in #98176 (sorry about that...). This hopefully fixes it. I did some rudimentary tests on my server, but my video card doesn't seem to support h264 encoding via VAAPI. Playing h264 videos with hardware decoding enabled seems to work fine, though.

@xwvvvvwx Can confirm that this adds hardware encoding / decoding back for you?

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg Bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Jan 2, 2021
@aanderse

aanderse commented Jan 3, 2021

Copy link
Copy Markdown
Member

I propose we stop providing our own systemd unit and start consuming the upstream unit instead. When someone wants to make changes to the unit they can submit a PR upstream and have the jellyfin developers review it. In my opinion NixOS shouldn't be making these types of decisions (excessive systemd hardening when we don't know how software will react to this) - we're just a distro.

@minijackson

Copy link
Copy Markdown
Member Author

Hello! In retrospect, I really should have tried to upstream these options first, instead of trusting my own very limited testing 😕

Is there an RFC that specifies which behavior maintainers should adopt when making a systemd service? (diverging from upstream, security considerations, NixOS-specific adjustments, ...) If not, I really think it would be useful to have, I believe there is a lot of things to be said from both sides of the argument, and we definitely need a coherent distribution behavior from all NixOS services.

@aanderse

aanderse commented Jan 3, 2021

Copy link
Copy Markdown
Member

Is there an RFC that specifies which behavior maintainers should adopt when making a systemd service? (diverging from upstream, security considerations, NixOS-specific adjustments, ...)

With the way NixOS is structured I think it will be difficult to find consensus on that 😄

My opinion is that systemd hardening upstream is the most ideal route, so we should put our effort into upstream PRs to make these changes. I concede that there are many projects with zero interest in systemd... so it is a balancing act of when to give up on upstream accepting changes and doing it ourselves in NixOS.

I've recently become a fan of the systemd.packages option in NixOS, which is a great choice when upstream is receptive.

Are you interested in creating a PR upstream to add your changes, then seeing if we can get the upstream unit to work on NixOS? Even if it doesn't work out it will still be a set of upstream eyes reviewing and commenting on your changes.

@GrahamcOfBorg test jellyfin

@stale

stale Bot commented Jul 4, 2021

Copy link
Copy Markdown

I marked this as stale due to inactivity. → More info

@stale stale Bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2021
@aanderse

aanderse commented Jul 4, 2021

Copy link
Copy Markdown
Member

@minijackson is this still an issue?

@stale stale Bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2021
@kevincox

kevincox commented Jul 7, 2021

Copy link
Copy Markdown
Contributor

I think when the upstream doesn't provide any hardening it is appropriate to add it manually. I agree that we should reach upstream as well.

@justinas

Copy link
Copy Markdown
Member

I can confirm that the equivalent of this works as a workaround (Intel QSV via VAAPI):

{
  systemd.services.jellyfin.serviceConfig = {
    DeviceAllow = lib.mkForce [ "char-drm rw" ];
    PrivateDevices = lib.mkForce false;
  };
}

I did not seem to require the video group, as on my machine the device seems to be world-writable. Not sure what this depends on.

$ ls -alh /dev/dri/renderD128
crw-rw-rw- 1 root render 226, 128 Oct 18 11:07 /dev/dri/renderD128

@matthiasdv

Copy link
Copy Markdown

I've recently become a fan of the systemd.packages option in NixOS, which is a great choice when upstream is receptive.

@aanderse Would you care to expand a little on how configuring using the upstream unit would work? I looked at those PRs you linked and played around with it for a bit but I have to admit I couldn't figure it out.

@aanderse

aanderse commented Dec 6, 2021

Copy link
Copy Markdown
Member

@matthiasdv sure! I continue to encourage the use of systemd units where it makes sense to. Thanks for creating that upstream issue. Bonus points awarded 🎉

I'll assume you already looked at our manual which doesn't do a great job explaining things.

How familiar are you with systemd units and overrides? On most systemd based distros you can include override files which end up being merged together with the distro provided unit. Read up on systemctl edit for any regular distro if you have never run across it. The documentation is good and it's a pretty simple concept for a normal distro.

When you include an upstream unit via systemd.packages you add this exact unit file as the "distro provided" unit. Any additional configuration added to systemd.services.jellyfin is not merged at the nix level, but instead added as a separate override file... like you would do on a non NixOS distro. So you take the base upstream unit and then any changes you make you can do in the NixOS module as overrides, considering any special override rules that systemd has. You can see this very clearly when you run systemctl cat my.service - this displays the upstream unit and your changes separately.

There are a few things to keep in mind. Some NixOS specific, others are just general systemd things to know. The biggest gotcha is to remember to add systemd.services.foo.wantedBy in your NixOS module because of #81138.

Did you have any specific questions, or specific examples that you need an explanation on?

@D4Delta

D4Delta commented Feb 3, 2022

Copy link
Copy Markdown
Contributor

This doesn't fix NVENC/NVDEC hardware acceleration
To fix it you need
DeviceAllow = [ "char-drm rw" "char-nvidia-frontend rw" "char-nvidia-uvm rw" ];
and
RestrictAddressFamilies = [ "AF_UNIX" "AF_NETLINK" "AF_INET" "AF_INET6" ];

@justinas

Copy link
Copy Markdown
Member

Did #163491 and #163602 solve this? These changes do not seem to include adding the video group.

@minijackson

Copy link
Copy Markdown
Member Author

#175439 should have superseeded this PR, so I'm closing this one

@minijackson
minijackson deleted the jellyfin-fix-hwaccel branch July 23, 2022 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants