[v2,2/7] gstreamer: Factor out video pool creation
diff mbox series

Message ID 20250604130741.9228-3-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • gstreamer: Miscellaneous cleanups + two fixes
Related show

Commit Message

Laurent Pinchart June 4, 2025, 1:07 p.m. UTC 
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@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(-)

--
Regards,

Laurent Pinchart

Comments

Nicolas Dufresne June 4, 2025, 5:08 p.m. UTC | #1 
Le mercredi 04 juin 2025 à 16:07 +0300, Laurent Pinchart a écrit :
> 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@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

Usually no s to buffer when saying "buffer pool". Its like "a noodle soup" versus
"a bowl of noodles".

> + * needed to accommodate stride mismatch. If the peer element supports the meta
> + * API, the stride will be negotiated, and no pool if needed.

if needed -> is 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.

Sounds great.

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> + */
> +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
Laurent Pinchart June 5, 2025, 8:50 a.m. UTC | #2 
On Wed, Jun 04, 2025 at 01:08:16PM -0400, Nicolas Dufresne wrote:
> Le mercredi 04 juin 2025 à 16:07 +0300, Laurent Pinchart a écrit :
> > 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@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
> 
> Usually no s to buffer when saying "buffer pool". Its like "a noodle soup" versus
> "a bowl of noodles".
> 
> > + * needed to accommodate stride mismatch. If the peer element supports the meta
> > + * API, the stride will be negotiated, and no pool if needed.
> 
> if needed -> is needed ? 

Oops. I've updated the comment to

 * This function creates and returns a video buffer pool for the given pad if
 * needed to accommodate stride mismatch. If the peer element supports stride
 * negotiation through the meta API, no pool is needed and the function will
 * return a null pool.
 *
 * \return A tuple containing the video buffers pool pointer and an error code

> > + *
> > + * \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.
> 
> Sounds great.
> 
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> 
> > + */
> > +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,
Nicolas Dufresne June 5, 2025, 12:49 p.m. UTC | #3 
Le jeudi 05 juin 2025 à 11:50 +0300, Laurent Pinchart a écrit :
> On Wed, Jun 04, 2025 at 01:08:16PM -0400, Nicolas Dufresne wrote:
> > Le mercredi 04 juin 2025 à 16:07 +0300, Laurent Pinchart a écrit :
> > > 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@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
> > 
> > Usually no s to buffer when saying "buffer pool". Its like "a noodle soup" versus
> > "a bowl of noodles".
> > 
> > > + * needed to accommodate stride mismatch. If the peer element supports the meta
> > > + * API, the stride will be negotiated, and no pool if needed.
> > 
> > if needed -> is needed ? 
> 
> Oops. I've updated the comment to
> 
>  * This function creates and returns a video buffer pool for the given pad if
>  * needed to accommodate stride mismatch. If the peer element supports stride
>  * negotiation through the meta API, no pool is needed and the function will
>  * return a null pool.
>  *
>  * \return A tuple containing the video buffers pool pointer and an error code

Ack, thanks.

> 
> > > + *
> > > + * \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.
> > 
> > Sounds great.
> > 
> > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > 
> > > + */
> > > +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,

Patch
diff mbox series 

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,