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

ffmpeg: disable xev{e,d} when under 7.1 #353198

Merged
merged 1 commit into from
Nov 3, 2024
Merged

Conversation

Atemu
Copy link
Member

@Atemu Atemu commented Nov 2, 2024

The update to xev caused an incompatibility with ffmpeg 7.0 which broke handbrake's ffmpeg.

Fixes #353072

Closes #353168

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.

The update to xev caused an incompatibility with ffmpeg 7.0 which broke
handbrake's ffmpeg.

Fixes NixOS#353072
@Atemu Atemu mentioned this pull request Nov 2, 2024
@jopejoe1 jopejoe1 mentioned this pull request Nov 2, 2024
13 tasks
@emilazy
Copy link
Member

emilazy commented Nov 2, 2024

Does Handbrake actually want -full? It seems dubious, and it’ll likely cause more annoying drift like this in future.

@itepastra
Copy link
Member

Does Handbrake actually want -full? It seems dubious, and it’ll likely cause more annoying drift like this in future.

Handbrake does compile and run without -full, but I don't know it well enough to know if any features are missing without it

@emilazy
Copy link
Member

emilazy commented Nov 2, 2024

We’ve been discouraging people from using it: #271863. I think some subset of CUDA stuff is the main thing some people want that isn’t in our main FFmpeg. I would be surprised if Handbrake relies on -full functionality, and if it does it’s perhaps something we should consider moving into the main variant.

@itepastra
Copy link
Member

itepastra commented Nov 2, 2024

I can quickly make a pr to move handbrake to without -full if you think that's a good plan @emilazy

@emilazy
Copy link
Member

emilazy commented Nov 2, 2024

It was added in #52453 (comment), “more codecs/containers for input and more processing possibilities, which seems sane for the rich video transcoder” – which I think would generally go against our modern advice.

@Atemu Does NVIDIA / AMD hardware‐accelerated encoding work with the stock FFmpeg? I think that’s the main thing we shouldn’t break, since I expect a lot of people will want fast encoding when using Handbrake. CUDA filters or whatever seem like something we can do without here.

@Atemu
Copy link
Member Author

Atemu commented Nov 2, 2024

VAAPI encode works but IDK about Nvidia. Jellyfin requires more features such as OpenCL for full hardware transcoding though. I've been meaning to figure that out and make our default ffmpeg have those features by default so that everyone can just use that.

@Atemu
Copy link
Member Author

Atemu commented Nov 3, 2024

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 353198


x86_64-linux

✅ 10 packages built:
  • handbrake
  • immich
  • jellyfin
  • jellyfin-ffmpeg
  • jellyfin-ffmpeg.bin
  • jellyfin-ffmpeg.data
  • jellyfin-ffmpeg.dev
  • jellyfin-ffmpeg.doc
  • jellyfin-ffmpeg.lib
  • jellyfin-ffmpeg.man

@Atemu Atemu merged commit ea1799e into NixOS:master Nov 3, 2024
25 of 26 checks passed
@Atemu Atemu deleted the ffmpeg-no-xev-in-7 branch November 3, 2024 00:05
@Atemu
Copy link
Member Author

Atemu commented Nov 3, 2024

Let's unbreak master for now, we can discuss removing the dependency on ffmpeg-full in #271863.

Thunderbottom added a commit to Thunderbottom/flakes that referenced this pull request Nov 3, 2024
ffmpeg_7-full is required by jellyfin-ffmpeg and is currently broken.

ref: NixOS/nixpkgs#353198

Signed-off-by: Chinmay D. Pai <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build failure: handbrake
3 participants