[x265] [PATCH 1 of 2] Add support for customizing logging

[Andrey Semashev]

On 03/18/18 20:04, Derek Buitenhuis wrote:
> On 3/16/2018 3:28 PM, Andrey Semashev wrote:
>> +/* x265_set_log:
>> + *       Sets a custom logger function */
>> +void x265_set_log(x265_log_t log);
> 
> I would like to voice my distaste for this implementation.
> 
> This is a global callback, and will introduce problems with e.g. multiple
> libraries or dependencies in a program have libx265 as a dependency, or even
> with multiple encoders in the same program are used.
> 
> Consider program A that has B as a dependency, both of which use libx265:
> 
> A - libx265
>    - B
>        - libx265
> 
> Either A or B could set the log callback for x265, and it would take
> over for both. This is not ideal, and it is the same design mistake
> we at FFmpeg made a long time ago, which has been haunting us for years.

Normally, there is only one _application_. There may be multiple 
_libraries_ using libx265 but none of them should be configuring its 
logging, unless it is supposed to be the only one using libx265 in the 
application (and generic libraries cannot make that assumption). It is 
the application that should configure logging in every component it uses.

That said, I myself would prefer to configure logging on per-encoder 
basis, although for different reasons. But the way x265 is currently 
written does not allow this because in many places logging is done 
without any encoder context. I'm not willing to rewrite x265 to support 
this as global logging customization is "good enough" for me. It 
certainly is much better than unconditionally logging to stdout.

> Further, defining user types ending in _t is not allowed in C (which this
> header is, and is used from), and x265 does not do this.

I'm not aware of this limitation. In particular, C11 section 7.1.3 
contains no such restriction. Can you provide a reference to the part of 
the C standard that says this?

Note also that the name starts with x265, so it clearly belongs to the 
x265 namespace and is unlikely to clash with anything.