{"id":19224,"url":"https://patchwork.libcamera.org/api/1.1/patches/19224/?format=json","web_url":"https://patchwork.libcamera.org/patch/19224/","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":"<20231122135406.14921-2-jaslo@ziska.de>","date":"2023-11-22T13:43:54","name":"[libcamera-devel,1/2] gstreamer: Move negotiation logic to separate function","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"81b236d9864ae0cf101571e3c02785b3700bdb54","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/19224/mbox/","series":[{"id":4079,"url":"https://patchwork.libcamera.org/api/1.1/series/4079/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4079","date":"2023-11-22T13:43:53","name":"gstreamer: Implement renegotiation","version":1,"mbox":"https://patchwork.libcamera.org/series/4079/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/19224/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/19224/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 85CEDC3220\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 22 Nov 2023 13:55:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3C1F5629BC;\n\tWed, 22 Nov 2023 14:55:21 +0100 (CET)","from mo4-p00-ob.smtp.rzone.de (mo4-p00-ob.smtp.rzone.de\n\t[81.169.146.221])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4827A61DAD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 22 Nov 2023 14:55:20 +0100 (CET)","from archlinux.fritz.box by smtp.strato.de (RZmta 49.9.1 AUTH)\n\twith ESMTPSA id j3f4eezAMDtJnQ1\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits))\n\t(Client did not present a certificate);\n\tWed, 22 Nov 2023 14:55:19 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700661321;\n\tbh=oWgg1wo9IWt/LYBkpHt4MOBNEMDRQsTaL5IYb22BJOI=;\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=U3ybrzVxD3bz5HpNgE/kYpxLJ1Ka7BUxixa2cFA2JoCI+h+lEGebmnhVuODC0Od/s\n\t5DOaB5mU2O7ARpbiPh08DWrmdcZn9FxvXqgE0rjW9AzEbAlAY/Al3fXrcy8NPTYjey\n\tSBRYuPGJTQlUoo2BBsaKHtDFm7sFCgB46stj2TLjoTi515xTkPp9XYzmtkqeaPAOn/\n\t0ok/VukpGBfsjd5nAlMdmmnXTpoVTpcJOViPvkchZDu0F+gJ5EJbBRbkWlne4T54d+\n\tBjVlIe+hLxMdFJSAq4PRZAe9lA/Bua4kwunkRZCqnEuu/f3Lbza91zjbGIgr1bwUR+\n\tz6DHnspPfdIfg==","v=1; a=rsa-sha256; c=relaxed/relaxed; t=1700661319;\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=fS0xtz0syGlJKn1HAlsQzRye2dlfXH4niWK15M5Wh0w=;\n\tb=Yf1/24NqRQgbf8fwCqPTCeCOk35qgILMBHXsGDxadplWyxidRbP88cwuCYkbAuDc56\n\tJ45bwOO0V8EdJG0MkhrqgK2AMsXQdV+Oz84Dv/kO2ZrCwH7/lFSIGl4iKQ8hk1Jyv4JU\n\tJtLLQYl0yZ983ObPG80TYAHW2vjnlnSQ1l3xBpLAA6/BYcW63fpvmUZHynGSb4hWmguI\n\tYxlDNAog0/XA239WkqiyKwQGM5CA+5g+aCtJs9PNpqOgYm50MLnFx+WmcF0sC/Sv8EYB\n\tlBeCkuKPnv/m0bvql51gfP6Qgi+TzsqdM9hAqz4N9pdfyNNyhPM8XFOlKf+1aY2bKESG\n\t2RfQ==","v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1700661319;\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=fS0xtz0syGlJKn1HAlsQzRye2dlfXH4niWK15M5Wh0w=;\n\tb=+f051/ws0gMdFdTqD3ATo6yUn1HY+qZFoci7MShMYqH/6eDZpYdweNswH+oacNR52p\n\ttuv9fUYDcfjJxRNLT3CQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=ziska.de header.i=@ziska.de\n\theader.b=\"Yf1/24Nq\"; \n\tdkim=permerror (0-bit key) header.d=ziska.de header.i=@ziska.de\n\theader.b=\"+f051/ws\"; dkim-atps=neutral","ARC-Seal":"i=1; a=rsa-sha256; t=1700661319; cv=none;\n\td=strato.com; s=strato-dkim-0002;\n\tb=beJUcUvboQ0Jnz3ZHkuu2AEIuHmCscvSgOiXPNWORvn2otEFDPvQ3W9r1TNeFwl/YK\n\tX68q8eJBMiGpAIgKjbhZRSAaIMEi/4zSBzjWNr5QMCQI6eo8YekOIHo89Bcqqt5YRjEa\n\tRmkWGxmSjJJ5cr+MgQHUtUCsX9NhXRoN5f0nEusFWsQin+Z9MT28GFmTKLXrICAsQ6Ek\n\tR4bxE1HZCcXR05QaZDU1T3LKTKa40dhxcSBOXZfS9Ki0PAQgD+fRlHuxoKJqoqR0dsXc\n\tOFIucUbsYAms8LRRIbCGrHoeMKwlDe8wcMPuh1tppB5+R9ZaFjMz6Ips2oQiQr/x5VRo\n\tKLAw==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; t=1700661319;\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=fS0xtz0syGlJKn1HAlsQzRye2dlfXH4niWK15M5Wh0w=;\n\tb=t1nrEcFDZhqi5ar1Yf25JpzLxJ3a5J3A1L2wbgT9cDCthnoxLsbai6FFv5hNsP+C4i\n\tumzHcaU6M8XgWpqeuH8gNT0TtFVIDyEhIMKBFd8WoIJM6+bpC8JigS+t3AMIaiFLQ7QZ\n\tXQ7rsAUfz3WcX+NGP93i9ozAZ681DyJIFekQp+YNjWu2zcczWmPpD9T2br4oiw/4/qqq\n\tpWD6JWRX4Mu1DvtH+PGGcgK6TtvD/BO+zTGocUyCk9+V8E0wwHMyMtIRPp+mntsARWE6\n\ttPl+kRGmSwFBetR3UyvH36wAUpNxysFDNhUB/0xQQKpsmRZvEPNNPqfsAep92Jht3Ope\n\t6W0Q==","ARC-Authentication-Results":"i=1; strato.com;\n    arc=none;\n    dkim=none","X-RZG-CLASS-ID":"mo00","X-RZG-AUTH":"\":Jm0XeU+IYfb0x77LHmrjN5Wlb7TBwusDqIM6Hizy8VdfzvKi4yoFC9cG0Yq6UfJaraj+i8jcRUV7OSd6ZI2VJZzV+od9\"","To":"libcamera-devel@lists.libcamera.org","Date":"Wed, 22 Nov 2023 14:43:54 +0100","Message-ID":"<20231122135406.14921-2-jaslo@ziska.de>","X-Mailer":"git-send-email 2.42.1","In-Reply-To":"<20231122135406.14921-1-jaslo@ziska.de>","References":"<20231122135406.14921-1-jaslo@ziska.de>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Content-Type":"text/plain; charset=\"us-ascii\"","Subject":"[libcamera-devel] [PATCH 1/2] gstreamer: Move negotiation logic to\n\tseparate 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 | 176 +++++++++++++++---------------\n 1 file changed, 88 insertions(+), 88 deletions(-)","diff":"diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\nindex 767017db..e7a49fef 100644\n--- a/src/gstreamer/gstlibcamerasrc.cpp\n+++ b/src/gstreamer/gstlibcamerasrc.cpp\n@@ -379,6 +379,87 @@ gst_libcamera_src_open(GstLibcameraSrc *self)\n \treturn true;\n }\n \n+static gboolean\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 +549,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,89 +578,22 @@ 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 \t\tgst_segment_init(&segment, GST_FORMAT_TIME);\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@@ -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","1/2"]}