[FFmpeg-devel] [PATCH 1/2] avcodec/nvenc: Include NVENC SDK header

[James Almer]

On 12/7/2015 3:09 PM, Timo Rothenpieler wrote:
>>> @@ -1603,6 +1603,7 @@ CONFIG_LIST="
>>>      memalign_hack
>>>      memory_poisoning
>>>      neon_clobber_test
>>> +    nvenc
>>
>> You forgot to remove nvenc from EXTERNAL_LIBRARY_LIST
> 
> Yep, forgot about that.
> 
>>> +case $target_os in
>>> +    mingw32*|mingw64*|win32|win64|linux|cygwin*)
>>> +        disabled nvenc || enable nvenc
>>> +        ;;
>>> +    *)
>>> +        disable nvenc
>>> +        ;;
>>> +esac
>>> +
>>> +if enabled nvenc; then
>>> +    {
>>> +        echo '#include "nvenc.h"'
>>> +        echo 'int main(void) { return 0; }'
>>> +    } | check_cc -I$source_path/libavcodec ||
>>> +        disable nvenc
>>> +fi
>>> +
>>
>> Is this check even needed when you're checking for compatible OSes above?
>> Not to mention nvenc.h only includes stdlib.h and then typedefs everything
>> it needs, which makes me think it should compile for any target out there.
> 
> It's kind of a sanity check to make sure it builds with the compiler
> that's beeing used, in case it doesn't work on some weird/old compiler.
> If that's not a concern, this check can be removed.

It seems to include stdlib.h for every target, then windows.h on _WIN32 and
stdint.h on MSVC.
It also apparently makes the assumption that stdlib.h alone is enough to get
all the fixed-width integer types, which may not be true for all ffmpeg targets,
so i guess this check is the safest thing to do after all.

> 
>> In any case, do instead something like
>>
>> enabled nvenc &&
>>     check_cc -I$source_path/libavcodec <<EOF || disable nvenc
>> #include "nvenc.h"
>> int x;
>> EOF
> 
> Doesn't check_cc need a main function? Or is it just about the EOF style
> vs. { echo }?

No, look at the dxva2api_h check below. And yes, it's about the EOF style, which
is more consistent with the rest of the checks.

>> [...]
>>
>>> diff --git a/libavcodec/nvenc.h b/libavcodec/nvenc.h
>>> new file mode 100644
>>> index 0000000..8b67c11
>>> --- /dev/null
>>> +++ b/libavcodec/nvenc.h
>>
>> compat/nvenc/nvenc.h? It's the proper place for non ffmpeg headers, like we
>> do with Avisynth.
> 
> Yes, will move it there.
> 
>> Or maybe both nvenc and the Avisynth stuff could be moved to a new "contrib"
>> or "thirdparty" folder.
> 
> That's out of the scope of this patch, but sounds like a good idea to me.
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>