Hi Stephen, On Tue, 13 Jan 2015 08:26:10 -0500 Stephen Hutchinson <qyo...@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@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 -- _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel