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

Marc Giger gigerstyle at gmx.ch
Tue Jan 13 19:31:33 CET 2015


Hi Stephen,

On Tue, 13 Jan 2015 08:26:10 -0500
Stephen Hutchinson <qyot27 at gmail.com> wrote:

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

Will have a look at this one.

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

Please ignore all of the avisynth stuff in my patch. It's a copy&paste
error from myside. Sorry for the confusion...

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

I also thought about this one but for simplicity I left it away for the
moment. Will have a look at it when there is a consens about the basic
stuff.

Also many thanks to you for your valuable feedback!

Marc


_______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


-- 
Lesson 1: Cryptographic protocols should not be developed by a
committee. -- Niels Ferguson and Bruce Schneier --


More information about the ffmpeg-devel mailing list