Skip to content

qt6.qtmultimedia: Enable ffmpeg/VAAPI backend #271922

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

Merged
K900 merged 4 commits into from
Dec 4, 2023
Merged

qt6.qtmultimedia: Enable ffmpeg/VAAPI backend #271922

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0ebea89
qt6.qtmultimedia: Enable Spatial Audio (Quick3D)
puetzk Nov 10, 2023
91b8e47
qt6.qtmultimedia: Compile ffmpeg multimedia plugin
puetzk Nov 8, 2023
d3de574
qt6.qtmultimedia: Compile hardware-accelerated VAAPI
puetzk Dec 3, 2023
7e88653
qt6.qtmultimedia: Fix failure to load libva.so
puetzk Dec 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions nixos/doc/manual/release-notes/rl-2405.section.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,5 @@ The pre-existing [services.ankisyncd](#opt-services.ankisyncd.enable) has been m

- The `hardware.pulseaudio` module now sets permission of pulse user home directory to 755 when running in "systemWide" mode. It fixes [issue 114399](https://github.com/NixOS/nixpkgs/issues/114399).

- QtMultimedia has changed its default backend to `QT_MEDIA_BACKEND=ffmpeg` (previously `gstreamer` on Linux or `darwin` on MacOS).
The previous native backends remain available but are now minimally maintained. Refer to [upstream documentation](https://doc.qt.io/qt-6/qtmultimedia-index.html#ffmpeg-as-the-default-backend) for further details about each platform.
Comment on lines +56 to +57
Copy link
Contributor Author

@puetzk puetzk Dec 3, 2023

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

@puetzk puetzk Dec 3, 2023

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

@SuperSandro2000 SuperSandro2000 Dec 3, 2023

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

@puetzk puetzk Dec 3, 2023

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.

12 changes: 9 additions & 3 deletions pkgs/development/libraries/qt-6/modules/qtmultimedia.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
, stdenv
, qtbase
, qtdeclarative
, qtquick3d
, qtshadertools
, qtsvg
, pkg-config
Expand All @@ -12,8 +13,11 @@
, gst-plugins-good
, gst-libav
, gst-vaapi
, ffmpeg_6
, libva
, libpulseaudio
, wayland
, libXrandr
, elfutils
, libunwind
, orc
Expand All @@ -23,12 +27,14 @@
qtModule {
pname = "qtmultimedia";
nativeBuildInputs = [ pkg-config ];
buildInputs = [ libunwind orc ]
++ lib.optionals stdenv.isLinux [ libpulseaudio elfutils alsa-lib wayland ];
propagatedBuildInputs = [ qtbase qtdeclarative qtsvg qtshadertools ]
buildInputs = [ libunwind orc ffmpeg_6 ]
++ lib.optionals stdenv.isLinux [ libpulseaudio elfutils alsa-lib wayland libXrandr libva ];
propagatedBuildInputs = [ qtbase qtdeclarative qtsvg qtshadertools qtquick3d ]
++ lib.optionals stdenv.isLinux [ gstreamer gst-plugins-base gst-plugins-good gst-libav gst-vaapi ]
++ lib.optionals stdenv.isDarwin [ VideoToolbox ];

cmakeFlags = [ "-DENABLE_DYNAMIC_RESOLVE_VAAPI_SYMBOLS=0" ];
Copy link
Contributor Author

@puetzk puetzk Dec 3, 2023

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.

nixpkgs/pkgs/applications/networking/browsers/firefox-bin/default.nix

Lines 90 to 92 in c2273e1

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

@puetzk puetzk Dec 3, 2023

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

@SuperSandro2000 SuperSandro2000 Dec 3, 2023

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

@K900 K900 Dec 3, 2023

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

@puetzk puetzk Dec 3, 2023

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...


env.NIX_CFLAGS_COMPILE = lib.optionalString stdenv.isDarwin
"-include AudioToolbox/AudioToolbox.h";
NIX_LDFLAGS = lib.optionalString stdenv.isDarwin
Expand Down