[PATCH v2 3/7] gstreamer: Reduce indentation in gst_libcamera_create_video_pool()

[Nicolas Dufresne]

Le mercredi 04 juin 2025 à 16:07 +0300, Laurent Pinchart a écrit :
> Now that video pool creation is handled by a dedicated function, the
> logic can be simplified by returning early instead of nesting scopes. Do
> so to decrease indentation and improve readability, and document the
> implementation of the function with comments.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>

Ack.

> ---
> Changes since v1:
> 
> - Fixed regression that didn't allocate a pool when the peer allocation
>   can't be queried
> - Document the implementation of the gst_libcamera_create_video_pool()
>   function with comments
> ---
>  src/gstreamer/gstlibcamerasrc.cpp | 57 ++++++++++++++++---------------
>  1 file changed, 30 insertions(+), 27 deletions(-)
> 
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 79b94c7ee7c2..b907a5759740 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -543,45 +543,48 @@ gst_libcamera_create_video_pool(GstLibcameraSrc *self, GstPad *srcpad,
>  {
>  	GstQuery *query = NULL;
>  	const gboolean need_pool = true;
> -	gboolean has_video_meta = false;
>  	GstBufferPool *video_pool = NULL;
>  
> +	/*
> +	 * Get the peer allocation hints to check if it supports the meta API.
> +	 * If so, the stride will be negotiated, and there's no need to create a
> +	 * video pool.
> +	 */
>  	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);
> +	else if (gst_query_find_allocation_meta(query, GST_VIDEO_META_API_TYPE, NULL))
> +		return { NULL, 0 };
>  
> -	if (!has_video_meta) {
> -		GstBufferPool *pool = NULL;
> +	GST_WARNING_OBJECT(self, "Downstream doesn't support video meta, need to copy frame.");
>  
> -		if (gst_query_get_n_allocation_pools(query) > 0)
> -			gst_query_parse_nth_allocation_pool(query, 0, &pool, NULL, NULL, NULL);
> +	/*
> +	 * If the allocation query has pools, use the first one. Otherwise,
> +	 * create a new pool.
> +	 */
> +	if (gst_query_get_n_allocation_pools(query) > 0)
> +		gst_query_parse_nth_allocation_pool(query, 0, &video_pool, NULL, NULL, NULL);
>  
> -		if (pool)
> -			video_pool = pool;
> -		else {
> -			GstStructure *config;
> -			guint min_buffers = 3;
> -			video_pool = gst_video_buffer_pool_new();
> +	if (!video_pool) {
> +		GstStructure *config;
> +		guint min_buffers = 3;
>  
> -			config = gst_buffer_pool_get_config(video_pool);
> -			gst_buffer_pool_config_set_params(config, caps, info->size, min_buffers, 0);
> +		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_DEBUG_OBJECT(self, "Own pool config is %" GST_PTR_FORMAT, config);
>  
> -			gst_buffer_pool_set_config(GST_BUFFER_POOL_CAST(video_pool), 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 };
> -		}
> +	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);