libvlc: simplify and clean up external logging API (fixes #8129)

 - Scope the callback to its instance.
 - Avoid leaking the layout of an internal data structure.
 - Future-proof passing of extra informations through an opaque pointer.

include/vlc/libvlc.h

@@ -336,79 +336,56 @@ enum libvlc_log_level
     LIBVLC_ERROR=4    /**< Error message */
 };
 
+typedef struct vlc_log_t libvlc_log_t;
+
 /**
  * Callback prototype for LibVLC log message handler.
- * \param data data pointer as given to libvlc_log_subscribe()
+ * \param data data pointer as given to libvlc_log_set()
  * \param level message level (@ref enum libvlc_log_level)
+ * \param ctx message context (meta-informations about the message)
  * \param fmt printf() format string (as defined by ISO C11)
  * \param args variable argument list for the format
  * \note Log message handlers <b>must</b> be thread-safe.
  */
-typedef void (*libvlc_log_cb)(void *data, int level, const char *fmt,
-                              va_list args);
+typedef void (*libvlc_log_cb)(void *data, int level, const libvlc_log_t *ctx,
+                              const char *fmt, va_list args);
 
 /**
- * Data structure for a LibVLC logging callbacks.
- * \note This structure contains exactly 4 pointers and will never change.
- * Nevertheless, it should not be accessed directly outside of LibVLC.
- * (In fact, doing so would fail the thread memory model.)
+ * Unsets the logging callback for a LibVLC instance. This is rarely needed:
+ * the callback is implicitly unset when the instance is destroyed.
+ * This function will wait for any pending callbacks invocation to complete
+ * (causing a deadlock if called from within the callback).
+ *
+ * \version LibVLC 2.1.0 or later
  */
-typedef struct libvlc_log_subscriber
-{
-    struct libvlc_log_subscriber *prev, *next;
-    libvlc_log_cb func;
-    void *opaque;
-} libvlc_log_subscriber_t;
+LIBVLC_API void libvlc_log_unset( libvlc_instance_t * );
 
 /**
- * Registers a logging callback to LibVLC.
- * This function is thread-safe.
+ * Sets the logging callback for a LibVLC instance.
+ * This function is thread-safe: it will wait for any pending callbacks
+ * invocation to complete.
  *
- * \param sub uninitialized subscriber structure
  * \param cb callback function pointer
  * \param data opaque data pointer for the callback function
  *
  * \note Some log messages (especially debug) are emitted by LibVLC while
- * initializing, before any LibVLC instance even exists.
- * Thus this function does not require a LibVLC instance parameter.
+ * is being initialized. These messages cannot be captured with this interface.
  *
- * \warning As a consequence of not depending on a LibVLC instance,
- * all logging callbacks are shared by all LibVLC instances within the
- * process / address space. This also enables log messages to be emitted
- * by LibVLC components that are not specific to any given LibVLC instance.
+ * \warning A deadlock may occur if this function is called from the callback.
  *
- * \warning Do not call this function from within a logging callback.
- * It would trigger a dead lock.
  * \version LibVLC 2.1.0 or later
  */
