display: make cache maintenance really MT-safe.

Make sure to initialize one GstVaapiDisplay at a time, even in threaded
environments. This makes sure the display cache is also consistent
during the whole display creation process. In the former implementation,
there were risks that display cache got updated in another thread.

gst-libs/gst/vaapi/gstvaapidisplay.c

@@ -85,7 +85,8 @@ enum
   N_PROPERTIES
 };
 
-static GstVaapiDisplayCache *g_display_cache = NULL;
+static GstVaapiDisplayCache *g_display_cache;
+static GMutex g_display_cache_lock;
 
 static GParamSpec *g_properties[N_PROPERTIES] = { NULL, };
 
@@ -122,29 +123,27 @@ libgstvaapi_init_once (void)
   g_once_init_leave (&g_once, TRUE);
 }
 
-static inline GstVaapiDisplayCache *
+static GstVaapiDisplayCache *
 get_display_cache (void)
 {
+  GstVaapiDisplayCache *cache = NULL;
+
+  g_mutex_lock (&g_display_cache_lock);
   if (!g_display_cache)
     g_display_cache = gst_vaapi_display_cache_new ();
-  return g_display_cache;
-}
-
-GstVaapiDisplayCache *
-gst_vaapi_display_get_cache (void)
-{
-  return get_display_cache ();
+  if (g_display_cache)
+    cache = gst_vaapi_display_cache_ref (g_display_cache);
+  g_mutex_unlock (&g_display_cache_lock);
+  return cache;
 }
 
 static void
 free_display_cache (void)
 {
-  if (!g_display_cache)
-    return;
-  if (gst_vaapi_display_cache_get_size (g_display_cache) > 0)
-    return;
-  gst_vaapi_display_cache_free (g_display_cache);
-  g_display_cache = NULL;
+  g_mutex_lock (&g_display_cache_lock);
+  if (g_display_cache && gst_vaapi_display_cache_is_empty (g_display_cache))
+    gst_vaapi_display_cache_replace (&g_display_cache, NULL);
+  g_mutex_unlock (&g_display_cache_lock);
 }
 
 /* GstVaapiDisplayType enumerations */
@@ -857,20 +856,21 @@ gst_vaapi_display_destroy (GstVaapiDisplay * display)
 
   gst_vaapi_display_replace_internal (&priv->parent, NULL);
 
-  if (g_display_cache) {
-    gst_vaapi_display_cache_remove (get_display_cache (), display);
-    free_display_cache ();
-  }
+  g_mutex_lock (&g_display_cache_lock);
+  if (priv->cache)
+    gst_vaapi_display_cache_remove (priv->cache, display);
+  g_mutex_unlock (&g_display_cache_lock);
+  gst_vaapi_display_cache_replace (&priv->cache, NULL);
+  free_display_cache ();
 }
 
 static gboolean
