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

[Stefan Oltmanns]

Am 22.06.24 um 20:23 schrieb Stephen Hutchinson:
> On 6/22/24 6:02 AM, Stefan Oltmanns via ffmpeg-devel wrote:
>> I don't know the extact reason, but VapourSynth is not just a library
>> like avisynth, but an application that uses Python, meaning a lot of
>> dependencies.
>
> If we want to be technical, then yes, VapourSynth is just a library,
> with bindings to integrate it into Python.  Lua bindings used to exist
> in mpv, but were removed several years ago; I don't know if those
> still exist somewhere externally.  Maybe someone's tried
> writing Ruby or JS bindings for it, crazier things have happened.
>
> Python is the thing that provides the ability to execute scripts,
> not VapourSynth/VSScript[.dll|.so|.dylib] itself.
>

Technically yes, but a library that is not mostly self-contained like
for example libx264, but one with massive dependencies to Python. That's
why you might not want to link to VapourSynth even if you have packaged it.

>> Additionally VapourSynth can nowadays be installed using pip, so no
>> reason to package it for distros anymore.
>>
>
>  > As distros probably won't package VapourSynth as it can be installed
>  > using pip, they probably won't build ffmpeg with VapourSynth support,
>  > because they don't have the headers on the build system.
>  > Even when someone decides to build ffmpeg for themself with VapourSynth
>  > support, it will fail unless they have manually build VapourSynth,
>  > because pip won't install any header.
>  > That was my motivation to include those headers. Is there a specific
>  > rule when it is allowed to include headers for external libaries?
>  >
>
> Distros don't really care about whether something is installable via
> pip before providing their own packages, and this was even acknowledged
> by upstream Python/pip with the recent move to the
> venv/--break-system-packages thing.  What a distro decides to package
> or not package in conjunction with enabling support for it in FFmpeg
> relates directly to their native repository system.
>
> This would also apply to a project like libdovi; if it's going to be
> provided and enabled in a distro's FFmpeg package, they aren't going
> to take the stance of 'users will just install it through their Rust
> environment first' - the distro will have to provide a package for it
> for the distro's FFmpeg package to enable it.
>

What I mean before a distro will include something there needs to be a
need for that and a maintainer for that package. If people will just use
pip no one will ask for that package to be included in the distro and
it's difficult to find someone to maintain it.
The pip vs. distro packet manager is in my experience mostly a result of
an application that is provided by the distro that has Python
dependencies to something like numpy. Later the user wants to install a
application with pip, but that has a dependency to a newer numpy
version. Then the solutions are what you have described.


>>>> I changed it, so that it loads the library at runtime when a
>>>> VapourSynth
>>>> script should be opened, just like AviSynth does.
>>>> On Windows the DLL from VapourSynth is not installed in the system
>>>> directory, but the location is stored in the Registry. Therefore I
>>>> added
>>>> some code to read that information from the registry.
>>>>
>>>
>>> That function is in the wrong place.
>>>
>>
>>
>> I thought that to, should it be in /compat as w32registry.h or something
>> like that?
>   >
>> But what exactly should it contain?
>>
>> I could make a function that would be used like
>>   get_w32_regsitry_str(HKEY_CURRENT_USER, L"SOFTWARE\\VapourSynth",
>>                        L"VSScriptDLL");
>> That would return a utf8 string on success and NULL on failure
>>
>
> I would say either unify the logic inside of vs_load_library with the
> minimum of Windows-specific things still cordoned off behind an ifdef,
> or move the function (again, still behind an ifdef) to just above where
> vs_load_library is.  It was mainly that putting it inside a big block
> of header #includes isn't the right place for that.

Ah ok, I see. I made it like this to not have platform-specific code at
different places, but if it's preferred at another place, that's fine.

>
> Have you tested whether simply adding the directory VapourSynth.dll
> and VSScript.dll reside in to the %PATH% gets around the need to
> read locations from the registry?  Without adding it to the %PATH%,
> you could use Windows' own DLL loading rules to test it (just plop
> ffmpeg.exe down in the same directory and run it when navigated into
> that directory).

Yes, that works and is fallback when no registry entries are found (for
portable packages). But when a user has installed VapourSynth and wants
to use ffmpeg with it it's quite inconvenient to force him to copy the
DLL somewhere else.
It might also create a problem: When the version of VSScript.dll does
not match the version of the installed Vapoursynth (because it was
updated) that could cause a problem.
Reading the registry entry to load the VSScript.dll is the way to go as
recommended by the VapourSynth auhtor.

>
>> But that would still contain 3 Windows-specific constants in the
>> function call. Also is it useful to write the literals as utf-8 just for
>> them to be converted to wchar again or should it just take WCHAR and
>> return utf8?
>> Or shoulds the entire function be located in
>> /compat/vapoursynth/w32_vsscript_dll.h as "get_w32_vsscript_dll" or
>> something similar?
>>
>
> I'm definitely not the person to ask about the text encoding stuff.

When I just move the code somewhere else that's not an issue. If there
should be a generalized function to read from the registry that would be
something to decide.