Skip to content

ffmpeg: merge ffmpeg and ffmpeg-full#204198

Closed
Atemu wants to merge 1074 commits intoNixOS:stagingfrom
Atemu:ffmpeg-ffmpeg-full-merge
Closed

ffmpeg: merge ffmpeg and ffmpeg-full#204198
Atemu wants to merge 1074 commits intoNixOS:stagingfrom
Atemu:ffmpeg-ffmpeg-full-merge

Conversation

@Atemu
Copy link
Member

@Atemu Atemu commented Dec 2, 2022 •
edited
Loading

Description of changes

Fixes #15930

  • Merge Derivations
  • Refactor build flags
  • Reach build input conformance
  • Reach configureFlags conformance
  • Outputs conformance
  • Build/Install script conformance
  • ffmpeg_5 conformance
  • config.h conformance
  • Make headless variant work
  • Define "minimal", "headless" and "full"
    • Should "full" have a lib output? It shouldn't be depended upon in Nixpkgs.
  • Remove asserts.
  • Ensure our we don't explicitly disable ffmpeg features that were implicitly enabled before (how?)
    • Configure report
    • Compare -formats, pix_fmts, etc.
  • Unify variants
    • Multi-output
    • Things not in ffmpeg-full
  • Untangle flags and dependencies
    • Figure out to which flags the rest of the buildInputs belong
  • Cleanup
    • Move code comments from dependencies to with flags
    • Why configurePlatforms = []?
    • Is the postFixup thingy still necessary?
  • Meta cleanup
    • Make Licenses respond to flags
    • Put maintainers back
  • Compare closure sizes
  • Verify that cross-compilation still works
  • Do we want our rtmpdump or the one built in?

Wants:

  • Put Darwin frameworks behind finer flags
  • Restore commented flags
  • Cleanup