-gst_vaapi_display_create (GstVaapiDisplay * display,
+gst_vaapi_display_create_unlocked (GstVaapiDisplay * display,
     GstVaapiDisplayInitType init_type, gpointer init_value)
 {
   GstVaapiDisplayPrivate *const priv = GST_VAAPI_DISPLAY_GET_PRIVATE (display);
   const GstVaapiDisplayClass *const klass =
       GST_VAAPI_DISPLAY_GET_CLASS (display);
-  GstVaapiDisplayCache *cache;
   gint major_version, minor_version;
   VAStatus status;
   GstVaapiDisplayInfo info;
@@ -909,10 +909,7 @@ gst_vaapi_display_create (GstVaapiDisplay * display,
   if (!priv->display)
     return FALSE;
 
-  cache = get_display_cache ();
-  if (!cache)
-    return FALSE;
-  cached_info = gst_vaapi_display_cache_lookup_by_va_display (cache,
+  cached_info = gst_vaapi_display_cache_lookup_by_va_display (priv->cache,
       info.va_display);
   if (cached_info) {
     gst_vaapi_display_replace_internal (&priv->parent, cached_info->display);
@@ -927,12 +924,32 @@ gst_vaapi_display_create (GstVaapiDisplay * display,
   }
 
   if (!cached_info) {
-    if (!gst_vaapi_display_cache_add (cache, &info))
+    if (!gst_vaapi_display_cache_add (priv->cache, &info))
       return FALSE;
   }
   return TRUE;
 }
 
+static gboolean
+gst_vaapi_display_create (GstVaapiDisplay * display,
+    GstVaapiDisplayInitType init_type, gpointer init_value)
+{
+  GstVaapiDisplayPrivate *const priv = GST_VAAPI_DISPLAY_GET_PRIVATE (display);
+  GstVaapiDisplayCache *cache;
+  gboolean success;
+
+  cache = get_display_cache ();
+  if (!cache)
+    return FALSE;
+  gst_vaapi_display_cache_replace (&priv->cache, cache);
+  gst_vaapi_display_cache_unref (cache);
+
+  g_mutex_lock (&g_display_cache_lock);
+  success = gst_vaapi_display_create_unlocked (display, init_type, init_value);
+  g_mutex_unlock (&g_display_cache_lock);
+  return success;
+}
+
 static void
 gst_vaapi_display_lock_default (GstVaapiDisplay * display)
 {

gst-libs/gst/vaapi/gstvaapidisplay_drm.c

@@ -219,12 +219,9 @@ gst_vaapi_display_drm_open_display (GstVaapiDisplay * display,
 {
   GstVaapiDisplayDRMPrivate *const priv =
       GST_VAAPI_DISPLAY_DRM_PRIVATE (display);
-  GstVaapiDisplayCache *cache;
+  GstVaapiDisplayCache *const cache = GST_VAAPI_DISPLAY_GET_CACHE (display);
   const GstVaapiDisplayInfo *info;
 
-  cache = gst_vaapi_display_get_cache ();
-  g_return_val_if_fail (cache != NULL, FALSE);
-
   if (!set_device_path (display, name))
     return FALSE;
 
@@ -271,15 +268,11 @@ gst_vaapi_display_drm_get_display_info (GstVaapiDisplay * display,
 {
   GstVaapiDisplayDRMPrivate *const priv =
       GST_VAAPI_DISPLAY_DRM_PRIVATE (display);
-  GstVaapiDisplayCache *cache;
+  GstVaapiDisplayCache *const cache = GST_VAAPI_DISPLAY_GET_CACHE (display);
   const GstVaapiDisplayInfo *cached_info;
 
   /* Return any cached info even if child has its own VA display */
-  cache = gst_vaapi_display_get_cache ();
-  if (!cache)
-    return FALSE;
-  cached_info =
-      gst_vaapi_display_cache_lookup_by_native_display (cache,
+  cached_info = gst_vaapi_display_cache_lookup_by_native_display (cache,
       GINT_TO_POINTER (priv->drm_device), GST_VAAPI_DISPLAY_TYPES (display));
   if (cached_info) {
     *info = *cached_info;

gst-libs/gst/vaapi/gstvaapidisplay_priv.h

@@ -128,9 +128,20 @@ typedef void (*GstVaapiDisplayGetSizeMFunc) (GstVaapiDisplay * display,
 #define GST_VAAPI_DISPLAY_HAS_VPP(display) \
     gst_vaapi_display_has_video_processing(GST_VAAPI_DISPLAY_CAST(display))
 
+/**
+ * GST_VAAPI_DISPLAY_CACHE:
+ * @display: a @GstVaapiDisplay
+ *
+ * Returns the #GstVaapiDisplayCache attached to the supplied @display object.
+ */
+#undef  GST_VAAPI_DISPLAY_GET_CACHE
+#define GST_VAAPI_DISPLAY_GET_CACHE(display) \
+    (GST_VAAPI_DISPLAY_GET_PRIVATE (display)->cache)
+
 struct _GstVaapiDisplayPrivate
 {
   GstVaapiDisplay *parent;
+  GstVaapiDisplayCache *cache;
   GRecMutex mutex;
   GstVaapiDisplayType display_type;
   VADisplay display;
@@ -214,9 +225,6 @@ GstVaapiDisplay *
 gst_vaapi_display_new (const GstVaapiDisplayClass * klass,
     GstVaapiDisplayInitType init_type, gpointer init_value);
 
-GstVaapiDisplayCache *
-gst_vaapi_display_get_cache (void);
-
 static inline guint
 gst_vaapi_display_get_display_types (GstVaapiDisplay * display)
 {

gst-libs/gst/vaapi/gstvaapidisplay_wayland.c

@@ -204,12 +204,9 @@ gst_vaapi_display_wayland_open_display (GstVaapiDisplay * display,
 {
   GstVaapiDisplayWaylandPrivate *const priv =
       GST_VAAPI_DISPLAY_WAYLAND_GET_PRIVATE (display);
-  GstVaapiDisplayCache *cache;
+  GstVaapiDisplayCache *const cache = GST_VAAPI_DISPLAY_GET_CACHE (display);
   const GstVaapiDisplayInfo *info;
 
-  cache = gst_vaapi_display_get_cache ();
-  g_return_val_if_fail (cache != NULL, FALSE);
-
   if (!set_display_name (display, name))
     return FALSE;
 
@@ -266,13 +263,10 @@ gst_vaapi_display_wayland_get_display_info (GstVaapiDisplay * display,
 {
   GstVaapiDisplayWaylandPrivate *const priv =
       GST_VAAPI_DISPLAY_WAYLAND_GET_PRIVATE (display);
-  GstVaapiDisplayCache *cache;
+  GstVaapiDisplayCache *const cache = GST_VAAPI_DISPLAY_GET_CACHE (display);
   const GstVaapiDisplayInfo *cached_info;
 
   /* Return any cached info even if child has its own VA display */
-  cache = gst_vaapi_display_get_cache ();
-  if (!cache)
-    return FALSE;
   cached_info = gst_vaapi_display_cache_lookup_by_native_display (cache,
       priv->wl_display, GST_VAAPI_DISPLAY_TYPES (display));
   if (cached_info) {

gst-libs/gst/vaapi/gstvaapidisplay_x11.c

@@ -177,12 +177,9 @@ gst_vaapi_display_x11_open_display (GstVaapiDisplay * base_display,
 {
   GstVaapiDisplayX11 *const display = GST_VAAPI_DISPLAY_X11_CAST (base_display);
   GstVaapiDisplayX11Private *const priv = &display->priv;
-  GstVaapiDisplayCache *cache;
+  GstVaapiDisplayCache *const cache = GST_VAAPI_DISPLAY_GET_CACHE (display);
   const GstVaapiDisplayInfo *info;
 
-  cache = gst_vaapi_display_get_cache ();
-  g_return_val_if_fail (cache != NULL, FALSE);
-
   if (!set_display_name (display, name))
     return FALSE;
 
@@ -258,15 +255,11 @@ gst_vaapi_display_x11_get_display_info (GstVaapiDisplay * display,
 {
   GstVaapiDisplayX11Private *const priv =
       GST_VAAPI_DISPLAY_X11_PRIVATE (display);
-  GstVaapiDisplayCache *cache;
+  GstVaapiDisplayCache *const cache = GST_VAAPI_DISPLAY_GET_CACHE (display);
   const GstVaapiDisplayInfo *cached_info;
 
   /* Return any cached info even if child has its own VA display */
-  cache = gst_vaapi_display_get_cache ();
-  if (!cache)
-    return FALSE;
-  cached_info =
-      gst_vaapi_display_cache_lookup_by_native_display (cache,
+  cached_info = gst_vaapi_display_cache_lookup_by_native_display (cache,
       priv->x11_display, GST_VAAPI_DISPLAY_TYPES (display));
   if (cached_info) {
     *info = *cached_info;

gst-libs/gst/vaapi/gstvaapidisplaycache.c

@@ -36,7 +36,7 @@ struct _CacheEntry
 
 struct _GstVaapiDisplayCache
 {
-  GMutex mutex;
+  GstVaapiMiniObject parent_instance;
   GList *list;
 };
 
@@ -103,7 +103,6 @@ cache_lookup_1 (GstVaapiDisplayCache * cache, GCompareFunc func,
 {
   GList *l;
 
-  g_mutex_lock (&cache->mutex);
   for (l = cache->list; l != NULL; l = l->next) {
     GstVaapiDisplayInfo *const info = &((CacheEntry *) l->data)->info;
     if (!is_compatible_display_type (info->display_type, display_types))
@@ -111,7 +110,6 @@ cache_lookup_1 (GstVaapiDisplayCache * cache, GCompareFunc func,
     if (func (info, data))
       break;
   }
-  g_mutex_unlock (&cache->mutex);
   return l;
 }
 
@@ -161,6 +159,30 @@ compare_display_name (gconstpointer a, gconstpointer b)
   return strcmp (info->display_name, display_name) == 0;
 }
 
+static void
+gst_vaapi_display_cache_finalize (GstVaapiDisplayCache * cache)
+{
+  GList *l;
+
+  if (!cache->list)
+    return;
+
+  for (l = cache->list; l != NULL; l = l->next)
+    cache_entry_free (l->data);
+  g_list_free (cache->list);
+  cache->list = NULL;
+}
+
+static const GstVaapiMiniObjectClass *
+gst_vaapi_display_cache_class (void)
+{
+  static const GstVaapiMiniObjectClass GstVaapiDisplayCacheClass = {
+    .size = sizeof (GstVaapiDisplayCache),
+    .finalize = (GDestroyNotify) gst_vaapi_display_cache_finalize
+  };
+  return &GstVaapiDisplayCacheClass;
+}
+
 /**
  * gst_vaapi_display_cache_new:
  *
@@ -171,59 +193,24 @@ compare_display_name (gconstpointer a, gconstpointer b)
 GstVaapiDisplayCache *
 gst_vaapi_display_cache_new (void)
 {
-  GstVaapiDisplayCache *cache;
-
-  cache = g_slice_new0 (GstVaapiDisplayCache);
-  if (!cache)
-    return NULL;
-
-  g_mutex_init (&cache->mutex);
-  return cache;
+  return (GstVaapiDisplayCache *)
+      gst_vaapi_mini_object_new0 (gst_vaapi_display_cache_class ());
 }
 
 /**
- * gst_vaapi_display_cache_new:
- * @cache: the #GstVaapiDisplayCache to destroy
- *
- * Destroys a VA display cache.
- */
-void
-gst_vaapi_display_cache_free (GstVaapiDisplayCache * cache)
-{
-  GList *l;
-
-  if (!cache)
-    return;
-
-  if (cache->list) {
-    for (l = cache->list; l != NULL; l = l->next)
-      cache_entry_free (l->data);
-    g_list_free (cache->list);
-    cache->list = NULL;
-  }
-  g_mutex_clear (&cache->mutex);
-  g_slice_free (GstVaapiDisplayCache, cache);
-}
-
-/**
- * gst_vaapi_display_cache_get_size:
+ * gst_vaapi_display_cache_is_empty:
  * @cache: the #GstVaapiDisplayCache
  *
- * Gets the size of the display cache @cache.
+ * Checks whether the display cache @cache is empty.
  *
- * Return value: the size of the display cache
+ * Return value: %TRUE if the display @cache is empty, %FALSE otherwise.
  */
-guint
-gst_vaapi_display_cache_get_size (GstVaapiDisplayCache * cache)
+gboolean
+gst_vaapi_display_cache_is_empty (GstVaapiDisplayCache * cache)
 {
-  guint size;
-
   g_return_val_if_fail (cache != NULL, 0);
 
-  g_mutex_lock (&cache->mutex);
-  size = g_list_length (cache->list);
-  g_mutex_unlock (&cache->mutex);
-  return size;
+  return cache->list == NULL;
 }
 
 /**
@@ -249,9 +236,7 @@ gst_vaapi_display_cache_add (GstVaapiDisplayCache * cache,
   if (!entry)
     return FALSE;
 
-  g_mutex_lock (&cache->mutex);
   cache->list = g_list_prepend (cache->list, entry);
-  g_mutex_unlock (&cache->mutex);
   return TRUE;
 }
 
@@ -274,9 +259,7 @@ gst_vaapi_display_cache_remove (GstVaapiDisplayCache * cache,
     return;
 
   cache_entry_free (m->data);
-  g_mutex_lock (&cache->mutex);
   cache->list = g_list_delete_link (cache->list, m);
-  g_mutex_unlock (&cache->mutex);
 }
 
 /**

gst-libs/gst/vaapi/gstvaapidisplaycache.h

@@ -25,6 +25,7 @@
 
 #include "libgstvaapi_priv_check.h"
 #include <gst/vaapi/gstvaapidisplay.h>
+#include "gstvaapiminiobject.h"
 
 typedef struct _GstVaapiDisplayCache            GstVaapiDisplayCache;
 
@@ -32,13 +33,18 @@ G_GNUC_INTERNAL
 GstVaapiDisplayCache *
 gst_vaapi_display_cache_new (void);
 
-G_GNUC_INTERNAL
-void
-gst_vaapi_display_cache_free (GstVaapiDisplayCache * cache);
+#define gst_vaapi_display_cache_ref(cache) \
+    ((GstVaapiDisplayCache *) gst_vaapi_mini_object_ref ( \
+        GST_VAAPI_MINI_OBJECT (cache)))
+#define gst_vaapi_display_cache_unref(cache) \
+    gst_vaapi_mini_object_unref (GST_VAAPI_MINI_OBJECT (cache))
+#define gst_vaapi_display_cache_replace(old_cache_ptr, new_cache) \
+    gst_vaapi_mini_object_replace ((GstVaapiMiniObject **) (old_cache_ptr), \
+        GST_VAAPI_MINI_OBJECT (new_cache))
 
 G_GNUC_INTERNAL
-guint
-gst_vaapi_display_cache_get_size (GstVaapiDisplayCache * cache);
+gboolean
+gst_vaapi_display_cache_is_empty (GstVaapiDisplayCache * cache);
 
 G_GNUC_INTERNAL
 gboolean