gl/bufferpool: add configuration to extend buffer lifetime before reuse

Fixes a potential GPU stall if an immediately freed texture/buffer is
attempted to be reused immediately by the CPU, e.g. when uploading.

Problematic scenario is this:
1. element does GPU processing reading from texture
2. frees the buffer back to the pool
3. pool acquire returns the just released buffer
4. GPU processing then has to wait for the previous GPU operation to
   complete causing a stall

If there was a reliable way to know whether a buffer had been finished
with across all GPU drivers, we would use it.  However as that does not
exist, this workaround is to keep the released buffer unusable until the
next released buffer.

This is the same approach as is used in the qml (Qt5) elements.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/5144>

girs/GstGL-1.0.gir

@@ -10526,6 +10526,20 @@ you are writing to OpenGL. Conversely, combining #GST_MAP_GL with
         </parameter>
       </parameters>
     </function>
+    <function name="buffer_pool_config_get_gl_min_free_queue_size" c:identifier="gst_buffer_pool_config_get_gl_min_free_queue_size" version="1.24">
+      <doc xml:space="preserve" filename="../subprojects/gst-plugins-base/gst-libs/gst/gl/gstglbufferpool.c">See gst_buffer_pool_config_set_gl_min_free_queue_size().</doc>
+      <source-position filename="../subprojects/gst-plugins-base/gst-libs/gst/gl/gstglbufferpool.h"/>
+      <return-value transfer-ownership="none">
+        <doc xml:space="preserve" filename="../subprojects/gst-plugins-base/gst-libs/gst/gl/gstglbufferpool.c">then number of buffers configured the free queue</doc>
+        <type name="guint" c:type="guint"/>
+      </return-value>
+      <parameters>
+        <parameter name="config" transfer-ownership="none">
+          <doc xml:space="preserve" filename="../subprojects/gst-plugins-base/gst-libs/gst/gl/gstglbufferpool.c">a buffer pool config</doc>
+          <type name="Gst.Structure" c:type="GstStructure*"/>
+        </parameter>
+      </parameters>
+    </function>
     <function name="buffer_pool_config_set_gl_allocation_params" c:identifier="gst_buffer_pool_config_set_gl_allocation_params">
       <doc xml:space="preserve" filename="../subprojects/gst-plugins-base/gst-libs/gst/gl/gstglbufferpool.c">Sets @params on @config</doc>
       <source-position filename="../subprojects/gst-plugins-base/gst-libs/gst/gl/gstglbufferpool.h"/>
@@ -10543,6 +10557,33 @@ you are writing to OpenGL. Conversely, combining #GST_MAP_GL with
         </parameter>
       </parameters>
     </function>
+    <function name="buffer_pool_config_set_gl_min_free_queue_size" c:identifier="gst_buffer_pool_config_set_gl_min_free_queue_size" version="1.24">
+      <doc xml:space="preserve" filename="../subprojects/gst-plugins-base/gst-libs/gst/gl/gstglbufferpool.c">Instructs the #GstGLBufferPool to keep @queue_size amount of buffers around
+before allowing them for reuse.
+
+This is helpful to allow GPU processing to complete before the CPU
+operations on the same buffer could start.  Particularly useful when
+uploading or downloading data to/from the GPU.
+
+A value of 0 disabled this functionality.
+
+This value must be less than the configured maximum amount of buffers for
+this @config.</doc>
+      <source-position filename="../subprojects/gst-plugins-base/gst-libs/gst/gl/gstglbufferpool.h"/>
+      <return-value transfer-ownership="none">
+        <type name="none" c:type="void"/>
+      </return-value>
+      <parameters>
+        <parameter name="config" transfer-ownership="none">
+          <doc xml:space="preserve" filename="../subprojects/gst-plugins-base/gst-libs/gst/gl/gstglbufferpool.c">a buffer pool config</doc>
+          <type name="Gst.Structure" c:type="GstStructure*"/>
+        </parameter>
+        <parameter name="queue_size" transfer-ownership="none">
+          <doc xml:space="preserve" filename="../subprojects/gst-plugins-base/gst-libs/gst/gl/gstglbufferpool.c">the number of buffers</doc>
+          <type name="guint" c:type="guint"/>
+        </parameter>
+      </parameters>
+    </function>
     <function name="context_get_gl_display" c:identifier="gst_context_get_gl_display" version="1.4">
       <source-position filename="../subprojects/gst-plugins-base/gst-libs/gst/gl/gstgldisplay.h"/>
       <return-value transfer-ownership="none">

subprojects/gst-plugins-base/ext/gl/gstgldownloadelement.c

