Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/jellyfin: fix hardware acceleration #108224

Closed
wants to merge 1 commit into from

Conversation

minijackson
Copy link
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.

@aanderse
Copy link
Member

aanderse commented Jan 3, 2021

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
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
Copy link
Member

aanderse commented Jan 3, 2021

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
Copy link

stale bot commented Jul 4, 2021

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
Copy link
Member

aanderse commented Jul 4, 2021

@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
Copy link
Contributor

kevincox commented Jul 7, 2021

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
Contributor

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

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
Copy link
Member

aanderse commented Dec 6, 2021

@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
Copy link
Contributor

D4Delta commented Feb 3, 2022

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
Contributor

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

@minijackson
Copy link
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants