[FFmpeg-devel] configure: improve logic and checks for nvenc

On 8/30/2016 8:00 AM, Timo Rothenpieler wrote:
> ---
>  configure | 37 +++++++++++++++++++++++++------------
>  1 file changed, 25 insertions(+), 12 deletions(-)
> 
> diff --git a/configure b/configure
> index 52931c3..bcfc9a8 100755
> --- a/configure
> +++ b/configure
> @@ -5992,20 +5992,33 @@ enabled vdpau && enabled xlib &&
>      check_lib2 "vdpau/vdpau.h vdpau/vdpau_x11.h" vdp_device_create_x11 -lvdpau &&
>      enable vdpau_x11
>  
> -case $target_os in
> -    mingw32*|mingw64*|win32|win64|linux|cygwin*)
> -        disabled nvenc || enable nvenc
> -        ;;
> -    *)
> -        disable nvenc
> -        ;;
> -esac
> -
> -if enabled nvenc; then
> +if ! enabled x86; then
> +    enabled nvenc && die "NVENC is only supported on x86"
> +    disable nvenc
> +elif ! disabled nvenc; then
>      {
>          echo '#include "compat/nvenc/nvEncodeAPI.h"'
> -        echo 'int main(void) { return 0; }'
> -    } | check_cc -I$source_path || disable nvenc
> +        echo 'NV_ENCODE_API_FUNCTION_LIST flist;'
> +        echo 'void f(void) { struct { const GUID guid; } s[] = { { NV_ENC_PRESET_HQ_GUID } }; }'

This will most likely prevent nvenc from being enabled for msvc 2012, but not old
mingw32, which is failing with the error:

src/libavcodec/nvenc.c:115:52: error: 'ENOBUFS' undeclared here (not in a function)
     { NV_ENC_ERR_NOT_ENOUGH_BUFFER,        AVERROR(ENOBUFS), "not enough buffer"        },

I think the easiest solution would be using AVERROR_BUFFER_TOO_SMALL if ENOBUFS is
not defined.
That or just disable nvenc if using mingw32 toolchains by checking "enabled
libc_mingw32", since disabling for target-os == mingw32 would also affect mingw-w64.

gcc-asan fails with

/usr/bin/ld: libavcodec/libavcodec.a(nvenc.o): undefined reference to symbol 'dlsym@@GLIBC_2.2.5'
/usr/lib/../lib/libdl.so.2: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status

I have no idea how to deal with this.

> +        echo 'int main(void) { f(); return 0; }'
> +    } | check_cc -I$source_path
> +    nvenc_check_res=$?
> +
> +    if [ $nvenc_check_res != 0 ] && enabled nvenc; then
> +        die "NVENC enabled but test-compile failed"
> +    fi
> +
> +    case $target_os in
> +        mingw32*|mingw64*|win32|win64|linux|cygwin*)
> +            [ $nvenc_check_res = 0 ] && enable nvenc
> +            ;;
> +        *)
> +            enabled nvenc && die "NVENC is only supported on Windows and Linux"
> +            disable nvenc
> +            ;;
> +    esac
> +
> +    unset nvenc_check_res

This test is different from other automatically detected features, and also
unnecessarily complex.
You should force enable nvenc earlier in the script like with other similar
features (including hardware codecs and accelerators), then disable it on
unsupported platforms and old/broken compilers with the corresponding checks
and tests.

Something like this:

------

------

Is IMO much simpler.

>> +        echo 'NV_ENCODE_API_FUNCTION_LIST flist;'
>> +        echo 'void f(void) { struct { const GUID guid; } s[] = { { NV_ENC_PRESET_HQ_GUID } }; }'
> 
> This will most likely prevent nvenc from being enabled for msvc 2012, but not old
> mingw32, which is failing with the error:
> 
> src/libavcodec/nvenc.c:115:52: error: 'ENOBUFS' undeclared here (not in a function)
>      { NV_ENC_ERR_NOT_ENOUGH_BUFFER,        AVERROR(ENOBUFS), "not enough buffer"        },
> 
> I think the easiest solution would be using AVERROR_BUFFER_TOO_SMALL if ENOBUFS is
> not defined.

