-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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: ffmpeg_6 -> ffmpeg_7 #337855
ffmpeg: ffmpeg_6 -> ffmpeg_7 #337855
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This generally LGTM but we need to do a little more QA as we should verify that we don't cause any additional build failures by doing this.
Building an entire eval on at least one Linux platform would be on order. Technically, I think building direct dependants ought to be enough but I don't immediately know how to realise that.
# Please make sure this is updated to the latest version on the next major | ||
# update to ffmpeg | ||
# Packages which use ffmpeg as a library, should pin to the relevant major | ||
# version number which the upstream support. | ||
# Please make sure this is updated to new major versions once they | ||
# build and work on all the major platforms. If absolutely necessary | ||
# due to severe breaking changes, the bump can wait a little bit to | ||
# give the most proactive users time to migrate, but don’t hold off | ||
# for too long. | ||
# | ||
# Packages which depend on FFmpeg should generally use these | ||
# unversioned aliases to allow for quicker migration to new releases, | ||
# but can pin one of the versioned variants if they do not work with | ||
# the current default version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK.
@@ -17,16 +17,16 @@ | |||
|
|||
buildPythonPackage rec { | |||
pname = "av"; | |||
version = "12.3.0"; | |||
version = "12.3.0-unstable-2024-08-19"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume a release is imminent and we wait for that?
Otherwise I say we do this in a separate PR. Unpinning is a secondary goal, we should get everything that isn't pinned upgraded first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, per the PR and commit message – it’s scheduled for the start of September, before the next staging cycle will start, hence this being a draft for now. The reason to do this at the same time is to deal with the vast majority of FFmpeg 7 problems at once, Python system included, rather than stretching it out over multiple cycles.
It’s normal for a bunch of stuff to break during the Building all direct reverse dependencies would be nice, but is hard to compute (I tried @trofi’s I am active in the Staging Matrix room and will be monitoring the reports for regressions this causes to make sure I can update, patch, or pin them before the staging cycle is merged. Hopefully the instructions I gave at the start of the PR will help distribute the work, too. (There will probably be some regressions left over that don’t block the channel and that nobody noticed, but that’s pretty normal for |
Now built on |
I’ve successfully built |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Big fan of unpinning packages where we don't think we need pinning.
After many hours of babysitting and restarting builds on the Darwin community builder I’ve found that So no hope of building that right now, but given that this is a simple alias switch and it built on all the other platforms I don’t anticipate any problems. I’ll bump to the final PyAV release once it’s out and mark this as ready for review. |
Took me a while to workaround eval regressions in
|
Thank you @trofi! It’s around ~7400 rebuilds on |
It’s paying dividends! Tracked down a |
GStreamer bump is up: #339147. |
a1e6d21
to
979fad0
Compare
The final PyAV release came out, and I verified that it still builds on |
Qt should be fine, as long as it builds. |
(Confirmed that Qt builds.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is time to get this merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, let's go. This PR isn't going to get any better by sitting around.
Sure; the AArch64 builder kept restarting on me when I was trying to kick off more builds, so I think it’s probably best to leave most of the triage to the |
It is best to keep as many FFmpeg dependencies unpinned as possible, so that only the packages that actually break are kept behind on old versions. I ran into many cases recently where a package was pinned to an old version of FFmpeg but worked fine with the latest one, making the older versions look more necessary than they actually were. This will make it easier to find packages that need intervention to be updated to newer versions and speed up the process of jettisoning old FFmpeg versions. The cost of potentially holding back the default version for a little while to let major parts of the ecosystem catch up is minor by comparison, especially since it has happened with 7 anyway due to Darwin problems.
Per the adjusted FFmpeg pinning advice, packages that work on the default version should use the unversioned variants to ease the migration to future versions and reduce the number of packages that end up referencing old versions. I have left HandBrake pinned as it builds a custom patched FFmpeg.
Includes support for FFmpeg 7.
979fad0
to
5ad11f7
Compare
Let’s do this :) I’m running the large build again and I’ve found at least one regression that I’ll send a follow‐up PR for, but this is as ready as it’ll ever be. |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/breaking-changes-announcement-for-unstable/17574/58 |
Regression caused by NixOS#337855
Description of changes
It’s finally time! 🎉
This PR is pretty mundane; the main points for review are the changes to the release notes and the comments about updating and pinning FFmpeg, which are based on my experiences getting rid of old FFmpeg dependencies over the past however long it’s been. The real fun will come during the staging cycle. I’ve documented my procedure for handling FFmpeg version problems below for the benefit of people working on the cycle.
I have successfully built
ffmpeg{,4,6}{,-headless,-full}
andpython{311,312}Packages.{av,ffmpeg-python}
onaarch64-linux
, and am in the process of building them on the other major platforms.I hope that FFmpeg 6 won’t stick around as long as previous versions have. Major distros like Arch have already moved over to FFmpeg 7 and removed FFmpeg 6, so I anticipate that the vast majority of packages won’t be stuck on 6 for long. I intend to do my bit to ensure we can remove it as soon as possible, although I suspect there’ll be enough stragglers that the 24.11 release will still include it.
How to fix things this change breaks
First off, everything this breaks should be able to be fixed by pinning FFmpeg 6. If you don’t have the time, just change the dependency to one of the
ffmpeg_6
packages and be done with it. Please ping me, on Matrix or GitHub, if you notice any suspiciously FFmpeg‐looking failures, or if you open a PR to fix one, so that I can review and keep track of what packages need looking into.If you want to do your part to get the package working on FFmpeg 7, though, here’s my procedure:
Check if there’s a new upstream stable release that builds with the newer FFmpeg.
Check if there’s unreleased upstream commits that fix the build with the newer FFmpeg that we can either patch in or bump to.
Check if any other distributions have made downstream patches for FFmpeg support. Repology is good for finding packages here. Arch, Debian, and even Cygwin have helped me here in the past.
Check if there are any open pull requests upstream to add support that look high‐quality enough for us to vendor.
Consider patching in support yourself, sending it upstream, and vendoring the changes in Nixpkgs for now. FFmpeg breaking changes are rarely very involved to fix, just tedious. (There’s some sharp edges for this bump, though; the channel layout stuff touches on their options API, which is not statically checked, so it’s easy to miss things.)
Give up and pin the older version.
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-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.