[Ffmpeg-devel] Re: av_log() bloatedness

Michael Niedermayer michaelni
Sun Jul 23 23:38:01 CEST 2006


Hi

On Sun, Jul 23, 2006 at 02:06:36PM -0400, Rich Felker wrote:
> On Sun, Jul 23, 2006 at 12:34:24PM +0200, Michael Niedermayer wrote:
> > Hi
> > 
> > On Sun, Jul 23, 2006 at 09:57:31AM +0100, M?ns Rullg?rd wrote:
> > > 
> > > Sigbj?rn Skj?ret said:
> > > > [..not subscribed to the list, will probably break thread, sorry..]
> > > >
> > > >>> supposedly somebody could like to have a custom callback() like a syslog
> > > >>> one.
> > > >> then he sets av_log= my_callback;
> > > >
> > > > Although I'm aware that lavc isn't recommended as a shared library atm I guess
> > > > it will some time in the future be allowed?
> > > 
> > > Despite what Rich says, shared libs are supported.
> > > 
> > > > If so then the above suggestion is not practical for non-dl methods as
> > > > variables are not exported .. my impression is that this is the reason
> > > > av_log_set_callback() exists (and it works great as well), it would be a pity
> > > > if this was removed...
> > > 
> > > It would break windows dlls, not proper shared libs.
> > 
> > and as such av_log_set_callback() belongs to the windows dlls but not proper
> > shared libs
> > personally i would have no objections to a seperate file which contains
> > some warpers and which is only compiled on windos, but i do have objections
> > to spread this mess all over the source
> 
> Michael, IMO this is much worse. It requires every application that
> wants to support lavc on both windows and real platforms to have
> #ifdef WIN32 crap in it, to use the different API on win32. So instead
> of a few bytes of wrapper 'bloat' in lavc, you have nasty #ifdef bloat
> in every calling application...

hmm, indeed thats just shifting and multiplying the mess :(


> 
> [Note: this could be solved with an av_log_set_callback macro in
> avutil.h that just does (av_log = (x)) on unix.. But..]
> 
> Actually using external variables from shared libraries is much more
> costly than using function calls on "real" platforms too. It requires
> copy relocations (the elf approach, which creates nasty abi/versioning
> issues) or textrels in the calling application. Unlike with function
> calls, normal PIC methods do not work unless the calling app was
> compiled with PIC.
> 
> IMO it would be the nicest if av_log function pointer were not stored
> in a global variable at all, but in the relevant context. This is the
> only approach that works in cases like:
> 
> 1. Main application directly links libavcodec
> 2. Application dynamic loads a plugin linked to libavcodec.
> 3. Plugin sets av_log to code in the plugin to avoid spamming the
>    calling app's stderr.
> 4. Main program no longer gets its log messages from its own use of
>    libavcodec.
> ....... and optionally .....
> 5. Main program unloads plugin.
> 6. Main program crashes when av_log gets called.
> 
> Number 5 and 6 can be avoided with a proper uninit function in the
> plugin, but things can still go horribly wrong if multiple plugins
> have been loaded and unloaded, all with this behavior.
> 
> The answer to all the problems, as always, is that global variables
> are evil and should not be used. :)

yes, but theres a problem, many av_log() calls exist without a context
grep 'av_log(NULL' {,*/,*/*/}*.{c,h} | wc -l
has 316 hits
until that changes, we will have to keep the global variable and forbid
anything but the application to override av_log()

furthermore theres the issue with complexity, current av_log is fairly
simple, but if its moved to some context then things will get more messy
1. we cannot just store the function pointer in AVCodecContext, 
   AVFormatContext, ... simply because it would be a nightmare to change
   all these duplicated pointers
2. we might end up having to pass some context pointer into speed-critical 
   functions
3. iam a little affraid of the situation you described above, that is with
   plugins overriding av_log() and unloading, somehow i think this is
   risky because if you by mistake passed a AVCodecContext or swscaler
   context or anything else out of that plugin and then unload it so the
   memory where a avLogContext or function pointer was is freed then
   at the time it gets called that area could be used by some other 
   thing, like for example a buffer holding a piece of exploit.avi ...


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is




More information about the ffmpeg-devel mailing list