Skip to content

FFmpeg 5.0 build fixes#158226

Merged
jonringer merged 4 commits intoNixOS:staging-nextfrom
vs49688:ffstaging
Feb 7, 2022
Merged

FFmpeg 5.0 build fixes#158226
jonringer merged 4 commits intoNixOS:staging-nextfrom
vs49688:ffstaging

Conversation

@vs49688
Copy link
Contributor

@vs49688 vs49688 commented Feb 5, 2022

Motivation for this change

Updating applications for FFmpeg 5.0.

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/)
  • 22.05 Release Notes (or backporting 21.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.

@vs49688 vs49688 mentioned this pull request Feb 5, 2022
13 tasks
@ofborg ofborg bot requested review from edolstra and jagajaga February 5, 2022 11:22
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Feb 5, 2022
@vcunat vcunat changed the base branch from staging to staging-next February 5, 2022 12:10
@github-actions github-actions bot added 6.topic: golang Go is a high-level general purpose programming language that is statically typed and compiled. 6.topic: printing Drivers, CUPS & Co. labels Feb 5, 2022
@vcunat vcunat changed the base branch from staging-next to staging February 5, 2022 12:10
@github-actions github-actions bot removed 6.topic: golang Go is a high-level general purpose programming language that is statically typed and compiled. 6.topic: printing Drivers, CUPS & Co. labels Feb 5, 2022
@vcunat
Copy link
Member

vcunat commented Feb 5, 2022

This should be based on staging-next and targeted there. Unfortunately it already contains some commits from staging-next..staging.

@vs49688 vs49688 changed the base branch from staging to staging-next February 5, 2022 12:20
@vs49688
Copy link
Contributor Author

vs49688 commented Feb 5, 2022

Yeah, my bad. Fixed.

@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: 1001-2500 This PR causes many rebuilds on Darwin and should most likely 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: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. and removed 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. labels Feb 5, 2022
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. and removed 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 1001-2500 This PR causes many rebuilds on Darwin and should most likely 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: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Feb 5, 2022
@ofborg ofborg bot added the 8.has: clean-up This PR removes packages or removes other cruft label Feb 6, 2022
@vs49688 vs49688 requested review from jonringer and vcunat and removed request for edolstra and jagajaga February 6, 2022 14:05
@ofborg ofborg bot requested review from edolstra and jagajaga February 6, 2022 14:16
@siraben
Copy link
Member

siraben commented Feb 6, 2022

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

3 packages failed to build:
  • gopro
  • mplayer
  • vcs

@vs49688
Copy link
Contributor Author

vs49688 commented Feb 6, 2022

Result of nixpkgs-review pr 158226 run on aarch64-darwin 1
3 packages failed to build:

* gopro

* mplayer

An error of x265:

[100%] Linking CXX executable x265
[100%] Built target cli
running tests
/nix/store/0qlq4s175mzlx9g0aqcdk95j92g1z7mf-stdenv-darwin/setup: line 1363: ./test/TestBench: No such file or directory
builder for '/nix/store/sgf9mmgk6c8b063182b6833k6g30is7x-x265-3.5.drv' failed with exit code 127
copying path '/nix/store/87xybaaksc3x34s0jy63qqlj8pa3lyb0-libpng-apng-1.6.37-dev' from 'https://cache.nixos.org'...
cannot build derivation '/nix/store/d6iwl2b54l4agfggkvmad59jw1r8wrgh-ffmpeg-5.0.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/b8s0lzds41629xdqxg7z42zj5585a4fk-mplayer-unstable-2022-02-03.drv': 1 dependencies couldn't be built
error: build of '/nix/store/b8s0lzds41629xdqxg7z42zj5585a4fk-mplayer-unstable-2022-02-03.drv' failed

@vs49688
Copy link
Contributor Author

vs49688 commented Feb 7, 2022

Just confirmed, once x265 build is fixed, all gopro, mplayer, vcs build fine.

#154347 (comment)

Needed for FFmpeg 5 compatibility. Upstream hasn't tagged a release
since 2019.
Needed for FFmpeg 5.0 compatibility. Last release was in 2019.
Is broken and long-abandoned by upstream.
Temporary to not break its dependencies.

See NixOS#154347 (comment)
@vs49688
Copy link
Contributor Author

vs49688 commented Feb 7, 2022

Result of nixpkgs-review run on x86_64-linux 1

12 packages built:
  • devede
  • ffmpegthumbnailer
  • frostwire
  • geeqie
  • gmtk
  • gnome_mplayer
  • gopro
  • kmplayer
  • mplayer
  • spaceFM
  • vcs
  • xfce.tumbler

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

LGTM

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

13 packages built:
  • devede
  • ffmpegthumbnailer
  • frostwire
  • geeqie
  • gmtk
  • gnome_mplayer
  • gopro
  • haskellPackages.mplayer-spot
  • kmplayer
  • mplayer
  • spaceFM
  • vcs
  • xfce.tumbler

gmailieer = lieer; # Added 2020-04-19
gmic_krita_qt = gmic-qt-krita; # Added 2019-09-07
gmvault = throw "gmvault has been removed because it is unmaintained, mostly broken, and insecure"; # Added 2021-03-08
gnash = throw "gnash has been removed; broken and abandoned upstream."; # added 2022-02-06
Copy link
Contributor

@rnhmjoj rnhmjoj Feb 12, 2022

Choose a reason for hiding this comment

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

  1. It's not broken:
λ git revert 79cb125a6d27f3846cd2e60ae71926de95bae78b
Auto-merging pkgs/top-level/all-packages.nix
Auto-merging pkgs/top-level/aliases.nix
[master 79247e69dc6] Revert "gnash: remove"
 4 files changed, 193 insertions(+), 1 deletion(-)
 create mode 100644 pkgs/misc/gnash/0001-fix-build-with-ffmepg-4.patch
 create mode 100644 pkgs/misc/gnash/default.nix
λ nix build -f. gnash
[1 built, 300 copied (1193.3 MiB), 250.5 MiB DL]
  1. It's not broken because several people are making patches, despite being unmaintained upstream.

  2. You shouldn't just remove packages without pinging their maintainers.

Copy link
Member

Choose a reason for hiding this comment

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

The switch of ffmpeg default was reverted later, so I assume that's why it isn't broken now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. It's not broken:

Like @vcunat said, it was broken with ffmpeg 5. That was reverted, so of course it's not broken anymore.

λ git revert 79cb125a6d27f3846cd2e60ae71926de95bae78b
Auto-merging pkgs/top-level/all-packages.nix
Auto-merging pkgs/top-level/aliases.nix
[master 79247e69dc6] Revert "gnash: remove"
 4 files changed, 193 insertions(+), 1 deletion(-)
 create mode 100644 pkgs/misc/gnash/0001-fix-build-with-ffmepg-4.patch
 create mode 100644 pkgs/misc/gnash/default.nix
λ nix build -f. gnash
[1 built, 300 copied (1193.3 MiB), 250.5 MiB DL]
2. It's not broken because several people are making patches, despite being unmaintained upstream.

Where? The most recent patch on the gnash-dev mailing list is a fix for building with ffmpeg 4 - look who the author is... Also, I checked upstream before removing it.

3. You shouldn't just remove packages without pinging their maintainers.

This is your only valid criticism, I didn't realise ofborg wouldn't ping maintainers when their package is removed. I'll be sure to check next time.

Copy link
Contributor

Choose a reason for hiding this comment

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

The switch of ffmpeg default was reverted later

Ah, I see.

Where? The most recent patch on the gnash-dev mailing list is a fix for building with ffmpeg 4 - look who the author is... Also, I checked upstream before removing it.

I guess I'll have to thank you. The package is on life support, but it's still useful, so
unless it's really impossible to compile with libraries in Nixpkgs I'd rather keep it alive.

I'll be sure to check next time.

Ok, Thank you.

@vs49688 vs49688 deleted the ffstaging branch February 12, 2022 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

8.has: clean-up This PR removes packages or removes other cruft 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants