[PATCH v2 2/7] gstreamer: Factor out video pool creation

[Laurent Pinchart]

The gst_libcamera_src_negotiate() function uses 5 identation levels,
causing long lines. Move video pool creation to a separate function to
increase readability.

Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
Changes since v1:

- Document the gst_libcamera_create_video_pool() function
- Keep the gst_libcamera_extrapolate_info() call out of the new function
- Reorder function arguments
---
 src/gstreamer/gstlibcamerasrc.cpp | 114 +++++++++++++++++++-----------
 1 file changed, 72 insertions(+), 42 deletions(-)

diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
index 6ede43b06a8a..79b94c7ee7c2 100644
--- a/src/gstreamer/gstlibcamerasrc.cpp
+++ b/src/gstreamer/gstlibcamerasrc.cpp
@@ -29,6 +29,7 @@

 #include <atomic>
 #include <queue>
+#include <tuple>
 #include <vector>

 #include <libcamera/camera.h>
@@ -521,6 +522,72 @@ gst_libcamera_src_open(GstLibcameraSrc *self)
 	return true;
 }

+/**
+ * \brief Create a video pool for a pad
+ * \param[in] self The libcamerasrc instance
+ * \param[in] srcpad The pad
+ * \param[in] caps The pad caps
+ * \param[in] info The video info for the pad
+ *
+ * This function creates and returns a video buffers pool for the given pad if
+ * needed to accommodate stride mismatch. If the peer element supports the meta
+ * API, the stride will be negotiated, and no pool if needed.
+ *
+ * \return A tuple containing the video buffers pool pointer and an error code.
+ * When no error occurs, the pool can be null if the peer element supports
+ * stride negotiation.
+ */
+static std::tuple<GstBufferPool *, int>
+gst_libcamera_create_video_pool(GstLibcameraSrc *self, GstPad *srcpad,
+				GstCaps *caps, const GstVideoInfo *info)
+{
+	GstQuery *query = NULL;
+	const gboolean need_pool = true;
+	gboolean has_video_meta = false;
+	GstBufferPool *video_pool = NULL;
+
+	query = gst_query_new_allocation(caps, need_pool);
+	if (!gst_pad_peer_query(srcpad, query))
+		GST_DEBUG_OBJECT(self, "Didn't get downstream ALLOCATION hints");
+	else
+		has_video_meta = gst_query_find_allocation_meta(query, GST_VIDEO_META_API_TYPE, NULL);
+
+	if (!has_video_meta) {
+		GstBufferPool *pool = NULL;
+
+		if (gst_query_get_n_allocation_pools(query) > 0)
+			gst_query_parse_nth_allocation_pool(query, 0, &pool, NULL, NULL, NULL);
+
+		if (pool)
+			video_pool = pool;
+		else {
+			GstStructure *config;
+			guint min_buffers = 3;
+			video_pool = gst_video_buffer_pool_new();
+
+			config = gst_buffer_pool_get_config(video_pool);
+			gst_buffer_pool_config_set_params(config, caps, info->size, min_buffers, 0);
+
+			GST_DEBUG_OBJECT(self, "Own pool config is %" GST_PTR_FORMAT, config);
+
+			gst_buffer_pool_set_config(GST_BUFFER_POOL_CAST(video_pool), config);
+		}
+
+		GST_WARNING_OBJECT(self, "Downstream doesn't support video meta, need to copy frame.");
+
+		if (!gst_buffer_pool_set_active(video_pool, true)) {
+			gst_caps_unref(caps);
+			GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,
+					  ("Failed to active buffer pool"),
+					  ("gst_libcamera_src_negotiate() failed."));
+			return { NULL, -EINVAL };
+		}
+	}
+
+	gst_query_unref(query);
+	return { video_pool, 0 };
+}
+
 /* Must be called with stream_lock held. */
 static bool
 gst_libcamera_src_negotiate(GstLibcameraSrc *self)
@@ -603,50 +670,13 @@ gst_libcamera_src_negotiate(GstLibcameraSrc *self)
 		/* Stride mismatch between camera stride and that calculated by video-info. */
 		if (static_cast<unsigned int>(info.stride[0]) != stream_cfg.stride &&
 		    GST_VIDEO_INFO_FORMAT(&info) != GST_VIDEO_FORMAT_ENCODED) {
-			GstQuery *query = NULL;
-			const gboolean need_pool = true;
-			gboolean has_video_meta = false;
-
 			gst_libcamera_extrapolate_info(&info, stream_cfg.stride);

-			query = gst_query_new_allocation(caps, need_pool);
-			if (!gst_pad_peer_query(srcpad, query))
-				GST_DEBUG_OBJECT(self, "Didn't get downstream ALLOCATION hints");
-			else
-				has_video_meta = gst_query_find_allocation_meta(query, GST_VIDEO_META_API_TYPE, NULL);
-
-			if (!has_video_meta) {
-				GstBufferPool *pool = NULL;
-
-				if (gst_query_get_n_allocation_pools(query) > 0)
-					gst_query_parse_nth_allocation_pool(query, 0, &pool, NULL, NULL, NULL);
-
-				if (pool)
-					video_pool = pool;
-				else {
-					GstStructure *config;
-					guint min_buffers = 3;
-					video_pool = gst_video_buffer_pool_new();
-
-					config = gst_buffer_pool_get_config(video_pool);
-					gst_buffer_pool_config_set_params(config, caps, info.size, min_buffers, 0);
-
-					GST_DEBUG_OBJECT(self, "Own pool config is %" GST_PTR_FORMAT, config);
-
-					gst_buffer_pool_set_config(GST_BUFFER_POOL_CAST(video_pool), config);
-				}
-
-				GST_WARNING_OBJECT(self, "Downstream doesn't support video meta, need to copy frame.");
-
-				if (!gst_buffer_pool_set_active(video_pool, true)) {
-					gst_caps_unref(caps);
-					GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,
-							  ("Failed to active buffer pool"),
-							  ("gst_libcamera_src_negotiate() failed."));
-					return false;
-				}
-			}
-			gst_query_unref(query);
+			std::tie(video_pool, ret) =
+				gst_libcamera_create_video_pool(self, srcpad,
+								caps, &info);
+			if (ret)
+				return false;
 		}

 		GstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,
--
Regards,

Laurent Pinchart