Yes, if that's all that's failing, I'll just do that.

> That or just disable nvenc if using mingw32 toolchains by checking "enabled
> libc_mingw32", since disabling for target-os == mingw32 would also affect mingw-w64.

> gcc-asan fails with
> 
> /usr/bin/ld: libavcodec/libavcodec.a(nvenc.o): undefined reference to symbol 'dlsym@@GLIBC_2.2.5'
> /usr/lib/../lib/libdl.so.2: error adding symbols: DSO missing from command line
> collect2: error: ld returned 1 exit status
> 
> I have no idea how to deal with this.

When and how are you seeing that error?
That usually means a wrong order of libraries/object-files on linker
command line.

>> +        echo 'int main(void) { f(); return 0; }'
>> +    } | check_cc -I$source_path
>> +    nvenc_check_res=$?
>> +
>> +    if [ $nvenc_check_res != 0 ] && enabled nvenc; then
>> +        die "NVENC enabled but test-compile failed"
>> +    fi
>> +
>> +    case $target_os in
>> +        mingw32*|mingw64*|win32|win64|linux|cygwin*)
>> +            [ $nvenc_check_res = 0 ] && enable nvenc
>> +            ;;
>> +        *)
>> +            enabled nvenc && die "NVENC is only supported on Windows and Linux"
>> +            disable nvenc
>> +            ;;
>> +    esac
>> +
>> +    unset nvenc_check_res
> 
> This test is different from other automatically detected features, and also
> unnecessarily complex.
> You should force enable nvenc earlier in the script like with other similar
> features (including hardware codecs and accelerators), then disable it on
> unsupported platforms and old/broken compilers with the corresponding checks
> and tests.
> 
> Something like this:
> [...]

Ah, so even if calling enable nvenc, --disable-nvenc on the command line
will still override it, and the "disabled nvenc" check will still work?
I wasn't aware of that, so yes, that makes it a lot simpler.

Forgot this, the idea with my approach is to handle the case where
--enable-nvenc is requested, but the compile-check fails.
Just silently disabling it then seems wrong.

2016-08-31 15:03 GMT+02:00 Timo Rothenpieler <timo@rothenpieler.org>:
> Forgot this, the idea with my approach is to handle the case where
> --enable-nvenc is requested, but the compile-check fails.

> Just silently disabling it then seems wrong.

But this is what we do for all auto-detected features except xcb. If changing
this comes at the price of far more complicated checks, I suggest we keep
the current logic.

Carl Eugen

> 2016-08-31 15:03 GMT+02:00 Timo Rothenpieler <timo@rothenpieler.org>:
>> Forgot this, the idea with my approach is to handle the case where
>> --enable-nvenc is requested, but the compile-check fails.
> 
>> Just silently disabling it then seems wrong.
> 
> But this is what we do for all auto-detected features except xcb. If changing
> this comes at the price of far more complicated checks, I suggest we keep
> the current logic.

Hm, just silently disabling stuff that's explicitly requested to be
enabled via enable seems broken.
It might also result in builds which show a feature to be enabled in the
configure line they show, while it's actually disabled because of a
failed check/missing library.

2016-08-31 15:26 GMT+02:00 Timo Rothenpieler <timo@rothenpieler.org>:
>> 2016-08-31 15:03 GMT+02:00 Timo Rothenpieler <timo@rothenpieler.org>:
>>> Forgot this, the idea with my approach is to handle the case where
>>> --enable-nvenc is requested, but the compile-check fails.
>>
>>> Just silently disabling it then seems wrong.

It's what FFmpeg does for more than a decade.

>> But this is what we do for all auto-detected features except xcb. If changing
>> this comes at the price of far more complicated checks, I suggest we keep
>> the current logic.
>
> Hm, just silently disabling stuff that's explicitly requested to be
> enabled via enable seems broken.
> It might also result in builds which show a feature to be enabled in the
> configure line they show, while it's actually disabled because of a
> failed check/missing library.

