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

[Ramiro Polla]

On Thu, Jul 18, 2024 at 2:48 PM Stefan Oltmanns via ffmpeg-devel
<ffmpeg-devel at ffmpeg.org> wrote:
> Am 18.07.24 um 13:08 schrieb Ramiro Polla:
> >>  [...]
> >> +
> >> +#include <vapoursynth/VSScript4.h>
> >> +
> >>  struct VSState {
> >>      VSScript *vss;
> >>  };
> >>
> >> +typedef const VSSCRIPTAPI *(*VSScriptGetAPIFunc)(int version);
> >> +
> >> +typedef struct VSScriptLibrary {
> >> +    void *library;
> >> +    const VSSCRIPTAPI *vssapi;
> >> +} VSScriptLibrary;
> >> +
> >>  typedef struct VSContext {
> >>      const AVClass *class;
> >>
> >>      AVBufferRef *vss_state;
> >>
> >>      const VSAPI *vsapi;
> >> -    VSCore *vscore;
> >>
> >> -    VSNodeRef *outnode;
> >> +    VSNode *outnode;
> >>      int is_cfr;
> >>      int current_frame;
> >>
> >> @@ -70,19 +94,72 @@ static const AVOption options[] = {
> >>      {NULL}
> >>  };
> >>
> >> +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.

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

> Fetching the path from the registry is the recommended method by the
> VaopourSynth author.

Sometimes the recommended method isn't necessarily the best method...

> Would it be an acceptable way to move the registry stuff to the Windows
> specific area (where for example the LoadLibrary stuff is) and create a
> function to get a UTF-8 string from the registry, so that there is no
> Windows-style code in the VapourSynth module?

It's best to just remove it.

[...]

Regards,
Ramiro