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/gpu-screen-recorder,gpu-screen-recorder{-,gtk}}: update to 4.1.11, remove cap_sys_nice #339874

Merged
merged 12 commits into from
Oct 14, 2024

Conversation

DontEatOreo
Copy link
Member

@DontEatOreo DontEatOreo commented Sep 5, 2024

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions 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/` labels Sep 5, 2024
@DontEatOreo DontEatOreo changed the title {nixos/gpu-screen-recorder,gpu-screen-recorder-{,gtk}}: update to {09-03,09-02}, add systemd serviceand nvidia fix to module {nixos/gpu-screen-recorder,gpu-screen-recorder-{,gtk}}: update to {09-03,09-02}, add systemd service and nvidia fix to module Sep 5, 2024
@DontEatOreo
Copy link
Member Author

Result of nixpkgs-review pr 339874 --extra-nixpkgs-config '{ allowAliases = false; allowBroken = false; allowUnfree = true; checkMeta = true; cudaSupport = false; cudaCapabilities = [ ]; }' run on x86_64-linux 1

1 package blacklisted:
  • nixos-install-tools
3 packages built:
  • disko
  • gpu-screen-recorder
  • gpu-screen-recorder-gtk

@DontEatOreo DontEatOreo force-pushed the update-gpu-screen-recorder branch from eef7b0c to eb7d78c Compare September 5, 2024 19:34
@ofborg ofborg bot requested a review from babbaj September 5, 2024 20:22
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Sep 5, 2024
@DontEatOreo DontEatOreo force-pushed the update-gpu-screen-recorder branch from eb7d78c to 5b1d68c Compare September 5, 2024 22:24
@sarcasticadmin sarcasticadmin mentioned this pull request Sep 7, 2024
13 tasks
@DontEatOreo DontEatOreo changed the title {nixos/gpu-screen-recorder,gpu-screen-recorder-{,gtk}}: update to {09-03,09-02}, add systemd service and nvidia fix to module {nixos/gpu-screen-recorder,gpu-screen-recorder{-,gtk}}: update to {09-03,09-02}, add systemd service and nvidia fix to module Sep 8, 2024
@DontEatOreo DontEatOreo force-pushed the update-gpu-screen-recorder branch from 5b1d68c to 94c83f9 Compare September 9, 2024 12:53
@DontEatOreo DontEatOreo force-pushed the update-gpu-screen-recorder branch from 94c83f9 to 22c443c Compare September 11, 2024 14:42
@keenanweaver
Copy link
Member

Should the packages use the unstable updater? I believe @dec05eba wants users to use the latest commit to master.

@DontEatOreo
Copy link
Member Author

Thanks for the additional reviews! Tomorrow, I will take a look and try to implement the suggestions

@dec05eba
Copy link
Contributor

dec05eba commented Oct 6, 2024

Should the packages use the unstable updater? I believe @dec05eba wants users to use the latest commit to master.

These days gpu screen recorder has proper versions and I now also created a tag for the latest release and will continue doing so for new releases. The tag is now visible here: https://git.dec05eba.com/gpu-screen-recorder/ (version 4.1.11). I recommend using the latest release instead of master now. There is also a changelog for versions available here: https://git.dec05eba.com/gpu-screen-recorder-gtk/tree/com.dec05eba.gpu_screen_recorder.appdata.xml#n82

Copy link
Member

@keenanweaver keenanweaver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll also need to close the finalAttrs portion, can't seem to add it to the Github review for whatever reason.

@DontEatOreo
Copy link
Member Author

I’ve decided to drop the module refactor from this PR. I'll need more time to think it through and implement it properly. I don’t want to slow down this PR any further, so I’ll create another PR in the near future for the module

@DontEatOreo DontEatOreo changed the title {nixos/gpu-screen-recorder,gpu-screen-recorder{-,gtk}}: update to {09-03,09-02}, add systemd service and nvidia fix to module {nixos/gpu-screen-recorder,gpu-screen-recorder{-,gtk}}: update to 4.1.11, remove cap_sys_nice Oct 6, 2024
@DontEatOreo DontEatOreo force-pushed the update-gpu-screen-recorder branch 2 times, most recently from eaae587 to 7b69d08 Compare October 6, 2024 12:30
@DontEatOreo
Copy link
Member Author

Result of nixpkgs-review pr 339874 --extra-nixpkgs-config '{ allowAliases = false; allowBroken = false; allowUnfree = true; checkMeta = true; cudaSupport = true; cudaCapabilities = [ "7.5" ]; }' run on x86_64-linux 1

1 package blacklisted:
  • nixos-install-tools
2 packages built:
  • gpu-screen-recorder
  • gpu-screen-recorder-gtk

@keenanweaver
Copy link
Member

Result of nixpkgs-review pr 339874 run on x86_64-linux 1

1 package blacklisted:
  • nixos-install-tools
2 packages built:
  • gpu-screen-recorder
  • gpu-screen-recorder-gtk

''
gappsWrapperArgs+=(--prefix PATH : ${wrapperDir})
gappsWrapperArgs+=(--suffix PATH : ${lib.makeBinPath [ gpu-screen-recorder-wrapped ]})
gappsWrapperArgs+=(--prefix LD_LIBRARY_PATH : ${
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot use patchelf?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not exactly sure how that would be implemented..

};
in
''
gappsWrapperArgs+=(--prefix PATH : ${wrapperDir})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can amend a list to the var

@keenanweaver
Copy link
Member

Trying to run it, I get this:

┌───────────────────>
│keenan in results/gpu-screen-recorder/bin🔒
❄️ bsh ›
└─>./gpu-screen-recorder -w screen -f 60 -a default_output -o "$HOME/Videos/video.mp4"
amdgpu: amdgpu_cs_ctx_create2 failed. (-13)
Info: using h264 encoder because a codec was not specified
gsr info: gsr_kms_client_init: waiting for server to connect
kms server info: connecting to the client
gsr info: gsr_kms_client_init: server connected
gsr info: replacing file-backed unix domain socket with socketpair
kms server info: connected to the client
kms server error: expected gpu screen recorder protocol version to be 2, but it's 4

^Cgsr error: gsr_kms_client_replace_connection: failed to receive response
gsr error: gsr_capture_start failed

Recording file doesn't get written. Don't know if it's because of my current gpu-screen-recorder instance. (I have a systemd service for replay autostart which I stopped for testing this.) But the package builds fine to 4.1.11. The 'current' gpu-screen-recorder records fine.

System info:

          ▗▄▄▄       ▗▄▄▄▄    ▄▄▄▖             keenan@nixos-desktop
          ▜███▙       ▜███▙  ▟███▛             --------------------
           ▜███▙       ▜███▙▟███▛              OS: NixOS 24.11.20241004.bc947f5 (Vicuna) x86_64
            ▜███▙       ▜██████▛               Kernel: Linux 6.11.1
     ▟█████████████████▙ ▜████▛     ▟▙         Uptime: 25 mins
    ▟███████████████████▙ ▜███▙    ▟██▙        Packages: 2614 (nix-system), 158 (flatpak-user)
           ▄▄▄▄▖           ▜███▙  ▟███▛        Shell: zsh 5.9
          ▟███▛             ▜██▛ ▟███▛         Display (XB271HU): 2560x1440 @ 165 Hz in 27″ [External]
         ▟███▛               ▜▛ ▟███▛          Display (LEN Y27q-20): 2560x1440 @ 165 Hz in 27″ [External] *
▟███████████▛                  ▟██████████▙    DE: KDE Plasma
▜██████████▛                  ▟███████████▛    WM: KWin (Wayland)
      ▟███▛ ▟▙               ▟███▛             WM Theme: Klassy
     ▟███▛ ▟██▙             ▟███▛              Theme: Klassy (CatppuccinMochaLavender) [Qt]
    ▟███▛  ▜███▙           ▝▀▀▀▀               Icons: Papirus-Dark [Qt], Papirus-Dark [GTK2]
    ▜██▛    ▜███▙ ▜██████████████████▛         Font: Inter (12pt) [Qt], Inter (12pt) [GTK2/3/4]
     ▜▛     ▟████▙ ▜████████████████▛          Cursor: breeze (24px)
           ▟██████▙       ▜███▙                Terminal: WezTerm 20240203-110809-5046fc22
          ▟███▛▜███▙       ▜███▙               Terminal Font: JetBrainsMono Nerd Font
         ▟███▛  ▜███▙       ▜███▙              CPU: AMD Ryzen 7 5800X (16) @ 4.85 GHz
         ▝▀▀▀    ▀▀▀▀▘       ▀▀▀▘              GPU: AMD Radeon RX 7900 XTX [Discrete]
                                               Memory: 8.14 GiB / 31.27 GiB (26%)
                                               Swap: 3.00 MiB / 7.82 GiB (0%)
                                               Disk (/): 1.23 MiB / 8.00 GiB (0%) - tmpfs
                                               Disk (/home/keenan/Games): 814.53 GiB / 1.82 TiB (44%) - btrfs
                                               Disk (/persist): 289.83 GiB / 1.82 TiB (16%) - btrfs
                                               Local IP (enp10s0): 10.20.1.8/24
                                               Locale: en_US.UTF-8

Can anyone else confirm similar issues?

@dec05eba
Copy link
Contributor

dec05eba commented Oct 6, 2024

Trying to run it, I get this:

You need to install the program rather than run it locally in the directory. gpu screen recorder will launch gsr-kms-server (also installed by gpu screen recorder) from PATH and the version of gsr-kms-server needs to match the version in gpu screen recorder. If you are running a local version of gpu screen recorder but have another version installed on the system then it can fail if there are changes between the versions.

@keenanweaver
Copy link
Member

Trying to run it, I get this:

You need to install the program rather than run it locally in the directory. gpu screen recorder will launch gsr-kms-server (also installed by gpu screen recorder) from PATH and the version of gsr-kms-server needs to match the version in gpu screen recorder. If you are running a local version of gpu screen recorder but have another version installed on the system then it can fail if there are changes between the versions.

Lo and behold, the author is correct! Working great for me in 'normal' and 'replay' modes.

Copy link
Member

@keenanweaver keenanweaver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bumping to latest

@dec05eba
Copy link
Contributor

vulkan-headers is a new dependency that should be added for gpu-screen-recorder (build dependency)

Copy link
Member

@keenanweaver keenanweaver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added vulkan-headers per #339874 (comment) Confirmed building

@DontEatOreo DontEatOreo force-pushed the update-gpu-screen-recorder branch from 0c23c16 to 04a6068 Compare October 14, 2024 12:55
@DontEatOreo
Copy link
Member Author

Result of nixpkgs-review pr 339874 --extra-nixpkgs-config '{ allowAliases = false; allowBroken = false; allowUnfree = true; checkMeta = true; cudaSupport = true; cudaCapabilities = [ "7.5" ]; }' run on x86_64-linux 1

1 package blacklisted:
  • nixos-install-tools
2 packages built:
  • gpu-screen-recorder
  • gpu-screen-recorder-gtk

@JohnRTitor JohnRTitor merged commit 18760e4 into NixOS:master Oct 14, 2024
28 of 29 checks passed
@DontEatOreo DontEatOreo deleted the update-gpu-screen-recorder branch October 14, 2024 21:11
@DontEatOreo
Copy link
Member Author

Superseeds #335546

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: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants