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.

diff --git a/gst-libs/gst/vaapi/gstvaapidisplay.c b/gst-libs/gst/vaapi/gstvaapidisplay.c
index 6cdc9ed..776ab68 100644 (file)
--- a/gst-libs/gst/vaapi/gstvaapidisplay.c
+++ b/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)
 {

diff --git a/gst-libs/gst/vaapi/gstvaapidisplay_drm.c b/gst-libs/gst/vaapi/gstvaapidisplay_drm.c
index da4fe2a..9114bfc 100644 (file)
--- a/gst-libs/gst/vaapi/gstvaapidisplay_drm.c
+++ b/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;

diff --git a/gst-libs/gst/vaapi/gstvaapidisplay_priv.h b/gst-libs/gst/vaapi/gstvaapidisplay_priv.h
index 92d466a..35f9113 100644 (file)
--- a/gst-libs/gst/vaapi/gstvaapidisplay_priv.h
+++ b/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)
 {

diff --git a/gst-libs/gst/vaapi/gstvaapidisplay_wayland.c b/gst-libs/gst/vaapi/gstvaapidisplay_wayland.c
index 4ed6dbe..d7c5c1f 100644 (file)
--- a/gst-libs/gst/vaapi/gstvaapidisplay_wayland.c
+++ b/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) {

diff --git a/gst-libs/gst/vaapi/gstvaapidisplay_x11.c b/gst-libs/gst/vaapi/gstvaapidisplay_x11.c
index f97d794..ba9ea26 100644 (file)
--- a/gst-libs/gst/vaapi/gstvaapidisplay_x11.c
+++ b/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;

diff --git a/gst-libs/gst/vaapi/gstvaapidisplaycache.c b/gst-libs/gst/vaapi/gstvaapidisplaycache.c
index 29156c2..e8c08ac 100644 (file)
--- a/gst-libs/gst/vaapi/gstvaapidisplaycache.c
+++ b/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,69 +159,58 @@ compare_display_name (gconstpointer a, gconstpointer b)
   return strcmp (info->display_name, display_name) == 0;
 }
 
-/**
- * gst_vaapi_display_cache_new:
- *
- * Creates a new VA display cache.
- *
- * Return value: the newly created #GstVaapiDisplayCache object
- */
-GstVaapiDisplayCache *
-gst_vaapi_display_cache_new (void)
+static void
+gst_vaapi_display_cache_finalize (GstVaapiDisplayCache * cache)
 {
-  GstVaapiDisplayCache *cache;
+  GList *l;
 
-  cache = g_slice_new0 (GstVaapiDisplayCache);
-  if (!cache)
-    return NULL;
+  if (!cache->list)
+    return;
 
-  g_mutex_init (&cache->mutex);
-  return cache;
+  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:
- * @cache: the #GstVaapiDisplayCache to destroy
  *
- * Destroys a VA display cache.
+ * Creates a new VA display cache.
+ *
+ * Return value: the newly created #GstVaapiDisplayCache object
  */
-void
-gst_vaapi_display_cache_free (GstVaapiDisplayCache * cache)
+GstVaapiDisplayCache *
+gst_vaapi_display_cache_new (void)
 {
-  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);
+  return (GstVaapiDisplayCache *)
+      gst_vaapi_mini_object_new0 (gst_vaapi_display_cache_class ());
 }
 
 /**
- * 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);
 }
 
 /**

diff --git a/gst-libs/gst/vaapi/gstvaapidisplaycache.h b/gst-libs/gst/vaapi/gstvaapidisplaycache.h
index 62f7b90..f727432 100644 (file)
--- a/gst-libs/gst/vaapi/gstvaapidisplaycache.h
+++ b/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