ndk icon indicating copy to clipboard operation
ndk copied to clipboard

[FR] Update Vulkan headers in sysroot of NDK r27 to enable FFmpeg's Vulkan integration

Open Javernaut opened this issue 1 year ago • 19 comments
trafficstars

Description

The latest NDK r27 beta 1 (27.0.11718014-beta1) has Vulkan headers updated to 275.

FFmpeg - a popular solution for working with media content - has Vulkan integration. However, the latest stable FFmpeg 7.0 requires the Vulkan headers of version 277. Is it possible to bump the Vulkan headers in r27 just a bit before it goes released?

In case the version of Vulkan in NDK is aligned with the Validation Layers, then probably 1.3.280 will be a better option.

Javernaut avatar Apr 18 '24 08:04 Javernaut

I can't go any faster than the OS, and this is what the OS currently has. Those updates are typically aligned with OS release cycles, so that's probably as new as it gets for r27. r28 may have something newer though (certainly we will eventually, it's just totally out of my hands so I can't say for certain when it will happen).

What specifically does ffmpeg need from the newer version? If it's new APIs that won't help you anyway, because those new APIs won't exist on any devices that you're targeting yet.

DanAlbert avatar Apr 18 '24 19:04 DanAlbert

(I'm confirming all that with the guy that actually does these updates, but I'm pretty sure those updates are expensive and so an out-of-cycle upgrade is not likely)

DanAlbert avatar Apr 18 '24 19:04 DanAlbert

A brief check of FFmpeg's history shows that in the commit where they bumped 255 to 277, they introduced the usage of VK_KHR_VIDEO_DECODE_AV1_EXTENSION_NAME extension. This extension isn't available in 275.

Screenshot 2024-04-18 at 22 09 43

However, the extension is considered opitonal. So I believe a simple bump of vulkan in NDK will make the latest FFmpeg compilable.

By the way, I have also checked the previous version of FFmpeg (6.1.1, requires vulkan 255). It is actually compilable with ndk r27-beta1 for arm64 and x86_64, but not for 32 bits.

Javernaut avatar Apr 18 '24 20:04 Javernaut

I'm waiting to hear back if it's possible, but the window for changes in r27 is closing fast, and I think vulkan upgrades in the OS are tricky.

DanAlbert avatar Apr 18 '24 20:04 DanAlbert

Of course, this is more about compilability of FFmpeg out of the box.

Vulkan versions on end devices fall short from the actual latest version. My private device with Android 14 has Vulkan 1.1.128 (5 y.o.), for example. Not complaining, just saying I understand that simple headers update in NDK won't deliver the updated Vulkan's binaries to end devices.

Javernaut avatar Apr 18 '24 20:04 Javernaut

Of course, this is more about compilability of FFmpeg out of the box.

Which is a big deal, IMO. We'll do it if we can. If FFmpeg often increases the minimum they require though, we'll always be lagging, especially in the LTS where the header will be 12 months old or more toward the end of the cycle.

There's some ongoing work to make the default path for vulkan to be getting the SDK straight from LunarG (which would mean you're no longer dependent on an old NDK having new tools), but I don't know if that included vulkan.h.

Vulkan versions on end devices fall short from the actual latest version

I actually hadn't seen that data before (not that it's surprising). Thanks!

DanAlbert avatar Apr 18 '24 21:04 DanAlbert

