d3d11bufferpool: Fix heavy CPU usage in case of fixed-size pool

There's no reason to release GstMemory manually at all.
If we do release GstMemory, corresponding GstBuffer will be
discarded by GstBufferPool baseclass because the size is changed
to zero.

Actual cause of heavy CPU usage in case of fixed-size pool
(i.e., decoder output buffer pool) and if we remove GstMemory from
GstBuffer is that GstBufferPool baseclass is doing busy wait in acquire_buffer()
for some reason. That needs to be investigated though, discarding
and re-alloc every GstBuffer is not ideal already.

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

subprojects/gst-plugins-bad/gst-libs/gst/d3d11/gstd3d11bufferpool.cpp

@@ -50,7 +50,6 @@ struct _GstD3D11BufferPoolPrivate
   GstD3D11Allocator *alloc[GST_VIDEO_MAX_PLANES];
 
   GstD3D11AllocationParams *d3d11_params;
-  gboolean texture_array_pool;
 
   gint stride[GST_VIDEO_MAX_PLANES];
   gsize offset[GST_VIDEO_MAX_PLANES];
@@ -66,10 +65,6 @@ static gboolean gst_d3d11_buffer_pool_set_config (GstBufferPool * pool,
     GstStructure * config);
 static GstFlowReturn gst_d3d11_buffer_pool_alloc_buffer (GstBufferPool * pool,
     GstBuffer ** buffer, GstBufferPoolAcquireParams * params);
-static GstFlowReturn gst_d3d11_buffer_pool_acquire_buffer (GstBufferPool * pool,
-    GstBuffer ** buffer, GstBufferPoolAcquireParams * params);
-static void gst_d3d11_buffer_pool_reset_buffer (GstBufferPool * pool,
-    GstBuffer * buffer);
 static gboolean gst_d3d11_buffer_pool_start (GstBufferPool * pool);
 static gboolean gst_d3d11_buffer_pool_stop (GstBufferPool * pool);
 
@@ -84,8 +79,6 @@ gst_d3d11_buffer_pool_class_init (GstD3D11BufferPoolClass * klass)
   bufferpool_class->get_options = gst_d3d11_buffer_pool_get_options;
   bufferpool_class->set_config = gst_d3d11_buffer_pool_set_config;
   bufferpool_class->alloc_buffer = gst_d3d11_buffer_pool_alloc_buffer;
-  bufferpool_class->acquire_buffer = gst_d3d11_buffer_pool_acquire_buffer;
-  bufferpool_class->reset_buffer = gst_d3d11_buffer_pool_reset_buffer;
   bufferpool_class->start = gst_d3d11_buffer_pool_start;
   bufferpool_class->stop = gst_d3d11_buffer_pool_stop;
 
@@ -244,10 +237,6 @@ gst_d3d11_buffer_pool_set_config (GstBufferPool * pool, GstStructure * config)
           max_buffers, max_array_size);
       max_buffers = max_array_size;
     }
-
-    priv->texture_array_pool = TRUE;
-  } else {
-    priv->texture_array_pool = FALSE;
   }
 
   offset = 0;