@@ -1453,6 +1453,7 @@ gst_gl_download_element_propose_allocation (GstBaseTransform * bt,
   /* the normal size of a frame */
   size = info.size;
   gst_buffer_pool_config_set_params (config, caps, size, 0, 0);
+  gst_buffer_pool_config_set_gl_min_free_queue_size (config, 1);
   gst_buffer_pool_config_add_option (config,
       GST_BUFFER_POOL_OPTION_GL_SYNC_META);
 

subprojects/gst-plugins-base/gst-libs/gst/gl/gstglbufferpool.c

@@ -28,6 +28,8 @@
 #include "gstglsyncmeta.h"
 #include "gstglutils.h"
 
+#define DEFAULT_FREE_QUEUE_MIN_DEPTH 0
+
 /**
  * SECTION:gstglbufferpool
  * @title: GstGLBufferPool
@@ -52,6 +54,11 @@ struct _GstGLBufferPoolPrivate
   GstCaps *caps;
   gboolean add_videometa;
   gboolean add_glsyncmeta;
+
+  gsize free_queue_min_depth;
+  /* work around the GPU still potentially executing a buffer after it has been
+   * freed by keeping N buffers before being able to reuse them */
+  GQueue *free_cache_buffers;
 };
 
 static void gst_gl_buffer_pool_finalize (GObject * object);
@@ -114,6 +121,18 @@ gst_gl_buffer_pool_set_config (GstBufferPool * pool, GstStructure * config)
   if (!gst_buffer_pool_config_get_allocator (config, &allocator, &alloc_params))
     goto wrong_config;
 
+  {
+    guint min_free_queue_size =
+        gst_buffer_pool_config_get_gl_min_free_queue_size (config);
+    if (min_buffers < min_free_queue_size) {
+      min_buffers = MAX (min_buffers, min_free_queue_size);
+    }
+    if (max_buffers != 0 && max_buffers < min_buffers)
+      goto wrong_buffer_count;
+
+    priv->free_queue_min_depth = min_free_queue_size;
+  }
+
   gst_caps_replace (&priv->caps, caps);
 
   if (priv->allocator)
@@ -255,6 +274,11 @@ wrong_allocator:
     GST_WARNING_OBJECT (pool, "Incorrect allocator type for this pool");
     return FALSE;
   }
+wrong_buffer_count:
+  {
+    GST_WARNING_OBJECT (pool, "Cannot achieve minimum buffer requirements");
+    return FALSE;
+  }
 }
 
 static gboolean
@@ -263,6 +287,20 @@ gst_gl_buffer_pool_start (GstBufferPool * pool)
   return GST_BUFFER_POOL_CLASS (parent_class)->start (pool);
 }
 
+static gboolean
+gst_gl_buffer_pool_stop (GstBufferPool * pool)
+{
+  GstGLBufferPool *glpool = GST_GL_BUFFER_POOL_CAST (pool);
+  GstGLBufferPoolPrivate *priv = glpool->priv;
+  GstBuffer *buffer;
+
+  while ((buffer = g_queue_pop_head (priv->free_cache_buffers))) {
+    GST_BUFFER_POOL_CLASS (parent_class)->release_buffer (pool, buffer);
+  }
+
+  return GST_BUFFER_POOL_CLASS (parent_class)->stop (pool);
+}
+
 /* This function handles GstBuffer creation */
 static GstFlowReturn
 gst_gl_buffer_pool_alloc (GstBufferPool * pool, GstBuffer ** buffer,
@@ -301,6 +339,27 @@ mem_create_failed:
   }
 }
 
+static void
+gst_gl_buffer_pool_release_buffer (GstBufferPool * pool, GstBuffer * buffer)
+{
+  GstGLBufferPool *glpool = GST_GL_BUFFER_POOL_CAST (pool);
+  GstGLBufferPoolPrivate *priv = glpool->priv;
+
+  if (priv->free_queue_min_depth == 0
+      && g_queue_get_length (priv->free_cache_buffers) == 0) {
+    GST_BUFFER_POOL_CLASS (parent_class)->release_buffer (pool, buffer);
+  } else {
+    g_queue_push_tail (priv->free_cache_buffers, buffer);
+
+    while (g_queue_get_length (priv->free_cache_buffers) >
+        priv->free_queue_min_depth) {
+      GstBuffer *release_buffer = g_queue_pop_head (priv->free_cache_buffers);
+      GST_BUFFER_POOL_CLASS (parent_class)->release_buffer (pool,
+          release_buffer);
+    }
+  }
+}
+
 /**
  * gst_gl_buffer_pool_new:
  * @context: the #GstGLContext to use
@@ -333,7 +392,9 @@ gst_gl_buffer_pool_class_init (GstGLBufferPoolClass * klass)
   gstbufferpool_class->get_options = gst_gl_buffer_pool_get_options;
   gstbufferpool_class->set_config = gst_gl_buffer_pool_set_config;
   gstbufferpool_class->alloc_buffer = gst_gl_buffer_pool_alloc;
+  gstbufferpool_class->release_buffer = gst_gl_buffer_pool_release_buffer;
   gstbufferpool_class->start = gst_gl_buffer_pool_start;
+  gstbufferpool_class->stop = gst_gl_buffer_pool_stop;
 }
 
 static void
@@ -348,6 +409,8 @@ gst_gl_buffer_pool_init (GstGLBufferPool * pool)
   priv->caps = NULL;
   priv->add_videometa = TRUE;
   priv->add_glsyncmeta = FALSE;
+  priv->free_queue_min_depth = DEFAULT_FREE_QUEUE_MIN_DEPTH;
+  priv->free_cache_buffers = g_queue_new ();
 }
 
 static void
@@ -361,6 +424,8 @@ gst_gl_buffer_pool_finalize (GObject * object)
   if (priv->caps)
     gst_caps_unref (priv->caps);
 
+  g_clear_pointer (&priv->free_cache_buffers, g_queue_free);
+
   G_OBJECT_CLASS (gst_gl_buffer_pool_parent_class)->finalize (object);
 
   /* only release the context once all our memory have been deleted */
