Hi Stefan,

On 7/6/24 23:08, Stefan Oltmanns via ffmpeg-devel wrote:
this is revised patch, to sum up the changes:

The current VapourSynth implementation is rarely used, as it links the
VapourSynth library at build time, making the resulting build unable to
run when VapourSynth is not installed. Therefore barely anyone compiles
with VapourSynth activated.

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.

As the V4 API is designed with dynamic loading in mind (only a single
import), I updated the implementation to V4 (changes are mostly
superficial, no structural changes). The V4 API is already several years
old, fully supported since R55 released in 2021.


Changes from first patch:
-Separated the Windows-specific function for getting the DLL location
from the platform-specific includes
-It is not enabled by default in configure
-The header files are not included anymore


I would like to include the header files for this reason:
While most Linux distributions ship ffmpeg, only very few contain
VapourSynth. Therefore ffmpeg won't be compiled with VapourSynth support
by these distributions, because no VapourSynth headers. Including the
headers in ffmpeg would mean they can compile with VapourSynth support
and if a user decided to install VapourSynth from somewhere else or
compile it by himself, ffmpeg support would be ready and no need for the
user to install another ffmpeg or compile one.
I'm not sure what the rules for shipping include files are, as there are
a few 3rd-party include files in ffmpeg. License is not an issue
(Vapourynth is LGPL v2.1 or later like ffmpeg).



make fate runs without any issue. I tested VapourSynth input scripts
with various color formats on different platforms:

Ubuntu 22.04
macOS 13 (x86_64)
macOS 13 (arm64)
Windows 10 (msys2/gcc)

It compiles on these platforms without any warning and runs without any
issues.

From 759b097865953ee66949ecbcdadbebfad623c29a Mon Sep 17 00:00:00 2001
From: Stefan Oltmanns <stefan-oltma...@gmx.net>
Date: Sat, 6 Jul 2024 22:56:53 +0200
Subject: [PATCH] avformat/vapoursynth: Update to API version 4, load library
 at runtime

Signed-off-by: Stefan Oltmanns <stefan-oltma...@gmx.net>
---
 configure                 |   3 +-
 libavformat/vapoursynth.c | 171 +++++++++++++++++++++++++++++---------
 2 files changed, 136 insertions(+), 38 deletions(-)

diff --git a/configure b/configure
index b28221f258..e43f3827ec 100755
--- a/configure
+++ b/configure
@@ -3575,6 +3575,7 @@ libxevd_decoder_deps="libxevd"
 libxeve_encoder_deps="libxeve"
 libxvid_encoder_deps="libxvid"
 libzvbi_teletext_decoder_deps="libzvbi"
+vapoursynth_deps_any="libdl LoadLibrary"
 vapoursynth_demuxer_deps="vapoursynth"
 videotoolbox_suggest="coreservices"
 videotoolbox_deps="corefoundation coremedia corevideo"
@@ -7080,7 +7081,7 @@ enabled rkmpp             && { require_pkg_config rkmpp 
rockchip_mpp  rockchip/r
                                { enabled libdrm ||
                                  die "ERROR: rkmpp requires --enable-libdrm"; }
                              }
-enabled vapoursynth       && require_pkg_config vapoursynth "vapoursynth-script >= 
42" VSScript.h vsscript_init
+enabled vapoursynth       && require_headers "vapoursynth/VSScript4.h 
vapoursynth/VapourSynth4.h"
if enabled gcrypt; then
diff --git a/libavformat/vapoursynth.c b/libavformat/vapoursynth.c
index 8a2519e19a..9c82f8f3b8 100644
--- a/libavformat/vapoursynth.c
+++ b/libavformat/vapoursynth.c
@@ -25,9 +25,6 @@
#include <limits.h> -#include <VapourSynth.h>
-#include <VSScript.h>
-
 #include "libavutil/avassert.h"
 #include "libavutil/avstring.h"
 #include "libavutil/eval.h"
@@ -40,19 +37,46 @@
 #include "demux.h"
 #include "internal.h"
+/* Platform-specific directives. */
+#ifdef _WIN32
+  #include <windows.h>
+  #include "compat/w32dlfcn.h"
+  #include "libavutil/wchar_filename.h"
+  #undef EXTERN_C
+  #define VSSCRIPT_LIB "VSScript.dll"
+  #define VS_DLOPEN() ({ void *handle = NULL; \
+                        char *dll_name = get_vs_script_dll_name(); \
+                        handle = dlopen(dll_name, RTLD_NOW | RTLD_GLOBAL); \
+                        av_free(dll_name); \
+                        handle; })
+#else
+  #include <dlfcn.h>
+  #define VSSCRIPT_NAME "libvapoursynth-script"
+  #define VSSCRIPT_LIB VSSCRIPT_NAME SLIBSUF
+  #define VS_DLOPEN() dlopen(VSSCRIPT_LIB, RTLD_NOW | RTLD_GLOBAL)
+#endif
+
+#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?

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

+
+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()?

[...]

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.

Ramiro
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to