Differences:

  • headless variant's pname now reflects the variant
  • Non-full variants have full's features explicitly disabled
  • Avresample is disabled/removed
  • nv-codec-headers are only included when necessary
  • Implicitly enabled features are now explicitly enabled
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 23.05 Release Notes (or backporting 22.11 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 2, 2022
@Atemu Atemu force-pushed the ffmpeg-ffmpeg-full-merge branch from a072ce9 to ab4bed7 Compare December 4, 2022 08:16
@Atemu Atemu mentioned this pull request Dec 4, 2022
Closed
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 4, 2022
@Atemu
Copy link
Member Author

Atemu commented Dec 6, 2022

If you normalise order, buildInputs and configureFlags closures are now identical for ffmpeg and ffmpeg-full on { x86_64, aarch64 } x { linux, darwin } except for configureFlags that:

  • now explicitly disable ffmpeg-full features in regular ffmpeg
  • are obviated (i.e. pkg-config discovery)
  • explicitly enable features that were implicitly enabled before (i.e. manpages)
  • were empty strings

and null buildInputs.

@Atemu Atemu force-pushed the ffmpeg-ffmpeg-full-merge branch from d247a16 to 2793c51 Compare December 9, 2022 11:50
@Atemu
Copy link
Member Author

Atemu commented Dec 9, 2022

ffmpeg-headless, ffmpeg and ffmpeg full build on my aarch64-linux systems now!

@ofborg ofborg bot added 8.has: clean-up This PR removes packages or removes other cruft 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Dec 9, 2022
Atemu
Atemu commented Dec 9, 2022
View reviewed changes
pkgs/development/libraries/ffmpeg/generic.nix Outdated
Comment on lines 7 to 22
Copy link
Member Author

@Atemu Atemu Dec 9, 2022

Choose a reason for hiding this comment

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

Please let me know whether this is a sane policy.

Copy link
Member

@SuperSandro2000 SuperSandro2000 Jan 19, 2023

Choose a reason for hiding this comment

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

LGTM fine to me. If we notice it does not work out we can always change it later again.

One minor thing: we should assert that it is only one of the three strings we expect.

@AndersonTorres
Copy link
Member

AndersonTorres commented Dec 13, 2022

Define "minimal", "headless" and "full"

What about small instead of a polemical minimal attribute name?

Should "full" have a lib output?

lib should be an output by itself, mostly independent from bin files.

AndersonTorres
AndersonTorres reviewed Dec 13, 2022
View reviewed changes
@Atemu Atemu mentioned this pull request Dec 24, 2022
Closed
@YellowOnion
Copy link
Contributor

YellowOnion commented Dec 28, 2022

Could we get libplacebo support as well?

@Atemu
Copy link
Member Author

Atemu commented Dec 28, 2022

Not in the scope of this PR.

It should certainly become easier to add support for more ffmpeg features with a cleaner derivation though.

@YellowOnion
Copy link
Contributor

YellowOnion commented Dec 28, 2022

Yeah I think I'll wait for this patch before attempting to add it, vulkan support on ffmpeg_4, ffmpeg_5 & ffmpeg_5Full seems to be broken, and needed for libplacebo.

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 6, 2023
@Atemu Atemu force-pushed the ffmpeg-ffmpeg-full-merge branch from de08bfa to d6fb6f0 Compare January 6, 2023 18:18
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 6, 2023
@ofborg ofborg bot requested review from codyopel and linsui January 6, 2023 18:49
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 501-1000 This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches. and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Jan 6, 2023
@Atemu
Copy link
Member Author

Atemu commented Jan 6, 2023

We're now at the point where the merge is mostly done with only some cleanup of the old ffmpeg-full derivation necessary but that could theoretically come later.

I'd love to have some more eyes on this.

Commits will probably be squashed in the end.

@Atemu Atemu marked this pull request as ready for review January 6, 2023 22:16
@github-actions github-actions bot added 6.topic: agda A dependently typed programming language / interactive theorem prover 6.topic: cinnamon Desktop environment 6.topic: emacs Text editor 6.topic: erlang General-purpose, concurrent, functional high-level programming language 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: golang Go is a high-level general purpose programming language that is statically typed and compiled. 6.topic: haskell General-purpose, statically typed, purely functional programming language 6.topic: kernel The Linux kernel 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: qt/kde Object-oriented framework for GUI creation 6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. 6.topic: stdenv Standard environment 6.topic: steam Steam game store/launcher (store.steampowered.com) 6.topic: systemd Software suite that provides an array of system components for Linux operating systems. 6.topic: TeX Issues regarding texlive and TeX in general 6.topic: vim Advanced text editor 6.topic: vscode A free and versatile code editor that supports almost every major programming language. 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` labels Jan 20, 2023
@Atemu
Copy link
Member Author

Atemu commented Jan 20, 2023

Fuck, master isn't in staging.

@Atemu Atemu closed this Jan 20, 2023
@Atemu Atemu mentioned this pull request Jan 20, 2023
Merged
13 tasks
@NixOS NixOS locked and limited conversation to collaborators Jan 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@mweinelt mweinelt mweinelt left review comments

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@Artturin Artturin Artturin left review comments

@codyopel codyopel Awaiting requested review from codyopel

@linsui linsui Awaiting requested review from linsui

@jonringer jonringer Awaiting requested review from jonringer

@AndersonTorres AndersonTorres Awaiting requested review from AndersonTorres

@Anton-Latukha Anton-Latukha Awaiting requested review from Anton-Latukha

@wmertens wmertens Awaiting requested review from wmertens

@IvarWithoutBones IvarWithoutBones Awaiting requested review from IvarWithoutBones

@zowoq zowoq Awaiting requested review from zowoq

@mkg20001 mkg20001 Awaiting requested review from mkg20001

@jtojnar jtojnar Awaiting requested review from jtojnar

@kalbasit kalbasit Awaiting requested review from kalbasit

@Mic92 Mic92 Awaiting requested review from Mic92

@mmahut mmahut Awaiting requested review from mmahut

@RaghavSood RaghavSood Awaiting requested review from RaghavSood

@aanderse aanderse Awaiting requested review from aanderse

@etu etu Awaiting requested review from etu

@globin globin Awaiting requested review from globin

@Ma27 Ma27 Awaiting requested review from Ma27

@talyz talyz Awaiting requested review from talyz

@figsoda figsoda Awaiting requested review from figsoda

@adisbladis adisbladis Awaiting requested review from adisbladis

@thoughtpolice thoughtpolice Awaiting requested review from thoughtpolice

@marsam marsam Awaiting requested review from marsam

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

@stigtsp stigtsp Awaiting requested review from stigtsp

@zakame zakame Awaiting requested review from zakame

@dasJ dasJ Awaiting requested review from dasJ

@cdepillabout cdepillabout Awaiting requested review from cdepillabout

@sternenseemann sternenseemann Awaiting requested review from sternenseemann

@maralorn maralorn Awaiting requested review from maralorn

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

@alyssais alyssais Awaiting requested review from alyssais

@nbp nbp Awaiting requested review from nbp

@edolstra edolstra Awaiting requested review from edolstra

@infinisil infinisil Awaiting requested review from infinisil

+1 more reviewer

@FRidh FRidh FRidh left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: agda A dependently typed programming language / interactive theorem prover 6.topic: cinnamon Desktop environment 6.topic: emacs Text editor 6.topic: erlang General-purpose, concurrent, functional high-level programming language 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: golang Go is a high-level general purpose programming language that is statically typed and compiled. 6.topic: haskell General-purpose, statically typed, purely functional programming language 6.topic: kernel The Linux kernel 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: qt/kde Object-oriented framework for GUI creation 6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. 6.topic: stdenv Standard environment 6.topic: steam Steam game store/launcher (store.steampowered.com) 6.topic: systemd Software suite that provides an array of system components for Linux operating systems. 6.topic: TeX Issues regarding texlive and TeX in general 6.topic: vim Advanced text editor 6.topic: vscode A free and versatile code editor that supports almost every major programming language. 8.has: changelog This PR adds or changes release notes 8.has: clean-up This PR removes packages or removes other cruft 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 501-1000 This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ffmpeg has two different implementations