-LIBVLC_API void libvlc_log_subscribe( libvlc_log_subscriber_t *sub,
+LIBVLC_API void libvlc_log_set( libvlc_instance_t *,
                                 libvlc_log_cb cb, void *data );
 
 
 /**
- * Registers a logging callback to a file.
+ * Sets up logging to a file.
  * \param stream FILE pointer opened for writing
- *         (the FILE pointer must remain valid until libvlc_log_unsubscribe())
- * \version LibVLC 2.1.0 or later
- */
-LIBVLC_API void libvlc_log_subscribe_file( libvlc_log_subscriber_t *sub,
-                                           FILE *stream );
-
-/**
- * Deregisters a logging callback from LibVLC.
- * This function is thread-safe.
- *
- * \note After (and only after) libvlc_log_unsubscribe() has returned,
- * LibVLC warrants that there are no more pending calls of the subscription
- * callback function.
- *
- * \warning Do not call this function from within a logging callback.
- * It would trigger a dead lock.
- *
- * \param sub initialized subscriber structure
+ *         (the FILE pointer must remain valid until libvlc_log_unset())
  * \version LibVLC 2.1.0 or later
  */
-LIBVLC_API void libvlc_log_unsubscribe( libvlc_log_subscriber_t *sub );
+LIBVLC_API void libvlc_log_set_file( libvlc_instance_t *, FILE *stream );
 
 /**
  * Always returns minus one.

include/vlc/libvlc_structures.h

@@ -52,9 +52,6 @@ typedef int64_t libvlc_time_t;
  * @{
  */
 
-/** This structure is opaque. It represents a libvlc log instance */
-typedef struct libvlc_log_t libvlc_log_t;
-
 /** This structure is opaque. It represents a libvlc log iterator */
 typedef struct libvlc_log_iterator_t libvlc_log_iterator_t;
 

lib/error.c

@@ -37,10 +37,7 @@ void libvlc_threads_init (void)
 {
     vlc_mutex_lock (&lock);
     if (refs++ == 0)
-    {
         vlc_threadvar_create (&context, free);
-        libvlc_log_init ();
-    }
     vlc_mutex_unlock (&lock);
 }
 
@@ -49,10 +46,7 @@ void libvlc_threads_deinit (void)
     vlc_mutex_lock (&lock);
     assert (refs > 0);
     if (--refs == 0)
-    {
-        libvlc_log_deinit ();
         vlc_threadvar_delete (&context);
-    }
     vlc_mutex_unlock (&lock);
 }
 

lib/libvlc.sym

@@ -45,8 +45,9 @@ libvlc_get_fullscreen
 libvlc_get_input_thread
 libvlc_get_log_verbosity
 libvlc_get_version
-libvlc_log_subscribe
-libvlc_log_unsubscribe
+libvlc_log_set
+libvlc_log_set_file
+libvlc_log_unset
 libvlc_log_clear
 libvlc_log_close
 libvlc_log_count

lib/libvlc_internal.h

@@ -72,6 +72,11 @@ struct libvlc_instance_t
     unsigned      ref_count;
     vlc_mutex_t   instance_lock;
     struct libvlc_callback_entry_list_t *p_callback_list;
+    struct
+    {
+        void (*cb) (void *, int, const libvlc_log_t *, const char *, va_list);
+        void *data;
+    } log;
 };
 
 

lib/log.c

@@ -34,88 +34,60 @@
 
 /*** Logging core dispatcher ***/
 
-static vlc_rwlock_t log_lock = VLC_STATIC_RWLOCK;
-static libvlc_log_subscriber_t *log_first = NULL;
-static msg_subscription_t sub;
-
-VLC_FORMAT(2,3)
-static void libvlc_log (int level, const char *fmt, ...)
+VLC_FORMAT(4,5)
+static void libvlc_log (libvlc_instance_t *inst, int level,
+                        const libvlc_log_t *ctx, const char *fmt, ...)
 {
-    libvlc_log_subscriber_t *sub;
     va_list ap;
 
-    switch (level)
-    {
-        case VLC_MSG_INFO: level = LIBVLC_NOTICE;  break;
-        case VLC_MSG_ERR:  level = LIBVLC_ERROR;   break;
-        case VLC_MSG_WARN: level = LIBVLC_WARNING; break;
-        case VLC_MSG_DBG:  level = LIBVLC_DEBUG;   break;
-    }
-
     va_start (ap, fmt);
-    vlc_rwlock_rdlock (&log_lock);
-    for (sub = log_first; sub != NULL; sub = sub->next)
-        sub->func (sub->opaque, level, fmt, ap);
-    vlc_rwlock_unlock (&log_lock);
+    inst->log.cb (inst->log.data, level, ctx, fmt, ap);
     va_end (ap);
 }
 
