[vlc-devel] [PATCH 1/2] libvlc: replaced libvlc_log_set with a formatted log callback
Rémi Denis-Courmont
remi at remlab.net
Tue May 21 09:30:53 CEST 2019
Hi,
I refuse to change the existing API which works, follows a well-known convention (indeed libavutil works the same way), and does not add heap overhead and failure modes - which is a MUST for *debug* logs.
Also this creates a reverse abstraction anti-pattern in the stdio handler and existing apps.
Le 20 mai 2019 23:58:40 GMT+03:00, "Jérémy VIGNELLES" <jeremy.vignelles at dev3i.fr> a écrit :
>---
> include/vlc/libvlc.h | 25 ++++++++++++-------------
> lib/libvlc.sym | 2 +-
> lib/libvlc_internal.h | 3 ++-
> lib/log.c | 29 +++++++++++++++++++----------
> 4 files changed, 34 insertions(+), 25 deletions(-)
>
>diff --git a/include/vlc/libvlc.h b/include/vlc/libvlc.h
>index 0ec0045..aac2c7f 100644
>--- a/include/vlc/libvlc.h
>+++ b/include/vlc/libvlc.h
>@@ -410,19 +410,18 @@ LIBVLC_API void libvlc_log_get_object(const
>libvlc_log_t *ctx,
> const char **name, const char **header, uintptr_t *id);
>
> /**
>- * Callback prototype for LibVLC log message handler.
>+ * Callback prototype for LibVLC preformatted log message handler.
> *
> * \param data data pointer as given to libvlc_log_set()
> * \param level message level (@ref libvlc_log_level)
> * \param ctx message context (meta-information about the message)
>- * \param fmt printf() format string (as defined by ISO C11)
>- * \param args variable argument list for the format
>+ * \param message the message, already formatted
> * \note Log message handlers <b>must</b> be thread-safe.
>- * \warning The message context pointer, the format string parameters
>and the
>- * variable arguments are only valid until the callback
>returns.
>+ * \warning The message context pointer, and the message string
>parameters
>+ * are only valid until the callback returns.
> */
>-typedef void (*libvlc_log_cb)(void *data, int level, const
>libvlc_log_t *ctx,
>- const char *fmt, va_list args);
>+typedef void (*libvlc_log_preformatted_cb)(void *data, int level,
>const libvlc_log_t *ctx,
>+ const char *message);
>
> /**
> * Unsets the logging callback.
>@@ -440,11 +439,13 @@ typedef void (*libvlc_log_cb)(void *data, int
>level, const libvlc_log_t *ctx,
> LIBVLC_API void libvlc_log_unset( libvlc_instance_t *p_instance );
>
> /**
>- * Sets the logging callback for a LibVLC instance.
>+ * Sets the logging callback for a LibVLC instance, that takes a
>preformatted message as input.
> *
> * This function is thread-safe: it will wait for any pending callbacks
> * invocation to complete.
> *
>+ * \param p_instance libvlc instance
>+ * \param min_level the callback will only be called if the message
>level is above this level, 0 means "get all messages" (@ref
>libvlc_log_level)
> * \param cb callback function pointer
> * \param data opaque data pointer for the callback function
> *
>@@ -453,12 +454,10 @@ LIBVLC_API void libvlc_log_unset(
>libvlc_instance_t *p_instance );
> *
>* \warning A deadlock may occur if this function is called from the
>callback.
> *
>- * \param p_instance libvlc instance
>- * \version LibVLC 2.1.0 or later
>+ * \version LibVLC 4.0.0 or later
> */
>-LIBVLC_API void libvlc_log_set( libvlc_instance_t *p_instance,
>- libvlc_log_cb cb, void *data );
>-
>+LIBVLC_API void libvlc_log_set_preformatted(libvlc_instance_t
>*p_instance, int min_level,
>+ libvlc_log_preformatted_cb
>cb, void* data );
>
> /**
> * Sets up logging to a file.
>diff --git a/lib/libvlc.sym b/lib/libvlc.sym
>index 456f9c2..59208c8 100644
>--- a/lib/libvlc.sym
>+++ b/lib/libvlc.sym
>@@ -55,8 +55,8 @@ libvlc_get_fullscreen
> libvlc_get_version
> libvlc_log_get_context
> libvlc_log_get_object
>-libvlc_log_set
> libvlc_log_set_file
>+libvlc_log_set_preformatted
> libvlc_log_unset
> libvlc_media_add_option
> libvlc_media_add_option_flag
>diff --git a/lib/libvlc_internal.h b/lib/libvlc_internal.h
>index 5c1107b..8c39aa2 100644
>--- a/lib/libvlc_internal.h
>+++ b/lib/libvlc_internal.h
>@@ -65,7 +65,8 @@ struct libvlc_instance_t
> struct libvlc_callback_entry_list_t *p_callback_list;
> struct
> {
>- void (*cb) (void *, int, const libvlc_log_t *, const char *,
>va_list);
>+ void (*cb) (void *, int, const libvlc_log_t *, const char *);
>+ int min_level;
> void *data;
> } log;
> struct
>diff --git a/lib/log.c b/lib/log.c
>index 66e0d99..1e57602 100644
>--- a/lib/log.c
>+++ b/lib/log.c
>@@ -73,7 +73,16 @@ static void libvlc_logf (void *data, int level,
>const vlc_log_t *item,
> case VLC_MSG_DBG: level = LIBVLC_DEBUG; break;
> }
>
>- inst->log.cb (inst->log.data, level, item, fmt, ap);
>+ if(level >= inst->log.min_level)
>+ {
>+ char* msg;
>+ int msgSize = vasprintf(&msg, fmt, ap);
>+ if (unlikely(msgSize < 0))
>+ return;
>+
>+ inst->log.cb(inst->log.data, level, item, msg);
>+ free(msg);
>+ }
> }
>
> static const struct vlc_logger_operations libvlc_log_ops = {
>@@ -85,28 +94,28 @@ void libvlc_log_unset (libvlc_instance_t *inst)
> vlc_LogSet (inst->p_libvlc_int, NULL, NULL);
> }
>
>-void libvlc_log_set (libvlc_instance_t *inst, libvlc_log_cb cb, void
>*data)
>+void libvlc_log_set_preformatted (libvlc_instance_t *p_instance, int
>min_level, libvlc_log_preformatted_cb cb, void* data )
> {
>- libvlc_log_unset (inst); /* <- Barrier before modifying the
>callback */
>- inst->log.cb = cb;
>- inst->log.data = data;
>- vlc_LogSet(inst->p_libvlc_int, &libvlc_log_ops, inst);
>+ libvlc_log_unset (p_instance); /* <- Barrier before modifying the
>callback */
>+ p_instance->log.cb = cb;
>+ p_instance->log.min_level = min_level;
>+ p_instance->log.data = data;
>+ vlc_LogSet(p_instance->p_libvlc_int, &libvlc_log_ops, p_instance);
> }
>
> /*** Helpers for logging to files ***/
>static void libvlc_log_file (void *data, int level, const libvlc_log_t
>*log,
>- const char *fmt, va_list ap)
>+ const char *msg)
> {
> FILE *stream = data;
>
> flockfile (stream);
>- vfprintf (stream, fmt, ap);
>- fputc ('\n', stream);
>+ fprintf (stream, "%s\n", msg);
> funlockfile (stream);
> (void) level; (void) log;
> }
>
> void libvlc_log_set_file (libvlc_instance_t *inst, FILE *stream)
> {
>- libvlc_log_set (inst, libvlc_log_file, stream);
>+ libvlc_log_set_preformatted (inst, LIBVLC_DEBUG, libvlc_log_file,
>stream);
> }
>--
>2.20.1
>
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel
--
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20190521/d47be1d9/attachment.html>
More information about the vlc-devel
mailing list