[libav-devel] [RFC] Getting rid of the global log callback

wm4 nfxjfg at googlemail.com
Mon Dec 11 14:48:07 CET 2017


The log callback, set with av_log_set_callback(), is global mutable
state, and as such not something we want in Libav, at all. but getting
rid of it is very complicated, because in most cases, av_log() does not
have enough context available to find per-context log callbacks.

av_log() has a context pointer as first argument. This is just a void*.
If it's non-NULL, then it's assumed to be a struct, with a AVClass*
field as first member (e.g. AVCodecContext.av_class). This points to
const memory, so it's not mutable state, and there's no way to put a
private log callback there.

So this whole problem boils down to how to change AVClass to store a
per-context log callback in structs using AVClass.

You can pass a NULL context to av_log() too. This would inherently
require a global log callback, so we have to deprecate this usage
entirely.

A log callback would include a pointer, and a user pointer:

struct AVLogCallback {
  void *user_context;
  // context is the caller (a struct with AVClass* as first field)
  // user_context as the same value as the field above.
  void (*callback)(void *context, void *user_context, int level,
                   const char *fmt, va_list vl);
}

Personally, I'd prefer if we had mandatory state per struct. Let's call
it AVObject:

struct AVObject {
  const AVClass *av_class;
  AVLogCallback log_callback:
}

(Sorry for the bad taste in names, but this whole AVClass nonsense
wasn't my idea anyway.)

We'd have a new AVClass field to signal presence of this:

struct AVClass {
  const char* class_name;
  ...
  int is_new_class;
}

The following function would work on my Libav context that works with
av_log() (provided all conversions have been done):

int av_class_set_log_callback(void *ctx, AVLogCallback cb);

// Example usage:
struct AVCodecContext {
  AVObject object;
  // ... other fields
}

AVCodecContext *avctx = ...;
av_class_set_log_callback(avctx, cb);


This probably won't work, because:

1. AVCodecContext.av_class probably can't go away
2. Would require an ABI bump too, which might be too late now, or not
   good for incremental conversion

So instead of AVObject/is_new_class we'd just have another field
offset, which I think is a bit clunky and possibly braindamaged, but
it would work. We would have:

struct AVClass {
  const char* class_name;
  ...
  // offset of AVClassState field
  int state_offset;
}

struct AVClassState {
  AVLogCallback log_callback:
}

And as an example:

struct AVCodecContext {
  const AVClass *av_class;
  
  ... // all other fields
  
  AVClassState *state;
}

static const AVClass av_codec_context_class = {
  .class_name = "AVCodecContext",
  .state_offset = offsetof(AVCodecContext, av_class_state),
  ...
}

AVClassState would probably be an opaque struct to make extending it
easier. That's also why the "state" field above is a pointer.

I'm arguing for such an extensible AVClassState, so we can stuff other
things like AVClass.log_level_offset_offset in there. Possibly settings
like CPU flags can go there too.

I'm also against something like a AVLibraryState struct, which would
imply to be globally shared across all contexts. I think it's better if
the state gets "inherited" by copying all the parameters as contexts
create sub-contexts. This is simpler and more flexible, and instead of
having the user pass around such an AVLibraryState, the user would just
configure the parameters with calls like av_class_set_log_callback() on
contexts.

Anyway, once these mechanisms are in place, we'd deprecate the global
log callback, and the possibility to pass NULL as first argument to
av_log().

Thoughts? I'd like to implement this stuff once we agree on how to
proceed.


More information about the libav-devel mailing list