[FFmpeg-devel] [PATCH v2] libavformat/vapoursynth: Update to API version 4, load library at runtime

[Stefan Oltmanns]

Am 18.07.24 um 16:21 schrieb Ramiro Polla:
> On Thu, Jul 18, 2024 at 3:41 PM Stefan Oltmanns via ffmpeg-devel
> <ffmpeg-devel at ffmpeg.org> wrote:
>> Am 18.07.24 um 15:04 schrieb Ramiro Polla:
>>>>>>   [...]
>>>>>>
>>>>>> +static VSScriptLibrary vs_script_library;
>>>>>
>>>>> Does vs_script_library have to be a global?
>>>>>
>>>>
>>>> I think it has to: ffmpeg allows multiple input files, in case you open
>>>> two VapourSynth files you want to load the Library only once.
>>>
>>> It should be possible to dlopen()/LoadLibrary() multiple times, and
>>> the library only gets really unloaded after the last call to
>>> dlclose()/FreeLibrary(). In that case you could move that struct to
>>> the context. If it's unavoidable to keep the global, at least add some
>>> locks to access it.
>>
>> Yes, that should be possible. I did a quick search at it seems that
>> dlopen()/LoadLibrary() is smart and will not open the same library
>> multiple times, but return the same pointer.
>> As dlclose won't be used anymore when removing the atexit handler, that
>> is not an issue at all.
>
> dlclose() will have to be called at some point (i.e.: in read_close).
>

The AviSynth patch to remove it by Stephen Hutchinson does not introduce
it somewhere else. It is now only called directly at the start in case a
needed function cannot be loaded from the DLL.
 From what I read dlclose is only needed if there are any C++
deconstructors or similar stuff that need to be called before exiting
the program.
dlclose usually won't unload the library anyway (the spec does not
require dlclose to do that)

>>>> This is exactly how it's done for AviSynth.
>>>
>>> Perhaps AviSynth is not the best example to follow :)

Is not using dlclose just another case?

>>>
>>>>>> +
>>>>>> +static int vs_atexit_called = 0;
>>>>>> +
>>>>>> +static av_cold void vs_atexit_handler(void);
>>>>>> +
>>>>>> +#ifdef _WIN32
>>>>>> +static av_cold char* get_vs_script_dll_name(void) {
>>>>>> +     LONG r;
>>>>>> +     WCHAR vss_path[512];
>>>>>> +     char *vss_path_utf8;
>>>>>> +     DWORD buf_size = sizeof(vss_path) - 2;
>>>>>> +     r = RegGetValueW(HKEY_CURRENT_USER, L"SOFTWARE\\VapourSynth",
>>>>>> +                      L"VSScriptDLL", RRF_RT_REG_SZ, NULL,
>>>>>> +                      &vss_path, &buf_size);
>>>>>> +     if (r == ERROR_SUCCESS && wchartoutf8(vss_path, &vss_path_utf8)
>>>>>> == 0)
>>>>>> +         return vss_path_utf8;
>>>>>> +     r = RegGetValueW(HKEY_LOCAL_MACHINE, L"SOFTWARE\\VapourSynth",
>>>>>> +                      L"VSScriptDLL", RRF_RT_REG_SZ, NULL,
>>>>>> +                      &vss_path, &buf_size);
>>>>>> +     if (r == ERROR_SUCCESS && wchartoutf8(vss_path, &vss_path_utf8)
>>>>>> == 0)
>>>>>> +         return vss_path_utf8;
>>>>>> +     if (wchartoutf8(L"VSScript.dll", &vss_path_utf8) == 0)
>>>>>> +         return vss_path_utf8;
>>>>>> +     return 0;
>>>>>> +}
>>>>>> +#endif
>>>>>
>>>>> Don't fetch the path on the registry on Windows. The user should set the
>>>>> path outside of FFmpeg.
>>>>
>>>> How exactly should the user do that? Additional option to ffmpeg?
>>>
>>> By making sure the libraries are accessible in the PATH environment
>>> variable. For example by adding the VapourSynth path to the PATH
>>> environment variable, or overriding PATH on the call to FFmpeg on a
>>> script. Either way that's outside the scope of FFmpeg.
>>
>> Well, the DLL directory is added to PATH by the VapourSynth installer,
>> but for safety reasons ffmpeg explictly tells the LoadLibrary function
>> to only search the application directory and system32, quote from
>> w32dlfcn.h:
>>
>>> /**
>>>   * Safe function used to open dynamic libs. This attempts to improve program security
>>>   * by removing the current directory from the dll search path. Only dll's found in the
>>>   * executable or system directory are allowed to be loaded.
>>>   * @param name  The dynamic lib name.
>>>   * @return A handle to the opened lib.
>>>   */
>> So ffmpeg prevents that solution on purpose. Or should that behavior be
>> changed in the w32dlfcn.h?
>
> Oh, bummer. I would expect that overriding the PATH environment
> variable would work kind of like how LD_LIBRARY_PATH works on Linux. I
> don't know why that was changed. I don't really follow what goes on in
> Windowsland anymore. Does anyone else care to comment on this? Martin,
> maybe?
>

Usually it would work on Windows that way (there is a list of all
directories it looks in what order). ffmpeg changes the default behavior.

Best regards
Stefan