ffmpeg_7-full: Enable xev{d,e} on Darwin systems

[toonn]

Description of changes

This enables xevd and xeve again in ffmpeg_7-full on all platforms but is dependent on #328422, hence still a draft.

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:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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.

[toonn]

Not a huge fan of the negative assertion TBH. Though the inverse of isx86 || isDarwin is a bit less clear IMO.

Kinda wish we could use !xevd.meta.broken.

[Atemu]

Kinda wish we could use !xevd.meta.broken.

Good point but why couldn't we?

[jopejoe1]

Could also make use of lib.meta.availableOn for that.

[emilazy]

Result of nixpkgs-review pr 328428 run on aarch64-darwin 1

1 package marked as broken and skipped:

• handbrake

7 packages built:

• ffmpeg_7-full
• ffmpeg_7-full.bin
• ffmpeg_7-full.data
• ffmpeg_7-full.dev
• ffmpeg_7-full.doc
• ffmpeg_7-full.lib
• ffmpeg_7-full.man

Result of nixpkgs-review pr 328428 run on x86_64-darwin 1

1 package marked as broken and skipped:

• handbrake

7 packages built:

• ffmpeg_7-full
• ffmpeg_7-full.bin
• ffmpeg_7-full.data
• ffmpeg_7-full.dev
• ffmpeg_7-full.doc
• ffmpeg_7-full.lib
• ffmpeg_7-full.man

I think it shouldn’t be hard to get this working on aarch64-linux; the builds succeed on aarch64-darwin and the error seems like just some weird static linking compiler flags thing stopping the build from even happening rather than a fundamental problem. That would obviate the need to have any conditional here, though I’m not asking you to spend any more time on these packages than you already have.

Anyway I’m fine with whatever condition is used here, but availableOn is the most elegant solution in my opinion.

[toonn]

@jopejoe1, I loved the idea of availableOn but it doesn't seem to look at meta.broken : /

@Atemu, kinda assumed we either wanted explicit control over what features should not be broken or that it causes more evaluation than we want or something. I'll ask around to see if anyone knows of a problem with doing this.

@emilazy, I'll mark this as ready for review as-is for now. If there's no problem with using meta.broken for the default value we could consider adding it to all (or just some) of the features. I'll try to get access to the community aarch64-linux to fix xev{d,e}, you nerdsniper -_-

[Atemu]

I loved the idea of availableOn but it doesn't seem to look at meta.broken : /

Perhaps we should introduce a worksOn that also takes into account meta.broken.

kinda assumed we either wanted explicit control over what features should not be broken or that it causes more evaluation than we want or something.

It depends. In this case, it was simply not enabled because it was broken which would in turn break ffmpeg. Using meta.broken/availableOn would have sent a clearer message.

[emilazy]

If there's no problem with using meta.broken for the default value we could consider adding it to all (or just some) of the features.

I’d rather just have way fewer feature flags, honestly. We build three variants of increasing size and have additional flags for licensing concerns; the current level of customizability is way over‐the‐top.

[Atemu]

It is a bit over the top but it has proven itself to be quite handy in the past and does not add too much maintenance overhead IME.

[toonn]

If defaulting the features to ? !xev{d,e}.meta.broken is preferred I'm perfectly fine with updating this PR. Just let me know what direction is preferred : )

[emilazy]

I’m genuinely curious as to how such fine‐grained configurability has proven helpful! The level of granularity for every single codec, which libraries to build, and whether to copy some .txt documentation seem far more than I can ever imagine finding use for, and combined with all the logic to try and ensure a sensible configuration they make the otherwise fairly pedestrian derivation much more intimidating to me, but you’ve been maintaining FFmpeg a lot longer than I have.

(Also we should probably just merge this.)

[toonn]

I do understand the idea behind ffmpeg_full TBH. If you build it with everything enabled then at least all the individual components build (and pass tests?), that gives some confidence as to any individual withFeature working.

As for the flags, I personally override libopus, lame, nonFreeLicensing, fdkaacExtlib and fdk_aac on top of ffmpeg_full. Just really convenient to never need to rebuild ffmpeg because there's a new format I want to use it with. If the feature flags go away would there still be a *_full? Would it still be possible to build an ffmpeg with a specific set of features?

[emilazy]

Oh, yeah, I’m not proposing we get rid of -headless or -full, or even the licensing flags (withGPL = false; or withUnfree = true; would get you FDK AAC, for instance). Just, like, almost everything else? Who is setting withSwScaleAlpha = false; and can we really support them better than an .overrideAttrs/patched derivation would? :)

[Atemu]

If defaulting the features to ? !xev{d,e}.meta.broken is preferred I'm perfectly fine with updating this PR. Just let me know what direction is preferred : )

I prefer it :)

I’m genuinely curious as to how such fine‐grained configurability has proven helpful!

It's helpful for precisely the situations such as this. Notice how @toonn didn't have to touch any implementation details whatsoever here but merely adjust the declarative policy? If you look at the implementation, notice that it is super simple and repetitive. We could perhaps even automate away most of the repetition. (Well, I was nerdsniped by this discussion just now and had an idea that I'll now attempt to implement.)

If you look at the flag defaults, there's lots of platform or version specific logic that we can adjust at any point in a manner like this. This is also true for external users.

This fine-grained flags system is what allows us to keep as close to a full feature set as possible without compromising other platforms or versions. Without it, we'd only be able to enable the intersection of flags that work on all platforms and versions.

Who is setting withSwScaleAlpha = false; and can we really support them better than an .overrideAttrs/patched derivation would? :)

