Git Product home page Git Product logo

Comments (19)

DanAlbert avatar DanAlbert commented on August 22, 2024 1

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!

from ndk.

DanAlbert avatar DanAlbert commented on August 22, 2024 1

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.

from ndk.

DanAlbert avatar DanAlbert commented on August 22, 2024

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.

from ndk.

DanAlbert avatar DanAlbert commented on August 22, 2024

(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)

from ndk.

Javernaut avatar Javernaut commented on August 22, 2024

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.

from ndk.

DanAlbert avatar DanAlbert commented on August 22, 2024

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.

from ndk.

Javernaut avatar Javernaut commented on August 22, 2024

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.

from ndk.

DanAlbert avatar DanAlbert commented on August 22, 2024

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.

from ndk.

DanAlbert avatar DanAlbert commented on August 22, 2024

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

from ndk.

Javernaut avatar Javernaut commented on August 22, 2024

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.

from ndk.

DanAlbert avatar DanAlbert commented on August 22, 2024

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.

from ndk.

Javernaut avatar Javernaut commented on August 22, 2024

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.

from ndk.

DanAlbert avatar DanAlbert commented on August 22, 2024

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.

from ndk.

enh-google avatar enh-google commented on August 22, 2024

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

from ndk.

DanAlbert avatar DanAlbert commented on August 22, 2024

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?

from ndk.

Javernaut avatar Javernaut commented on August 22, 2024

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.

from ndk.

enh-google avatar enh-google commented on August 22, 2024

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

from ndk.

Javernaut avatar Javernaut commented on August 22, 2024

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.

from ndk.

DanAlbert avatar DanAlbert commented on August 22, 2024

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.

from ndk.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.