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.

Sign up for GitHub

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

Jump to bottom

mpv: 0.36.0 -> 0.37.0 #270691

Merged
merged 6 commits into from
Feb 11, 2024
Merged

mpv: 0.36.0 -> 0.37.0 #270691

merged 6 commits into from
Feb 11, 2024

Conversation

AndersonTorres
Copy link
Member

@AndersonTorres AndersonTorres commented Nov 28, 2023 •
edited
Loading

Description of changes

Closes #270002
Waiting #269415

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

Priorities

Add a 👍 reaction to pull requests you find important.

@AndersonTorres AndersonTorres marked this pull request as ready for review November 28, 2023 16:42
@AndersonTorres AndersonTorres marked this pull request as draft November 28, 2023 16:44
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 11-100 10.rebuild-linux: 101-500 labels Nov 28, 2023
@arthsmn
Copy link
Member

arthsmn commented Jan 17, 2024

@AndersonTorres the PR is finally merged! You can procede with the update.

@fpletz
Copy link
Member

fpletz commented Jan 17, 2024

It's on staging though, so we have to wait a bit. I'll keep an eye on this since I'm also interested in the mpv version bump.

@rhododendrox
Copy link
Contributor

@AndersonTorres the pr made it's way into master, you can merge this if you want.

@AndersonTorres
Copy link
Member Author

AndersonTorres commented Jan 28, 2024 •
edited
Loading

No, I need to wait.
For now I'll update first.

@AndersonTorres AndersonTorres mentioned this pull request Jan 28, 2024
Closed
13 tasks
@AndersonTorres AndersonTorres marked this pull request as ready for review January 28, 2024 16:15
@AndersonTorres AndersonTorres mentioned this pull request Jan 28, 2024
Merged
13 tasks
@AndersonTorres AndersonTorres changed the title Mpv updates mpv: 0.36.0 -> 0.37.0 Jan 28, 2024
@AndersonTorres
Copy link
Member Author

Shot by splicing again!

@fpletz
Copy link
Member

fpletz commented Jan 29, 2024

Code looks ok, but darwin fails: https://github.com/NixOS/nixpkgs/pull/270691/checks?check_run_id=20964475886

@AndersonTorres
Copy link
Member Author

Fails by not finding libplacebo?
Is this lib available on Darwrn?

@fpletz
Copy link
Member

fpletz commented Jan 30, 2024

libplacebo seems to fail on darwin. Not sure why mpv is searching for this even though vulkan support is only enabled on Linux. I don't have access to a darwin machine to debug this.

@fpletz
Copy link
Member

fpletz commented Jan 30, 2024

Looking at the meson options, this commit could maybe fix the darwin build (cannot push onto your branch): f56f464

@ofborg ofborg bot requested a review from fpletz January 30, 2024 02:15
@AndersonTorres
Copy link
Member Author

mental note: fixup authorship of last commit

@fpletz
Copy link
Member

fpletz commented Jan 30, 2024