@@ -439,3 +504,56 @@ gst_buffer_pool_config_set_gl_allocation_params (GstStructure * config,
   gst_structure_set (config, "gl-allocation-params",
       GST_TYPE_GL_ALLOCATION_PARAMS, params, NULL);
 }
+
+/**
+ * gst_buffer_pool_config_set_gl_min_free_queue_size:
+ * @config: a buffer pool config
+ * @queue_size: the number of buffers
+ *
+ * Instructs the #GstGLBufferPool to keep @queue_size amount of buffers around
+ * before allowing them for reuse.
+ *
+ * This is helpful to allow GPU processing to complete before the CPU
+ * operations on the same buffer could start.  Particularly useful when
+ * uploading or downloading data to/from the GPU.
+ *
+ * A value of 0 disabled this functionality.
+ *
+ * This value must be less than the configured maximum amount of buffers for
+ * this @config.
+ *
+ * Since: 1.24
+ */
+void
+gst_buffer_pool_config_set_gl_min_free_queue_size (GstStructure * config,
+    guint queue_size)
+{
+  g_return_if_fail (config != NULL);
+
+  gst_structure_set (config, "gl-min-free-queue-size",
+      G_TYPE_UINT, queue_size, NULL);
+}
+
+/**
+ * gst_buffer_pool_config_get_gl_min_free_queue_size:
+ * @config: a buffer pool config
+ *
+ * See gst_buffer_pool_config_set_gl_min_free_queue_size().
+ *
+ * Returns: then number of buffers configured the free queue
+ *
+ * Since: 1.24
+ */
+guint
+gst_buffer_pool_config_get_gl_min_free_queue_size (GstStructure * config)
+{
+  guint queue_size = 0;
+
+  g_return_val_if_fail (config != NULL, 0);
+
+  if (!gst_structure_get (config, "gl-min-free-queue-size",
+          G_TYPE_UINT, &queue_size, NULL))
+    queue_size = 0;
+
+  return queue_size;
+}

subprojects/gst-plugins-base/gst-libs/gst/gl/gstglbufferpool.h

@@ -77,6 +77,11 @@ GstGLAllocationParams * gst_buffer_pool_config_get_gl_allocation_params    (GstS
 GST_GL_API
 void                    gst_buffer_pool_config_set_gl_allocation_params    (GstStructure * config,
                                                                             const GstGLAllocationParams * params);
+GST_GL_API
+guint                   gst_buffer_pool_config_get_gl_min_free_queue_size  (GstStructure * config);
+GST_GL_API
+void                    gst_buffer_pool_config_set_gl_min_free_queue_size  (GstStructure * config,
+                                                                            guint queue_size);
 
 G_END_DECLS
 

subprojects/gst-plugins-base/gst-libs/gst/gl/gstglupload.c

@@ -458,7 +458,6 @@ _gl_memory_upload_propose_allocation (gpointer impl, GstQuery * decide_query,
     GstVideoInfo info;
     gsize size;
 
-
     if (!gst_video_info_from_caps (&info, caps))
       goto invalid_caps;
 
@@ -468,6 +467,8 @@ _gl_memory_upload_propose_allocation (gpointer impl, GstQuery * decide_query,
     /* the normal size of a frame */
     size = info.size;
     gst_buffer_pool_config_set_params (config, caps, size, 0, 0);
+    /* keep one buffer around before allowing acquire */
+    gst_buffer_pool_config_set_gl_min_free_queue_size (config, 1);
     gst_buffer_pool_config_add_option (config,
         GST_BUFFER_POOL_OPTION_GL_SYNC_META);
     if (upload->upload->priv->out_caps) {

subprojects/gst-plugins-base/tests/check/elements/glimagesink.c

@@ -29,6 +29,7 @@
 #include <gst/gst.h>
 #include <gst/video/video.h>
 #include <gst/check/gstcheck.h>
+#include <gst/gl/gl.h>
 
 static GstStaticPadTemplate srctemplate = GST_STATIC_PAD_TEMPLATE ("src",
     GST_PAD_SRC,
@@ -181,6 +182,7 @@ GST_START_TEST (test_query_drain)
   config = gst_buffer_pool_get_config (originpool);
   gst_buffer_pool_config_set_params (config, caps, size, maxbuffers,
       maxbuffers);
+  gst_buffer_pool_config_set_gl_min_free_queue_size (config, 0);
   fail_unless (gst_buffer_pool_set_config (originpool, config));
 
   /* The gl pool is setup and ready to be activated. */