[FFmpeg-devel] [PATCH] configure: disable vulkan if min version insufficient

[Tristan Matthews]

On Fri, Sep 29, 2023 at 3:26 PM Timo Rothenpieler <timo at rothenpieler.org> wrote:
>
> On 29.09.2023 20:10, Tristan Matthews wrote:
> > On Fri, Sep 29, 2023 at 1:37 PM Timo Rothenpieler <timo at rothenpieler.org> wrote:
> >>
> >> On 29.09.2023 15:52, Tristan Matthews wrote:
> >>> Fixes: https://trac.ffmpeg.org/ticket/10596
> >>> ---
> >>>    configure | 3 ++-
> >>>    1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/configure b/configure
> >>> index 20db1801ed..50ba6f772f 100755
> >>> --- a/configure
> >>> +++ b/configure
> >>> @@ -7154,7 +7154,8 @@ enabled crystalhd && check_lib crystalhd "stdint.h libcrystalhd/libcrystalhd_if.
> >>>
> >>>    if enabled vulkan; then
> >>>        check_pkg_config_header_only vulkan "vulkan >= 1.3.255" "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 >= 255)"
> >>> +        check_cpp_condition vulkan "vulkan/vulkan.h" "defined(VK_VERSION_1_4) || (defined(VK_VERSION_1_3) && VK_HEADER_VERSION >= 255)" ||
> >>> +        warn "Disabling vulkan" && disable vulkan
> >>>    fi
> >>
> >> Doesn't that just always disable vulkan if any of the previous checks
> >> succeed? The logic looks weird to me.
> >
> > No, it will only disable vulkan if all the previous checks fail as
> > this conditional shortcircuits (so to get to here it would have to be
> > false || false || false || false || false || warn "Disable vulkan" &&
> > disable vulkan). If I hack the version number to match mine (changing
> > it to 1.3.239), vulkan won't be disabled (that's why I added the
> > warning message) and you'll hit the build breakage I and others are
> > hitting currently.
>
> My shell disagrees:
>
> $ function warn() { echo $1; }
> $ true || false || warn "Disabling vulkan" && echo DISABLED
> DISABLED
> $ false || true || warn "Disabling vulkan" && echo DISABLED
> DISABLED
> $ false || false || warn "Disabling vulkan" && echo DISABLED
> Disabling vulkan
> DISABLED
> $ true || true || warn "Disabling vulkan" && echo DISABLED
> DISABLED
>
> No matter the results of the checks, this will always disable Vulkan,
> since the chain before it will always have a positive outcome, so &&
> continues.

Oh I see, I should have parens around the last bit, e.g.:
false || true || (warn "Disabling vulkan" && echo DISABLED)

but as Hendrik mentioned, the warning can be dropped in which case
this would become:
check_pkg_config_header_only vulkan ... || check_cpp_condition || disable vulkan

As mentioned earlier in the thread if someone has a better idea of how
to avoid explicitly calling disable vulkan like this (i.e. why isn't
this already working) that would of course be preferable.

Best,
-t