[FFmpeg-devel] [RFC] Support dynamic loading of third-party libs

[Stephen Hutchinson]

On general principle, the idea would be nice.  I'll leave broader critiques on
the code to others, but I do have one thing to say:

>diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>index 1ec5cae..2520e69 100644
>--- a/libavcodec/utils.c
>+++ b/libavcodec/utils.c
>@@ -69,6 +69,25 @@
> #include "libavutil/ffversion.h"
> const char av_codec_ffversion[] = "FFmpeg version " FFMPEG_VERSION;
>
>+#if CONFIG_DYNAMIC_LOADING
>+  #ifdef _WIN32
>+    #include <windows.h>
>+    #undef EXTERN_C
>+    #define AVISYNTH_LIB "avisynth"
>+  #else
>+    #include <dlfcn.h>
>+    #if defined (__APPLE__)
>+      #define AVISYNTH_LIB "libavxsynth.dylib"
>+    #else
>+      #define AVISYNTH_LIB "libavxsynth.so"
>+    #endif /* __APPLE__ */
>+
>+    #define LoadLibrary(x) dlopen(x, RTLD_LAZY)
>+    #define GetProcAddress dlsym
>+    #define FreeLibrary dlclose
>+  #endif /* _WIN32 */
>+#endif /* CONFIG_DYNAMIC_LOADING */
>+
> #if HAVE_PTHREADS || HAVE_W32THREADS || HAVE_OS2THREADS
> static int default_lockmgr_cb(void **arg, enum AVLockOp op)
> {

What?

One, that's a weird place to put a pruned-down version of the lib/header
handler from libavformat/avisynth.c. And IMO, dynamic loading setup
for a specific library belongs in that library's wrapper, since too much
is liable to vary from library-to-library in what you need to be aware of.

Two, AviSynth and AvxSynth are *always* dynamically loaded as it is.  You
cannot even build them as static (well, AvxSynth might be capable of that,
but standard AviSynth is not, and currently, standard AviSynth can only be
built by MSVC; good luck trying to link that against a MinGW-GCC built
FFmpeg). Putting them inside this new block would be incorrect.

Three, it changes the RTLD value once again?  It just got changed to
RTLD_LOCAL a week or two ago.

The loading block in libavofrmat/avisynth.c already handles the AviSynth
case correctly.  It also has examples of how to deal with different versions,
since it supports two different versions of AviSynth (2.5 and AvxSynth on
one hand, and 2.6 and AviSynth+ on the other).

I would generally recommend looking at how libavformat/avisynth.c is set
up to get a feel for how to do this for other libraries, but the support for
AviSynth using dynamic loading should not rely on other libraries doing
so.  That's just not how AviSynth is intended to work.

It would also possibly make sense to allow the user to specify a list of
libs to dynamically load, just in case they want to use loading for some
but not all.

In essence,
--enable-dynamic-loading would default to 'all', but the user could also do
--enable-dynamic-loading=libname which would only activate it for X lib.