Skip to content

ffmpeg: Patch for compatibility with 10.12 AVFoundation#328424

Merged
Atemu merged 1 commit intoNixOS:masterfrom
toonn:ffmpeg_7-avfoundation-compat
Jul 22, 2024
Merged

ffmpeg: Patch for compatibility with 10.12 AVFoundation#328424
Atemu merged 1 commit intoNixOS:masterfrom
toonn:ffmpeg_7-avfoundation-compat

Conversation

@toonn
Copy link
Contributor

@toonn toonn commented Jul 19, 2024

Description of changes

This patch conflicts mildly with #322724, if that is merged first this one should be updated not to disable AVFoundation, if this is merged first that PR should be updated not to introduce the disabling.

The vendored patch can be dropped as soon as we start building ffmpeg against a newer SDK but the effort to make it compatible with 10.12 was very minor.

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.

@toonn toonn mentioned this pull request Jul 19, 2024
13 tasks
@emilazy
Copy link
Member

emilazy commented Jul 19, 2024

Do you have any plans to try upstreaming this to FFmpeg or would you prefer someone else brave the rapids?

@ofborg ofborg bot requested review from Atemu, arthsmn and jopejoe1 July 19, 2024 13:26
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jul 19, 2024
@FlameFlag
Copy link
Member

FlameFlag commented Jul 19, 2024

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

1 package marked as broken and skipped:
  • handbrake
7 packages failed to build:
  • ffmpeg_7-full
  • ffmpeg_7-full.bin
  • ffmpeg_7-full.data
  • ffmpeg_7-full.dev
  • ffmpeg_7-full.doc
  • ffmpeg_7-full.lib
  • ffmpeg_7-full.man
15 packages built:
  • ffmpeg_7
  • ffmpeg_7-headless
  • ffmpeg_7-headless.bin
  • ffmpeg_7-headless.data
  • ffmpeg_7-headless.dev
  • ffmpeg_7-headless.doc
  • ffmpeg_7-headless.lib
  • ffmpeg_7-headless.man
  • ffmpeg_7.bin
  • ffmpeg_7.data
  • ffmpeg_7.dev
  • ffmpeg_7.doc
  • ffmpeg_7.lib
  • ffmpeg_7.man
  • metadata

Log: https://pub.microbin.eu/raw/ape-bear-gecko

@toonn
Copy link
Contributor Author

toonn commented Jul 19, 2024

@emilazy, I think we should respect upstream's decision to support a minimum of 10.13, TBH. That's already quite generous. This is just a temporary band-aid for us.

@emilazy
Copy link
Member

emilazy commented Jul 19, 2024

Maybe we should follow suit :')

@toonn
Copy link
Contributor Author

toonn commented Jul 20, 2024

For clarity the failures on ffmpeg_7-full are expected. That requires more fixes.

Comment on lines +18 to +19
-static NSArray* getDevicesWithMediaType(AVMediaType mediaType) {
+static NSArray* getDevicesWithMediaType(NSString * mediaType) {
Copy link
Member

Choose a reason for hiding this comment

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

For how long will this be forward compatible?

Copy link
Member

Choose a reason for hiding this comment

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

It looks like AVMediaType was introduced in 10.13, and is documented as an alias, so probably for a while unless there’s explicit notice that depending on it being an alias is deprecated?

@Atemu Atemu merged commit 3f8971c into NixOS:master Jul 22, 2024
toonn added a commit to toonn/nixpkgs that referenced this pull request Jul 23, 2024
This was disabled in NixOS#322724 after NixOS#328424 provided a patch fixing it.
@toonn toonn mentioned this pull request Jul 23, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants