Skip to content
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

Fixes for FFmpeg 7, round 1 #340640

Merged
merged 3 commits into from
Sep 16, 2024
Merged

Fixes for FFmpeg 7, round 1 #340640

merged 3 commits into from
Sep 16, 2024

Conversation

emilazy
Copy link
Member

@emilazy emilazy commented Sep 9, 2024

Description of changes

Fixes build regressions from #337855, without introducing any FFmpeg 6 pins. I have built all the packages I directly touched here on top of staging (except liquidsoap).

I didn’t know Xine was still developed, although it seems they’ve gone almost two years without a release. @AndersonTorres would appreciate if you could test it out (rebasing on top of master and changing the dependencies to ffmpeg_7 will work to avoid mass rebuilds).

@dali99 @ehmry I don’t use Liquidsoap. I updated the FFmpeg binding package they maintain to the current Git HEAD, as recommended by upstream for those using FFmpeg 7; they are holding off on cutting a new release as their bindings cannot support both FFmpeg 6 and 7. I have had to bump Liquidsoap to the rolling-release-v2.3.x branch to pick up support for the new bindings version. According to their README, our current 2.2.x branch will be retired soon, and minor releases should not cause significant compatibility issues, but there is not a strict guarantee, and they do describe 2.3.x as being in the alpha stage and recommend testing new versions. Please let me know what you think; the alternative is to pin FFmpeg 6 for now.

The upstream WebRTC patch has already been deployed to Qt 5 WebEngine without incident, so I’m pretty confident about the Telegram clients.

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:
  • 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.

@@ -7,18 +7,23 @@

let
pname = "liquidsoap";
version = "2.2.5";
version = "2.3.x-unstable-2024-09-02";
Copy link
Member

Choose a reason for hiding this comment

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

What is .x here doing?

Copy link
Member Author

Choose a reason for hiding this comment

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

The version is from the rolling-release-v2.3.x branch, per https://github.com/savonet/liquidsoap/tree/rolling-release-v2.3.x?tab=readme-ov-file#release-details. 2.2.4-unstable-… felt misleading as upstream already has a version designation for this separate branch that they would expect users to reference.

Copy link
Member

Choose a reason for hiding this comment

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

In that case, I think 2.3-unstable... would be more appropriate.

Copy link
Member Author

Choose a reason for hiding this comment

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

It’s a bit subtle – usually VERSION-unstable-YYYY-MM-DD means an unstable version after VERSION. In this case, the version was after the birth of the rolling-release-v2.3.x branch and the marking of the 2.3.x release “series” as being available in alpha, but before the release of 2.3.0. So 2.3-unstable-… feels a little misleading to me, as it would be prior to the release of 2.3.0, and we could conceivably use e.g. 2.3.0-unstable-… to designate a commit from the rolling release branch after the release of 2.3.0. (Anyway, I dropped this from the PR.)

pkgs/tools/audio/liquidsoap/full.nix Outdated Show resolved Hide resolved
@dali99
Copy link
Member

dali99 commented Sep 9, 2024

Thanks for doing this! I think I'd prefer just a pin for liquidsoap for now. We'll bump when the new version get released properly. liquidsoap is developed like a lot of other software and only really maintains one release at a time, so the "being retired soon" is mostly a reflection of that. In any case we likely wont have to wait too long to unpin.

I'll experiment with the new version to see if anything will break horribly in the future as well. I just don't think nixpkgs-unstable should force all their users to do that experiment. Again thanks!

@emilazy
Copy link
Member Author

emilazy commented Sep 9, 2024

Alright, I’ve pinned it for now. Feel free to use 0954c30, 8c9979c, and fbbf2b8 when you do package the new version, if you keep the attribution or add a Co-authored-by :)

@AndersonTorres
Copy link
Member

Since the patches for Xine work well in Master for both FFMPEGs, I put them here: #340813

These are forks of WebRTC, so include the same upstream patch we use
for FFmpeg 7 support in Qt 5’s WebEngine.

I didn’t bother deduplicating it across these two copy‐pasted
expressions since the original authors didn’t either.
@emilazy
Copy link
Member Author

emilazy commented Sep 10, 2024

Dropped the Xine commits since they’ve been split off.

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

LGTM

@emilazy emilazy merged commit d101d27 into NixOS:staging Sep 16, 2024
25 of 27 checks passed
@emilazy emilazy deleted the push-xmyvtxwtlzmz branch September 16, 2024 16:56
@emilazy emilazy mentioned this pull request Oct 3, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants