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

[Stefan Oltmanns]

Hi Ramiro,

Am 18.07.24 um 13:08 schrieb Ramiro Polla:
> Hi Stefan,
>
>>  [...]
>> +
>> +#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.
This is exactly how it's done for AviSynth.

>> +
>> +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?
Fetching the path from the registry is the recommended method by the
VaopourSynth author.
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?

>
>> +
>> +static av_cold int vs_load_library(void)
>> +{
>> +    VSScriptGetAPIFunc get_vs_script_api;
>> +    vs_script_library.library = VS_DLOPEN();
>> +    if (!vs_script_library.library)
>> +        return -1;
>> +    get_vs_script_api =
>> (VSScriptGetAPIFunc)dlsym(vs_script_library.library,
>> +                                                  "getVSScriptAPI");
>> +    if (!get_vs_script_api) {
>> +        dlclose(vs_script_library.library);
>> +        return -2;
>> +    }
>> +    vs_script_library.vssapi = get_vs_script_api(VSSCRIPT_API_VERSION);
>> +    if (!vs_script_library.vssapi) {
>> +        dlclose(vs_script_library.library);
>> +        return -3;
>> +    }
>> +    atexit(vs_atexit_handler);
>
> Can you get rid of the call to atexit()?

Yes, that should be possible. I just read it was only included in
AviSynth to work around a crash at exit caused one specific AviSynth
variant. So it's probably save to remove.

>
> [...]
>
> Could you split the patch into first moving to API 4 and then working on
> the runtime loading? The first part might be reviewed and merged faster.

I can do that.

Best regards,
Stefan