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

[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).

> >> This is exactly how it's done for AviSynth.
> >
> > Perhaps AviSynth is not the best example to follow :)
> >
> >>>> +
> >>>> +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?

Ramiro