ffmpeg: use drv version for aribcaption configureFlags logic#280784
ffmpeg: use drv version for aribcaption configureFlags logic#280784Atemu merged 1 commit intoNixOS:masterfrom
Conversation
`withAribcaption` would be true because, for the flags in the function argument, ffmpeg_6-full's version is the deciding one. We will likely need a better pattern here. Fixes NixOS#280645
|
cc @anund |
|
@Atemu I think this should also be applied to the patches, otherwise it'll probably not build correctly on darwin. |
|
For now this only affects jellyfin-ffmpeg where the aribcaption flag makes it break and that package is not needed on Darwin. We need a better pattern for the version anyways that does not have this issue to begin with. |
How so it's not needed? It is available on darwin. |
|
jellyfin-ffmpeg is a fork of the regular ffmpeg specifically for use in jellyfin. I don't even know if jellyfin builds on Darwin. I'd also like to see it fixed but, again, I'd prefer to see that happen using a better pattern for version etc. |
It does build for darwin, but I understand. |
|
Regarding patches I see the generic package doing similar things to nixpkgs/pkgs/development/libraries/ffmpeg/generic.nix Lines 351 to 365 in 66afe6c The first won't apply to jellyfin now and the second should only apply in future. Likely need a separate change to just swap all |
| (enableFeature withAlsa "alsa") | ||
| # FIXME: see if jellyfin-ffmpeg is already on a version >= 6.1 to use enableFeature | ||
| (optionalString withAribcaption "--enable-libaribcaption") | ||
| (optionalString (withAribcaption && lib.versionAtLeast finalAttrs.version "6.1") "--enable-libaribcaption") |
There was a problem hiding this comment.
Consider duplicating this change on the later buildInputs reference.
There was a problem hiding this comment.
Good point. I'll do that should my refactor preventing this issue in the first place fail. Let's merge this first to unbreak jellyfin.
|
To be clear: general version changes can be done in a separate PR as building but slightly wrong is better than not building at the moment. |
|
Follow-up and revert: #280904 |
withAribcaptionwould be true because, for the flags in the function argument, ffmpeg_6-full's version is the deciding one. We will likely need a better pattern here.Fixes #280645
Description of changes
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.