From d7cb68bfc72925d136699270ae0fec911e46bc66 Mon Sep 17 00:00:00 2001
From: Arun Raghavan <arun@asymptotic.io>
Date: Wed, 26 Mar 2025 11:55:13 -0400
Subject: [PATCH] gst: pool: Some refinements to min/max handling

A number of changes for correctness.

  1) We expose the actualy min and max values we support in the
     allocation query.

  2) We don't support max_buffers as 0, as unlimited buffers is not an
     option

  3) In ParamBuffers, we request the max_buffers from bufferpool config,
     as we cannot dynamically allocate buffers
---
 src/gst/gstpipewirepool.c | 10 ++++++++++
 src/gst/gstpipewirepool.h |  3 +++
 src/gst/gstpipewiresink.c | 17 +++++++++++------
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/src/gst/gstpipewirepool.c b/src/gst/gstpipewirepool.c
index 6c6e2dcab..ec0b9bc59 100644
--- a/src/gst/gstpipewirepool.c
+++ b/src/gst/gstpipewirepool.c
@@ -255,6 +255,16 @@ set_config (GstBufferPool * pool, GstStructure * config)
     return FALSE;
   }
 
+  /* We don't support unlimited buffers */
+  if (max_buffers == 0)
+    max_buffers = PIPEWIRE_POOL_MAX_BUFFERS;
+  /* Pick a sensible min to avoid starvation */
+  if (min_buffers == 0)
+    min_buffers = PIPEWIRE_POOL_MIN_BUFFERS;
+
+  if (min_buffers < PIPEWIRE_POOL_MIN_BUFFERS || max_buffers > PIPEWIRE_POOL_MAX_BUFFERS)
+    return FALSE;
+
   structure = gst_caps_get_structure (caps, 0);
   if (g_str_has_prefix (gst_structure_get_name (structure), "video/") ||
       g_str_has_prefix (gst_structure_get_name (structure), "image/")) {
diff --git a/src/gst/gstpipewirepool.h b/src/gst/gstpipewirepool.h
index fb00a100c..15b7f0d59 100644
--- a/src/gst/gstpipewirepool.h
+++ b/src/gst/gstpipewirepool.h
@@ -18,6 +18,9 @@ G_BEGIN_DECLS
 #define GST_TYPE_PIPEWIRE_POOL (gst_pipewire_pool_get_type())
 G_DECLARE_FINAL_TYPE (GstPipeWirePool, gst_pipewire_pool, GST, PIPEWIRE_POOL, GstBufferPool)
 
+#define PIPEWIRE_POOL_MIN_BUFFERS 2u
+#define PIPEWIRE_POOL_MAX_BUFFERS 16u
+
 typedef struct _GstPipeWirePoolData GstPipeWirePoolData;
 struct _GstPipeWirePoolData {
   GstPipeWirePool *pool;
diff --git a/src/gst/gstpipewiresink.c b/src/gst/gstpipewiresink.c
index 4457e2eac..2afcbbd78 100644
--- a/src/gst/gstpipewiresink.c
+++ b/src/gst/gstpipewiresink.c
@@ -40,8 +40,6 @@ GST_DEBUG_CATEGORY_STATIC (pipewire_sink_debug);
 #define DEFAULT_PROP_SLAVE_METHOD GST_PIPEWIRE_SINK_SLAVE_METHOD_NONE
 #define DEFAULT_PROP_USE_BUFFERPOOL USE_BUFFERPOOL_AUTO
 
-#define MIN_BUFFERS     8u
-
 enum
 {
   PROP_0,
@@ -167,7 +165,8 @@ gst_pipewire_sink_propose_allocation (GstBaseSink * bsink, GstQuery * query)
   GstPipeWireSink *pwsink = GST_PIPEWIRE_SINK (bsink);
 
   if (pwsink->use_bufferpool != USE_BUFFERPOOL_NO)
-    gst_query_add_allocation_pool (query, GST_BUFFER_POOL_CAST (pwsink->stream->pool), 0, 0, 0);
+    gst_query_add_allocation_pool (query, GST_BUFFER_POOL_CAST (pwsink->stream->pool), 0,
+        PIPEWIRE_POOL_MIN_BUFFERS, PIPEWIRE_POOL_MAX_BUFFERS);
 
   gst_query_add_allocation_meta (query, GST_VIDEO_META_API_TYPE, NULL);
   return TRUE;
@@ -310,6 +309,12 @@ gst_pipewire_sink_update_params (GstPipeWireSink *sink)
   config = gst_buffer_pool_get_config (GST_BUFFER_POOL (pool));
   gst_buffer_pool_config_get_params (config, &caps, &size, &min_buffers, &max_buffers);
 
+  /* We cannot dynamically grow the pool */
+  if (max_buffers == 0) {
+    GST_WARNING_OBJECT (sink, "cannot support unlimited buffers in pool");
+    max_buffers = PIPEWIRE_POOL_MAX_BUFFERS;
+  }
+
   spa_pod_builder_init (&b, buffer, sizeof (buffer));
   spa_pod_builder_push_object (&b, &f, SPA_TYPE_OBJECT_ParamBuffers, SPA_PARAM_Buffers);
   spa_pod_builder_add (&b,
@@ -324,10 +329,10 @@ gst_pipewire_sink_update_params (GstPipeWireSink *sink)
 
   spa_pod_builder_add (&b,
       SPA_PARAM_BUFFERS_stride,  SPA_POD_CHOICE_RANGE_Int(0, 0, INT32_MAX),
+      /* At this stage, we will request as many buffers as we _might_ need as
+       * the default, since we can't grow the pool once this is set */
       SPA_PARAM_BUFFERS_buffers, SPA_POD_CHOICE_RANGE_Int(
-              SPA_MAX(MIN_BUFFERS, min_buffers),
-              SPA_MAX(MIN_BUFFERS, min_buffers),
-              max_buffers ? max_buffers : INT32_MAX),
+              max_buffers, min_buffers, max_buffers),
       SPA_PARAM_BUFFERS_dataType, SPA_POD_CHOICE_FLAGS_Int(
                                                 (1<<SPA_DATA_MemFd) |
                                                 (1<<SPA_DATA_MemPtr)),