[{"id":28246,"web_url":"https://patchwork.libcamera.org/comment/28246/","msgid":"<249600da78d9ad94ec591d94f4fc3fa85161ddbe.camel@ndufresne.ca>","date":"2023-12-05T16:10:42","subject":"Re: [libcamera-devel] [PATCH v2 1/3] gstreamer: Move negotiation\n\tlogic to separate function","submitter":{"id":30,"url":"https://patchwork.libcamera.org/api/people/30/","name":"Nicolas Dufresne","email":"nicolas@ndufresne.ca"},"content":"Le jeudi 30 novembre 2023 à 16:43 +0100, Jaslo Ziska via libcamera-devel a\nécrit :\n> Move the code which negotiates all the source pad caps into a separate\n> function called gst_libcamera_src_negotiate. When the negotiation fails\n> this function will return FALSE and TRUE otherwise.\n\n                            true and false\n\nSince it has been ported, but just a nit, I suppose maintainers can deal with\nsuch edit.\n\n> \n> Use this function instead of doing the negotiation manually in\n> gst_libcamera_src_task_enter and remove the now redundant error handling\n> code accordingly.\n> \n> Signed-off-by: Jaslo Ziska <jaslo@ziska.de>\n\nReviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n\n> ---\n>  src/gstreamer/gstlibcamerasrc.cpp | 178 +++++++++++++++---------------\n>  1 file changed, 89 insertions(+), 89 deletions(-)\n> \n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index a6f240f5..e57ba52f 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -379,6 +379,88 @@ gst_libcamera_src_open(GstLibcameraSrc *self)\n>  \treturn true;\n>  }\n>  \n> +/* Must be called with stream_lock held. */\n> +static bool\n> +gst_libcamera_src_negotiate(GstLibcameraSrc *self)\n> +{\n> +\tGstLibcameraSrcState *state = self->state;\n> +\n> +\tg_autoptr(GstStructure) element_caps = gst_structure_new_empty(\"caps\");\n> +\n> +\tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n> +\t\tGstPad *srcpad = state->srcpads_[i];\n> +\t\tStreamConfiguration &stream_cfg = state->config_->at(i);\n> +\n> +\t\t/* Retrieve the supported caps. */\n> +\t\tg_autoptr(GstCaps) filter = gst_libcamera_stream_formats_to_caps(stream_cfg.formats());\n> +\t\tg_autoptr(GstCaps) caps = gst_pad_peer_query_caps(srcpad, filter);\n> +\t\tif (gst_caps_is_empty(caps))\n> +\t\t\treturn false;\n> +\n> +\t\t/* Fixate caps and configure the stream. */\n> +\t\tcaps = gst_caps_make_writable(caps);\n> +\t\tgst_libcamera_configure_stream_from_caps(stream_cfg, caps);\n> +\t\tgst_libcamera_get_framerate_from_caps(caps, element_caps);\n> +\t}\n> +\n> +\t/* Validate the configuration. */\n> +\tif (state->config_->validate() == CameraConfiguration::Invalid)\n> +\t\treturn false;\n> +\n> +\tint ret = state->cam_->configure(state->config_.get());\n> +\tif (ret) {\n> +\t\tGST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> +\t\t\t\t  (\"Failed to configure camera: %s\", g_strerror(-ret)),\n> +\t\t\t\t  (\"Camera::configure() failed with error code %i\", ret));\n> +\t\treturn false;\n> +\t}\n> +\n> +\t/* Check frame duration bounds within controls::FrameDurationLimits */\n> +\tgst_libcamera_clamp_and_set_frameduration(state->initControls_,\n> +\t\t\t\t\t\t  state->cam_->controls(), element_caps);\n> +\n> +\t/*\n> +\t * Regardless if it has been modified, create clean caps and push the\n> +\t * caps event. Downstream will decide if the caps are acceptable.\n> +\t */\n> +\tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n> +\t\tGstPad *srcpad = state->srcpads_[i];\n> +\t\tconst StreamConfiguration &stream_cfg = state->config_->at(i);\n> +\n> +\t\tg_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg);\n> +\t\tgst_libcamera_framerate_to_caps(caps, element_caps);\n> +\n> +\t\tif (!gst_pad_push_event(srcpad, gst_event_new_caps(caps)))\n> +\t\t\treturn false;\n> +\n> +\t}\n> +\n> +\tif (self->allocator)\n> +\t\tg_clear_object(&self->allocator);\n> +\n> +\tself->allocator = gst_libcamera_allocator_new(state->cam_, state->config_.get());\n> +\tif (!self->allocator) {\n> +\t\tGST_ELEMENT_ERROR(self, RESOURCE, NO_SPACE_LEFT,\n> +\t\t\t\t  (\"Failed to allocate memory\"),\n> +\t\t\t\t  (\"gst_libcamera_allocator_new() failed.\"));\n> +\t\treturn false;\n> +\t}\n> +\n> +\tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n> +\t\tGstPad *srcpad = state->srcpads_[i];\n> +\t\tconst StreamConfiguration &stream_cfg = state->config_->at(i);\n> +\n> +\t\tGstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> +\t\t\t\t\t\t\t\tstream_cfg.stream());\n> +\t\tg_signal_connect_swapped(pool, \"buffer-notify\",\n> +\t\t\t\t\t G_CALLBACK(gst_task_resume), self->task);\n> +\n> +\t\tgst_libcamera_pad_set_pool(srcpad, pool);\n> +\t}\n> +\n> +\treturn true;\n> +}\n> +\n>  static void\n>  gst_libcamera_src_task_run(gpointer user_data)\n>  {\n> @@ -468,11 +550,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n>  \tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(user_data);\n>  \tGLibRecLocker lock(&self->stream_lock);\n>  \tGstLibcameraSrcState *state = self->state;\n> -\tGstFlowReturn flow_ret = GST_FLOW_OK;\n>  \tgint ret;\n>  \n> -\tg_autoptr(GstStructure) element_caps = gst_structure_new_empty(\"caps\");\n> -\n>  \tGST_DEBUG_OBJECT(self, \"Streaming thread has started\");\n>  \n>  \tgint stream_id_num = 0;\n> @@ -500,62 +579,16 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n>  \t}\n>  \tg_assert(state->config_->size() == state->srcpads_.size());\n>  \n> -\tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n> -\t\tGstPad *srcpad = state->srcpads_[i];\n> -\t\tStreamConfiguration &stream_cfg = state->config_->at(i);\n> -\n> -\t\t/* Retrieve the supported caps. */\n> -\t\tg_autoptr(GstCaps) filter = gst_libcamera_stream_formats_to_caps(stream_cfg.formats());\n> -\t\tg_autoptr(GstCaps) caps = gst_pad_peer_query_caps(srcpad, filter);\n> -\t\tif (gst_caps_is_empty(caps)) {\n> -\t\t\tflow_ret = GST_FLOW_NOT_NEGOTIATED;\n> -\t\t\tbreak;\n> -\t\t}\n> -\n> -\t\t/* Fixate caps and configure the stream. */\n> -\t\tcaps = gst_caps_make_writable(caps);\n> -\t\tgst_libcamera_configure_stream_from_caps(stream_cfg, caps);\n> -\t\tgst_libcamera_get_framerate_from_caps(caps, element_caps);\n> -\t}\n> -\n> -\tif (flow_ret != GST_FLOW_OK)\n> -\t\tgoto done;\n> -\n> -\t/* Validate the configuration. */\n> -\tif (state->config_->validate() == CameraConfiguration::Invalid) {\n> -\t\tflow_ret = GST_FLOW_NOT_NEGOTIATED;\n> -\t\tgoto done;\n> -\t}\n> -\n> -\tret = state->cam_->configure(state->config_.get());\n> -\tif (ret) {\n> -\t\tGST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> -\t\t\t\t  (\"Failed to configure camera: %s\", g_strerror(-ret)),\n> -\t\t\t\t  (\"Camera::configure() failed with error code %i\", ret));\n> +\tif (!gst_libcamera_src_negotiate(self)) {\n> +\t\tstate->initControls_.clear();\n> +\t\tGST_ELEMENT_FLOW_ERROR(self, GST_FLOW_NOT_NEGOTIATED);\n>  \t\tgst_task_stop(task);\n> -\t\tflow_ret = GST_FLOW_NOT_NEGOTIATED;\n> -\t\tgoto done;\n> +\t\treturn;\n>  \t}\n>  \n> -\t/* Check frame duration bounds within controls::FrameDurationLimits */\n> -\tgst_libcamera_clamp_and_set_frameduration(state->initControls_,\n> -\t\t\t\t\t\t  state->cam_->controls(), element_caps);\n> -\n> -\t/*\n> -\t * Regardless if it has been modified, create clean caps and push the\n> -\t * caps event. Downstream will decide if the caps are acceptable.\n> -\t */\n> -\tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n> -\t\tGstPad *srcpad = state->srcpads_[i];\n> -\t\tconst StreamConfiguration &stream_cfg = state->config_->at(i);\n> -\n> -\t\tg_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg);\n> -\t\tgst_libcamera_framerate_to_caps(caps, element_caps);\n> -\n> -\t\tif (!gst_pad_push_event(srcpad, gst_event_new_caps(caps))) {\n> -\t\t\tflow_ret = GST_FLOW_NOT_NEGOTIATED;\n> -\t\t\tbreak;\n> -\t\t}\n> +\tself->flow_combiner = gst_flow_combiner_new();\n> +\tfor (GstPad *srcpad : state->srcpads_) {\n> +\t\tgst_flow_combiner_add_pad(self->flow_combiner, srcpad);\n>  \n>  \t\t/* Send an open segment event with time format. */\n>  \t\tGstSegment segment;\n> @@ -563,28 +596,6 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n>  \t\tgst_pad_push_event(srcpad, gst_event_new_segment(&segment));\n>  \t}\n>  \n> -\tself->allocator = gst_libcamera_allocator_new(state->cam_, state->config_.get());\n> -\tif (!self->allocator) {\n> -\t\tGST_ELEMENT_ERROR(self, RESOURCE, NO_SPACE_LEFT,\n> -\t\t\t\t  (\"Failed to allocate memory\"),\n> -\t\t\t\t  (\"gst_libcamera_allocator_new() failed.\"));\n> -\t\tgst_task_stop(task);\n> -\t\treturn;\n> -\t}\n> -\n> -\tself->flow_combiner = gst_flow_combiner_new();\n> -\tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n> -\t\tGstPad *srcpad = state->srcpads_[i];\n> -\t\tconst StreamConfiguration &stream_cfg = state->config_->at(i);\n> -\t\tGstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> -\t\t\t\t\t\t\t\tstream_cfg.stream());\n> -\t\tg_signal_connect_swapped(pool, \"buffer-notify\",\n> -\t\t\t\t\t G_CALLBACK(gst_task_resume), task);\n> -\n> -\t\tgst_libcamera_pad_set_pool(srcpad, pool);\n> -\t\tgst_flow_combiner_add_pad(self->flow_combiner, srcpad);\n> -\t}\n> -\n>  \tif (self->auto_focus_mode != controls::AfModeManual) {\n>  \t\tconst ControlInfoMap &infoMap = state->cam_->controls();\n>  \t\tif (infoMap.find(&controls::AfMode) != infoMap.end()) {\n> @@ -605,17 +616,6 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n>  \t\tgst_task_stop(task);\n>  \t\treturn;\n>  \t}\n> -\n> -done:\n> -\tstate->initControls_.clear();\n> -\tswitch (flow_ret) {\n> -\tcase GST_FLOW_NOT_NEGOTIATED:\n> -\t\tGST_ELEMENT_FLOW_ERROR(self, flow_ret);\n> -\t\tgst_task_stop(task);\n> -\t\tbreak;\n> -\tdefault:\n> -\t\tbreak;\n> -\t}\n>  }\n>  \n>  static void","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 B55B5C3226\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  5 Dec 2023 16:10:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2938E61D9B;\n\tTue,  5 Dec 2023 17:10:46 +0100 (CET)","from mail-qt1-x82d.google.com (mail-qt1-x82d.google.com\n\t[IPv6:2607:f8b0:4864:20::82d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 646FB61D9B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  5 Dec 2023 17:10:44 +0100 (CET)","by mail-qt1-x82d.google.com with SMTP id\n\td75a77b69052e-42540e3520bso26808511cf.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 05 Dec 2023 08:10:44 -0800 (PST)","from nicolas-tpx395.localdomain ([2606:6d00:17:b5c::7a9])\n\tby smtp.gmail.com with ESMTPSA id\n\th17-20020a0cedb1000000b0067aa7461aafsm3461703qvr.99.2023.12.05.08.10.42\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 05 Dec 2023 08:10:42 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1701792646;\n\tbh=TDJT7sCCypr/IzaYlGgFzpa6hi04umK04jceJwfQYFg=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=f0Lmd4jeTYyhHjrc+d8CKwZ5fUmYF4FeltcOshooMwWIQANfY227lezsV429Cb2I5\n\tqBjG1uxv01WThKsH7vREcMWaUqgKTUhOoHZTSurrhavaadlz+rh+HN82nzzrLq4Cnp\n\tO1F5GZ0LNCDUMiQjPMTCmRNjaMvJF+OrMMK61ZmmQQ5TGehgMjk7udjxIGnNt38PWc\n\teP+R5X6A1sOiSYmHf/LLWYE9Tw0wb2OxCEUCto1pYYK65zucI4nkFE/qo69lHoQOOs\n\tHqFki4B1VqN9IyuxjE9aPbEb5j3gQ3QvBzuGTegNAbBEiSEUEykOg1SDPS452zhgTN\n\tcw7J2UHoT2S9Q==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ndufresne-ca.20230601.gappssmtp.com; s=20230601; t=1701792643;\n\tx=1702397443; darn=lists.libcamera.org; \n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:to:from:subject:message-id:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=BVRW0A6bd9x6iJAEo9Bj+LB+dBPNumjmKKldOfERE/M=;\n\tb=kjJa1d8qSwruJ4cZpbsTx0LPvvVYE3zx7mo5vxUK08TWsmPog8iRcFGBRmyylJe7vP\n\tR7VnbOLZfVocoWbjPZBVUkPSMWNW4So2r0Yce7oOL4C/ELOowBaE57ErnocDvPkiiXEN\n\t9Ah9nnVTrFyJfyUcYagesmAUhwyGMNU6G2UniTOk8qdao8t6DBO4GpfpkfZ1TZUuo4Mb\n\tyEb2PEcslSN2de0P28QS6ec3asgRYZOloH+KFWN1arW1JVn4DMqFQBll4Z5Zk0btGvOc\n\t99+9MuSdkYD8jDLzp6BjkjJIvIylLVzTRDoAyemaHvquBJmVBqjn9vG/bA29jmw+y2cN\n\t1XJw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ndufresne-ca.20230601.gappssmtp.com\n\theader.i=@ndufresne-ca.20230601.gappssmtp.com header.b=\"kjJa1d8q\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1701792643; x=1702397443;\n\th=mime-version:user-agent:content-transfer-encoding:references\n\t:in-reply-to:date:to:from:subject:message-id:x-gm-message-state:from\n\t:to:cc:subject:date:message-id:reply-to;\n\tbh=BVRW0A6bd9x6iJAEo9Bj+LB+dBPNumjmKKldOfERE/M=;\n\tb=GxWDBQ4R7fPofTT900eqxOpkkPnzCjKgvkJmxzILxg2VG3GLfK0F47WKJC+OUw4WSJ\n\t4SGUEmnzeC6jqJteAS57EFFrf73M2ZmOc+soAfsKnFeOIZMb9kFQPIWG+ljd+X1vtGEE\n\t3xvVYk2IHoYalYLlHBSwNdzdMiKai7+JFIUXGLmd5o/3j3BT/On10JkRejemtxCyzZ4R\n\tlZudmcacr3Ob1BcU+60G9Kmk9goyYgMZkaG8ntHByb9h0izMqGIVmL0jglkGOj+tL7PZ\n\tCGJynQOXI4FtE8JRY6b8zG1mGuj24+nTNmSTzBrS2FlpcOpmmAK5YdjBnrV+HSzPElf2\n\t8uaw==","X-Gm-Message-State":"AOJu0YypzyeecdGCVDu8XQinYJLq/rIn33SCmy5XiZgGoELZXMspB/3f\n\thuGZAucFXVUTvYtpVNPYFY1qWpg/AwZifdKmi8A=","X-Google-Smtp-Source":"AGHT+IF+9Yl+THpB88AXT/UfSOoOBJoUQwULQ0fd16PAezaBElNimmVZ3EXZDuD15Q5Mb2bJsAA7Yw==","X-Received":"by 2002:a05:6214:2482:b0:67a:b459:b594 with SMTP id\n\tgi2-20020a056214248200b0067ab459b594mr1696412qvb.119.1701792643103; \n\tTue, 05 Dec 2023 08:10:43 -0800 (PST)","Message-ID":"<249600da78d9ad94ec591d94f4fc3fa85161ddbe.camel@ndufresne.ca>","To":"Jaslo Ziska <jaslo@ziska.de>, libcamera-devel@lists.libcamera.org","Date":"Tue, 05 Dec 2023 11:10:42 -0500","In-Reply-To":"<20231130155323.13259-2-jaslo@ziska.de>","References":"<20231130155323.13259-1-jaslo@ziska.de>\n\t<20231130155323.13259-2-jaslo@ziska.de>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","User-Agent":"Evolution 3.48.4 (3.48.4-1.fc38) ","MIME-Version":"1.0","Subject":"Re: [libcamera-devel] [PATCH v2 1/3] gstreamer: Move negotiation\n\tlogic to separate function","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Nicolas Dufresne via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Nicolas Dufresne <nicolas@ndufresne.ca>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28268,"web_url":"https://patchwork.libcamera.org/comment/28268/","msgid":"<20231206232010.GF29417@pendragon.ideasonboard.com>","date":"2023-12-06T23:20:10","subject":"Re: [libcamera-devel] [PATCH v2 1/3] gstreamer: Move negotiation\n\tlogic to separate function","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Dec 05, 2023 at 11:10:42AM -0500, Nicolas Dufresne via libcamera-devel wrote:\n> Le jeudi 30 novembre 2023 à 16:43 +0100, Jaslo Ziska via libcamera-devel a écrit :\n> > Move the code which negotiates all the source pad caps into a separate\n> > function called gst_libcamera_src_negotiate. When the negotiation fails\n> > this function will return FALSE and TRUE otherwise.\n> \n>                             true and false\n> \n> Since it has been ported, but just a nit, I suppose maintainers can deal with\n> such edit.\n\nSure, I'll update the commit message. I'll also add () after function\nnames, as customary.\n\n> > Use this function instead of doing the negotiation manually in\n> > gst_libcamera_src_task_enter and remove the now redundant error handling\n> > code accordingly.\n> > \n> > Signed-off-by: Jaslo Ziska <jaslo@ziska.de>\n> \n> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> \n> > ---\n> >  src/gstreamer/gstlibcamerasrc.cpp | 178 +++++++++++++++---------------\n> >  1 file changed, 89 insertions(+), 89 deletions(-)\n> > \n> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > index a6f240f5..e57ba52f 100644\n> > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > @@ -379,6 +379,88 @@ gst_libcamera_src_open(GstLibcameraSrc *self)\n> >  \treturn true;\n> >  }\n> >  \n> > +/* Must be called with stream_lock held. */\n> > +static bool\n> > +gst_libcamera_src_negotiate(GstLibcameraSrc *self)\n> > +{\n> > +\tGstLibcameraSrcState *state = self->state;\n> > +\n> > +\tg_autoptr(GstStructure) element_caps = gst_structure_new_empty(\"caps\");\n> > +\n> > +\tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n> > +\t\tGstPad *srcpad = state->srcpads_[i];\n> > +\t\tStreamConfiguration &stream_cfg = state->config_->at(i);\n> > +\n> > +\t\t/* Retrieve the supported caps. */\n> > +\t\tg_autoptr(GstCaps) filter = gst_libcamera_stream_formats_to_caps(stream_cfg.formats());\n> > +\t\tg_autoptr(GstCaps) caps = gst_pad_peer_query_caps(srcpad, filter);\n> > +\t\tif (gst_caps_is_empty(caps))\n> > +\t\t\treturn false;\n> > +\n> > +\t\t/* Fixate caps and configure the stream. */\n> > +\t\tcaps = gst_caps_make_writable(caps);\n> > +\t\tgst_libcamera_configure_stream_from_caps(stream_cfg, caps);\n> > +\t\tgst_libcamera_get_framerate_from_caps(caps, element_caps);\n> > +\t}\n> > +\n> > +\t/* Validate the configuration. */\n> > +\tif (state->config_->validate() == CameraConfiguration::Invalid)\n> > +\t\treturn false;\n> > +\n> > +\tint ret = state->cam_->configure(state->config_.get());\n> > +\tif (ret) {\n> > +\t\tGST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> > +\t\t\t\t  (\"Failed to configure camera: %s\", g_strerror(-ret)),\n> > +\t\t\t\t  (\"Camera::configure() failed with error code %i\", ret));\n> > +\t\treturn false;\n> > +\t}\n> > +\n> > +\t/* Check frame duration bounds within controls::FrameDurationLimits */\n> > +\tgst_libcamera_clamp_and_set_frameduration(state->initControls_,\n> > +\t\t\t\t\t\t  state->cam_->controls(), element_caps);\n> > +\n> > +\t/*\n> > +\t * Regardless if it has been modified, create clean caps and push the\n> > +\t * caps event. Downstream will decide if the caps are acceptable.\n> > +\t */\n> > +\tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n> > +\t\tGstPad *srcpad = state->srcpads_[i];\n> > +\t\tconst StreamConfiguration &stream_cfg = state->config_->at(i);\n> > +\n> > +\t\tg_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg);\n> > +\t\tgst_libcamera_framerate_to_caps(caps, element_caps);\n> > +\n> > +\t\tif (!gst_pad_push_event(srcpad, gst_event_new_caps(caps)))\n> > +\t\t\treturn false;\n> > +\n\nExtra blank line, I'll remove it when applying the series if no other\nchanges require a new version to be posted.\n\nBy the way, Jaslo, we have a tool called checkstyle.py to detect coding\nstyle issues. You can enable it as a git commit hook as explained at the\nend of Documentation/coding-style.rst.\n\nI've tried to review the patch to the best of my knowledge of the\nGStreamer element. The result looks cleaner. There are some behavioural\nchanges that I can't really judge, but nothing appears wrong.\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> > +\t}\n> > +\n> > +\tif (self->allocator)\n> > +\t\tg_clear_object(&self->allocator);\n> > +\n> > +\tself->allocator = gst_libcamera_allocator_new(state->cam_, state->config_.get());\n> > +\tif (!self->allocator) {\n> > +\t\tGST_ELEMENT_ERROR(self, RESOURCE, NO_SPACE_LEFT,\n> > +\t\t\t\t  (\"Failed to allocate memory\"),\n> > +\t\t\t\t  (\"gst_libcamera_allocator_new() failed.\"));\n> > +\t\treturn false;\n> > +\t}\n> > +\n> > +\tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n> > +\t\tGstPad *srcpad = state->srcpads_[i];\n> > +\t\tconst StreamConfiguration &stream_cfg = state->config_->at(i);\n> > +\n> > +\t\tGstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> > +\t\t\t\t\t\t\t\tstream_cfg.stream());\n> > +\t\tg_signal_connect_swapped(pool, \"buffer-notify\",\n> > +\t\t\t\t\t G_CALLBACK(gst_task_resume), self->task);\n> > +\n> > +\t\tgst_libcamera_pad_set_pool(srcpad, pool);\n> > +\t}\n> > +\n> > +\treturn true;\n> > +}\n> > +\n> >  static void\n> >  gst_libcamera_src_task_run(gpointer user_data)\n> >  {\n> > @@ -468,11 +550,8 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> >  \tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(user_data);\n> >  \tGLibRecLocker lock(&self->stream_lock);\n> >  \tGstLibcameraSrcState *state = self->state;\n> > -\tGstFlowReturn flow_ret = GST_FLOW_OK;\n> >  \tgint ret;\n> >  \n> > -\tg_autoptr(GstStructure) element_caps = gst_structure_new_empty(\"caps\");\n> > -\n> >  \tGST_DEBUG_OBJECT(self, \"Streaming thread has started\");\n> >  \n> >  \tgint stream_id_num = 0;\n> > @@ -500,62 +579,16 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> >  \t}\n> >  \tg_assert(state->config_->size() == state->srcpads_.size());\n> >  \n> > -\tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n> > -\t\tGstPad *srcpad = state->srcpads_[i];\n> > -\t\tStreamConfiguration &stream_cfg = state->config_->at(i);\n> > -\n> > -\t\t/* Retrieve the supported caps. */\n> > -\t\tg_autoptr(GstCaps) filter = gst_libcamera_stream_formats_to_caps(stream_cfg.formats());\n> > -\t\tg_autoptr(GstCaps) caps = gst_pad_peer_query_caps(srcpad, filter);\n> > -\t\tif (gst_caps_is_empty(caps)) {\n> > -\t\t\tflow_ret = GST_FLOW_NOT_NEGOTIATED;\n> > -\t\t\tbreak;\n> > -\t\t}\n> > -\n> > -\t\t/* Fixate caps and configure the stream. */\n> > -\t\tcaps = gst_caps_make_writable(caps);\n> > -\t\tgst_libcamera_configure_stream_from_caps(stream_cfg, caps);\n> > -\t\tgst_libcamera_get_framerate_from_caps(caps, element_caps);\n> > -\t}\n> > -\n> > -\tif (flow_ret != GST_FLOW_OK)\n> > -\t\tgoto done;\n> > -\n> > -\t/* Validate the configuration. */\n> > -\tif (state->config_->validate() == CameraConfiguration::Invalid) {\n> > -\t\tflow_ret = GST_FLOW_NOT_NEGOTIATED;\n> > -\t\tgoto done;\n> > -\t}\n> > -\n> > -\tret = state->cam_->configure(state->config_.get());\n> > -\tif (ret) {\n> > -\t\tGST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n> > -\t\t\t\t  (\"Failed to configure camera: %s\", g_strerror(-ret)),\n> > -\t\t\t\t  (\"Camera::configure() failed with error code %i\", ret));\n> > +\tif (!gst_libcamera_src_negotiate(self)) {\n> > +\t\tstate->initControls_.clear();\n> > +\t\tGST_ELEMENT_FLOW_ERROR(self, GST_FLOW_NOT_NEGOTIATED);\n> >  \t\tgst_task_stop(task);\n> > -\t\tflow_ret = GST_FLOW_NOT_NEGOTIATED;\n> > -\t\tgoto done;\n> > +\t\treturn;\n> >  \t}\n> >  \n> > -\t/* Check frame duration bounds within controls::FrameDurationLimits */\n> > -\tgst_libcamera_clamp_and_set_frameduration(state->initControls_,\n> > -\t\t\t\t\t\t  state->cam_->controls(), element_caps);\n> > -\n> > -\t/*\n> > -\t * Regardless if it has been modified, create clean caps and push the\n> > -\t * caps event. Downstream will decide if the caps are acceptable.\n> > -\t */\n> > -\tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n> > -\t\tGstPad *srcpad = state->srcpads_[i];\n> > -\t\tconst StreamConfiguration &stream_cfg = state->config_->at(i);\n> > -\n> > -\t\tg_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg);\n> > -\t\tgst_libcamera_framerate_to_caps(caps, element_caps);\n> > -\n> > -\t\tif (!gst_pad_push_event(srcpad, gst_event_new_caps(caps))) {\n> > -\t\t\tflow_ret = GST_FLOW_NOT_NEGOTIATED;\n> > -\t\t\tbreak;\n> > -\t\t}\n> > +\tself->flow_combiner = gst_flow_combiner_new();\n> > +\tfor (GstPad *srcpad : state->srcpads_) {\n> > +\t\tgst_flow_combiner_add_pad(self->flow_combiner, srcpad);\n> >  \n> >  \t\t/* Send an open segment event with time format. */\n> >  \t\tGstSegment segment;\n> > @@ -563,28 +596,6 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> >  \t\tgst_pad_push_event(srcpad, gst_event_new_segment(&segment));\n> >  \t}\n> >  \n> > -\tself->allocator = gst_libcamera_allocator_new(state->cam_, state->config_.get());\n> > -\tif (!self->allocator) {\n> > -\t\tGST_ELEMENT_ERROR(self, RESOURCE, NO_SPACE_LEFT,\n> > -\t\t\t\t  (\"Failed to allocate memory\"),\n> > -\t\t\t\t  (\"gst_libcamera_allocator_new() failed.\"));\n> > -\t\tgst_task_stop(task);\n> > -\t\treturn;\n> > -\t}\n> > -\n> > -\tself->flow_combiner = gst_flow_combiner_new();\n> > -\tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n> > -\t\tGstPad *srcpad = state->srcpads_[i];\n> > -\t\tconst StreamConfiguration &stream_cfg = state->config_->at(i);\n> > -\t\tGstLibcameraPool *pool = gst_libcamera_pool_new(self->allocator,\n> > -\t\t\t\t\t\t\t\tstream_cfg.stream());\n> > -\t\tg_signal_connect_swapped(pool, \"buffer-notify\",\n> > -\t\t\t\t\t G_CALLBACK(gst_task_resume), task);\n> > -\n> > -\t\tgst_libcamera_pad_set_pool(srcpad, pool);\n> > -\t\tgst_flow_combiner_add_pad(self->flow_combiner, srcpad);\n> > -\t}\n> > -\n> >  \tif (self->auto_focus_mode != controls::AfModeManual) {\n> >  \t\tconst ControlInfoMap &infoMap = state->cam_->controls();\n> >  \t\tif (infoMap.find(&controls::AfMode) != infoMap.end()) {\n> > @@ -605,17 +616,6 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,\n> >  \t\tgst_task_stop(task);\n> >  \t\treturn;\n> >  \t}\n> > -\n> > -done:\n> > -\tstate->initControls_.clear();\n> > -\tswitch (flow_ret) {\n> > -\tcase GST_FLOW_NOT_NEGOTIATED:\n> > -\t\tGST_ELEMENT_FLOW_ERROR(self, flow_ret);\n> > -\t\tgst_task_stop(task);\n> > -\t\tbreak;\n> > -\tdefault:\n> > -\t\tbreak;\n> > -\t}\n> >  }\n> >  \n> >  static void","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 91B85C322E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  6 Dec 2023 23:20:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D86FF629CE;\n\tThu,  7 Dec 2023 00:20:05 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C60B961D9B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  7 Dec 2023 00:20:04 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 62FC1674;\n\tThu,  7 Dec 2023 00:19:23 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1701904805;\n\tbh=6KhEGkbOt5tB6lte9ThqBkOngJhY8RLG5x1YiqgXdBY=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=V9rg38dKJ0hRZSKid1fF27CWZTzgl17v0Z6CX/7cF1fXybMmn9V6fZWN9TZLbSXFp\n\tpY9G09AfM3dFm9DceyLRs1SCG8YDfCPvhXAGo7BzGkGoc0Z3m3GqhBlSC91SPYfcP3\n\tLUU0Xk99fgn8WIFUGTZsfvZZS10EcgacsSKj7T15DxQXyzM4g+0PD3Rjx90Zc8ijAl\n\tpt/Nhe7mZI9BwYFNy1ugQ2EBKOICxlaI3PHnrp2uLWBJQnXNwo1THvkpyd+1HAW4g+\n\t+eRkqpCk8q4aGtSU6R1BlQ7hpewGtDUZmpuHZeQKGS3v4atTnmYgCrzEX/gSgjiQkT\n\tnzK/fN8YKLjAw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1701904763;\n\tbh=6KhEGkbOt5tB6lte9ThqBkOngJhY8RLG5x1YiqgXdBY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=h52iYmTkgXx+GsvGwevzUc6kO/VBKnJbFYtDIVxwivivmBjIKeLM762cHWxxqyaTK\n\tjq5xe0IzLSO3bu0cw2vxsu4OgCHQV5p4xlLJg5+R7TNldrS5BCGc28Ywtyn7MGPj8v\n\tuw+yB5tXSX1Gq5LiJwHwZlmQC6L2mX9Rb5tE4nXA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"h52iYmTk\"; dkim-atps=neutral","Date":"Thu, 7 Dec 2023 01:20:10 +0200","To":"Nicolas Dufresne <nicolas@ndufresne.ca>","Message-ID":"<20231206232010.GF29417@pendragon.ideasonboard.com>","References":"<20231130155323.13259-1-jaslo@ziska.de>\n\t<20231130155323.13259-2-jaslo@ziska.de>\n\t<249600da78d9ad94ec591d94f4fc3fa85161ddbe.camel@ndufresne.ca>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<249600da78d9ad94ec591d94f4fc3fa85161ddbe.camel@ndufresne.ca>","Subject":"Re: [libcamera-devel] [PATCH v2 1/3] gstreamer: Move negotiation\n\tlogic to separate function","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":28324,"web_url":"https://patchwork.libcamera.org/comment/28324/","msgid":"<87jzpgojle.fsf@ziska.de>","date":"2023-12-14T16:02:55","subject":"Re: [libcamera-devel] [PATCH v2 1/3] gstreamer: Move negotiation\n\tlogic to separate function","submitter":{"id":173,"url":"https://patchwork.libcamera.org/api/people/173/","name":"Jaslo Ziska","email":"jaslo@ziska.de"},"content":"Hi Laurent,\n\nLaurent Pinchart <laurent.pinchart@ideasonboard.com> writes:\n> On Tue, Dec 05, 2023 at 11:10:42AM -0500, Nicolas Dufresne via \n> libcamera-devel wrote:\n>> Le jeudi 30 novembre 2023 à 16:43 +0100, Jaslo Ziska via \n>> libcamera-devel a écrit :\n>> > Move the code which negotiates all the source pad caps into a \n>> > separate\n>> > function called gst_libcamera_src_negotiate. When the \n>> > negotiation fails\n>> > this function will return FALSE and TRUE otherwise.\n>>\n>>                             true and false\n>>\n>> Since it has been ported, but just a nit, I suppose maintainers \n>> can deal with\n>> such edit.\n>\n> Sure, I'll update the commit message. I'll also add () after \n> function\n> names, as customary.\n>\n>> > Use this function instead of doing the negotiation manually \n>> > in\n>> > gst_libcamera_src_task_enter and remove the now redundant \n>> > error handling\n>> > code accordingly.\n>> >\n>> > Signed-off-by: Jaslo Ziska <jaslo@ziska.de>\n>>\n>> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n>>\n>> > ---\n>> >  src/gstreamer/gstlibcamerasrc.cpp | 178 \n>> >  +++++++++++++++---------------\n>> >  1 file changed, 89 insertions(+), 89 deletions(-)\n>> >\n>> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp \n>> > b/src/gstreamer/gstlibcamerasrc.cpp\n>> > index a6f240f5..e57ba52f 100644\n>> > --- a/src/gstreamer/gstlibcamerasrc.cpp\n>> > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n>> > @@ -379,6 +379,88 @@ gst_libcamera_src_open(GstLibcameraSrc \n>> > *self)\n>> >  \treturn true;\n>> >  }\n>> >\n>> > +/* Must be called with stream_lock held. */\n>> > +static bool\n>> > +gst_libcamera_src_negotiate(GstLibcameraSrc *self)\n>> > +{\n>> > +\tGstLibcameraSrcState *state = self->state;\n>> > +\n>> > +\tg_autoptr(GstStructure) element_caps = \n>> > gst_structure_new_empty(\"caps\");\n>> > +\n>> > +\tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n>> > +\t\tGstPad *srcpad = state->srcpads_[i];\n>> > +\t\tStreamConfiguration &stream_cfg = \n>> > state->config_->at(i);\n>> > +\n>> > +\t\t/* Retrieve the supported caps. */\n>> > +\t\tg_autoptr(GstCaps) filter = \n>> > gst_libcamera_stream_formats_to_caps(stream_cfg.formats());\n>> > +\t\tg_autoptr(GstCaps) caps = \n>> > gst_pad_peer_query_caps(srcpad, filter);\n>> > +\t\tif (gst_caps_is_empty(caps))\n>> > +\t\t\treturn false;\n>> > +\n>> > +\t\t/* Fixate caps and configure the stream. */\n>> > +\t\tcaps = gst_caps_make_writable(caps);\n>> > +\t\tgst_libcamera_configure_stream_from_caps(stream_cfg, \n>> > caps);\n>> > +\t\tgst_libcamera_get_framerate_from_caps(caps, \n>> > element_caps);\n>> > +\t}\n>> > +\n>> > +\t/* Validate the configuration. */\n>> > +\tif (state->config_->validate() == \n>> > CameraConfiguration::Invalid)\n>> > +\t\treturn false;\n>> > +\n>> > +\tint ret = state->cam_->configure(state->config_.get());\n>> > +\tif (ret) {\n>> > +\t\tGST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n>> > +\t\t\t\t  (\"Failed to configure camera: %s\", \n>> > g_strerror(-ret)),\n>> > +\t\t\t\t  (\"Camera::configure() failed with error code \n>> > %i\", ret));\n>> > +\t\treturn false;\n>> > +\t}\n>> > +\n>> > +\t/* Check frame duration bounds within \n>> > controls::FrameDurationLimits */\n>> > + \n>> > gst_libcamera_clamp_and_set_frameduration(state->initControls_,\n>> > +\t\t\t\t\t\t  state->cam_->controls(), \n>> > element_caps);\n>> > +\n>> > +\t/*\n>> > +\t * Regardless if it has been modified, create clean caps \n>> > and push the\n>> > +\t * caps event. Downstream will decide if the caps are \n>> > acceptable.\n>> > +\t */\n>> > +\tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n>> > +\t\tGstPad *srcpad = state->srcpads_[i];\n>> > +\t\tconst StreamConfiguration &stream_cfg = \n>> > state->config_->at(i);\n>> > +\n>> > +\t\tg_autoptr(GstCaps) caps = \n>> > gst_libcamera_stream_configuration_to_caps(stream_cfg);\n>> > +\t\tgst_libcamera_framerate_to_caps(caps, element_caps);\n>> > +\n>> > +\t\tif (!gst_pad_push_event(srcpad, \n>> > gst_event_new_caps(caps)))\n>> > +\t\t\treturn false;\n>> > +\n>\n> Extra blank line, I'll remove it when applying the series if no \n> other\n> changes require a new version to be posted.\n>\n> By the way, Jaslo, we have a tool called checkstyle.py to detect \n> coding\n> style issues. You can enable it as a git commit hook as \n> explained at the\n> end of Documentation/coding-style.rst.\n>\n> I've tried to review the patch to the best of my knowledge of \n> the\n> GStreamer element. The result looks cleaner. There are some \n> behavioural\n> changes that I can't really judge, but nothing appears wrong.\n>\n> Reviewed-by: Laurent Pinchart \n> <laurent.pinchart@ideasonboard.com>\n>\n\nThanks for the reviews and taking for care of this. I did not \nnotice the checkstyle.py tool before, thanks for the tip.\n\nRegards,\n\nJaslo\n\n>> > +\t}\n>> > +\n>> > +\tif (self->allocator)\n>> > +\t\tg_clear_object(&self->allocator);\n>> > +\n>> > +\tself->allocator = gst_libcamera_allocator_new(state->cam_, \n>> > state->config_.get());\n>> > +\tif (!self->allocator) {\n>> > +\t\tGST_ELEMENT_ERROR(self, RESOURCE, NO_SPACE_LEFT,\n>> > +\t\t\t\t  (\"Failed to allocate memory\"),\n>> > +\t\t\t\t  (\"gst_libcamera_allocator_new() failed.\"));\n>> > +\t\treturn false;\n>> > +\t}\n>> > +\n>> > +\tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n>> > +\t\tGstPad *srcpad = state->srcpads_[i];\n>> > +\t\tconst StreamConfiguration &stream_cfg = \n>> > state->config_->at(i);\n>> > +\n>> > +\t\tGstLibcameraPool *pool = \n>> > gst_libcamera_pool_new(self->allocator,\n>> > +\t\t\t\t\t\t\t\tstream_cfg.stream());\n>> > +\t\tg_signal_connect_swapped(pool, \"buffer-notify\",\n>> > +\t\t\t\t\t G_CALLBACK(gst_task_resume), self->task);\n>> > +\n>> > +\t\tgst_libcamera_pad_set_pool(srcpad, pool);\n>> > +\t}\n>> > +\n>> > +\treturn true;\n>> > +}\n>> > +\n>> >  static void\n>> >  gst_libcamera_src_task_run(gpointer user_data)\n>> >  {\n>> > @@ -468,11 +550,8 @@ gst_libcamera_src_task_enter(GstTask \n>> > *task, [[maybe_unused]] GThread *thread,\n>> >  \tGstLibcameraSrc *self = GST_LIBCAMERA_SRC(user_data);\n>> >  \tGLibRecLocker lock(&self->stream_lock);\n>> >  \tGstLibcameraSrcState *state = self->state;\n>> > -\tGstFlowReturn flow_ret = GST_FLOW_OK;\n>> >  \tgint ret;\n>> >\n>> > -\tg_autoptr(GstStructure) element_caps = \n>> > gst_structure_new_empty(\"caps\");\n>> > -\n>> >  \tGST_DEBUG_OBJECT(self, \"Streaming thread has started\");\n>> >\n>> >  \tgint stream_id_num = 0;\n>> > @@ -500,62 +579,16 @@ gst_libcamera_src_task_enter(GstTask \n>> > *task, [[maybe_unused]] GThread *thread,\n>> >  \t}\n>> >  \tg_assert(state->config_->size() == \n>> >  state->srcpads_.size());\n>> >\n>> > -\tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n>> > -\t\tGstPad *srcpad = state->srcpads_[i];\n>> > -\t\tStreamConfiguration &stream_cfg = \n>> > state->config_->at(i);\n>> > -\n>> > -\t\t/* Retrieve the supported caps. */\n>> > -\t\tg_autoptr(GstCaps) filter = \n>> > gst_libcamera_stream_formats_to_caps(stream_cfg.formats());\n>> > -\t\tg_autoptr(GstCaps) caps = \n>> > gst_pad_peer_query_caps(srcpad, filter);\n>> > -\t\tif (gst_caps_is_empty(caps)) {\n>> > -\t\t\tflow_ret = GST_FLOW_NOT_NEGOTIATED;\n>> > -\t\t\tbreak;\n>> > -\t\t}\n>> > -\n>> > -\t\t/* Fixate caps and configure the stream. */\n>> > -\t\tcaps = gst_caps_make_writable(caps);\n>> > -\t\tgst_libcamera_configure_stream_from_caps(stream_cfg, \n>> > caps);\n>> > -\t\tgst_libcamera_get_framerate_from_caps(caps, \n>> > element_caps);\n>> > -\t}\n>> > -\n>> > -\tif (flow_ret != GST_FLOW_OK)\n>> > -\t\tgoto done;\n>> > -\n>> > -\t/* Validate the configuration. */\n>> > -\tif (state->config_->validate() == \n>> > CameraConfiguration::Invalid) {\n>> > -\t\tflow_ret = GST_FLOW_NOT_NEGOTIATED;\n>> > -\t\tgoto done;\n>> > -\t}\n>> > -\n>> > -\tret = state->cam_->configure(state->config_.get());\n>> > -\tif (ret) {\n>> > -\t\tGST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,\n>> > -\t\t\t\t  (\"Failed to configure camera: %s\", \n>> > g_strerror(-ret)),\n>> > -\t\t\t\t  (\"Camera::configure() failed with error code \n>> > %i\", ret));\n>> > +\tif (!gst_libcamera_src_negotiate(self)) {\n>> > +\t\tstate->initControls_.clear();\n>> > +\t\tGST_ELEMENT_FLOW_ERROR(self, GST_FLOW_NOT_NEGOTIATED);\n>> >  \t\tgst_task_stop(task);\n>> > -\t\tflow_ret = GST_FLOW_NOT_NEGOTIATED;\n>> > -\t\tgoto done;\n>> > +\t\treturn;\n>> >  \t}\n>> >\n>> > -\t/* Check frame duration bounds within \n>> > controls::FrameDurationLimits */\n>> > - \n>> > gst_libcamera_clamp_and_set_frameduration(state->initControls_,\n>> > -\t\t\t\t\t\t  state->cam_->controls(), \n>> > element_caps);\n>> > -\n>> > -\t/*\n>> > -\t * Regardless if it has been modified, create clean caps \n>> > and push the\n>> > -\t * caps event. Downstream will decide if the caps are \n>> > acceptable.\n>> > -\t */\n>> > -\tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n>> > -\t\tGstPad *srcpad = state->srcpads_[i];\n>> > -\t\tconst StreamConfiguration &stream_cfg = \n>> > state->config_->at(i);\n>> > -\n>> > -\t\tg_autoptr(GstCaps) caps = \n>> > gst_libcamera_stream_configuration_to_caps(stream_cfg);\n>> > -\t\tgst_libcamera_framerate_to_caps(caps, element_caps);\n>> > -\n>> > -\t\tif (!gst_pad_push_event(srcpad, \n>> > gst_event_new_caps(caps))) {\n>> > -\t\t\tflow_ret = GST_FLOW_NOT_NEGOTIATED;\n>> > -\t\t\tbreak;\n>> > -\t\t}\n>> > +\tself->flow_combiner = gst_flow_combiner_new();\n>> > +\tfor (GstPad *srcpad : state->srcpads_) {\n>> > +\t\tgst_flow_combiner_add_pad(self->flow_combiner, \n>> > srcpad);\n>> >\n>> >  \t\t/* Send an open segment event with time format. */\n>> >  \t\tGstSegment segment;\n>> > @@ -563,28 +596,6 @@ gst_libcamera_src_task_enter(GstTask \n>> > *task, [[maybe_unused]] GThread *thread,\n>> >  \t\tgst_pad_push_event(srcpad, \n>> >  gst_event_new_segment(&segment));\n>> >  \t}\n>> >\n>> > -\tself->allocator = gst_libcamera_allocator_new(state->cam_, \n>> > state->config_.get());\n>> > -\tif (!self->allocator) {\n>> > -\t\tGST_ELEMENT_ERROR(self, RESOURCE, NO_SPACE_LEFT,\n>> > -\t\t\t\t  (\"Failed to allocate memory\"),\n>> > -\t\t\t\t  (\"gst_libcamera_allocator_new() failed.\"));\n>> > -\t\tgst_task_stop(task);\n>> > -\t\treturn;\n>> > -\t}\n>> > -\n>> > -\tself->flow_combiner = gst_flow_combiner_new();\n>> > -\tfor (gsize i = 0; i < state->srcpads_.size(); i++) {\n>> > -\t\tGstPad *srcpad = state->srcpads_[i];\n>> > -\t\tconst StreamConfiguration &stream_cfg = \n>> > state->config_->at(i);\n>> > -\t\tGstLibcameraPool *pool = \n>> > gst_libcamera_pool_new(self->allocator,\n>> > -\t\t\t\t\t\t\t\tstream_cfg.stream());\n>> > -\t\tg_signal_connect_swapped(pool, \"buffer-notify\",\n>> > -\t\t\t\t\t G_CALLBACK(gst_task_resume), task);\n>> > -\n>> > -\t\tgst_libcamera_pad_set_pool(srcpad, pool);\n>> > -\t\tgst_flow_combiner_add_pad(self->flow_combiner, \n>> > srcpad);\n>> > -\t}\n>> > -\n>> >  \tif (self->auto_focus_mode != controls::AfModeManual) {\n>> >  \t\tconst ControlInfoMap &infoMap = \n>> >  state->cam_->controls();\n>> >  \t\tif (infoMap.find(&controls::AfMode) != infoMap.end()) \n>> >  {\n>> > @@ -605,17 +616,6 @@ gst_libcamera_src_task_enter(GstTask \n>> > *task, [[maybe_unused]] GThread *thread,\n>> >  \t\tgst_task_stop(task);\n>> >  \t\treturn;\n>> >  \t}\n>> > -\n>> > -done:\n>> > -\tstate->initControls_.clear();\n>> > -\tswitch (flow_ret) {\n>> > -\tcase GST_FLOW_NOT_NEGOTIATED:\n>> > -\t\tGST_ELEMENT_FLOW_ERROR(self, flow_ret);\n>> > -\t\tgst_task_stop(task);\n>> > -\t\tbreak;\n>> > -\tdefault:\n>> > -\t\tbreak;\n>> > -\t}\n>> >  }\n>> >\n>> >  static void","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 5BABBC31E9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 14 Dec 2023 16:06:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9514061D97;\n\tThu, 14 Dec 2023 17:06:00 +0100 (CET)","from mo4-p00-ob.smtp.rzone.de (mo4-p00-ob.smtp.rzone.de\n\t[81.169.146.161])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4E6CC61D97\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 14 Dec 2023 17:05:58 +0100 (CET)","from archlinux by smtp.strato.de (RZmta 49.10.0 AUTH)\n\twith ESMTPSA id t8bebczBEG5uoHp\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits))\n\t(Client did not present a certificate);\n\tThu, 14 Dec 2023 17:05:56 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1702569960;\n\tbh=wogCwyVL+KUel8JB4f6asRw2d1cBaC2XjksOY77VB/g=;\n\th=References:To:Date:In-reply-to:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=h/89YSbq2KCqHBmRFTYSmP3uVqt3uQROcXWM4IteY+YyoGfnvqj7yNR5y15J365Wf\n\tGD3XCWoW2/b43rFFOmTTPZUKfesw5knApp3WB5p9VH4qFVypR7mSlz8/ZtYZXicoQ7\n\tYUQwcIeAitnuVJzY0VZrX5LMm6ShIWc3E8YvybIxkQVjon3DZxxPhl2BoAUEYKdlHi\n\tuG4USPC9Gl081GplWPzRuTKlHtQGVaKs7Zz8xNLxWSCwTEs55TvGLjdMYnrKQ6sRWN\n\td/knlusQ+kdHInqHB/ooQxetLtf8V8O0ezK1bzG8gur4zuwLoQHToOnRJ2uyq3BIeK\n\tOhBe1CyflEh0g==","v=1; a=rsa-sha256; c=relaxed/relaxed; t=1702569956;\n\ts=strato-dkim-0002; d=ziska.de;\n\th=Message-ID:In-reply-to:Date:Subject:Cc:To:From:References:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=JMMaWsYkGUXUzBvUWFqoPZ4mph1Ce7DAhsqHzWAFRKI=;\n\tb=Nwg2EVx9hw0d0DUKeIyWqEYA3Hkxkx/WFhdXeEYXYDGSumvSn4s4vCaYwJiuEajqME\n\teBRurnQsGlH/Mk0+aoOuK3X5pDHslFYe7zrcrtAGd9yjfziUz5ehwDUyEYYCB3ETiRi5\n\tYCJ49tju1gAKOW0QYdiJbJSlngAvwzQrj1X3PXsYt6ZWR1N4qUsDihay+yxpYi0kXbsn\n\taXff7rNH23pFEdW2+++jCAtndMRA90QCeIU9O0za3uJ8rvKIdtLKWDGsR3HbM58yiJuc\n\txUg7KhooOUhQjItb3rb7HqxqSzvLP9ajLzKhrm8wO2RLUvskbAtI8LLYNeH4RDF2cLmX\n\tPtUg==","v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1702569956;\n\ts=strato-dkim-0003; d=ziska.de;\n\th=Message-ID:In-reply-to:Date:Subject:Cc:To:From:References:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=JMMaWsYkGUXUzBvUWFqoPZ4mph1Ce7DAhsqHzWAFRKI=;\n\tb=EdJ/9YKBU4E+Fii7HQGYf2RFOwWG56EzkFrGLjTualGZOEethWxXatAjVn1uq7z7W8\n\ttIJwRZI1q9HqoX5SlyDA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=ziska.de header.i=@ziska.de\n\theader.b=\"Nwg2EVx9\"; \n\tdkim=permerror (0-bit key) header.d=ziska.de header.i=@ziska.de\n\theader.b=\"EdJ/9YKB\"; dkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1702569956; cv=none;\n\td=strato.com; s=strato-dkim-0002;\n\tb=rYx7QoSC3foNA7d4TxGsf1XgiUkMlctDUFi5xbBowVhkdAmNd7UdG8EvoIMIZrkT0C\n\tQL5sbnp0NtK1isZjbl3j/p+ecSKkr16ZF2Sqjqx4k3fFq2dLkRwieNVzShgFjzM1QfWK\n\tePedgvhqFkRzHnblvg13I5CooR1h3WbAyJkgLc9IOCsrtjncxYpcE8pQ9gwaw/uGghC6\n\tFuPmTjYDSOPjeFh8uLZayapcWF0AXHYCH2PmAa3U2NAe2N90MJxCu0M6w70PxPAAAMzj\n\tm98f0dcIMnCTKt/HurTESEP80ZmSlu3iA0GfdGYxviZLggXkvw8/DBJUSJqIa0AqgLmJ\n\tqXAg==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; t=1702569956;\n\ts=strato-dkim-0002; d=strato.com;\n\th=Message-ID:In-reply-to:Date:Subject:Cc:To:From:References:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=JMMaWsYkGUXUzBvUWFqoPZ4mph1Ce7DAhsqHzWAFRKI=;\n\tb=jWbnzLsnm7vTvkpdg2pTFT/SoZA/ewhznT2a8vzjslretIOBMKMikMuoLjRRST442S\n\tim/02yA6L5sD55sHXVFcZkAKiw58Hda2JrBFTbXHeocYVbCgCMnkOfy+zZh5x36Kfadi\n\tT1ucIGc9KTwxI2/JS8k1vZmRby0aQFrjCuXSf2mn2NNbyvYw7HESjWvHgEqe3O6uvpe/\n\tBMR+HyZh+AydA5vZpA8V26L8pD7s8A5Ao1PkYj87tnYUA+kTuklu47lhg3kY2PPX3Wgh\n\tzdSHH/agW0N+yrZGye77FwxRYxfVVV04bdsUJ2J988HzKT6s8HQ5VeskeVB+cA6Adv16\n\tSnLQ==","ARC-Authentication-Results":"i=1; strato.com;\n    arc=none;\n    dkim=none","X-RZG-CLASS-ID":"mo00","X-RZG-AUTH":"\":Jm0XeU+IYfb0x77LHmrjN5Wlb7TBwusQvJpnsql/jCNNVfDSgNWRnQi1f6u5kqcOqCfkVJLitp7MLcUjCYMneSjI70SVxuSh\"","References":"<20231130155323.13259-1-jaslo@ziska.de>\n\t<20231130155323.13259-2-jaslo@ziska.de>\n\t<249600da78d9ad94ec591d94f4fc3fa85161ddbe.camel@ndufresne.ca>\n\t<20231206232010.GF29417@pendragon.ideasonboard.com>","User-agent":"mu4e 1.10.8; emacs 29.1","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Date":"Thu, 14 Dec 2023 17:02:55 +0100","In-reply-to":"<20231206232010.GF29417@pendragon.ideasonboard.com>","Message-ID":"<87jzpgojle.fsf@ziska.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH v2 1/3] gstreamer: Move negotiation\n\tlogic to separate function","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Jaslo Ziska via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jaslo Ziska <jaslo@ziska.de>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]