{"id":19257,"url":"https://patchwork.libcamera.org/api/1.1/patches/19257/?format=json","web_url":"https://patchwork.libcamera.org/patch/19257/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20231130155323.13259-2-jaslo@ziska.de>","date":"2023-11-30T15:43:08","name":"[libcamera-devel,v2,1/3] gstreamer: Move negotiation logic to separate function","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"6186812091b22e49b1bac20a21d455cc238ff8d1","submitter":{"id":173,"url":"https://patchwork.libcamera.org/api/1.1/people/173/?format=json","name":"Jaslo Ziska","email":"jaslo@ziska.de"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/19257/mbox/","series":[{"id":4093,"url":"https://patchwork.libcamera.org/api/1.1/series/4093/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4093","date":"2023-11-30T15:43:07","name":"gstreamer: Implement renegotiation","version":2,"mbox":"https://patchwork.libcamera.org/series/4093/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/19257/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/19257/checks/","tags":{},"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 C61C8BDE6B\n\tfor ;\n\tThu, 30 Nov 2023 15:54:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 85495629C2;\n\tThu, 30 Nov 2023 16:54:33 +0100 (CET)","from mo4-p00-ob.smtp.rzone.de (mo4-p00-ob.smtp.rzone.de\n\t[85.215.255.22])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5EAB361DA0\n\tfor ;\n\tThu, 30 Nov 2023 16:54:31 +0100 (CET)","from archlinux.fritz.box by smtp.strato.de (RZmta 49.9.7 AUTH)\n\twith ESMTPSA id n54b84zAUFsU0SA\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits))\n\t(Client did not present a certificate);\n\tThu, 30 Nov 2023 16:54:30 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1701359673;\n\tbh=sub7EUP7Zgfkfnw+cI5Q6+nxIAuZewzIe/zvYkn6eZk=;\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:Cc:\n\tFrom;\n\tb=rmnmvxSNHX9wyiNanZ7M8hc/auqAAW+MC3HPYopHyLTVOVtWtpFpR+2tUyFm3Jscx\n\t7X3lNSocqXuTfb2S4XEAr46kWSyOak0431S8y4D5xBkdmUuwtTCBGe5AvCFx9aQSGc\n\tO4tK3g/mPqtCa0BxXy1Dczl3GOREYL3YQEcYbWN8O6yKasPty8Tcv9SIOAtKQ4w/eO\n\toIkvyoQtkqzGIy4qr6vj8OCmIWVrJLtlI4/S1/CzNv1f2zCbApOglairNyNWC7kCIu\n\tVIrInmevsuJPgtvXU5i7y6uh1HghzckifSat6RPVL/wME//B+gCGq38ehpLNwea4gW\n\tR/wmjILcuaAyw==","v=1; a=rsa-sha256; c=relaxed/relaxed; t=1701359670;\n\ts=strato-dkim-0002; d=ziska.de;\n\th=References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=Oyty/H1OP9OtthVhCEO8NVCcN5ESVTneZvxybMh1zCM=;\n\tb=gfIWpS1cPG8dLWRT87o7sS9WjSJhn0UK+Zyz8gp4r2BHMnc3Q7Hu0nfhnWwQCEt0i+\n\tsHejkB6L/PfL9aTnjAtep10CPu1Hu+wd6tNZg+tRIRu7mP55QEgkJwISXdDKBD9wNh4E\n\tD2ML4RzyxYb1jab3/wU+RL/Je2IDIz9UAQNPEEiNhc2jiQlutIgXcbxvqVZtboL9NoKi\n\tktga+2ydr8FBqRvp6HbC1eMVT03AGljLXS+9NOxkM/jFpxE86JHgFC6/xX0rs88aOdts\n\tpfnGZUWCD8g/R5MmcOZ7abN+vq3Z/4hiWEvOJRX53fJF5zGJj4qWkfhmfVZ9VZxUM/aO\n\tMq5w==","v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1701359670;\n\ts=strato-dkim-0003; d=ziska.de;\n\th=References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=Oyty/H1OP9OtthVhCEO8NVCcN5ESVTneZvxybMh1zCM=;\n\tb=c2EcKEd6WP55Tq2aC9kxRMiH4q53PTtoQY+mKvsaAvvJ0icfe8dJCbUiKuLHxhOTpI\n\tZHGMA4bvOLw/BAIZWLBA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=ziska.de header.i=@ziska.de\n\theader.b=\"gfIWpS1c\"; \n\tdkim=permerror (0-bit key) header.d=ziska.de header.i=@ziska.de\n\theader.b=\"c2EcKEd6\"; dkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1701359670; cv=none;\n\td=strato.com; s=strato-dkim-0002;\n\tb=kRGk4cQdDddh28Z0QVurZV9LR6iOz5wE95ZNYuAqkkYVQJCdGw9BGoPXwiUR8ShzOW\n\tzRbgdJv6xeQFmIoWuanhNWAWPGDzIDr+4xITQZz1saz0r1rheDtHoQEhRR7bbUBFnQGU\n\tINixfJZWT++GzyVSYyTw0f0Kvq3OGJkebYd4aNxxMw0D8dUiW8i62gC846DfXM/PjoqP\n\tLDDf0fCQQiM2eP2nOyfsvqROF+LygBnYpVBmd6wqqOzUepJXik7aqNMcueKIultmVDhI\n\tmGMFtwEELorUsse6YVfc//t11nJbPL1HIbtAx6Ne/L/9UVOjAzILe3BWIHIxuCB1v8zI\n\tYNhg==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; t=1701359670;\n\ts=strato-dkim-0002; d=strato.com;\n\th=References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From:Cc:Date:\n\tFrom:Subject:Sender;\n\tbh=Oyty/H1OP9OtthVhCEO8NVCcN5ESVTneZvxybMh1zCM=;\n\tb=JuhYXYQNxXt1jW9WOyyy0WN07gmxAKM+FPbuFaZhibWLc0kLqtRZSbt3UCKqdSdp+E\n\t9Kn0SizapZDG4grcnm5dBYOD74xsFsUq1UJ/MASxybNTR9U0X7QzZ+FM9QETTWwIbszM\n\tBhLLPKp3QUIogmathxWPlEV3naOb1KrCeDypr4ZU7U5zANkQ2elNKBxOj0RX/YUlN4mb\n\tJjnTbTJlALTYAiw0etBJ5QrYn/Ce6OWIb6J6w3tZ8jsaN+ttYohmfYdDFDhQKp2Co60c\n\tcsA7gUqTLkh6M1BgkPEK+Mp/xNigM4LGH1bXyYZIWnfcK9ajrMX+X/OqjBbP6hnFGf2G\n\t67Ww==","ARC-Authentication-Results":"i=1; strato.com;\n arc=none;\n dkim=none","X-RZG-CLASS-ID":"mo00","X-RZG-AUTH":"\":Jm0XeU+IYfb0x77LHmrjN5Wlb7TBwusDqIM6Hizy8VdfzvKi4yoFC9cH04q6BfJa07bS3ov6I8QOM/V5x+uTrCUtBrk=\"","To":"libcamera-devel@lists.libcamera.org","Date":"Thu, 30 Nov 2023 16:43:08 +0100","Message-ID":"<20231130155323.13259-2-jaslo@ziska.de>","X-Mailer":"git-send-email 2.43.0","In-Reply-To":"<20231130155323.13259-1-jaslo@ziska.de>","References":"<20231130155323.13259-1-jaslo@ziska.de>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Content-Type":"text/plain; charset=\"us-ascii\"","Subject":"[libcamera-devel] [PATCH v2 1/3] gstreamer: Move negotiation logic\n\tto separate function","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","From":"Jaslo Ziska via libcamera-devel ","Reply-To":"Jaslo Ziska ","Cc":"Jaslo Ziska ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"Move the code which negotiates all the source pad caps into a separate\nfunction called gst_libcamera_src_negotiate. When the negotiation fails\nthis function will return FALSE and TRUE otherwise.\n\nUse this function instead of doing the negotiation manually in\ngst_libcamera_src_task_enter and remove the now redundant error handling\ncode accordingly.\n\nSigned-off-by: Jaslo Ziska \n---\n src/gstreamer/gstlibcamerasrc.cpp | 178 +++++++++++++++---------------\n 1 file changed, 89 insertions(+), 89 deletions(-)","diff":"diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\nindex 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\n","prefixes":["libcamera-devel","v2","1/3"]}