This is exactly why I ask Zeranoe (and Alexis) since several years to
remove "--enable-zlib --enable-bzlib" from their configure lines, so far
my success was limiited;-(

I'd like to repeat that if this new feature comes at the price of
significantly more complicated checks in the configure script,
we should probably not change the established logic.
(Correct me if I misremember: It was tried already but resulted in
completely broken configure?)

Carl Eugen

> 2016-08-31 15:26 GMT+02:00 Timo Rothenpieler <timo@rothenpieler.org>:
>>> 2016-08-31 15:03 GMT+02:00 Timo Rothenpieler <timo@rothenpieler.org>:
>>>> Forgot this, the idea with my approach is to handle the case where
>>>> --enable-nvenc is requested, but the compile-check fails.
>>>
>>>> Just silently disabling it then seems wrong.
> 
> It's what FFmpeg does for more than a decade.

I'll follow along with nvenc for now, and might try to tackle it at a
more general level.

>>> But this is what we do for all auto-detected features except xcb. If changing
>>> this comes at the price of far more complicated checks, I suggest we keep
>>> the current logic.
>>
>> Hm, just silently disabling stuff that's explicitly requested to be
>> enabled via enable seems broken.
>> It might also result in builds which show a feature to be enabled in the
>> configure line they show, while it's actually disabled because of a
>> failed check/missing library.
> 
> This is exactly why I ask Zeranoe (and Alexis) since several years to
> remove "--enable-zlib --enable-bzlib" from their configure lines, so far
> my success was limiited;-(
> 
> I'd like to repeat that if this new feature comes at the price of
> significantly more complicated checks in the configure script,
> we should probably not change the established logic.
> (Correct me if I misremember: It was tried already but resulted in
> completely broken configure?)
> 
> Carl Eugen

The idea I'd have for this it to simply store a second variable while
parsing the enable/disable options, which states user_enabled/user_disabled.
That way checking for it becomes a mere

user_enabled feature && disabled feature && die "..."

Which could even be reduced further by introducing a function that does
exactly that.

Could even go over all disabled features at the end of configure, and
throw a warning or even an error in case something is user_enabled but
finally set to disabled.

On 8/31/2016 5:26 AM, Timo Rothenpieler wrote:
>>> +        echo 'NV_ENCODE_API_FUNCTION_LIST flist;'
>>> +        echo 'void f(void) { struct { const GUID guid; } s[] = { { NV_ENC_PRESET_HQ_GUID } }; }'
>>
>> This will most likely prevent nvenc from being enabled for msvc 2012, but not old
>> mingw32, which is failing with the error:
>>
>> src/libavcodec/nvenc.c:115:52: error: 'ENOBUFS' undeclared here (not in a function)
>>      { NV_ENC_ERR_NOT_ENOUGH_BUFFER,        AVERROR(ENOBUFS), "not enough buffer"        },
>>
>> I think the easiest solution would be using AVERROR_BUFFER_TOO_SMALL if ENOBUFS is
>> not defined.
> 
> Yes, if that's all that's failing, I'll just do that.
> 
>> That or just disable nvenc if using mingw32 toolchains by checking "enabled
>> libc_mingw32", since disabling for target-os == mingw32 would also affect mingw-w64.
> 
>> gcc-asan fails with
>>
>> /usr/bin/ld: libavcodec/libavcodec.a(nvenc.o): undefined reference to symbol 'dlsym@@GLIBC_2.2.5'
>> /usr/lib/../lib/libdl.so.2: error adding symbols: DSO missing from command line
>> collect2: error: ld returned 1 exit status
>>
>> I have no idea how to deal with this.
> 
> When and how are you seeing that error?
> That usually means a wrong order of libraries/object-files on linker
> command line.

http://fate.ffmpeg.org/history.cgi?slot=x86_64-archlinux-gcc-asan

Check the "Compile" logs for the failing runs.
A simple "configure --toolchain=gcc-asan" seems to be enough to get this error.