[EXT] [PATCH 4/4] gstreamer: Reduce indentation in gst_libcamera_create_video_pool()

[Qi Hou]

Hi,

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Sent: 2025年5月22日 20:55
> To: libcamera-devel at lists.libcamera.org
> Cc: Qi Hou <qi.hou at nxp.com>; Nicolas Dufresne
> <nicolas.dufresne at collabora.com>
> Subject: [EXT] [PATCH 4/4] gstreamer: Reduce indentation in
> gst_libcamera_create_video_pool()
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> 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.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/gstreamer/gstlibcamerasrc.cpp | 50 ++++++++++++++-----------------
>  1 file changed, 23 insertions(+), 27 deletions(-)
> 
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp
> b/src/gstreamer/gstlibcamerasrc.cpp
> index 380f8368af8b..e0eb6e7b4227 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -528,47 +528,43 @@ gst_libcamera_create_video_pool(GstLibcameraSrc
> *self,  {
>         g_autoptr(GstQuery) query = NULL;
>         const gboolean need_pool = true;
> -       gboolean has_video_meta = false;
>         GstBufferPool *video_pool = NULL;
> 
>         gst_libcamera_extrapolate_info(info, stream_cfg.stride);
> 
>         query = gst_query_new_allocation(caps, need_pool);
> -       if (!gst_pad_peer_query(srcpad, query))
> +       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);
> +               return { NULL, 0 };
> +       }

I think the failure of gst_pad_peer_query() is not a problem, because connected plugin such as filesink does not provide propose_allocation(). This case is like that of query success but downstream doesn't support videometa. Both require creating internal video pool.

Regards,
Qi Hou

> 
> -       if (!has_video_meta) {
> -               GstBufferPool *pool = NULL;
> +       if (gst_query_find_allocation_meta(query,
> GST_VIDEO_META_API_TYPE, NULL))
> +               return { NULL, 0 };
> 
> -               if (gst_query_get_n_allocation_pools(query) > 0)
> -                       gst_query_parse_nth_allocation_pool(query, 0,
> &pool, NULL, NULL, NULL);
> +       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.");
> +       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 };
>         }
> 
>         return { video_pool, 0 };
> --
> Regards,
> 
> Laurent Pinchart