Patch Detail
Show a patch.
GET /api/1.1/patches/19257/?format=api
{ "id": 19257, "url": "https://patchwork.libcamera.org/api/1.1/patches/19257/?format=api", "web_url": "https://patchwork.libcamera.org/patch/19257/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "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=api", "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=api", "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": "<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 C61C8BDE6B\n\tfor <parsemail@patchwork.libcamera.org>;\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 <libcamera-devel@lists.libcamera.org>;\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": "<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": "Jaslo Ziska <jaslo@ziska.de>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "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 <jaslo@ziska.de>\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" ] }