No worries. Unfortunately, no luck there. :(

@davidkna
Copy link
Contributor

Because it might be useful, I tried enabling vulkanSupport on darwin-aarch64, and it builds fine on top of #284256 with if you also add a dependency on VideoToolbox, but the app bundling needs additional modifications. The python script tries to sign by itself now, but sigtool doesn't have --deep.

davidkna added a commit to davidkna/mpv that referenced this pull request Jan 31, 2024
@davidkna
osxbundle: avoid running codesign with deprecated --deep argument
7032b6e 
`--deep` is deprecated as of macos 13.0.
It is also not supported by alternative `codesign`
implementations like
[sigtool](https://github.com/thefloweringash/sigtool).

Related: NixOS/nixpkgs#270691
@davidkna davidkna mentioned this pull request Jan 31, 2024
Merged
@reckenrode
Copy link
Contributor

@davidkna Thank you for the explanation. I misunderstood it as referencing a codesign argument.

(I mentioned enabling Vulkan support because the commit of yours I used did not have it enabled, but it was needed in the derivation to avoid the libplacebo error.)

@Tungsten842
Copy link
Member

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

1 package marked as broken and skipped:
  • feishin
2 packages failed to build:
  • mov-cli
  • mov-cli.dist
78 packages built:
  • adl
  • ani-cli
  • anime-downloader
  • anime-downloader.dist
  • anki
  • anki.dist
  • anki.doc
  • anki.man
  • celluloid
  • cplay-ng
  • cplay-ng.dist
  • curseradio
  • curseradio.dist
  • deepin.dde-gsettings-schemas
  • deepin.deepin-movie-reborn
  • deepin.deepin-movie-reborn.dev
  • dmlive
  • dra-cla
  • ff2mpv
  • ff2mpv-go
  • gonic
  • gtk-pipe-viewer
  • gtk-pipe-viewer.devdoc
  • haruna
  • hydrus
  • hydrus.doc
  • hypnotix
  • invidtui
  • jellyfin-media-player
  • jellyfin-mpv-shim
  • jellyfin-mpv-shim.dist
  • jftui
  • klipperscreen
  • libsForQt5.plasmatube (plasma5Packages.plasmatube)
  • libsForQt5.tokodon (plasma5Packages.tokodon)
  • linux-wallpaperengine
  • memento
  • minitube
  • mnemosyne
  • mnemosyne.dist
  • mpc-qt
  • mpv
  • mpv-unwrapped
  • mpv-unwrapped.doc
  • mpv-unwrapped.man
  • mpvScripts.acompressor
  • mpvScripts.autocrop
  • mpvScripts.autodeint
  • mpvScripts.autoload
  • mpvScripts.inhibit-gnome
  • mpvScripts.mpris
  • mpvpaper
  • photoqt
  • pipe-viewer
  • pipe-viewer.devdoc
  • plex-media-player
  • plex-mpv-shim
  • plex-mpv-shim.dist
  • python311Packages.mpv
  • python311Packages.mpv.dist
  • python312Packages.mpv
  • python312Packages.mpv.dist
  • qimgv
  • radioboat
  • somafm-cli
  • spotube
  • stremio
  • sublime-music
  • sublime-music.dist
  • subtitleedit
  • supersonic
  • supersonic-wayland
  • svp
  • tomato-c
  • wtwitch
  • youtube-tui
  • ytfzf
  • ytui-music

@AndersonTorres
Copy link
Member Author

Still not working for Darwin...

@reckenrode
Copy link
Contributor

It’s failing because libplacebo is conditioned on Vulkan support in the derivation, but the build requires it regardless. It needs to be added unconditionally to mpv’s build inputs. (Whether to enable Vulkan by default on Darwin is a separate question. It appears to work, but I don’t know what upstream expects or requires from it on Darwin.)

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/1428

@Tungsten842
Copy link
Member

It’s failing because libplacebo is conditioned on Vulkan support in the derivation, but the build requires it regardless. It needs to be added unconditionally to mpv’s build inputs. (Whether to enable Vulkan by default on Darwin is a separate question. It appears to work, but I don’t know what upstream expects or requires from it on Darwin.)

Libplacebo supports vulkan on darwin through: MoltenVK
https://code.videolan.org/videolan/libplacebo#hardware-requirements

@geri1701 geri1701 self-requested a review February 4, 2024 17:08
@geri1701
Copy link
Member

geri1701 commented Feb 4, 2024

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

1 package marked as broken and skipped:
  • feishin
4 packages failed to build:
  • mov-cli
  • mov-cli.dist
  • sublime-music
  • sublime-music.dist
76 packages built:
  • adl
  • ani-cli
  • anime-downloader
  • anime-downloader.dist
  • anki
  • anki.dist
  • anki.doc
  • anki.man
  • celluloid
  • cplay-ng
  • cplay-ng.dist
  • curseradio
  • curseradio.dist
  • deepin.dde-gsettings-schemas
  • deepin.deepin-movie-reborn
  • deepin.deepin-movie-reborn.dev
  • dmlive
  • dra-cla
  • ff2mpv
  • ff2mpv-go
  • gonic
  • gtk-pipe-viewer
  • gtk-pipe-viewer.devdoc
  • haruna
  • hydrus
  • hydrus.doc
  • hypnotix
  • invidtui
  • jellyfin-media-player
  • jellyfin-mpv-shim
  • jellyfin-mpv-shim.dist
  • jftui
  • klipperscreen
  • libsForQt5.plasmatube (plasma5Packages.plasmatube)
  • libsForQt5.tokodon (plasma5Packages.tokodon)
  • linux-wallpaperengine
  • memento
  • minitube
  • mnemosyne
  • mnemosyne.dist
  • mpc-qt
  • mpv
  • mpv-unwrapped
  • mpv-unwrapped.doc
  • mpv-unwrapped.man
  • mpvScripts.acompressor
  • mpvScripts.autocrop
  • mpvScripts.autodeint
  • mpvScripts.autoload
  • mpvScripts.inhibit-gnome
  • mpvScripts.mpris
  • mpvpaper
  • photoqt
  • pipe-viewer
  • pipe-viewer.devdoc
  • plex-media-player
  • plex-mpv-shim
  • plex-mpv-shim.dist
  • python311Packages.mpv
  • python311Packages.mpv.dist
  • python312Packages.mpv
  • python312Packages.mpv.dist
  • qimgv
  • radioboat
  • somafm-cli
  • spotube
  • stremio
  • subtitleedit
  • supersonic
  • supersonic-wayland
  • svp
  • tomato-c
  • wtwitch
  • youtube-tui
  • ytfzf
  • ytui-music

@reckenrode
Copy link
Contributor

reckenrode commented Feb 4, 2024 •
edited
Loading

It’s failing because libplacebo is conditioned on Vulkan support in the derivation, but the build requires it regardless. It needs to be added unconditionally to mpv’s build inputs. (Whether to enable Vulkan by default on Darwin is a separate question. It appears to work, but I don’t know what upstream expects or requires from it on Darwin.)

Libplacebo supports vulkan on darwin through: MoltenVK https://code.videolan.org/videolan/libplacebo#hardware-requirements

I wasn’t trying to suggest it didn’t or shouldn’t, and I apologize if I gave that impression. I wanted to address the failing build without getting sidetracked discussing what the default for vulkanSupport should be in the mpv derivation.

It seems like it probably should be the default, but I’d prefer to defer to the others here who have been giving good feedback and providing fixes for the issues. My only contribution really to this PR was the switch to rcodesign.

@AndersonTorres
Copy link
Member Author

The things are getting worse. Now two packages don't build.

@wegank
Copy link
Member

wegank commented Feb 4, 2024

@ofborg build sublime-music

@wegank
Copy link
Member

wegank commented Feb 4, 2024

The things are getting worse. Now two packages don't build.

mov-cli doesn't build on master either.

@AndersonTorres
Copy link
Member Author

The things are getting worse. Now two packages don't build.

mov-cli doesn't build on master either.

yes, I know. #284587

Now the point is about sublime-music.

@wegank
Copy link
Member

wegank commented Feb 4, 2024

Now the point is about sublime-music.

Looks good according to ofborg, probably some flaky tests failed/timed out?

AndersonTorres and others added 6 commits February 9, 2024 21:28
@AndersonTorres
mpv: 0.36.0 -> 0.37.0
af97e98 
And some refactors:
- Remove dev output (cycle detected in build of <mpv> in the references of
  output 'dev' from output 'out')
- Env vars inside env
- Remove nested with
@AndersonTorres @fpletz @davidkna
mpv: extra dependencies for Darwin
3d1ef6d 
Namely, libplacebo (since it is now unconditional) and VideoToolbox.

Co-authored-by: Franz Pletz <fpletz@fnordicwalking.de>
Co-authored-by: David Knaack <davidkna@users.noreply.github.com>
@davidkna @AndersonTorres
mpv: fix app-bundling
ce6ca5e
@AndersonTorres @reckenrode
mpv: use rcodesign instead of darwin.codesign
7e96adf 
Co-authored-by: Randy Eckenrode <randy@largeandhighquality.com>
@AndersonTorres @SuperSandro2000
mpv: split dev output via patching
a88842e 
Co-authored-by: Sandro Jäckel <sandro.jaeckel@gmail.com>
@AndersonTorres
mpv: remove env.NIX_LDFLAGS setting
390daad
@fpletz fpletz merged commit ff46dbb into NixOS:master Feb 11, 2024
22 of 23 checks passed
davidkna added a commit to davidkna/mpv that referenced this pull request Mar 2, 2024
@davidkna
osxbundle: avoid running codesign with deprecated --deep argument
0b94473 
`--deep` is deprecated as of macos 13.0.
It is also not supported by alternative `codesign`
implementations like
[sigtool](https://github.com/thefloweringash/sigtool).

Related: NixOS/nixpkgs#270691
Akemi pushed a commit to mpv-player/mpv that referenced this pull request Mar 7, 2024
@davidkna @Akemi
osxbundle: avoid running codesign with deprecated --deep argument
68fbdc8 
`--deep` is deprecated as of macos 13.0.
It is also not supported by alternative `codesign`
implementations like
[sigtool](https://github.com/thefloweringash/sigtool).

Related: NixOS/nixpkgs#270691
xrun1 pushed a commit to xrun1/mpv-svp-fix that referenced this pull request Mar 12, 2024
@davidkna @xrun1
osxbundle: avoid running codesign with deprecated --deep argument
a82c375 
`--deep` is deprecated as of macos 13.0.
It is also not supported by alternative `codesign`
implementations like
[sigtool](https://github.com/thefloweringash/sigtool).

Related: NixOS/nixpkgs#270691
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@geri1701 geri1701 geri1701 approved these changes

@wegank wegank wegank approved these changes

@fpletz fpletz fpletz approved these changes

@Ma27 Ma27 Awaiting requested review from Ma27

@primeos primeos Awaiting requested review from primeos

@globin globin Awaiting requested review from globin

@tadeokondrak tadeokondrak Awaiting requested review from tadeokondrak

Assignees
No one assigned
Labels
10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update request: mpv 0.36.0 → 0.37.0