Is the vulkan integration a default part of ffmpeg, or is that an extension you're trying to enable (I know they have a lot of config knobs). Apparently none of the vulkan video APIs will work on Android anyway. If it's in the default config for ffmpeg, it's annoying that it won't build out of the box (and I'd suggest that ffmpeg switch to default off for Android), but the runtime behavior with either the fix or explicitly disabling vulkan video during configure would be identical. If it's default off and you're passing an explicit enable flag, there's apparently no point in doing that.

That said, they say it's probably possible to get the header updated in time for r27. They agree with me though that apps requiring latest vulkan headers are always going to run into this sort of problem though :( We can fix it now, but it might break again shortly after release.

DanAlbert avatar Apr 18 '24 22:04 DanAlbert

Being tracked internally at http://b/335709592, I'll forward anything interesting here, but I don't expect anything interesting to happen other than either a fix or a "next release".

DanAlbert avatar Apr 18 '24 22:04 DanAlbert

Thanks @DanAlbert

Is the vulkan integration a default part of ffmpeg

No, FFmpeg can live without Vulkan being enabled. Actually, for a long time it was easier to disable it completely. Here is the statistics of Vulkan in Android from 2021.

As I understand, the top FFmpeg version (with Vulkan enabled) that can be compiled with NDK r26 is 6.0. By bumping Vulkan in NDK r27 to 277+, both FFmpeg 6.1 and 7.0 will be covered.

FFmpeg considers optional features of Vulkan and its integration started with 1.1.97, which is actually supported by many Android devices nowadays. And I think enabling 'at least something' of Vulkan is better than disabling it completely.

Of course, it is possible to stick to older FFmpeg release, it is just 7.0 got Android's content protocol support, attracting more attention.

Javernaut avatar Apr 19 '24 09:04 Javernaut

Okay, so the default configuration of ffmpeg (that is, no --with-feature flags when you run configure) does require the updated header, even if vulkan won't be used at runtime? I'm trying to confirm whether the latest ffmpeg is incompatible with the current header out of the box, or only in some configurations. Sounds like yes, by default.

DanAlbert avatar Apr 19 '24 18:04 DanAlbert

From the start Vulkan integration was disabled by default. At some point it was changed to 'autodetect'. The autodetection works like this: the configure script checks the Vulkan’s availability and if it is available, then it will be considered enabled for the build. The autodetection isn’t specific to Vulkan, all external libraries are checked in some way. For Vulkan specifically the configure script initially would check the presence of the vulkan/vulkan.h file and later the check for its version was added.

Frankly speaking up until now I overlooked the fact that Vulkan is autodetected rather than enabled by default.

When Vulkan in NDK passes the check of configure script, then the real compilation of Vulkan’s integration happens. And from the tests I’ve done I can say (as funny as it is) that NDK r27 already cripples FFmpeg 6.1.x default compilation, since Vulkan is autodetected as 'enabled’, but the compilation fails for 32 bit archs. With NDK r26 the FFmpeg 6.1.x doesn’t detect the Vulkan and everything is green. If Vulkan is updated to 277+, then FFmpeg 7.0 will also autodetect it to 'enabled' and potentially fail for some archs.

That being said, I still think updating of Vulkan in NDK is something desirable and the compilation errors are more to FFmpeg itself to fix. And these errors are out of the scope of this particular issue. Consumers still can use --disable-vulkan as a fix for some time.

Javernaut avatar Apr 20 '24 08:04 Javernaut

Yes, up to date Vulkan headers are of course something we want :) No argument there.

It does sound like ffmpeg's autodetection is wrong though. Presence of those headers has nothing to do with the availability of the API at runtime. This isn't just pedantic, from what I'm told, those APIs do not work on any Android device.

DanAlbert avatar Apr 22 '24 19:04 DanAlbert

It does sound like ffmpeg's autodetection is wrong though.

at the very least, it seems like they should be looking for the newest constant/function/type they actually need, since they're obviously dependent on a specific version (just to build), rather than just "does the header exist?".

enh-google avatar Apr 22 '24 20:04 enh-google

It looks like it's trying to do that, actually:

if enabled vulkan; then
  check_pkg_config_header_only vulkan "vulkan >= 1.3.277" "vulkan/vulkan.h" "defined VK_VERSION_1_3" ||
      check_cpp_condition vulkan "vulkan/vulkan.h" "defined(VK_VERSION_1_4) || (defined(VK_VERSION_1_3) && VK_HEADER_VERSION >= 277)"
fi

Is that wrongly finding and passing with the host's header?

DanAlbert avatar Apr 22 '24 20:04 DanAlbert

Is that wrongly finding and passing with the host's header?

When the cross-compilation of FFmpeg for Android is set one must use the --sysroot parameter for the configure script. One should use the sysroot from NDK ${NDK}/toolchains/llvm/prebuilt/${HOST_TAG}/sysroot, right? This is where the Vulkan is found - in NDK. So this is not the host's header, but exatly the NDK's.

