vd_lavc: add Vulkan hardware decoding to autoprobe

[github-actions[bot]]

Download the artifacts for this pull request:

Windows

* [mpv-i686-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/1862621529.zip) * [mpv-x86_64-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/1862625772.zip) * [mpv-x86_64-windows-msvc](https://nightly.link/mpv-player/mpv/actions/artifacts/1862629562.zip)

macOS

* [mpv-macos-12-intel](https://nightly.link/mpv-player/mpv/actions/artifacts/1862614470.zip) * [mpv-macos-13-intel](https://nightly.link/mpv-player/mpv/actions/artifacts/1862620877.zip) * [mpv-macos-14-arm](https://nightly.link/mpv-player/mpv/actions/artifacts/1862612916.zip)

[Andarwinux]

vulkan hwdec still crashes on nvidia.

[sfan5]

Breaks build on Android:

../video/out/vulkan/context.c:193:9: error: use of undeclared identifier 'VK_KHR_VIDEO_DECODE_QUEUE_EXTENSION_NAME'
        VK_KHR_VIDEO_DECODE_QUEUE_EXTENSION_NAME,
        ^
../video/out/vulkan/context.c:194:9: error: use of undeclared identifier 'VK_KHR_VIDEO_DECODE_H264_EXTENSION_NAME'
        VK_KHR_VIDEO_DECODE_H264_EXTENSION_NAME,
        ^
../video/out/vulkan/context.c:195:9: error: use of undeclared identifier 'VK_KHR_VIDEO_DECODE_H265_EXTENSION_NAME'
        VK_KHR_VIDEO_DECODE_H265_EXTENSION_NAME,
        ^
../video/out/vulkan/context.c:196:9: error: use of undeclared identifier 'VK_KHR_VIDEO_QUEUE_EXTENSION_NAME'
        VK_KHR_VIDEO_QUEUE_EXTENSION_NAME,
        ^
../video/out/vulkan/context.c:231:25: error: use of undeclared identifier 'VK_QUEUE_VIDEO_DECODE_BIT_KHR'
        .extra_queues = VK_QUEUE_VIDEO_DECODE_BIT_KHR,
                        ^
../video/out/vulkan/context.c:233:31: error: invalid application of 'sizeof' to an incomplete type 'const char *[]'
        .num_opt_extensions = MP_ARRAY_SIZE(opt_extensions),
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../common/common.h:48:33: note: expanded from macro 'MP_ARRAY_SIZE'
#define MP_ARRAY_SIZE(s) (sizeof(s) / sizeof((s)[0]))
                                ^~~
6 errors generated.
[...]

The newest android NDK ships with headers for 1.3.237. Even if they update I'm not sure if Google will be allowing vk video on Android anyway.

[dyphire]

vulkan hwdec still crashes on nvidia.

This can be confirmed on windows.

[Jules-A]

Vulkan HWDec has a decent bit lower performance on my 6700XT (on Windows 10), not as bad as crashing at least but software performs much better (though irrelevant if they are enabling hwdec) and so does vulkan-copy (slightly) with rather heavy shaders.

[kasper93]

vulkan hwdec still crashes on nvidia.

It has been know issue for months, there have been patches for ffmpeg, clearly not going to be fixed, so what should we do? Wait till next decade? Noted, though.

The newest android NDK ships with headers for 1.3.237.

Good, so need to check that still. I was wondering what version they have.

Even if they update I'm not sure if Google will be allowing vk video on Android anyway.

No, they won't allow Vulkan Video implementations, but it is runtime limitation, headers will still contain those definitions.

https://source.android.com/docs/compatibility/14/android-14-cdd#7142_vulkan

[C-1-11] MUST NOT enumerate support for the VK_KHR_video_queue, VK_KHR_video_decode_queue, or VK_KHR_video_encode_queue extensions.

EDIT:

Vulkan HWDec has a decent bit lower performance on my 6700XT (on Windows 10), not as bad as crashing at least but software performs much better (though irrelevant if they are enabling hwdec) and so does vulkan-copy (slightly) with rather heavy shaders.

hwdec is often slower, it is not the objective for hardware decoding. Power consumption is. Also I'm not proposing enabling hwdec by default. Your point is moot.

EDIT2:

The newest android NDK ships with headers for 1.3.237.

r27 RC 1 has 1.3.275. So next release we have it.

[Andarwinux]

It has been know issue for months, there have been patches for ffmpeg, clearly not going to be fixed, so what should we do? Wait till next decade? Noted, though.

vkvideo is currently just cause crashes or build issues on multiple platforms, vaapi has been improved recently and vkvideo doesn't even provide any performance advantage, I'm not sure what the point of allow vkvideo by default is exactly in this case, everyone have to build mpv or ffmpeg with ot patch.

hwdec is often slower, it is not the objective for hardware decoding. Power consumption is. Also I'm not proposing enabling hwdec by default. Your point is moot.

For those who just use hwdec=auto, this is obviously a regression as well.

[kasper93]

vkvideo is currently just cause crashes or build issues on multiple platforms, vaapi has been improved recently and vkvideo doesn't even provide any performance advantage, I'm not sure what the point of allow vkvideo by default is exactly in this case, everyone have to build mpv or ffmpeg with ot patch.

In this case Vulkan Video is bloat and should be removed from mpv. In other case it should be consistent and selected as documentation says, not hidden implicitly, because it was not added to autoprobe.

For those who just use hwdec=auto, this is obviously a regression as well.

What do you mean? Do you think d3d11va-copy is faster than native vulkan without copy?

[Dudemanguy]

Adding it to the autoprobe is fine, but it seems premature to put it in the safe whitelist and I don't see a reason it should be first in the list. Maybe before or after vaapi imo.

[kasper93]

Adding it to the autoprobe is fine, but it seems premature to put it in the safe whitelist and I don't see a reason it should be first in the list.

Not sure I agree, if it is unstable, it is unstable and shouldn't be used at all. I don't see any development around this topic lately, it really stabilized in the sense things are implemented. So unless someone put pressure to fix remaining bugs it will just stagnate.

I don't see a reason it should be first in the list.

Unlike all other APIs, and similar to d3d11va and dxva, vulkan video is tied to gpu-api, at least to some degree. So it should be probed first, to select it. Else it will never be used, because we will fallback to some -copy variant or something else. On Windows we would use d3d11va-copy which is always available, on *nix we likely will just use vaapi, so whatever.

Either way, I don't push for this. But it seems weird to have high visibility feature is intentionally hidden, it is not our fault it has problems and it highly depends on platform/hardware. We don't have any driver/vendor specific exclusions, so here it shouldn't matter either. I understand there were transition period until things are implemented/integrated, stable version of software is released, but we are past that point.

hwdec always get push back.

[kasper93]

/remindme in 5 years

[norinoriko]

I think it's worth considering that most proprietary gpu vendors aren't going to bother updating or fixing vulkan video if the feature isn't visible in the first place. Do vendors care about dedicated media players more than hardware decoding on browsers? Probably not, but it's still worth thinking about.

[Dudemanguy]

I can't speak for the situation on windows but on *nix it's entirely possible to autoprobe vulkan. In fact, my own card would autoprobe vulkan as long as I am using --gpu-api=vulkan because it doesn't have the modifier support necessary for vaapi to work on vulkan. --hwdec=vulkan works just fine though.

[kasper93]

To explain myself, because I may be ranting too much. Vulkan Video suppose to be this one API that rule them all, the one that is cross platform, support everything and doesn't need additional libraries, except Vulkan itself. That's why I think it is good candidate to be positioned as this one API to use.

I might update this just without safe flag, to keep it for --hwdec=auto.

because it doesn't have the modifier support necessary for vaapi to work on vulkan

So it will fallback to vaapi-copy and that's all.

[Dudemanguy]

There's no reason it couldn't be before vaapi-copy.

[philipl]

It is certainly reasonable to make it work for auto. We already caveat that mode enough that none of these issues are blockers.

Is there any value in moving the minimum vulkan headers up to 1.3.237? I forget if we have any other conditions that would simplify.

[kasper93]

Is there any value in moving the minimum vulkan headers up to 1.3.237? I forget if we have any other conditions that would simplify.

I don't think so. We can wait for next NDK, it will have 275.

[na-na-hi]

Unlike all other APIs, and similar to d3d11va and dxva, vulkan video is tied to gpu-api, at least to some degree. So it should be probed first, to select it.

The problem is that with d3d11, there are no other usable hwdec other than d3d11va and dxva, so it's fine to place it first. But for vulkan there are other hwdec usable. I see no reason why it should be placed above nvdec for example which works perfectly with vulkan.

[kasper93]

The problem is that with d3d11, there are no other usable hwdec other than d3d11va and dxva, so it's fine to place it first. But for vulkan there are other hwdec usable. I see no reason why it should be placed above nvdec for example which works perfectly with vulkan.

I see no reason why we should prefer one vendor (nvidia) specific codec over more open and free alternatives.

[na-na-hi]

I think vendor-specific codecs should be priortized over vendor-neutral alternatives since these are usually implemented as a less efficient wrapper over the vendor-specific ones. There is no reason not to use them since these code already exist.

The only benefit I see with vendor-neutral alternatives is being able to remove vendor-specific codecs. Is this in the plan?

[kasper93]

vendor-neutral alternatives since these are usually implemented as a less efficient wrapper over the vendor-specific ones

I don't think this is valid, generally they are implemented in driver anyway, but the thing is vendor specific may have more features, better support, in case things are not standardized or standardized in different ways.

The only benefit I see with vendor-neutral alternatives is being able to remove vendor-specific codecs. Is this in the plan?

In perfect world, but as we can see Vulkan is unusable and this will likely not changes. So no, there is no plans like that.

[philipl]

I think vendor-specific codecs should be priortized over vendor-neutral alternatives since these are usually implemented as a less efficient wrapper over the vendor-specific ones. There is no reason not to use them since these code already exist.

The only benefit I see with vendor-neutral alternatives is being able to remove vendor-specific codecs. Is this in the plan?

The exact implementation is going to vary by vendor, but we know that in Mesa, all the vulkan video implementations are not wrappers around vaapi, and we know that nvidia's implementation is directly against the low level nvdec api - meaning that is more efficient than the public cuda based nvdec api.

In practice, the way vulkan video works makes it incredibly difficult to implement as a wrapper over other implementations because of the way resource management is done.

[Andarwinux]

What do you mean? Do you think d3d11va-copy is faster than native vulkan without copy?

This is a fact, see 13309.

I see no reason why we should prefer one vendor (nvidia) specific codec over more open and free alternatives.

At least this prevents crashes on nvidia. Of course, intel windows will still have problems.

[sfan5]

I don't think so. We can wait for next NDK, it will have 275.

That's still not a reason to do it. I think the result of weighing "we can save 4 LOC" against "users potentially can't build mpv without annoying workaround" should be obvious.

another counterpoint: libplacebo itself only requires vk1.2 and then mpv should force the user to install vk1.3 headers for no reason?

[kasper93]

I think the result of weighing "we can save 4 LOC" against "users potentially can't build mpv without annoying workaround" should be obvious.

I have already explained. Even Debian stable has the correct headers. NDK is the last feasible target that doesn't have them, and we can wait for it. If you are concerned about any specific platform or user, feel free to let me know, but I won't respond to mythical cases that don't exist in the real world.

It is not about saving 4 LOC, it is about shipping mpv binaries that are not crippled, by disabling features that only require build time headers to work.

another counterpoint: libplacebo itself only requires vk1.2 and then mpv should force the user to install vk1.3 headers for no reason?

libplacebo requires 1.3 headers. FFmpeg requires 1.3.277.

[sfan5]

I don't have a concrete platform in mind. But by merging these features what you are saying is "if you don't have vk1.3 headers then you can't use even just the vulkan rendering", for which there is no technical basis and which I think is unnecessary.

It is not about saving 4 LOC, it is about shipping mpv binaries that are not crippled, by disabling features that only require build time headers to work.

Well, where are these crippled mpv builds in the real world?

libplacebo required 1.3 headers.

libplacebo has a transparent way of using vendored headers in case the OS headers are too old. This would alleviate the concerns but do you want to port that to mpv?

[kasper93]

if you don't have vk1.3 headers then you can't use even just the vulkan rendering

Exactly, same as in FFmpeg case.

libplacebo has a transparent way of using vendored headers in case the OS headers are too old. This would alleviate the concerns but do you want to port that to mpv?

I don't think this is necessary, as we don't even know what platform this solution should target. Like I said, let me know, I will add it to CI, so we can track the availability and think about solutions.

[sfan5]

Exactly, same as in FFmpeg case.

Not really. The thing is: excluding more fringe cases like -vf hwupload,yadif_cuda,hwdownload ffmpeg has exactly one use case for being compiled with vulkan and that is Vulkan Video Decoding. Meanwhile mpv has a big and obvious use for vulkan, which works perfectly fine completely independent from hwdec (which isn't even default). And hwdec is what requires headers >=1.3.227.

And again: Where are these real world crippled mpv builds that you want to avoid? Basing changes on real world situation rather than theory is reasonable, but this also applies to the argument for doing it in the first place.

[kasper93]

ffmpeg has exactly one use case for being compiled with vulkan and that is Vulkan Video Decoding.

Not really, ffmpeg has 13 Vulkan filters, which are directly comparable to our rendering. https://ffmpeg.org/ffmpeg-filters.html#Vulkan-Video-Filters Those does not require Vulkan Video.

Yet, they are not splitting it into each use-case, simply because the cost of acquiring the correct headers is almost nothing and doesn't depend on anything that may block such update. And maintaining support for old headers is only a burden. Sure in mpv case those are "4 LOC" but that's not the point.

And again: Where are these real world crippled mpv builds that you want to avoid?

I though it is rhetorical question. I already told that almost everywhere the correct headers are available, so the check is not needed. But yes, I think it is badly hidden, that you can enable Vulkan in mpv, have both ffmpeg and libplacebo compiled with Vulkan Video support and yet, mpv can be not functional in this area. There is no reason to do so.

I already agreed that we should wait for next NDK release, even if Vulkan Video doesn't matter there. But apart from that I just see no reason to keep our workaround.

[sfan5]

I am still against including the second commit.

[kasper93]

I am still against including the second commit.

It fixes compilation errors though.

[sfan5]

If we have code that depends on vk1.3 headers without being behind HAVE_VULKAN_INTEROP we should fix that.

[kasper93]

If we have code that depends on vk1.3 headers without being behind HAVE_VULKAN_INTEROP we should fix that.

I don't know how to fix that. https://github.com/mpv-player/mpv-build/issues/234 Feel free to send patches for ffmpeg to support older Vulkan headers. I think Lynne has no interest in maintaining that, given those header versions are available virtually anywhere, but if there is maintainer for older version compatibility code, they should accept that.

It could also be some mechanism to negotiate/advertise what version of headers is required to use certain headers, so libplacebo doesn't include those headers and fallback to no vulkan, but this would require to agree on some way to do that between ffmpeg/libplacebo/mpv.

[sfan5]

I don't know how to fix that. mpv-player/mpv-build#234

ffmpeg brings an implicit requirement that hwcontext_vulkan.h only works with vk1.3 headers. however libplacebo unconditionally includes it. so this looks like the issue lies there and AFAICT this PR will not fix the build error.

[kasper93]

It fixes it by requiring correct headers. A mixed header version is what causes problems. It is way more readable for users to notify about it in meson instead error out during build.

[sfan5]

Ah I get it now. libplacebo vendors headers and builds fine on its own, but the headers it exports still depend on vk1.3. This is tricky and I don't see a good way out of it, plus it doesn't really matter. Consider my concern solved then.

[kasper93]

libplacebo vendors headers and builds fine on its own, but the headers it exports still depend on vk1.3. This is tricky and I don't see a good way out of it, plus it doesn't really matter. Consider my concern solved then.

This is even more tricky, because libplacebo itself doesn't do anything wrong, it depends on libavutil/hwcontext_vulkan.h and it was working fine, but since very recently this header from ffmpeg requires vk1.3, specifically VkVideoCodecOperationFlagBitsKHR definition which is added in 1.3.238.

And there is even a check to see if we have headers https://code.videolan.org/videolan/libplacebo/-/blob/82bf46ae8b4cacd2523f994da292e4d12312c026/src/include/libplacebo/utils/libav_internal.h#L42-45

Doing something like, might make it work. I didn't think much, because my idea was to just sync up required header version between 3 projects and avoid weirdness. @haasn do you have comment on that? Other libplacebo clients might be affected, so regardless what mpv does, it is fragile.

diff --git a/src/include/libplacebo/utils/libav_internal.h b/src/include/libplacebo/utils/libav_internal.h
index a754e98b..7c849ae4 100644
--- a/src/include/libplacebo/utils/libav_internal.h
+++ b/src/include/libplacebo/utils/libav_internal.h
@@ -39,8 +39,9 @@
 # endif
 #endif

-#if LIBAVUTIL_VERSION_INT >= AV_VERSION_INT(57, 8, 100) && \
-    defined(PL_HAVE_VULKAN) && defined(VK_API_VERSION_1_2)
+#if defined(PL_HAVE_VULKAN) && \
+    (LIBAVUTIL_VERSION_INT >= AV_VERSION_INT(57, 8, 100) && defined(VK_API_VERSION_1_2) ||
+     LIBAVUTIL_VERSION_INT >= AV_VERSION_INT(59, 34, 100) && VK_HEADER_VERSION_COMPLETE >= VK_MAKE_API_VERSION(1, 3, 238))
 # define PL_HAVE_LAV_VULKAN
 # include <libavutil/hwcontext_vulkan.h>
 # include <libplacebo/vulkan.h>

[kasper93]

To summary recent discussions with @jeeb and @haasn. We think that libplacebo / ffmpeg could do better to not impose dependency in public headers on new vulkan headers or at least have a mechanism to error out during configure time on our end. But it is unclear what would be the best solution. One idea is to move away from header only libav helper and add a separate target that wouldn't transitively import lavu headers. Either way, this is not a small hotfix and likely won't be release soon as stable libplacebo. (well it is a small change, but would be breaking for all users, so not really back-portable to current release)

I understand bumping header dependency globally is maybe not that great, but I think the availability is good enough to allow us do that.

Currently not only build is an issue, but also the fact that after building -Dvulkan=enabled -Dvulkan-interop=disabled there is not even an warning message why vulkan decoding doesn't work. All we get is info vd: Could not create device.. And obviously this is fixable by adding more checks, maybe even completely disabling vulkan decode in this case, but frankly this work is just shaving a yak. That's why I still support removing the vulkan-interop option.

EDIT:

This response is prompted by discussion here https://github.com/mpv-player/mpv/discussions/13909?sort=old#discussioncomment-10468656 Fedora 40, which having headers 1.3.275 still have -Dvulkan-interop=disabled, while it could be enabled to support.

EDIT2:

Actually I have small idea how to improve meson.build. Might try later.

[kasper93]

I think reducing the complexity in buildsystem options in itself makes this changes worthwhile. Required headers are wildly available.