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.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