Javernaut avatar Apr 23 '24 05:04 Javernaut

do you have whatever the cmake equivalent of the configure.log file is? because it doesn't make sense that

if enabled vulkan; then
  check_pkg_config_header_only vulkan "vulkan >= 1.3.277" "vulkan/vulkan.h" "defined VK_VERSION_1_3" ||
      check_cpp_condition vulkan "vulkan/vulkan.h" "defined(VK_VERSION_1_4) || (defined(VK_VERSION_1_3) && VK_HEADER_VERSION >= 277)"
fi

would pass with our current headers...

enh-google avatar Apr 23 '24 14:04 enh-google

No CMake equivalent, only this explanation:

During the configure script execution the ffbuild/config.log file is generated. For the FFmpeg 6.1.1 (which expects the Vulkan 255) and NDK r27-beta1 (which has Vulkan 275) the config.log file will have such section:

check_pkg_config_cpp vulkan vulkan >= 1.3.255 vulkan/vulkan.h defined VK_VERSION_1_3
test_pkg_config_cpp vulkan vulkan >= 1.3.255 vulkan/vulkan.h defined VK_VERSION_1_3
/opt/homebrew/bin/pkg-config --exists --print-errors vulkan >= 1.3.255
Package vulkan was not found in the pkg-config search path.
Perhaps you should add the directory containing `vulkan.pc'
to the PKG_CONFIG_PATH environment variable
No package 'vulkan' found
check_cpp_condition vulkan vulkan/vulkan.h defined(VK_VERSION_1_4) || (defined(VK_VERSION_1_3) && VK_HEADER_VERSION >= 255)
test_cpp_condition vulkan/vulkan.h defined(VK_VERSION_1_4) || (defined(VK_VERSION_1_3) && VK_HEADER_VERSION >= 255)
test_cpp
BEGIN /var/temp_dir/test.c
    1	#include <vulkan/vulkan.h>
    2	#if !(defined(VK_VERSION_1_4) || (defined(VK_VERSION_1_3) && VK_HEADER_VERSION >= 255))
    3	#error "unsatisfied condition: defined(VK_VERSION_1_4) || (defined(VK_VERSION_1_3) && VK_HEADER_VERSION >= 255)"
    4	#endif
END /var/temp_dir/test.c
/path/to/ndk/27.0.11718014/toolchains/llvm/prebuilt/darwin-x86_64/bin/armv7a-linux-androideabi21-clang --sysroot=/path/to/ndk/27.0.11718014/toolchains/llvm/prebuilt/darwin-x86_64/sysroot -D_ISOC99_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Dstrtod=avpriv_strtod -DPIC -O3 -fPIC -I/path/to/additional/but/empty/include -std=c11 -fPIE -fomit-frame-pointer -fPIC -marm -pthread -E -o /var/temp_dir/test.o /var/temp_dir/test.c

The original check includes check_pkg_config_header_only() and check_cpp_condition(), where the first fails, but the second actually succeeds. And that makes the configure script believe the proper Vulkan sources are available.

Just in case, all the functions like check_pkg_config_header_only(), check_cpp_condition(), test_cpp_condition() and others are defined in the same configure file.

If you try compiling FFmpeg 7.0 (which expects Vulkan 277) with the same NDK r27-beta1, then the second check fails too, as the test.c file contains 277 as the value to check with. No Vulkan support is considered after that.

Javernaut avatar Apr 23 '24 20:04 Javernaut

They've been working on it but ran into some issues that slowed down the update, and it looks like it won't happen in time for r27.

DanAlbert avatar Jun 25 '24 19:06 DanAlbert

Looks like Android's still on 275. I'll leave the bug here in case that's updated before we stop taking sysroot updates for r28, but I expect this will have to be punted.

A fix on the ffmpeg side the not require that decl is probably the faster solution. The vulkan header in the NDK belongs to the OS, and the OS doesn't have a need to upgrade more than annually. Unless something goes horribly wrong that would make the OS skip upgrading for a release, it'll certainly be fixed some time in the next year, but I unfortunately can't be more accurate than that because it's out of my hands.

DanAlbert avatar Aug 01 '24 21:08 DanAlbert