I agree that some of the flags could probably be removed. When I refactored the ffmpeg derivations, I opted to simply retain the existing ones and keep the result as close as possible. (I even had the drvs be 1:1 the same modulo configureFlags and buildInputs order.)

I'd like to challenge the second thought of yours however: Go try and remove a feature flag aswell a dependency using .overrideAttrs. You'd have to construct non-trivial filters for either and that's all but idiomatic or convenient.

Meanwhile having these flags present costs us nearly nothing. If we had something like #312432 we would even be able to spin out this slight complexity into a separate code unit using a few rather idiomatic patterns.

.overrideAttrs can be likened to a scalpel. It's a precise cutting tool that requires skill and knowledge to use properly but it is unsuited for rougher work such as chopping vegetables where larger knifes or specialised tools reign supreme. You could cut an apple using a scalpel but wouldn't you rather use one of these?

[toonn]

Pushed the change, rebased on master and altered the commit message. Still seems to build fine on aarch64-darwin.

[emilazy]

Okay, I think clearly I’ll have to write up my position on the feature flags in another issue sometime. For now though let’s get this merged.

Result of nixpkgs-review pr 328428 run on x86_64-darwin 1

1 package marked as broken and skipped:

• handbrake

7 packages built:

• ffmpeg_7-full
• ffmpeg_7-full.bin
• ffmpeg_7-full.data
• ffmpeg_7-full.dev
• ffmpeg_7-full.doc
• ffmpeg_7-full.lib
• ffmpeg_7-full.man

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

Result of nixpkgs-review pr 328428 run on aarch64-linux 1

[toonn]

One drawback of going with broken is that Ffmpeg does carry some weight. So Ffmpeg being broken because of a dependency motivates fixing the dependency. I wouldn't've fixed Quirc and Xev{d,e} for Darwin if it weren't for Ffmpeg.

The flipside of that is that it's not quite fair to put the burden of dealing with such breakages on Ffmpeg maintainers.

[emilazy]

As soon as xev{d,e} work on aarch64-linux I think we should remove the conditional, because yes, silent breakage is bad.

[Atemu]

Someone will have to manually set meta.broken when the build of some dep actually breaks, so that will not be silent.

I'm generally of the opinion that the people who care about some feature working should be the one maintaining it and that everyone else should take a rather hands-off approach, only doing the bare minimum (e.g. marking as broken and notifying interested people of the breakage).

[emilazy]

Well, but do you want to add .broken conditionals to every single package FFmpeg uses…? Then we never really know or explicitly state what condition our FFmpeg package is in. The assumption should be that dependencies we‘ve chosen for a given variant are present, and if they break that should be handled exceptionally.

[toonn]

I actually think *-full is a bit of an exceptional case. It's pretty much "Enable all the options you can." Without necessarily an expectation that that will be all the possible options.

In the ~minimal variant it makes sense to have tight control over what is enabled. With *-full it turns a bit into a game of whack-a-mole disabling a feature for packages marked broken and reenabling them when the packages get fixed.

I do still think conditioning ~everything on broken will have the knock-on effect of fewer people trying to get things to work. I certainly didn't have any reason to fix Quirc and Xev{d,e} other than "Huh, I wonder why ffmpeg_7-full is not building for aarch64-darwin?" But I also think it's very reasonable to not require ffmpeg maintainers to play Feature-Flag Manager Tycoon. Maybe a change of name to *-fullest or *-full-ish would communicate the intent better?

[emilazy]

I don‘t think -full is intended to be that literal. FFmpeg supports a mind‐boggling number of libraries of wildly varying quality and usefulness and I at least hope it’s not unintentional that there are plenty of them that aren’t exposed in our package definition at all. My assumption is that it is just meant to be “all the libraries you could reasonably expect to want to use”, which isn’t really that much more porous than “all the libraries the common cases want”.

[Atemu]

I do still think conditioning ~everything on broken will have the knock-on effect of fewer people trying to get things to work. I certainly didn't have any reason to fix Quirc and Xev{d,e} other than "Huh, I wonder why ffmpeg_7-full is not building for aarch64-darwin?"

That effect is not necessarily a positive one. Not all things need to get fixed in a timely manner. These codecs here are irrelevant in practice and almost nobody would be likely to miss them on macOS. There are far better uses of your time than to fix these packages w.r.t. your own goals and the ecosystem.

If you think fixing these is fun and/or it fulfils you then go ahead and knock yourself out of course but you shouldn't feel compelled to do so because it's seen as necessary.

IMHO, leave it to the people who actually want/need this codec to look into fixing it.

FFmpeg supports a mind‐boggling number of libraries of wildly varying quality and usefulness and I at least hope it’s not unintentional that there are plenty of them that aren’t exposed in our package definition at all.

Keep in mind that it's ffmpeg-full, not ffmpeg-all.

IMV it should be a "fully featured" build of ffmpeg that includes all functionality we can reasonably support in Nixpkgs, no matter how niche or outdated it is with little regard for closure size.
If a feature is available in ffmpeg and we have the required deps in working condition in Nixpkgs, it should be in full.

There's some exceptions like for instance torch which IIRC had such a huge closure that ffmpeg-full would be mostly torch with a little ffmpeg on the side.

My assumption is that it is just meant to be “all the libraries you could reasonably expect to want to use”, which isn’t really that much more porous than “all the libraries the common cases want”.

That's my vision for the regular ffmpeg.

It's a principle of least surprise thing for me. If a feature being absent in the default ffmpeg package would surprise you, it should be present.

For instance, you'd expect ffmpeg to come with AV1 encoders these days but nobody would really expect MPEG's newest DOA patent traps or an advanced effects scripting library to be included. Since we do have them in Nixpkgs though, those should be included in ffmpeg-full.