search

haasn / libplacebo

Official mirror of libplacebo
http://libplacebo.org/
GNU Lesser General Public License v2.1
524 stars 63 forks source link

Vulkan-Headers not recent enough for FFmpeg master #196

Closed zeromind closed 10 months ago

zeromind commented 10 months ago

Since recently, FFmpeg requires at least version 1.3.255 of the Vulkan-Headers: https://patchwork.ffmpeg.org/project/ffmpeg/patch/NbeCLQn--3-9@lynne.ee/

libplacebo currently still has 1.3.243 as submodule, which causes failures trying to build ffmpeg master: e.g. mpv-build with --enabled-libplacebo for ffmpeg

[...]
CC  libavfilter/vf_libplacebo.o
In file included from src/libavutil/vulkan.h:30,
                 from src/libavfilter/vulkan.h:22,
                 from src/libavfilter/vulkan_filter.h:26,
                 from src/libavfilter/vf_libplacebo.c:29:
src/libavutil/vulkan_functions.h:223:5: error: unknown type name 'PFN_vkGetPhysicalDeviceCooperativeMatrixPropertiesKHR'
  223 |     PFN_vk##name name;
      |     ^~~~~~
src/libavutil/vulkan_functions.h:84:5: note: in expansion of macro 'PFN_DEF'
   84 |     MACRO(1, 0, FF_VK_EXT_COOP_MATRIX,          GetPhysicalDeviceCooperativeMatrixPropertiesKHR) \
      |     ^~~~~
src/libavutil/vulkan_functions.h:227:5: note: in expansion of macro 'FN_LIST'
  227 |     FN_LIST(PFN_DEF)
      |     ^~~~~~~
In file included from src/libavutil/vulkan.h:32:
src/libavutil/vulkan_loader.h: In function 'ff_vk_extensions_to_mask':
src/libavutil/vulkan_loader.h:49:11: error: 'VK_KHR_COOPERATIVE_MATRIX_EXTENSION_NAME' undeclared (first use in this function); did you mean 'VK_NV_COOPERATIVE_MATRIX_EXTENSION_NAME'?
   49 |         { VK_KHR_COOPERATIVE_MATRIX_EXTENSION_NAME,        FF_VK_EXT_COOP_MATRIX            },
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |           VK_NV_COOPERATIVE_MATRIX_EXTENSION_NAME
src/libavutil/vulkan_loader.h:49:11: note: each undeclared identifier is reported only once for each function it appears in
src/libavutil/vulkan.h: At top level:
src/libavutil/vulkan.h:239:5: error: unknown type name 'VkPhysicalDeviceCooperativeMatrixPropertiesKHR'
  239 |     VkPhysicalDeviceCooperativeMatrixPropertiesKHR coop_matrix_props;
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/libavutil/vulkan.h:245:5: error: unknown type name 'VkCooperativeMatrixPropertiesKHR'
  245 |     VkCooperativeMatrixPropertiesKHR *coop_mat_props;
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
make: *** [/home/zeromind/src/github.com/mpv-player/mpv-build3/ffmpeg/ffbuild/common.mak:81: libavfilter/vf_libplacebo.o] Error 1
make: Leaving directory '/home/zeromind/src/github.com/mpv-player/mpv-build3/ffmpeg_build'

Could you please bump the headers/submodule?

haasn commented 10 months ago

As mentioned on IRC, I don't think this is a libplacebo issue. The headers we compile vulkan against internally have no reason to leak outside of the libplacebo build system.

I think mpv-build is mistaken in somehow propagating these headers to the rest of the system.

kasper93 commented 10 months ago

Just to clarify, because it may seem strange to ffmpeg uses libplacebo's provided headers.

This only happens if libplacebo itself did not found vulkan headers during build and used internal ones as a fallback. Can be workaround by providing proper vulkan headers during build.

As mentioned on IRC, I don't think this is a libplacebo issue. The headers we compile vulkan against internally have no reason to leak outside of the libplacebo build system.

I think mpv-build is mistaken in somehow propagating these headers to the rest of the system.

Not mpv-build's fault. Meson adds dependency headers to pkg-config file. Because libplacebo depends on vulkan headers in public headers. So otherwise it would be unusable after build, if there are no other headers in the system, which we know because libplacebo did not found ones.

haasn commented 10 months ago

Not mpv-build's fault. Meson adds dependency headers to pkg-config file. Because libplacebo depends on vulkan headers in public headers. So otherwise it would be unusable after build, if there are no other headers in the system, which we know because libplacebo did not found ones.

This makes no sense - we only use this header for compiling internal code. Public code wouldn't have a reason to include <libplacebo/vulkan.h> if no vulkan headers are available. Can we disable this meson behavior? I consider it a bug to leak internal headers like this.

kasper93 commented 10 months ago

Public code wouldn't have a reason to include <libplacebo/vulkan.h> if no vulkan headers are available.

What does this even mean? There is no difference between the internal headers build and external headers one. If that's intended behavior we shouldn't ship internal headers.

Can we disable this meson behavior? I consider it a bug to leak internal headers like this.

Yes, l see your point. But conceptually it is some header dependency that we have/define. It happens to be vulkan, but can be anything else that is not available in the system.

I guess what can be done is not to declare it as dependency and instead add include dir manually to args. Then pc file wouldn't generate it. But then again it forces user to provide headers manually instead.

It is annoying without https://github.com/mesonbuild/meson/pull/10122 but it is meson, everything is annoying at one point, it is not really robust...

kasper93 commented 10 months ago

Ok, nevermind... we prefer our own headers over system ones. It is a bug.

zeromind commented 10 months ago

This only happens if libplacebo itself did not found vulkan headers during build and used internal ones as a fallback.

I have 1.3.261 of the headers installed via system packages, and the build still failed. Seems libplacebo prefers the 3rdparty/submodule headers over ones provided by the system: https://github.com/haasn/libplacebo/blob/d6f1db186d535f3d99d56165bb468de71d58dc6e/src/vulkan/meson.build#L7-L16

mpv-build inits all submodules by default, whereas it seem libplacebo expects them to only be present if they are not provided by the system, or the internal ones should be preferred.

haasn commented 10 months ago

Ok, nevermind... we prefer our own headers over system ones. It is a bug.

I'm not convinced this is a bug. As long as we don't leak internal headers to pc, preferring our internal headers (that we know are safe to use) shouldn't be a bad thing.

But, I agree that swapping the order of this dependency resolution may be the simplest work-around to the meson limitation.

kasper93 commented 10 months ago

I'm not convinced this is a bug. As long as we don't leak internal headers to pc, preferring our internal headers (that we know are safe to use) shouldn't be a bad thing.

No, I mean the leaking headers in this case is a bug. Because there are system ones that we ignore and force internal upon users.

kasper93 commented 10 months ago

This should help https://code.videolan.org/videolan/libplacebo/-/merge_requests/565

I must say I really don't like that meson has no concept of private/public dependencies like cmake. Need to pass around the args like caveman...

haasn commented 10 months ago

Fixed by 5914fb6d

monarc99 commented 10 months ago

With the 3rdparty/Vulkan-Headers submodule initialized /cloned, it compiles fine.

If the 3rdparty/Vulkan-Headers submodule is not initialized (dir empty), it finds the system vulkan headers on my system, but stops with an error.

... Run-time dependency vulkan found: YES 1.3.255

src/vulkan/meson.build:18:44: ERROR: Unknown variable "vulkan_headers_inc". ...