mpv: clean‐ups and Darwin improvements#350674
Merged
fpletz merged 21 commits intoNixOS:staging-nextfrom Oct 24, 2024
Merged
Conversation
Oh, this is satisfying. Swift already propagates its corresponding SDK, so we don’t need the patch.
mpv currently doesn’t even build successfully on macOS without Swift, and there’s no use of Swift outside of the platform.
Using `--vo=gpu` or `--vo=gpu-next` with MoltenVK is now supported and has replaced the previous `cocoa-cb` as the default.
This is now detected correctly as part of the SDK, as it should be.
Does nothing, removed upstream in 2012(!): <mpv-player/mpv@1fde09d>.
Does nothing, removed upstream in 2012(!): <mpv-player/mpv@1fde09d>.
Does nothing, removed upstream in 2012(!): <mpv-player/mpv@6a26b4a>.
This is required as part of X11 support since the Meson transition.
Removed upstream: <mpv-player/mpv@200992f>.
It builds fine on macOS.
Upstream’s Meson defaults, upstream’s CI binaries, and Arch don’t enable this.
This prints out a big warning about how you shouldn’t be using it. Upstream CI builds and Arch don’t enable it. There are other backends of questionable worth that we don’t expose options for, so let’s ditch this one.
No reason for FreeBSD to miss out on the fun.
423c415 to
888c460
Compare
Since we’re doing auto‐detection anyway, this is unnecessary.
On macOS, many things won’t work properly unless mpv is executed from the app bundle, such as spatial audio with `--ao=avfoundation`. This ensures that those features work reliably and also avoids duplicating the entire `mpv` binary.
888c460 to
12ce8fd
Compare
Member
Author
|
@AndersonTorres @fpletz would be great to get this reviewed before the freeze tonight if you have the time ^^ |
fpletz
approved these changes
Oct 24, 2024
Member
fpletz
left a comment
There was a problem hiding this comment.
Thank you very much for the cleanups. Much appreciated! ❤️
|
|
||
| patches = [ | ||
| # Fix build with Darwin SDK 11 | ||
| ./0001-fix-darwin-build.patch |
Member
There was a problem hiding this comment.
Was this file removed from Nixpkgs repo?
Member
Author
There was a problem hiding this comment.
I intended to, but it’s possible it got lost in rebasing. Will open a PR soon if nobody beats me to it (currently racing against time for the 24.11 freeze).
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This ports mpv over to the new SDK format from #346043 (so much less cruft! yay!), enables the new upstream default of Vulkan on Darwin, and cleans up a bunch of flags and dependencies, including removing flags that had no effect, broke the build to toggle, or are just obsolete, and adjusting some defaults to be less Linux‐specific or more inline with upstream and other distros.
There are still things to investigate with the macOS build (e.g. VideoToolbox seems to support fewer pixel output formats compared to their official CI binaries), but I wanted to get this batch with the theoretically‐breaking changes posted before the freeze.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-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.