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

[Laurent Pinchart]

On Fri, May 23, 2025 at 05:58:36AM +0000, Qi Hou wrote:
> On 2025年5月22日 20:55, Laurent Pinchart wrote:
> > 
> > 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.

You're right, I introduced a change in behaviour by mistake here. I'll
fix it. Thank you for noticing.

> > -       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