@@ -370,17 +359,10 @@ gst_d3d11_buffer_pool_alloc_buffer (GstBufferPool * pool, GstBuffer ** buffer,
   GstFlowReturn ret = GST_FLOW_OK;
 
   buf = gst_buffer_new ();
-  /* In case of texture-array, we are releasing memory objects in
+  ret = gst_d3d11_buffer_pool_fill_buffer (self, buf);
-   * the GstBufferPool::reset_buffer() so that GstD3D11Memory objects can be
+  if (ret != GST_FLOW_OK) {
-   * returned to the GstD3D11PoolAllocator. So, underlying GstD3D11Memory
+    gst_buffer_unref (buf);
-   * will be filled in the later GstBufferPool::acquire_buffer() call.
+    return ret;
-   * Don't fill memory here for non-texture-array therefore */
-  if (!priv->texture_array_pool) {
-    ret = gst_d3d11_buffer_pool_fill_buffer (self, buf);
-    if (ret != GST_FLOW_OK) {
-      gst_buffer_unref (buf);
-      return ret;
-    }
   }
 
   gst_buffer_add_video_meta_full (buf, GST_VIDEO_FRAME_FLAG_NONE,
@@ -393,48 +375,6 @@ gst_d3d11_buffer_pool_alloc_buffer (GstBufferPool * pool, GstBuffer ** buffer,
   return GST_FLOW_OK;
 }
 
-static GstFlowReturn
-gst_d3d11_buffer_pool_acquire_buffer (GstBufferPool * pool,
-    GstBuffer ** buffer, GstBufferPoolAcquireParams * params)
-{
-  GstD3D11BufferPool *self = GST_D3D11_BUFFER_POOL (pool);
-  GstD3D11BufferPoolPrivate *priv = self->priv;
-  GstFlowReturn ret;
-
-  ret = GST_BUFFER_POOL_CLASS (parent_class)->acquire_buffer (pool,
-      buffer, params);
-
-  if (ret != GST_FLOW_OK)
-    return ret;
-
-  /* Don't need special handling for non-texture-array case */
-  if (!priv->texture_array_pool)
-    return ret;
-
-  /* Baseclass will hold empty buffer in this case, fill GstMemory */
-  g_assert (gst_buffer_n_memory (*buffer) == 0);
-
-  return gst_d3d11_buffer_pool_fill_buffer (self, *buffer);
-}
-
-static void
-gst_d3d11_buffer_pool_reset_buffer (GstBufferPool * pool, GstBuffer * buffer)
-{
-  GstD3D11BufferPool *self = GST_D3D11_BUFFER_POOL (pool);
-  GstD3D11BufferPoolPrivate *priv = self->priv;
-
-  /* If we are using texture array, we should return GstD3D11Memory to
-   * to the GstD3D11PoolAllocator, so that the allocator can wake up
-   * if it's waiting for available memory object */
-  if (priv->texture_array_pool) {
-    GST_LOG_OBJECT (self, "Returning memory to allocator");
-    gst_buffer_remove_all_memory (buffer);
-  }
-
-  GST_BUFFER_POOL_CLASS (parent_class)->reset_buffer (pool, buffer);
-  GST_BUFFER_FLAGS (buffer) = 0;
-}
-
 static gboolean
 gst_d3d11_buffer_pool_start (GstBufferPool * pool)
 {

subprojects/gst-plugins-bad/tests/check/libs/d3d11memory.c

@@ -0,0 +1,233 @@
+/*
+ * GStreamer
+ * Copyright (C) 2023 Seungha Yang <seungha@centricular.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 51 Franklin St, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <gst/gst.h>
+#include <gst/check/gstcheck.h>
+#include <gst/d3d11/gstd3d11.h>
+#include <gst/video/video.h>
+#include <string.h>
+
+static GstD3D11Device *device = NULL;
+static GQuark memory_tester_quark;
+
+static void
+setup_func (void)
+{
+  device = gst_d3d11_device_new (0, D3D11_CREATE_DEVICE_BGRA_SUPPORT);
+  fail_unless (GST_IS_D3D11_DEVICE (device));
+
+  memory_tester_quark = g_quark_from_static_string ("gst-d3d11-memory-tester");
+}
+
+static void
+teardown_func (void)
+{
+  gst_object_unref (device);
+}
+
+static void
+allocator_finalize_cb (gboolean * alloc_finalized)
+{
+  *alloc_finalized = TRUE;
+}
+
+GST_START_TEST (test_free_active_allocator)
+{
+  GstD3D11PoolAllocator *alloc;
+  GstMemory *mem = NULL;
+  gboolean ret;
+  GstFlowReturn flow_ret;
+  gboolean alloc_finalized = FALSE;
+  D3D11_TEXTURE2D_DESC desc;
+
+  memset (&desc, 0, sizeof (desc));
+
+  desc.Width = 16;
+  desc.Height = 16;
+  desc.MipLevels = 1;
+  desc.ArraySize = 1;
+  desc.Format = DXGI_FORMAT_R8G8B8A8_UNORM;
+  desc.SampleDesc.Count = 1;
+  desc.SampleDesc.Quality = 0;
+  desc.Usage = D3D11_USAGE_DEFAULT;
+
+  alloc = gst_d3d11_pool_allocator_new (device, &desc);
+  fail_unless (alloc);
+
+  g_object_set_qdata_full (G_OBJECT (alloc), memory_tester_quark,
+      &alloc_finalized, (GDestroyNotify) allocator_finalize_cb);
+
+  /* inactive pool should return flusing */
+  flow_ret = gst_d3d11_pool_allocator_acquire_memory (alloc, &mem);
+  fail_unless (flow_ret == GST_FLOW_FLUSHING);
+  fail_if (mem);
+
+  ret = gst_d3d11_allocator_set_active (GST_D3D11_ALLOCATOR (alloc), TRUE);
+  fail_unless (ret);
+
+  flow_ret = gst_d3d11_pool_allocator_acquire_memory (alloc, &mem);
+  fail_unless (flow_ret == GST_FLOW_OK);
+  fail_unless (mem);
+
+  gst_object_unref (alloc);
+  /* Only memory should hold refcount at this moment */
+  fail_unless (G_OBJECT (alloc)->ref_count == 1);
+  fail_if (alloc_finalized);
+
+  /* allocator should be finalized as well */
+  gst_memory_unref (mem);
+  fail_unless (alloc_finalized);
+}
+
+GST_END_TEST;
+
+typedef struct
+{
+  GMutex lock;
+  GCond cond;
+  gboolean blocked;
+  GstBufferPool *pool;
+} UnblockTestData;
+
+static gpointer
+alloc_thread (UnblockTestData * data)
+{
+  GstBuffer *buffers[2];
+  GstBuffer *flush_buf = NULL;
+  GstFlowReturn ret;
+
+  g_mutex_lock (&data->lock);
+  ret = gst_buffer_pool_acquire_buffer (data->pool, &buffers[0], NULL);
+  fail_unless (ret == GST_FLOW_OK);
+
+  ret = gst_buffer_pool_acquire_buffer (data->pool, &buffers[1], NULL);
+  fail_unless (ret == GST_FLOW_OK);
+
+  /* below call will be blocked by buffer pool */
+  data->blocked = TRUE;
+  g_cond_signal (&data->cond);
+  g_mutex_unlock (&data->lock);
+
+  ret = gst_buffer_pool_acquire_buffer (data->pool, &flush_buf, NULL);
+  fail_unless (ret == GST_FLOW_FLUSHING);
+
+  gst_buffer_unref (buffers[0]);
+  gst_buffer_unref (buffers[1]);
+
+  return NULL;
+}
+
+GST_START_TEST (test_unblock_on_stop)
+{
+  GstStructure *config;
+  GstVideoInfo info;
+  GstCaps *caps;
+  GstD3D11AllocationParams *params;
+  UnblockTestData data;
+  GThread *thread;
+
+  data.blocked = FALSE;
+  g_mutex_init (&data.lock);
+  g_cond_init (&data.cond);
+
+  gst_video_info_set_format (&info, GST_VIDEO_FORMAT_NV12, 16, 16);
+  caps = gst_video_info_to_caps (&info);
+  fail_unless (caps);
+
+  data.pool = gst_d3d11_buffer_pool_new (device);
+  fail_unless (data.pool);
+
+  config = gst_buffer_pool_get_config (data.pool);
+  fail_unless (config);
+
+  params = gst_d3d11_allocation_params_new (device,
+      &info, GST_D3D11_ALLOCATION_FLAG_TEXTURE_ARRAY, 0, 0);
+  fail_unless (params);
+
+  params->desc[0].ArraySize = 2;
+
+  gst_buffer_pool_config_set_d3d11_allocation_params (config, params);
+  gst_d3d11_allocation_params_free (params);
+  gst_buffer_pool_config_set_params (config, caps, info.size, 0, 2);
+  gst_caps_unref (caps);
+
+  fail_unless (gst_buffer_pool_set_config (data.pool, config));
+  fail_unless (gst_buffer_pool_set_active (data.pool, TRUE));
+
+  thread = g_thread_new (NULL, (GThreadFunc) alloc_thread, &data);
+
+  g_mutex_lock (&data.lock);
+  while (!data.blocked)
+    g_cond_wait (&data.cond, &data.lock);
+  g_mutex_unlock (&data.lock);
+
+  /* Wait 1 second for the alloc thread to be actually blocked */
+  Sleep (1000);
+
+  fail_unless (gst_buffer_pool_set_active (data.pool, FALSE));
+  g_thread_join (thread);
+
+  gst_object_unref (data.pool);
+  g_mutex_clear (&data.lock);
+  g_cond_clear (&data.cond);
+}
+
+GST_END_TEST;
+
+static gboolean
+check_d3d11_device (void)
+{
+  GstD3D11Device *device;
+
+  device = gst_d3d11_device_new (0, D3D11_CREATE_DEVICE_BGRA_SUPPORT);
+  if (device) {
+    gst_object_unref (device);
+    return TRUE;
+  }
+
+  return FALSE;
+}
+
+static Suite *
+d3d11memory_suite (void)
+{
+  Suite *s;
+  TCase *tc_chain;
+
+  s = suite_create ("d3d11memory");
+  tc_chain = tcase_create ("general");
+
+  suite_add_tcase (s, tc_chain);
+  tcase_add_checked_fixture (tc_chain, setup_func, teardown_func);
+
+  if (!check_d3d11_device ())
+    return s;
+
+  tcase_add_test (tc_chain, test_free_active_allocator);
+  tcase_add_test (tc_chain, test_unblock_on_stop);
+
+  return s;
+}
+
+GST_CHECK_MAIN (d3d11memory);

subprojects/gst-plugins-bad/tests/check/meson.build

@@ -104,6 +104,7 @@ base_tests = [
   [['libs/vkinstance.c'], not gstvulkan_dep.found(), [gstvulkan_dep]],
   [['libs/vkimagebufferpool.c'], not gstvulkan_dep.found(), [gstvulkan_dep]],
   [['libs/d3d11device.cpp'], not gstd3d11_dep.found(), [gstd3d11_dep]],
+  [['libs/d3d11memory.c'], not gstd3d11_dep.found(), [gstd3d11_dep]],
   [['libs/cudamemory.c'], not gstcuda_dep.found(), [gstcuda_dep, gstcuda_stub_dep]],
 ]