[{"id":34416,"web_url":"https://patchwork.libcamera.org/comment/34416/","msgid":"<35ecbe156745637a51bc2f2bd64363530729cde8.camel@collabora.com>","date":"2025-06-04T17:11:21","subject":"Re: [PATCH v2 3/7] gstreamer: Reduce indentation in\n\tgst_libcamera_create_video_pool()","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Le mercredi 04 juin 2025 à 16:07 +0300, Laurent Pinchart a écrit :\n> Now that video pool creation is handled by a dedicated function, the\n> logic can be simplified by returning early instead of nesting scopes. Do\n> so to decrease indentation and improve readability, and document the\n> implementation of the function with comments.\n> \n> Signed-off-by: Laurent Pinchart \n> Reviewed-by: Nicolas Dufresne \n\nAck.\n\n> ---\n> Changes since v1:\n> \n> - Fixed regression that didn't allocate a pool when the peer allocation\n>   can't be queried\n> - Document the implementation of the gst_libcamera_create_video_pool()\n>   function with comments\n> ---\n>  src/gstreamer/gstlibcamerasrc.cpp | 57 ++++++++++++++++---------------\n>  1 file changed, 30 insertions(+), 27 deletions(-)\n> \n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 79b94c7ee7c2..b907a5759740 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -543,45 +543,48 @@ gst_libcamera_create_video_pool(GstLibcameraSrc *self, GstPad *srcpad,\n>  {\n>  \tGstQuery *query = NULL;\n>  \tconst gboolean need_pool = true;\n> -\tgboolean has_video_meta = false;\n>  \tGstBufferPool *video_pool = NULL;\n>  \n> +\t/*\n> +\t * Get the peer allocation hints to check if it supports the meta API.\n> +\t * If so, the stride will be negotiated, and there's no need to create a\n> +\t * video pool.\n> +\t */\n>  \tquery = gst_query_new_allocation(caps, need_pool);\n> +\n>  \tif (!gst_pad_peer_query(srcpad, query))\n>  \t\tGST_DEBUG_OBJECT(self, \"Didn't get downstream ALLOCATION hints\");\n> -\telse\n> -\t\thas_video_meta = gst_query_find_allocation_meta(query, GST_VIDEO_META_API_TYPE, NULL);\n> +\telse if (gst_query_find_allocation_meta(query, GST_VIDEO_META_API_TYPE, NULL))\n> +\t\treturn { NULL, 0 };\n>  \n> -\tif (!has_video_meta) {\n> -\t\tGstBufferPool *pool = NULL;\n> +\tGST_WARNING_OBJECT(self, \"Downstream doesn't support video meta, need to copy frame.\");\n>  \n> -\t\tif (gst_query_get_n_allocation_pools(query) > 0)\n> -\t\t\tgst_query_parse_nth_allocation_pool(query, 0, &pool, NULL, NULL, NULL);\n> +\t/*\n> +\t * If the allocation query has pools, use the first one. Otherwise,\n> +\t * create a new pool.\n> +\t */\n> +\tif (gst_query_get_n_allocation_pools(query) > 0)\n> +\t\tgst_query_parse_nth_allocation_pool(query, 0, &video_pool, NULL, NULL, NULL);\n>  \n> -\t\tif (pool)\n> -\t\t\tvideo_pool = pool;\n> -\t\telse {\n> -\t\t\tGstStructure *config;\n> -\t\t\tguint min_buffers = 3;\n> -\t\t\tvideo_pool = gst_video_buffer_pool_new();\n> +\tif (!video_pool) {\n> +\t\tGstStructure *config;\n> +\t\tguint min_buffers = 3;\n>  \n> -\t\t\tconfig = gst_buffer_pool_get_config(video_pool);\n> -\t\t\tgst_buffer_pool_config_set_params(config, caps, info->size, min_buffers, 0);\n> +\t\tvideo_pool = gst_video_buffer_pool_new();\n> +\t\tconfig = gst_buffer_pool_get_config(video_pool);\n> +\t\tgst_buffer_pool_config_set_params(config, caps, info->size, min_buffers, 0);\n>  \n> -\t\t\tGST_DEBUG_OBJECT(self, \"Own pool config is %\" GST_PTR_FORMAT, config);\n> +\t\tGST_DEBUG_OBJECT(self, \"Own pool config is %\" GST_PTR_FORMAT, config);\n>  \n> -\t\t\tgst_buffer_pool_set_config(GST_BUFFER_POOL_CAST(video_pool), config);\n> -\t\t}\n> +\t\tgst_buffer_pool_set_config(GST_BUFFER_POOL_CAST(video_pool), config);\n> +\t}\n>  \n> -\t\tGST_WARNING_OBJECT(self, \"Downstream doesn't support video meta, need to copy frame.\");\n> -\n> -\t\tif (!gst_buffer_pool_set_active(video_pool, true)) {\n> -\t\t\tgst_caps_unref(caps);\n> -\t\t\tGST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> -\t\t\t\t\t  (\"Failed to active buffer pool\"),\n> -\t\t\t\t\t  (\"gst_libcamera_src_negotiate() failed.\"));\n> -\t\t\treturn { NULL, -EINVAL };\n> -\t\t}\n> +\tif (!gst_buffer_pool_set_active(video_pool, true)) {\n> +\t\tgst_caps_unref(caps);\n> +\t\tGST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> +\t\t\t\t  (\"Failed to active buffer pool\"),\n> +\t\t\t\t  (\"gst_libcamera_src_negotiate() failed.\"));\n> +\t\treturn { NULL, -EINVAL };\n>  \t}\n>  \n>  \tgst_query_unref(query);","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A3B87C31E9\n\tfor ;\n\tWed, 4 Jun 2025 17:11:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 861AB68DBE;\n\tWed, 4 Jun 2025 19:11:25 +0200 (CEST)","from bali.collaboradmins.com (bali.collaboradmins.com\n\t[148.251.105.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2729A68DA8\n\tfor ;\n\tWed, 4 Jun 2025 19:11:24 +0200 (CEST)","from [IPv6:2606:6d00:10:5285::5ac] (unknown\n\t[IPv6:2606:6d00:10:5285::5ac])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (4096 bits))\n\t(No client certificate requested) (Authenticated sender: nicolas)\n\tby bali.collaboradmins.com (Postfix) with ESMTPSA id 1974A17E0FE7;\n\tWed, 4 Jun 2025 19:11:22 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=collabora.com header.i=@collabora.com\n\theader.b=\"RacGvKD5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com;\n\ts=mail; t=1749057083;\n\tbh=qL06aj/BgUpfZLqBo89o/+sFx4lLvTGK2BhZ6RSutmk=;\n\th=Subject:From:To:Cc:Date:In-Reply-To:References:From;\n\tb=RacGvKD5R1ySeN9FNU7WZNRr1gNwvAR1OwIWfi0NIdWMmUl6nd8bjvnQ0TGZJLRny\n\tA/6B6AZIYY1+lsfoWbrA8niAWY/uez/gFc42Qeye2HUlsfNb+IAPpSYFqzdc5lugsf\n\t3/ucD2G4J5dug7iC50i12cbt9EWlyiaASjsvR66QSDgulzT0ECsQ+YHW9JhzeKjVyW\n\taHpXqNwOspBLeoNNtvehYBeT5+joG8nNxKcoO8wRV5ITfSp1Aj0hxF6VKnioswQTs7\n\tOS9yh8FrLiSupKOK8OIiX9BIrheBjgWOqvdvLt8WDIGhvPB33uOzAsqc/qex+yXIX3\n\tRVa5sIHkEA0LA==","Message-ID":"<35ecbe156745637a51bc2f2bd64363530729cde8.camel@collabora.com>","Subject":"Re: [PATCH v2 3/7] gstreamer: Reduce indentation in\n\tgst_libcamera_create_video_pool()","From":"Nicolas Dufresne ","To":"Laurent Pinchart , \n\tlibcamera-devel@lists.libcamera.org","Cc":"Hou Qi ","Date":"Wed, 04 Jun 2025 13:11:21 -0400","In-Reply-To":"<20250604130741.9228-4-laurent.pinchart@ideasonboard.com>","References":"<20250604130741.9228-1-laurent.pinchart@ideasonboard.com>\n\t<20250604130741.9228-4-laurent.pinchart@ideasonboard.com>","Organization":"Collabora Canada","Content-Type":"text/plain; charset=\"UTF-8\"","User-Agent":"Evolution 3.56.2 (3.56.2-1.fc42) ","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]