-static void libvlc_logf (void *dummy, int level, const vlc_log_t *item,
+static void libvlc_logf (void *data, int level, const vlc_log_t *item,
                          const char *fmt, va_list ap)
 {
     char *msg;
 
     if (unlikely(vasprintf (&msg, fmt, ap) == -1))
-        msg = NULL;
+        return;
+
+    switch (level)
+    {
+        case VLC_MSG_INFO: level = LIBVLC_NOTICE;  break;
+        case VLC_MSG_ERR:  level = LIBVLC_ERROR;   break;
+        case VLC_MSG_WARN: level = LIBVLC_WARNING; break;
+        case VLC_MSG_DBG:  level = LIBVLC_DEBUG;   break;
+    }
+
     if (item->psz_header != NULL)
-        libvlc_log (level, "[%p] [%s]: %s %s %s", (void *)item->i_object_id,
-                    item->psz_header, item->psz_module, item->psz_object_type,
-                    msg ? msg : "Not enough memory");
+        libvlc_log (data, level, item, "[%p] [%s]: %s %s %s",
+                    (void *)item->i_object_id, item->psz_header,
+                    item->psz_module, item->psz_object_type, msg);
     else
-        libvlc_log (level, "[%p]: %s %s %s", (void *)item->i_object_id,
-                    item->psz_module, item->psz_object_type,
-                    msg ? msg : "Not enough memory");
+        libvlc_log (data, level, item, "[%p]: %s %s %s",
+                    (void *)item->i_object_id, item->psz_module,
+                    item->psz_object_type, msg);
     free (msg);
-    (void) dummy;
-}
-
-void libvlc_log_init (void)
-{
-    vlc_Subscribe (&sub, libvlc_logf, NULL);
 }
 
-void libvlc_log_deinit (void)
+void libvlc_log_unset (libvlc_instance_t *inst)
 {
-    vlc_Unsubscribe (&sub);
+    vlc_LogSet (inst->p_libvlc_int, NULL, NULL);
 }
 
-void libvlc_log_subscribe (libvlc_log_subscriber_t *sub,
-                           libvlc_log_cb cb, void *data)
+void libvlc_log_set (libvlc_instance_t *inst, libvlc_log_cb cb, void *data)
 {
-    sub->prev = NULL;
-    sub->func = cb;
-    sub->opaque = data;
-    vlc_rwlock_wrlock (&log_lock);
-    sub->next = log_first;
-    log_first = sub;
-    vlc_rwlock_unlock (&log_lock);
-}
-
-void libvlc_log_unsubscribe( libvlc_log_subscriber_t *sub )
-{
-    vlc_rwlock_wrlock (&log_lock);
-    if (sub->next != NULL)
-        sub->next->prev = sub->prev;
-    if (sub->prev != NULL)
-        sub->prev->next = sub->next;
-    else
-        log_first = sub->next;
-    vlc_rwlock_unlock (&log_lock);
+    libvlc_log_unset (inst); /* <- Barrier before modifying the callback */
+    inst->log.cb = cb;
+    inst->log.data = data;
+    vlc_LogSet (inst->p_libvlc_int, libvlc_logf, inst);
 }
 
 /*** Helpers for logging to files ***/
-static void libvlc_log_file (void *data, int level, const char *fmt,
-                             va_list ap)
+static void libvlc_log_file (void *data, int level, const libvlc_log_t *log,
+                             const char *fmt, va_list ap)
 {
     FILE *stream = data;
 
@@ -123,12 +95,12 @@ static void libvlc_log_file (void *data, int level, const char *fmt,
     vfprintf (stream, fmt, ap);
     fputc ('\n', stream);
     funlockfile (stream);
-    (void) level;
+    (void) level; (void) log;
 }
 
-void libvlc_log_subscribe_file (libvlc_log_subscriber_t *sub, FILE *stream)
+void libvlc_log_set_file (libvlc_instance_t *inst, FILE *stream)
 {
-    libvlc_log_subscribe (sub, libvlc_log_file, stream);
+    libvlc_log_set (inst, libvlc_log_file, stream);
 }
 
 /*** Stubs for the old interface ***/