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

ffmpeg_4: Enable support for AV1 decoding via dav1d by default #66527

Merged

Conversation

@primeos
Copy link
Member

commented Aug 12, 2019

This is e.g. required for mpv (depends on ffmpeg_4) to play AV1 videos.
Fixes #54990.

But since dav1d is only a AV1 decoder this doesn't support AV1 encoding
as well (that would require an additional dependency on libaom).

TODOs for review:

  • should we rename the option to av1DecodingSupport (or add libaom as well)?
    • Removed the option entirely but made it possible to set dav1d to null.
  • is AV1 decoding relevant enough to enable it by default? The file contains the following note:
 * THIS IS A MINIMAL BUILD OF FFMPEG, do not include dependencies unless
 * a build that depends on ffmpeg requires them to be compiled into ffmpeg,
 * see `ffmpeg-full' for an ffmpeg build with all features included.
  • Seems to be relevant enough, the AV1 adoption should proceed "rapidly" (hopefully :D).
  • test how well (performance) this works on different platforms. E.g. on my laptop this works pretty well (smooth 1080p playback), but on my older AMD Phenom II X6 1100T CPU this didn't work that well (too many frames dropped and high CPU usage for 1080p, depending on the Video - IIRC it worked better with more frame and tile threads).

TODOs:

  • created this commit yesterday and forgot that it doesn't only affect ffmpeg_4 which causes the many rebuilds (IIRC ~2.5k), that might be avoidable (I'll take a look later). -> It affects the other versions because ifMinVer evaluates to null and therefore changes the list (could avoid this but that wouldn't match the existing file/code structure).
    • Decided to base this on staging.
Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

@adisbladis

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

should we rename the option to av1DecodingSupport (or add libaom as well)?

Imo this is mostly a closure size issue, it looks like we can amend the libaom derivation the reduce the closure size enough that adding libaom is a no-brainer.
If that's really the case we should just add libaom too. I'll investigate!

is AV1 decoding relevant enough to enable it by default? The file contains the following note:

Yes, my take at least on the ffmpeg packages is that ffmpeg_4 should support playback of pretty much anything not too esoteric. The fact that youtube now serves av1 is reason enough alone to enable it by default.

@primeos

This comment has been minimized.

Copy link
Member Author

commented Aug 21, 2019

@adisbladis thanks for the feedback :)

Imo this is mostly a closure size issue, it looks like we can amend the libaom derivation the reduce the closure size enough that adding libaom is a no-brainer.
If that's really the case we should just add libaom too. I'll investigate!

Any updates?

Yes, my take at least on the ffmpeg packages is that ffmpeg_4 should support playback of pretty much anything not too esoteric. The fact that youtube now serves av1 is reason enough alone to enable it by default.

Sounds good :)

And the adoption of AV1 will hopefully progress "fast" (for a codec): https://en.wikipedia.org/wiki/AV1#Adoption

I'm just a bit concerned about the performance without proper hardware acceleration, but that'll hopefully change soon as well :)

@primeos

This comment has been minimized.

Copy link
Member Author

commented Aug 21, 2019

If that's really the case we should just add libaom too. I'll investigate!

@adisbladis I just gave libaom a quick test with a few AV1 videos (Netflix/YouTube) and wasn't able to play any of them. At least the decoder currently doesn't seem to perform well at all (I didn't test the encoder though). See #54990 (comment) and #54990 (comment).

@xantoz

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2019

The stable version of libaom is unusable grade outdated. On my personal system I build it from a git snapshot (rev 32185d165e0a238a32b20c5bbd59e360bd46d067 currently. Could probably go a bit newer). At least for my mpv closure.

See: https://github.com/xantoz/nixconfig/blob/80d35cbac842e1a47ffdf72b1cca648655a6629c/overlays/local/pkgs/default.nix#L65

Unfortunately libaom API seems to be quite unstable, and bumping it globally might break other users of libaom (that expect 1.0.0) than ffpmeg.

My proposal would be that we either build ffmpeg_4 with a known good git revision of libaom, or simply don't support libaom in ffmpeg_4 for the time being (so no encoding, unfortunately), and focus on libdav1d which is both more stable API-wise and faster at decoding. The latter has the benefit of making dav1d the default decoder for AV1 in e.g. mpv (last time I checked it would default to the slower libaom unless you run mpv --vd=libdav1d)

Edit: Misc clarifications. Change the link into my nixconfig, as it was pointing to a revision of it with not quite working libaom

@primeos

This comment has been minimized.

Copy link
Member Author

commented Aug 25, 2019

The stable version of libaom is unusable grade outdated.

Agreed, I already checked what other distributions are doing but it seems like most distributions are also packaging the stable releases: https://repology.org/project/aom/versions
IIRC there where ~2.5k commits between v1.0.0-errata1 and master (AFAIK also many performance improvements).

Unfortunately libaom API seems to be quite unstable, and bumping it globally might break other users of libaom (that expect 1.0.0) than ffpmeg.

Most packages would hopefully only use it indirectly via ffmpeg which should be fine if ffmpeg still builds (AFAIK).

Currently, the only other package with a direct dependency on libaom is apparently gst-plugins-bad:

michael@jarvis> git grep libaom
pkgs/development/libraries/ffmpeg-full/default.nix:, libaom ? null # AV1 encoder
pkgs/development/libraries/ffmpeg-full/default.nix:  patches = [ ./prefer-libdav1d-over-libaom.patch ];
pkgs/development/libraries/ffmpeg-full/default.nix:    (enableFeature (libaom != null) "libaom")
pkgs/development/libraries/ffmpeg-full/default.nix:    libjack2 ladspaH lame libaom libass libbluray libbs2b libcaca libdc1394 libmodplug libmysofa
pkgs/development/libraries/ffmpeg-full/prefer-libdav1d-over-libaom.patch: extern AVCodec ff_libaom_av1_decoder;
pkgs/development/libraries/ffmpeg-full/prefer-libdav1d-over-libaom.patch: extern AVCodec ff_libaom_av1_encoder;
pkgs/development/libraries/gstreamer/bad/default.nix:, libaom
pkgs/development/libraries/gstreamer/bad/default.nix:    libaom
pkgs/development/libraries/libaom/default.nix:  name = "libaom-${version}";
pkgs/top-level/all-packages.nix:  libaom = callPackage ../development/libraries/libaom { };

My proposal would be that we either build ffmpeg_4 with a known good git revision of libaom, or simply don't support libaom in ffmpeg_4 for the time being (so no encoding, unfortunately), and focus on libdav1d which is both more stable API-wise and faster at decoding.

I'll update this PR to focus on AV1 decoding via dav1d for now, but I have no objections if anyone else wants to look into adding libaom as well.

The latter has the benefit of making dav1d the default decoder for AV1 in e.g. mpv (last time I checked it would default to the slower libaom unless you run mpv --vd=libdav1d)

Yeah, that's a bit annoying - would be great if dav1d would be the upstream default. I've added a small patch to ffmpeg-full to prefer dav1d (which we could use here as well).

@primeos primeos force-pushed the primeos:ffmpeg_4-add-support-for-av1-decoding branch from 7ca1e64 to a1a43d9 Aug 25, 2019
This is e.g. required for mpv (depends on ffmpeg_4) to play AV1 videos.
Fixes #54990.

But since dav1d is only a AV1 decoder this doesn't support AV1 encoding
as well (that would require an additional dependency on libaom).
The dependency on dav1d can be disabled by overriding it to null.
@primeos primeos changed the base branch from master to staging Aug 25, 2019
@primeos primeos force-pushed the primeos:ffmpeg_4-add-support-for-av1-decoding branch from a1a43d9 to aed9694 Aug 25, 2019
@primeos primeos merged commit f2023c7 into NixOS:staging Aug 26, 2019
15 of 16 checks passed
15 of 16 checks passed
ffmpeg_4 on x86_64-darwin Timed out, unknown build status
Details
Evaluation Performance Report Evaluator Performance Report
Details
ffmpeg_4 on aarch64-linux Success
Details
ffmpeg_4 on x86_64-linux Success
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.