[Ffmpeg-devel] Re: av_log() bloatedness

[Rich Felker]

On Sun, Jul 23, 2006 at 11:38:01PM +0200, Michael Niedermayer wrote:
> > 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

Ah. Why so many places that need debug output without a context?! I
would think almost all functions, among the ones that aren't so
speed-critical that logging is out of the question, would have some
sort of context..

> until that changes, we will have to keep the global variable and forbid
> anything but the application to override av_log()

Yes.

> 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

Hm? Why would they need to be changed after they were initialized? My
idea is that you would just set the log function immediately upon
allocating the context. If you wanted to be able to switch outputs
later, your application could do that in its own log function, to
which all the contexts' av_log pointers point.

> 2. we might end up having to pass some context pointer into speed-critical 
>    functions

IMO a speed-critical function can't have calls to av_log anyway, not
even conditioned on a debug variable..

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

Obviously this is a problem, but IMO it results from a major error in
programming, rather than an issue that you can't really predict..
Still it's good you raised it for consideration.

Rich