[vlc-devel] [PATCH 1/2] libvlc: replaced libvlc_log_set with a formatted log callback

Steve Lhomme robux4 at ycbcr.xyz
Tue May 21 09:57:02 CEST 2019


On 2019-05-21 9:30, Rémi Denis-Courmont wrote:
> 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.

We can provide 2 API's and let the users decide if they want to take the 
memory hit in libvlc or in their app (in their own language/wrapper). I 
have a feeling noone will want to deal it by themselves.

> Also this creates a reverse abstraction anti-pattern in the stdio 
> handler and existing apps.

I did not understand this sentence.

> 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);
>       }
> 
> 
> -- 
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser 
> ma brièveté.
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
> 


More information about the vlc-devel mailing list