LogCallback implementation not thread-safe

[lknock]

I see that the new LogCallback class is an alternative to the Callback_Pointer_int_String_Pointer and Callback_Pointer_int_BytePointer_Pointer callbacks. Most people don't want to reinvent the wheel and simply want to redirect the log messages to wherever, so this is a welcome addition indeed.

There are two issues with its current implementation, though. First of all, the AV_LOG_SKIP_REPEATED flag isn't handled. People using this new callback will suddenly find that setting this flag will do nothing to avoid redundant log messages. It's an unfortunate flaw in FFmpeg's API that any custom callback has to handle this itself.

But, more importantly, the callback is not thread-safe which it has to be according to the documentation:

The callback must be thread safe, even if the application does not use threads itself as some codecs are multithreaded.

[saudet]

It's not just an alternative: There's no standard way to dereference a va_list.

If you'd like to implement something for AV_LOG_SKIP_REPEATED it would be very welcome. Please send a pull request.

FFmpegLogCallback just forwards everything to SLF4J, and the loggers it supports are usually thread-safe, so there should be no problem. If you find an issue with the way things are though, please do explain, thanks!

[saudet]

Is it that av_log_format_line() isn't thread-safe? That would be problematic indeed... Any suggestions?

[lknock]

Actually the use of av_log_format_line is thread-safe. That isn't to say the function itself is. It checks the global log flags variable but there's little we can do about that.

The real issue here is that people should either be aware that their custom callback must be thread-safe or forwarding the log messages should be done in a thread-safe manner.

The problem with implementing AV_LOG_SKIP_REPEATED is that it will have be thread-safe, as we'd have no choice but to compare to and update a static variable that holds the previous line.

In that case, we might as well lock the call to custom callback and guarantee thread-safety in LogCallback itself. If you're okay with that, I'll have a go at it.

[saudet]

Yeah, that'd be great! 👍 Thanks

Looking inside log.c, FFmpeg seems to assume that everyone is going to use pthread, and I think that holds for all currently supported platforms, even Windows since it's built with MinGW so I guess that we could use pthread as well in log_callback.h. Though, we might have to modify the properties of JavaCPP to add a "compiler.option.threads" entry like the one for "cpp11"...

[saudet]

Hi, have you made any progress since?