Skip to content

qt6.qtmultimedia: Enable ffmpeg/VAAPI backend#271922

Merged
K900 merged 4 commits intoNixOS:masterfrom
puetzk:qt6-qtmultimedia-ffmpeg
Dec 4, 2023
Merged

qt6.qtmultimedia: Enable ffmpeg/VAAPI backend#271922
K900 merged 4 commits intoNixOS:masterfrom
puetzk:qt6-qtmultimedia-ffmpeg

Conversation

@puetzk
Copy link
Contributor

@puetzk puetzk commented Dec 3, 2023

Description of changes

Enable new QtMultimedia plugins to match upstream recommendations/defaults for Qt6.6

See also #194167 (closed unmerged) which had started to make similar changes back in Qt 6.4; I presume it was rejected/abandoned because at that point these new backends were still technology previews and not enabled by default in the upstream releases either. Now they are the recommended defaults.

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.05 Release Notes (or backporting 23.05 and 23.11 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.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: changelog This PR adds or changes release notes labels Dec 3, 2023
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another possible approach here would presumably be to somehow set an rpath, but runtimeDependencies = [ libva.out ] didn't work. I am not very familiar with this stuff, but found that pattern in e.g.

runtimeDependencies = [
curl
libva.out

I presume it worked there but not here because firefox's derivation owns the top-level executable, but qtmultimedia only has libraries/plugins and DT_RUNPATH isn't transitive.

Either solution would cause qtmultimedia-6.6.1/lib/qt-6/plugins/multimedia/libffmpegmediaplugin.so to menton libva's store path, and thus make it part of the closure. And in fact we had it in the closure already anyway (via part of ffmpeg-6.0-lib). And squashing this kind of "use if it it happens to be found" variability is a big part of nix's purpose.

So it seems simplest to just to tell Qt we would prefer that it simply link QFFmpegMediaPlugin to LIBRARIES VAAPI::VAAPI
resulting in an ordinary DT_NEEDED.

But I'm still relatively new to nix, and even more to nixpkgs, so I'd welcome any reviewer feedback on what's preferred in cases where upstream uses dlopen as a way to have optional features...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rebased to fix the merge conflict vs #269475 (in release notes) - which, coincidentally, may be somewhat related to the question of how we would like QtMultimedia (and ffmpeg itself) to find hardware-acceleration dependencies like CUDA/VAAPI (and possibly libvdpau in the future, though QtMultimedia currently has AV_HWDEVICE_TYPE_VDPAU disabled.

Copy link
Member

Choose a reason for hiding this comment

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

but runtimeDependencies = [ libva.out ] didn't work.

By default this is not used in stdenv.

Is this just about which libva is being used or the graphics driver? If it is just the former I don't know why we shouldn't hardcode it.

Copy link
Contributor

Choose a reason for hiding this comment

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

libva loads backends dynamically (and impurely), so this is totally fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The warnings I mentioned in this commit are about about libffmpegmediaplugin.so loading libva.so with dlopen: see https://github.com/qt/qtmultimedia/blob/v6.6.1/src/plugins/multimedia/ffmpeg/qffmpegvaapisymbols.cpp#L22-L38

But the Qt CMake logic gives you a choice (via DYNAMIC_RESOLVE_VAAPI_SYMBOLS) to actually link to it instead (which is what I chose to do):

https://github.com/qt/qtmultimedia/blob/v6.6.1/cmake/FindFFmpeg.cmake#L197-L214
https://github.com/qt/qtmultimedia/blob/v6.6.1/src/plugins/multimedia/ffmpeg/CMakeLists.txt#L111-L121

Which seemed fine since libva does the backends separately anyhow (and because nix's ffmpeg already just linked it), but I appreciate the confirmation, since I'm still learning the nix conventions...

Comment on lines +54 to +57
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm also not sure whether to ask that this be backported to 23.11 - I'd personally like it to be, and it kind of seems like it should given that this is upstream's default and they are pretty negative on continued support for gstreamer:

https://doc.qt.io/qt-6.5/qtmultimedia-index.html#backend-support
Furthermore, even some major issues with the gstreamer backend (on Linux) won't be fixed since gstreamer is difficult to debug and is further complicated as it is dependent on Linux distributions.

But it does affect the set of supported codecs (though mostly for the better...).

Opinions welcome on that too - for now I only added to 24.05 release notes "Notable changes"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose another possibility would be to set it up as a multiple-output derivation (so qt6.qtmultimedia.ffmpeg and qt6.qtmultimedia.gstreamer could be separated and not have the closure include both when only one or the other will be used). Then 23.11 and 24.05 could offer both choices, but with a different "default" (which one is propagated a build input on qt6.qtmultimedia).

But I'm not familiar with doing this, nor do I see anything similar being done in the other Qt packages. E.g. qtbase could have split the xcb vs wayland QPA plugins in a similar fashion, since it's unlikely anybody really wants both. But I didn't find any pattern to imitate...

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to duplicate this and since qt6 will only start to take off with plasma6 which will not be backported, I don't think we need to backport this either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

easy enough to use an override in the case I cared about. If there isn't actually much moving toward Qt6 until 24.05 that's a sane reason to leave well enough alone.

This was Tech Preview in Qt 6.4, released in 6.5
@puetzk puetzk force-pushed the qt6-qtmultimedia-ffmpeg branch from 035059d to 6cf1504 Compare December 3, 2023 22:35
@SuperSandro2000 SuperSandro2000 requested a review from K900 December 3, 2023 22:52
@K900 K900 requested a review from NickCao December 3, 2023 22:54
@K900
Copy link
Contributor

K900 commented Dec 3, 2023

Overall change seems good, but may need to retarget to staging depending on rebuild count impact?

This is now the default recommendation upstream for linux platforms

> https://doc.qt.io/qt-6.6/qtmultimedia-index.html#ffmpeg-as-the-default-backend
> In this release the FFmpeg framework is set as the default backend on
> Windows, macOS, Android, and Linux except Yocto distribution.

> The version shipped with Qt binary packages is FFmpeg 6.0
> and is tested by the maintainers.

libXrandr is required to compile support QT_WINDOW_CAPTURE_BACKEND=x11
QtMultimedia 6.6.0 would select dynamic VAAPI on linux, then warns
during build (even though it chose this on purpose):

> QT_FEATURE_vaapi is found but ffmpeg doesn't include vaapi,
> however dynamic symbols resolve is possible

The nuisance warning was fixed for 6.7 and backported to 6.6.1:
https://codereview.qt-project.org/c/qt/qtmultimedia/+/517333

However, tracing it helped me figure out why vaapi actually wasn't
working: nix doesn't end up with an rpath such that dlopen("va")
can actually find libva.so in the nix store, thus failing at runtime:

> qt.multimedia.plugin: loading backend "ffmpeg"
> qt.core.library: "/nix/store/i9fkjks6dfjj1p9qvj5633sxbrf5rbd8-qtmultimedia-6.6.1/lib/qt-6/plugins/multimedia/libffmpegmediaplugin.so" loaded library
> qt.multimedia.ffmpeg.libsymbolsresolver: Start VAAPI symbols resolving: 39 symbols
> qt.core.library: "va" cannot load: Cannot load library va: (va: cannot open shared object file: No such file or directory)
> qt.multimedia.ffmpeg.libsymbolsresolver: Couldn't load VAAPI library
@puetzk puetzk force-pushed the qt6-qtmultimedia-ffmpeg branch from 6cf1504 to 7e88653 Compare December 3, 2023 22:59
@puetzk
Copy link
Contributor Author

puetzk commented Dec 3, 2023

Overall change seems good, but may need to retarget to staging depending on rebuild count impact?

it hasn't finished yet, but from nixpkgs-review it isn't insanely big:

63 packages updated:
abracadabra activitywatch anki aw-qt beamerpresenter beamerpresenter calibre citra-canary citra-nightly cutter feather focuswriter goldendict-ng haka jami khoj libremines maestral-qt mandown mediaelch mnemosyne nanovna-saver nixos-install-tools normcap olive-editor-unstable onedrivegui ostinato pyside6 pyside6 python3.10-echo python3.10-glueviz python3.10-OpenUSD python3.10-pvextractor python3.10-PyQt6 python3.10-PyQt6_Charts python3.10-PyQt6_WebEngine python3.11-echo python3.11-glueviz python3.11-hydrus python3.11-OpenUSD python3.11-pvextractor python3.11-PyQt6 python3.11-PyQt6_Charts python3.11-PyQt6_WebEngine qmmp qt-full qt3d qtmultimedia qtspeech qutebrowser qzxing retext retool-unstable rpcs3 rz-ghidra smb3-foundry stellarium streamdeck-ui syncplay unbook wireshark-qt yuzu-early-access yuzu-mainline

Happy to retarget if that's preferred

@puetzk
Copy link
Contributor Author

puetzk commented Dec 4, 2023

One failure from nixpkgs-review (so far), on olive-editor-unstable

/nix/store/4lxc6c85wgyc68rz8fl2b4mjg9h23pgn-openimageio-2.5.5.0-dev/include/OpenImageIO/Imath.h:26:13: fatal error: OpenEXR/ImathColor.h: No such file or directory
26 | # include <OpenEXR/ImathColor.h>

This seems unrelated and already known (#260603 (comment) saw it on master last week), and has its own PR #271909 going already.

@puetzk
Copy link
Contributor Author

puetzk commented Dec 4, 2023

nixpkgs-review completed (no failures except olive-editor)

@puetzk puetzk changed the title Qt6 qtmultimedia ffmpeg qt6.qtmultimedia: Enable ffmpeg/VAAPI backend Dec 4, 2023
@ofborg ofborg bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Dec 4, 2023
@K900 K900 merged commit f0b6f1f into NixOS:master Dec 4, 2023
@puetzk puetzk deleted the qt6-qtmultimedia-ffmpeg branch December 4, 2023 14:31
@overtoneblue overtoneblue mentioned this pull request Aug 24, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants