[vlc-devel] [PATCH 0/2] Libvlc formatted log callback

[Alexandre Janniaux]

Hi,

On 2019-05-17 11:35, Jeremy Vignelles wrote:
>> Le 17 mai 2019 à 11:23, Romain Vimont <rom1v at videolabs.io> a écrit :
>> 
>> 
>> On Fri, May 17, 2019 at 10:04:59AM +0200, Jérémy VIGNELLES wrote:
>> > Hi,
>> >
>> > In libvlc, there is currently a `libvlc_log_set` function, that can register
>> > log callback in a vprintf style, that is, with a `va_list` argument.
>> 
>> I agree that we should avoid va_list in callbacks :)
>> > This is nice when you are trying to call from C, but when using other languages
>> > like C# or Java, va_list is impossible to get right, and even if it did,
>> > those languages do not provide ways to format that kind of stuff.
>> 
>> (not counter-arguments, just comments)
>> 
>> I don't know C#, but in Java you will need a JNI layer in C, so it can
>> be handled here.
> 
> I don't really know java, but that JNI layer will still need to be
> built for the target platform you're building for. Wouldn't that make
> a lot of combinations too? I think it would be similar to what I did
> in libvlcLogInterop

What Romain said is that you always need a layer between libvlc and JNI, 
be
it your own layer or the libvlc-jni that we provide, which already 
targets
a lot of different platforms. So it is not really difficult for the 
binding
layer to provide adaptation of the va_list and for the application to
implement this adaptor idiomatically.

However, it becomes an issue for languages making bindings through FFI, 
like
Rust or C#. It's workaroundable but it's getting ugly.

Thank you for removing these va_list.

>> BTW, in pure Java, you can format using String.format():
>> 
>>  String s = String.format("%s %d", "hello", 42).
> 
> C# has a
> string.Format("{0} {1}", "hello", 42)
> 
> But that doesn't make it easier to consume, because a va_list doesn't
> map well to the C# version (object[] params)
> 
>> > The best attempt so far seems to write an external native project to do that,
>> > like this one : https://github.com/jeremyVignelles/libvlcLogInterop, but it has
>> > a major drawback: it needs to be compiled for every host triplet that vlc supports,
>> > which is a lot of work (if even possible), and requires the correct v*printf to be
>> > available on the standard library.
>> >
>> > I'm attempting here another approach : Modify libvlc v4 to include a preformatted
>> > version of libvlc_log_set. The caller is now able to get log message as a single string,
>> > and can filter by log level before formatting.
>> >
>> > The new API requires a context to be created and destroyed, in order to keep compatibility with
>> > libvlc_log_set.
>> >
>> > I thought about modifying the libvlc_instance_t.log structure and put my context there,
>> > but I was unsure about how that would interact with the existing things
>> >
>> > Comments welcome :)
>> >
>> > Jérémy VIGNELLES (2):
>> >  libvlc: added a formatted log callback
>> >  test: libvlc: Added a formatted log callback test
>> >
>> >  include/vlc/libvlc.h | 64 ++++++++++++++++++++++++++++++++++++++++++++
>> >  lib/libvlc.sym | 3 +++
>> >  lib/log.c | 55 +++++++++++++++++++++++++++++++++++++
>> >  test/libvlc/core.c | 27 +++++++++++++++++++
>> >  4 files changed, 149 insertions(+)
>> >
>> > --
>> > 2.21.0.windows.1
>> >
>> > _______________________________________________
>> > vlc-devel mailing list
>> > To unsubscribe or modify your subscription options:
>> > https://mailman.videolan.org/listinfo/vlc-devel
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel