[{"id":37244,"web_url":"https://patchwork.libcamera.org/comment/37244/","msgid":"<854ipy7l92.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-12-10T09:13:13","subject":"Re: [PATCH v4 15/20] libcamera: software_isp: debayer: Introduce a\n\tstart() / stop() methods to the debayer object","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Bryan O'Donoghue writes:\n\n> In order to initialise and deinitialise gpuisp we need to be able to setup\n> EGL in the same thread as Debayer::process() happens in.\n>\n> In order to do this we need to extend the Debayer object to provide start\n> and stop methods which are triggered through invokeMethod in the same way\n> as process() is.\n>\n> Introduce start() and stop() methods to the debayer class. Trigger those\n> methods as described above via invokeMethod. The debayer_egl class will\n> take care of initialising and de-initialising as necessary. Debayer CPU\n> sees no functional change.\n>\n> [bod: Made method blocking not queued per Robert's bugfixes]\n> Signed-off-by: Bryan O'Donoghue \n> ---\n> src/libcamera/software_isp/debayer.cpp | 18 ++++++++++++++++++\n> src/libcamera/software_isp/debayer.h | 2 ++\n> src/libcamera/software_isp/software_isp.cpp | 8 ++++++--\n> 3 files changed, 26 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp\n> index 1d135b278..32a2c8378 100644\n> --- a/src/libcamera/software_isp/debayer.cpp\n> +++ b/src/libcamera/software_isp/debayer.cpp\n> @@ -336,6 +336,24 @@ Debayer::~Debayer()\n> * debayer processing.\n> */\n> \n> +/**\n> + * \\fn int Debayer::start()\n> + * \\brief Execute a start signal in the debayer object from workerthread context.\n> + *\n> + * In the DebayerCPU case this is an empty stub function but\n> + * for the GPU case this does something useful. The stub here is to\n> + * ensure the right version of the virtual gets called.\n\nI think this paragraph can be omitted. Instead, the purpose and the\nreturn value of the method should be described; it's unclear what \"start\nsignal\" is. Perhaps something similar to SoftwareIsp::start()\ndocstring could be used.\n\n> + */\n> +\n> +/**\n> + * \\fn void Debayer::stop()\n> + * \\brief Stop the debayering process and perform cleanup\n> + *\n> + * In the DebayerCPU case this is an empty stub function but\n> + * for the GPU case this does something useful. The stub here is to\n> + * ensure the right version of the virtual gets called.\n\nSimilarly, let's replace this paragraph with something else.\n\n> + */\n> +\n> /**\n> * \\fn void Debayer::setParams(DebayerParams ¶ms)\n> * \\brief Select the bayer params to use for the next frame debayer\n> diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h\n> index ff4a92c15..5c0cb3914 100644\n> --- a/src/libcamera/software_isp/debayer.h\n> +++ b/src/libcamera/software_isp/debayer.h\n> @@ -48,6 +48,8 @@ public:\n> \tstrideAndFrameSize(const PixelFormat &outputFormat, const Size &size) = 0;\n> \n> \tvirtual void process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, DebayerParams params) = 0;\n> +\tvirtual int start() { return 0; }\n> +\tvirtual void stop() { }\n> \n> \tvirtual SizeRange sizes(PixelFormat inputFormat, const Size &inputSize) = 0;\n> \n> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n> index 928a2520c..7c9ad9160 100644\n> --- a/src/libcamera/software_isp/software_isp.cpp\n> +++ b/src/libcamera/software_isp/software_isp.cpp\n> @@ -347,7 +347,9 @@ int SoftwareIsp::start()\n> \t\treturn ret;\n> \n> \tispWorkerThread_.start();\n> -\treturn 0;\n> +\n> +\treturn debayer_->invokeMethod(&DebayerCpu::start,\n> +\t\t\t\t ConnectionTypeBlocking);\n\nDebayerCpu or Debayer?\n\n> }\n> \n> /**\n> @@ -358,9 +360,11 @@ int SoftwareIsp::start()\n> */\n> void SoftwareIsp::stop()\n> {\n> +\tdebayer_->invokeMethod(&DebayerCpu::stop,\n> +\t\t\t ConnectionTypeBlocking);\n\nDebayerCpu or Debayer?\n\n> +\n> \tispWorkerThread_.exit();\n> \tispWorkerThread_.wait();\n> -\tispWorkerThread_.removeMessages(debayer_.get());\n\nWhy removing this here?\n\n> \n> \tThread::current()->dispatchMessages(Message::Type::InvokeMessage, this);","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 0B2D0C3257\n\tfor ;\n\tWed, 10 Dec 2025 09:13:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 505D66144D;\n\tWed, 10 Dec 2025 10:13:21 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4C1476142F\n\tfor ;\n\tWed, 10 Dec 2025 10:13:19 +0100 (CET)","from mail-wr1-f71.google.com (mail-wr1-f71.google.com\n\t[209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-554-rOVMItAIN66VMHPceEAl1Q-1; Wed, 10 Dec 2025 04:13:16 -0500","by mail-wr1-f71.google.com with SMTP id\n\tffacd0b85a97d-42e2e3c2cccso4894353f8f.3\n\tfor ;\n\tWed, 10 Dec 2025 01:13:15 -0800 (PST)","from mzamazal-thinkpadp1gen7.tpbc.csb ([213.175.46.86])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-42f7cbe8a7bsm36526157f8f.4.2025.12.10.01.13.13\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 10 Dec 2025 01:13:13 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"hkhuYL1D\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1765357998;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=r68jw3QxAMordkfehTSbbZZZKTKzWUlf61b42s4i548=;\n\tb=hkhuYL1D55gGWi/9k14AJsEJ0IVjV5zS928Sp3ENix/zsHpaGHhsGADebACTJBXY6FbZ6+\n\tUIF8PM5re8yCwRVTZD0VlsT758vY2cfU58JxVt8xptZ5mAHTfXFAHhJpMsmH/5j5GV43Hq\n\t2DTGP/WViwdCTi6JQxEZ3K5YHAe940g=","X-MC-Unique":"rOVMItAIN66VMHPceEAl1Q-1","X-Mimecast-MFC-AGG-ID":"rOVMItAIN66VMHPceEAl1Q_1765357995","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1765357995; x=1765962795;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject\n\t:date:message-id:reply-to;\n\tbh=r68jw3QxAMordkfehTSbbZZZKTKzWUlf61b42s4i548=;\n\tb=bF9ODDPtHNTYGQYAuXKGZxZruNRoP2siotyI1MQoaPWlVePoA3wqU1O2YY60q//RIS\n\tk8wwNbf0t/UHVv9pL5Ju4ZaOpUF2LyXCNEWy4eVa4w3/ykCZmm/I4wW33NmIxI9vzJY+\n\tRo8qIBP6SnmBDT1KWNjcY3HsFQVu6HNadGapybXQSTeeSXGnIiqwH4ElbDIa4d588dzB\n\tA0lpgyPvIJx8MESKnNYeKSh8B7N+i6LAqe1EImmZ00gq9eIswgVj19li3oBs+oB1SgCe\n\tdV6B75S3klhQal97KGV2pfV2wZcoriz5xoWYUgWQJdx1m3Jhq/x1SluEf5DeeZjtt1ZU\n\tuUoA==","X-Gm-Message-State":"AOJu0YypOTdPxD7OA0ztxL3GyIOfvawLB5tqdcf8X5VcUeQJbf1I3oho\n\tORvRirAZRefGvacKTJCcYeTkT0ybMZwgvMHCk+Uj/IY90ayZNASHV1hxf+5WEFqMhQXHU8v12bC\n\tVrL6jEQzUBDbqv8VyQOpgCv3RecJGbGCT649zAcLakixSFbnA859MQVxY/QluRHxpJ/uY9EyYGj\n\t8=","X-Gm-Gg":"AY/fxX6w4RYlz4uMvT8bjui7obrOhjgUW6U+UgLmC5pvDgAs8DCqCO6V1DQzpGFY9qT\n\tNc1Xab54CzWWcY4CkmkZ3amTP7uQi3QJNxhXy+omauazAQr0NMP/Cm7NFT/l1x31tYEyqoLAbWx\n\tsW/9Fas751LqpkJJ1PbT0Rq8Wh8bhA8Mfril3ku5rkW3uvRdzR1Mox5KMAS+JbPt1+3JaHb1oVB\n\tkRmznmHKNoGVSsyanVs+1UZ0gm6rZcaT+DrjTYOX3KbqMKkfrNLYkNOC53gJLUzjZS8mSgGlokx\n\tg4OhFfpYqPp5DmH563FF8Hxoxaqd9Vtb/1V7RYgkKUyqQ7QWWvDkKsGnj2srArMqS8F1qLNkadk\n\tN5xgxkjTVzGbz8MkRAOlHYvXa8w==","X-Received":["by 2002:a05:6000:4308:b0:42f:9e75:85ed with SMTP id\n\tffacd0b85a97d-42fa39cf2b8mr1747195f8f.11.1765357994589; \n\tWed, 10 Dec 2025 01:13:14 -0800 (PST)","by 2002:a05:6000:4308:b0:42f:9e75:85ed with SMTP id\n\tffacd0b85a97d-42fa39cf2b8mr1747159f8f.11.1765357994082; \n\tWed, 10 Dec 2025 01:13:14 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IEmIejlb6OWXPJfrREIs9C6lPIb29Yb5jTRIOaNz+AFH9ezqonWT/ogMLMVhcP4TemttToUng==","From":"Milan Zamazal ","To":"Bryan O'Donoghue ","Cc":"libcamera-devel@lists.libcamera.org, pavel@ucw.cz","Subject":"Re: [PATCH v4 15/20] libcamera: software_isp: debayer: Introduce a\n\tstart() / stop() methods to the debayer object","In-Reply-To":"<20251210003139.43606-16-bryan.odonoghue@linaro.org> (Bryan\n\tO'Donoghue's message of \"Wed, 10 Dec 2025 00:31:32 +0000\")","References":"<20251210003139.43606-1-bryan.odonoghue@linaro.org>\n\t<20251210003139.43606-16-bryan.odonoghue@linaro.org>","Date":"Wed, 10 Dec 2025 10:13:13 +0100","Message-ID":"<854ipy7l92.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"Z6pIEOHckiOkCXQA31fixCryvM4W1dJvGLxMvqBVwDA_1765357995","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37253,"web_url":"https://patchwork.libcamera.org/comment/37253/","msgid":"<03c78fb7-7e2a-4071-8a36-843aceb3f4b3@linaro.org>","date":"2025-12-10T14:26:32","subject":"Re: [PATCH v4 15/20] libcamera: software_isp: debayer: Introduce a\n\tstart() / stop() methods to the debayer object","submitter":{"id":175,"url":"https://patchwork.libcamera.org/api/people/175/","name":"Bryan O'Donoghue","email":"bryan.odonoghue@linaro.org"},"content":"On 10/12/2025 09:13, Milan Zamazal wrote:\n> Bryan O'Donoghue writes:\n> \n>> In order to initialise and deinitialise gpuisp we need to be able to setup\n>> EGL in the same thread as Debayer::process() happens in.\n>>\n>> In order to do this we need to extend the Debayer object to provide start\n>> and stop methods which are triggered through invokeMethod in the same way\n>> as process() is.\n>>\n>> Introduce start() and stop() methods to the debayer class. Trigger those\n>> methods as described above via invokeMethod. The debayer_egl class will\n>> take care of initialising and de-initialising as necessary. Debayer CPU\n>> sees no functional change.\n>>\n>> [bod: Made method blocking not queued per Robert's bugfixes]\n>> Signed-off-by: Bryan O'Donoghue \n>> ---\n>> src/libcamera/software_isp/debayer.cpp | 18 ++++++++++++++++++\n>> src/libcamera/software_isp/debayer.h | 2 ++\n>> src/libcamera/software_isp/software_isp.cpp | 8 ++++++--\n>> 3 files changed, 26 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp\n>> index 1d135b278..32a2c8378 100644\n>> --- a/src/libcamera/software_isp/debayer.cpp\n>> +++ b/src/libcamera/software_isp/debayer.cpp\n>> @@ -336,6 +336,24 @@ Debayer::~Debayer()\n>> * debayer processing.\n>> */\n>> \n>> +/**\n>> + * \\fn int Debayer::start()\n>> + * \\brief Execute a start signal in the debayer object from workerthread context.\n>> + *\n>> + * In the DebayerCPU case this is an empty stub function but\n>> + * for the GPU case this does something useful. The stub here is to\n>> + * ensure the right version of the virtual gets called.\n> \n> I think this paragraph can be omitted. Instead, the purpose and the\n> return value of the method should be described; it's unclear what \"start\n> signal\" is. Perhaps something similar to SoftwareIsp::start()\n> docstring could be used.\n> \n>> + */\n>> +\n>> +/**\n>> + * \\fn void Debayer::stop()\n>> + * \\brief Stop the debayering process and perform cleanup\n>> + *\n>> + * In the DebayerCPU case this is an empty stub function but\n>> + * for the GPU case this does something useful. The stub here is to\n>> + * ensure the right version of the virtual gets called.\n> \n> Similarly, let's replace this paragraph with something else.\n\nSure.\n\n> \n>> + */\n>> +\n>> /**\n>> * \\fn void Debayer::setParams(DebayerParams ¶ms)\n>> * \\brief Select the bayer params to use for the next frame debayer\n>> diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h\n>> index ff4a92c15..5c0cb3914 100644\n>> --- a/src/libcamera/software_isp/debayer.h\n>> +++ b/src/libcamera/software_isp/debayer.h\n>> @@ -48,6 +48,8 @@ public:\n>> \tstrideAndFrameSize(const PixelFormat &outputFormat, const Size &size) = 0;\n>> \n>> \tvirtual void process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, DebayerParams params) = 0;\n>> +\tvirtual int start() { return 0; }\n>> +\tvirtual void stop() { }\n>> \n>> \tvirtual SizeRange sizes(PixelFormat inputFormat, const Size &inputSize) = 0;\n>> \n>> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n>> index 928a2520c..7c9ad9160 100644\n>> --- a/src/libcamera/software_isp/software_isp.cpp\n>> +++ b/src/libcamera/software_isp/software_isp.cpp\n>> @@ -347,7 +347,9 @@ int SoftwareIsp::start()\n>> \t\treturn ret;\n>> \n>> \tispWorkerThread_.start();\n>> -\treturn 0;\n>> +\n>> +\treturn debayer_->invokeMethod(&DebayerCpu::start,\n>> +\t\t\t\t ConnectionTypeBlocking);\n> \n> DebayerCpu or Debayer?\n\nDebayerCPU.\n\nThe next patch changes the type to virtual base class. This patch adds \nthe methods.\n\n> \n>> }\n>> \n>> /**\n>> @@ -358,9 +360,11 @@ int SoftwareIsp::start()\n>> */\n>> void SoftwareIsp::stop()\n>> {\n>> +\tdebayer_->invokeMethod(&DebayerCpu::stop,\n>> +\t\t\t ConnectionTypeBlocking);\n> \n> DebayerCpu or Debayer?\n> \n>> +\n>> \tispWorkerThread_.exit();\n>> \tispWorkerThread_.wait();\n>> -\tispWorkerThread_.removeMessages(debayer_.get());\n> \n> Why removing this here?\n> \n>> \n>> \tThread::current()->dispatchMessages(Message::Type::InvokeMessage, this);\n> \n\nPer Barnabas comment in the previous version, making the call blocking, \nmeans we should remove the removeMessages() call.\n\n---\nbod","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 E678DBD1F1\n\tfor ;\n\tWed, 10 Dec 2025 14:26:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5E73461479;\n\tWed, 10 Dec 2025 15:26:40 +0100 (CET)","from mail-pf1-x433.google.com (mail-pf1-x433.google.com\n\t[IPv6:2607:f8b0:4864:20::433])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7480E613CB\n\tfor ;\n\tWed, 10 Dec 2025 15:26:39 +0100 (CET)","by mail-pf1-x433.google.com with SMTP id\n\td2e1a72fcca58-7e2762ad850so7204448b3a.3\n\tfor ;\n\tWed, 10 Dec 2025 06:26:39 -0800 (PST)","from [10.237.118.45] (M106185144161.v4.enabler.ne.jp.\n\t[106.185.144.161]) by smtp.gmail.com with ESMTPSA id\n\td2e1a72fcca58-7e2afa31d4esm19055812b3a.62.2025.12.10.06.26.34\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tWed, 10 Dec 2025 06:26:37 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"bc8lcJ1G\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=linaro.org; s=google; t=1765376798; x=1765981598;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:in-reply-to:from:content-language\n\t:references:cc:to:subject:user-agent:mime-version:date:message-id\n\t:from:to:cc:subject:date:message-id:reply-to;\n\tbh=M4wK/C4TAW9qsWXVkNBH64Vt/PlHn3zrNZcHskgN6RM=;\n\tb=bc8lcJ1GfUw2xr1ZQPTI1xvgbH/1Sup9btX0ax5WyvPXy8VGdutJghAHUFoxxk5rj0\n\t4M2X6DPdtchwwD3pdGzNgBWfdn1spW0Y58bkd+oC88HNL+49VWOTCnxzvkyFN/Jj2Yh3\n\tZzOLUiPTsxR6UQ2h3jNB15ScBjJhUJXdc4gVFu+CfeUDi34UgDPR8iv8wtQxMuiHWE9G\n\tzGwWO4zPauPDNQYGijsZmyaCVRlOpAkSTBUR7zIUFrxvWeOnTHBrnsT6vN24bJ8SAeT+\n\t7kSNtbdm6tMf9Cv85WihcyXcO//WRHi7WiOvMoJcoirfDgnfOzP1/sTzj8SW8tFWmram\n\txnkA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1765376798; x=1765981598;\n\th=content-transfer-encoding:in-reply-to:from:content-language\n\t:references:cc:to:subject:user-agent:mime-version:date:message-id\n\t:x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=M4wK/C4TAW9qsWXVkNBH64Vt/PlHn3zrNZcHskgN6RM=;\n\tb=VYbn8qqJlRUe5PplGfTVrA3Z30xorXw3C3mqrCYI5C7Ojb37YbJALDSNRzXvsUwTIN\n\tX8aihbodFmbi2lu1QJQVGFkDJcKifSvewwh/MAK5xb42w977jz7/EiTzEBfg0+yyrbtZ\n\tQbA01vJmX46QPyLk+BFk/HswXSYxtZ0epiZzUqB/fMQeJvGEJIc0UV4sJQ1uTSLPHePC\n\t6I7Z9J1XpV44U0Rvz/N+R1jHC6IFB//Jw5th1Ar0+ipTsLTLj0i7nMBBM60L0bVzPZYs\n\tdscH8yQw/DVpWAR2eLiyZMr1kJ5zXD2JVM/U9uPsfL1IFPHByvbWQn9pHjEiE0u9Z3HA\n\tUeyA==","X-Gm-Message-State":"AOJu0YyVhWPakxd5y/VI1BoJXpESvEjJAUErJebfOP+mi9ZdRt0amaQw\n\tdWX9lfuOn/4+2f/exy9JQtprzWJTm3w1lSjFVrSCGxDTIiL7q6tXIwrc2Asgl+luyco=","X-Gm-Gg":"AY/fxX6/8CS67is88MWDi+czZA8ZJ+C+NuXG2yoy4+8XwpAmqCCAmih8E0PGat0h7hA\n\tZWK/qC1+Fkz8D6BSSqk5jGPp078pTcNIcGSIqhJpuj4KYEeewdANF+KCQaF4YTSxXWr72PsyRo0\n\tALNFO260+P6AqCkDInFEOM5lSMabE05CmF0RjOUYjkA1D1JYKW/qgSeKkrizOOOqOsoS5IyedG7\n\tcea8+once3WgNrAIYfWeVKmHUgohvS/RJ/kppGl139yv6brlSPmLx+Fc12RPEuuHQA0vDwlcGgr\n\twY0F35t0xdOJHMvfrjSJZCbit3tX18mJLaxpyJxAfYMtBZ3VoT363iTSif6p6RF5Frc7F2yPXgZ\n\ti1KyGBcX3dKGqqvCPUUdBLuEZ2d7xhabW3QMY2bRKP1qtuNN8ifG814tgvFyBldlR2K5a4TLBEi\n\tK1LrTaQcTKp5c0XjkUpGa2UD59NSO469q6IJzLMOVx3jjbz4JLO9yR8T0zacI=","X-Google-Smtp-Source":"AGHT+IFzXJVhGhHyptpvC6w8KS/k4zpr8dXrcreBTMljM+I0l4WgF5TwociK5hwp05mQ9WOULnT93A==","X-Received":"by 2002:a05:6a00:1494:b0:7e2:c0d5:372d with SMTP id\n\td2e1a72fcca58-7f22d9cbc17mr2607756b3a.11.1765376797650; \n\tWed, 10 Dec 2025 06:26:37 -0800 (PST)","Message-ID":"<03c78fb7-7e2a-4071-8a36-843aceb3f4b3@linaro.org>","Date":"Wed, 10 Dec 2025 14:26:32 +0000","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 15/20] libcamera: software_isp: debayer: Introduce a\n\tstart() / stop() methods to the debayer object","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org, pavel@ucw.cz","References":"<20251210003139.43606-1-bryan.odonoghue@linaro.org>\n\t<20251210003139.43606-16-bryan.odonoghue@linaro.org>\n\t<854ipy7l92.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Content-Language":"en-US","From":"Bryan O'Donoghue ","In-Reply-To":"<854ipy7l92.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":37258,"web_url":"https://patchwork.libcamera.org/comment/37258/","msgid":"<85pl8m4avw.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-12-10T15:25:23","subject":"Re: [PATCH v4 15/20] libcamera: software_isp: debayer: Introduce a\n\tstart() / stop() methods to the debayer object","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Bryan O'Donoghue writes:\n\n> On 10/12/2025 09:13, Milan Zamazal wrote:\n>> Bryan O'Donoghue writes:\n>> \n>\n>>> In order to initialise and deinitialise gpuisp we need to be able to setup\n>>> EGL in the same thread as Debayer::process() happens in.\n>>>\n>>> In order to do this we need to extend the Debayer object to provide start\n>>> and stop methods which are triggered through invokeMethod in the same way\n>>> as process() is.\n>>>\n>>> Introduce start() and stop() methods to the debayer class. Trigger those\n>>> methods as described above via invokeMethod. The debayer_egl class will\n>>> take care of initialising and de-initialising as necessary. Debayer CPU\n>>> sees no functional change.\n>>>\n>>> [bod: Made method blocking not queued per Robert's bugfixes]\n>>> Signed-off-by: Bryan O'Donoghue \n>>> ---\n>>> src/libcamera/software_isp/debayer.cpp | 18 ++++++++++++++++++\n>>> src/libcamera/software_isp/debayer.h | 2 ++\n>>> src/libcamera/software_isp/software_isp.cpp | 8 ++++++--\n>>> 3 files changed, 26 insertions(+), 2 deletions(-)\n>>>\n>>> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp\n>>> index 1d135b278..32a2c8378 100644\n>>> --- a/src/libcamera/software_isp/debayer.cpp\n>>> +++ b/src/libcamera/software_isp/debayer.cpp\n>>> @@ -336,6 +336,24 @@ Debayer::~Debayer()\n>>> * debayer processing.\n>>> */\n>>> +/**\n>>> + * \\fn int Debayer::start()\n>>> + * \\brief Execute a start signal in the debayer object from workerthread context.\n>>> + *\n>>> + * In the DebayerCPU case this is an empty stub function but\n>>> + * for the GPU case this does something useful. The stub here is to\n>>> + * ensure the right version of the virtual gets called.\n>> I think this paragraph can be omitted. Instead, the purpose and the\n>> return value of the method should be described; it's unclear what \"start\n>> signal\" is. Perhaps something similar to SoftwareIsp::start()\n>> docstring could be used.\n>> \n>>> + */\n>>> +\n>>> +/**\n>>> + * \\fn void Debayer::stop()\n>>> + * \\brief Stop the debayering process and perform cleanup\n>>> + *\n>>> + * In the DebayerCPU case this is an empty stub function but\n>>> + * for the GPU case this does something useful. The stub here is to\n>>> + * ensure the right version of the virtual gets called.\n>> Similarly, let's replace this paragraph with something else.\n>\n> Sure.\n>\n>> \n>>> + */\n>>> +\n>>> /**\n>>> * \\fn void Debayer::setParams(DebayerParams ¶ms)\n>>> * \\brief Select the bayer params to use for the next frame debayer\n>>> diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h\n>>> index ff4a92c15..5c0cb3914 100644\n>>> --- a/src/libcamera/software_isp/debayer.h\n>>> +++ b/src/libcamera/software_isp/debayer.h\n>>> @@ -48,6 +48,8 @@ public:\n>>> \tstrideAndFrameSize(const PixelFormat &outputFormat, const Size &size) = 0;\n>>> \tvirtual void process(uint32_t frame, FrameBuffer *input, FrameBuffer *output, DebayerParams params) = 0;\n>>> +\tvirtual int start() { return 0; }\n>>> +\tvirtual void stop() { }\n>>> \tvirtual SizeRange sizes(PixelFormat inputFormat, const Size &inputSize) = 0;\n>>> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\n>>> index 928a2520c..7c9ad9160 100644\n>>> --- a/src/libcamera/software_isp/software_isp.cpp\n>>> +++ b/src/libcamera/software_isp/software_isp.cpp\n>>> @@ -347,7 +347,9 @@ int SoftwareIsp::start()\n>>> \t\treturn ret;\n>>> \tispWorkerThread_.start();\n>>> -\treturn 0;\n>>> +\n>>> +\treturn debayer_->invokeMethod(&DebayerCpu::start,\n>>> +\t\t\t\t ConnectionTypeBlocking);\n>> DebayerCpu or Debayer?\n>\n> DebayerCPU.\n>\n> The next patch changes the type to virtual base class. This patch adds the methods.\n>\n>> \n>>> }\n>>> /**\n>>> @@ -358,9 +360,11 @@ int SoftwareIsp::start()\n>>> */\n>>> void SoftwareIsp::stop()\n>>> {\n>>> +\tdebayer_->invokeMethod(&DebayerCpu::stop,\n>>> +\t\t\t ConnectionTypeBlocking);\n>> DebayerCpu or Debayer?\n>> \n>>> +\n>>> \tispWorkerThread_.exit();\n>>> \tispWorkerThread_.wait();\n>>> -\tispWorkerThread_.removeMessages(debayer_.get());\n>> Why removing this here?\n>> \n>>> \tThread::current()->dispatchMessages(Message::Type::InvokeMessage, this);\n>> \n>\n> Per Barnabas comment in the previous version, making the call blocking, means we should remove the removeMessages() call.\n\nI see. It would be worth to mention this in the commit message.\n\n> ---\n> bod","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 A958FC3257\n\tfor ;\n\tWed, 10 Dec 2025 15:25:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1BDCF6148D;\n\tWed, 10 Dec 2025 16:25:30 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6A26261480\n\tfor ;\n\tWed, 10 Dec 2025 16:25:28 +0100 (CET)","from mail-wm1-f72.google.com (mail-wm1-f72.google.com\n\t[209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-609-Pg4yZRaCP0ynr-bbWAVUOw-1; Wed, 10 Dec 2025 10:25:26 -0500","by mail-wm1-f72.google.com with SMTP id\n\t5b1f17b1804b1-4775d110fabso61888545e9.1\n\tfor ;\n\tWed, 10 Dec 2025 07:25:25 -0800 (PST)","from mzamazal-thinkpadp1gen7.tpbc.csb ([213.175.46.86])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-47a82d7f778sm58362545e9.11.2025.12.10.07.25.23\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 10 Dec 2025 07:25:23 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"G+k5XnYM\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1765380327;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=nYYqBB9vA1kkCO0KM141wmz0Sz+e2PAoBZ/VZEDNqdg=;\n\tb=G+k5XnYMDclBwD3gqV1qHan31lrWp5Cs4U/hdrN/Cs6IgtMRtO+4TVh3yYXgGUnb3ynR5B\n\tvne3MF5/eTeVODYfIwrUaDIe2Jsh3Kt7j8aSBvHa01x1MvT9x4jrcnFft6JhBz9nh9j5EY\n\toNKYHZ0wyfJbLEF1nH6GPZzD5Aq9j4U=","X-MC-Unique":"Pg4yZRaCP0ynr-bbWAVUOw-1","X-Mimecast-MFC-AGG-ID":"Pg4yZRaCP0ynr-bbWAVUOw_1765380325","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1765380325; x=1765985125;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject\n\t:date:message-id:reply-to;\n\tbh=nYYqBB9vA1kkCO0KM141wmz0Sz+e2PAoBZ/VZEDNqdg=;\n\tb=bWyRPWCZ3tluK2jycLOzkcbof29tAwORuKeTmXuSSStyTTLvnhzikGTt0UbhbiOooi\n\t33txNGBoWTbKxw3cD53V5Wz0aPR0YGhXCGCzyPrrGrwqTPRVUriOKZaef7dSPt628Zr2\n\tzJfruGYExXPDFtQTypAHw/ELnU7B4EGxUO/as2CCm/X6rQ4E+BEW7FnrdKI9+7ALf6Zr\n\tjtNvvxwZZJF1nbYqF17nUjAKAHggGbciY7HCFubpwl40bUHIBcHhVcArsUqwjggziYw0\n\tPXTP02W4lDi1sytof589ThiECLC82oE3HUObFf4ZZIWC7qPCy/PtfCd4gh28tHRIVu4C\n\tnXYQ==","X-Gm-Message-State":"AOJu0YxCMNQp6HTKYVqwJ+EUq7UYsL2BijC7YEG7iqgCq2pJHxvb1quk\n\t5Na4s+e92wBIJ7dPIiv3hWw3t/CbHf0VxnRKV6e5IQKjDe9k+v5CTDZTZ3CwZ7ammC0I2+Jaz/z\n\tFrU3HMKcPCIVOab1dh0IXfctWsSFk6auy/FWCZ6om46uLMRtZqloYbVYsXzVbUlMyrLHA34aVfq\n\trDSmx1Zjo=","X-Gm-Gg":"ASbGnctRhkvftKHvgoGAo8VJqoTt6bTV7RlTf8dMjmkf2IiaC6e0zEvXzJ8GFsdepzC\n\ttquSs2khTPMNQAv4GtP7hi4pkqcPeAZinC/wTopMiyD6uRpTaVFEz/AeMXXGHBUmsJxAEQ6SW3m\n\t/W7vwFSxXsgbUbHg/KHM7l97zkB6ByfwoYvbELmbRaJBRN5KTO7FwirEhxKKzhNPIy7IHv2RStk\n\t3QaRDei0qa6iRDq6v1aruZHTe3QW2o7+L3HuJ5S+ZYFjztNZ4GYaTP6/GB2c0s/oUil7hQaS4cV\n\tk2Mp1SPu3SbnD24pAUU1661OQ8B2sTaJBfPG/1OXndstXoxAvhjNiNtOvBSma69k3H4s55hcl3o\n\tB/aPMRCWoJPWZWtC/opgfNm055w==","X-Received":["by 2002:a05:600c:8b6a:b0:477:7a95:b971 with SMTP id\n\t5b1f17b1804b1-47a8384e027mr30917305e9.31.1765380324698; \n\tWed, 10 Dec 2025 07:25:24 -0800 (PST)","by 2002:a05:600c:8b6a:b0:477:7a95:b971 with SMTP id\n\t5b1f17b1804b1-47a8384e027mr30917095e9.31.1765380324300; \n\tWed, 10 Dec 2025 07:25:24 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IEVPL4krKG3F2uLErEWZwPD6X+hB3aXZ0BI/t+BSTvcxUtUBT5cKcGD9Hz5rZ49lwSTuQhYUQ==","From":"Milan Zamazal ","To":"Bryan O'Donoghue ","Cc":"libcamera-devel@lists.libcamera.org, pavel@ucw.cz","Subject":"Re: [PATCH v4 15/20] libcamera: software_isp: debayer: Introduce a\n\tstart() / stop() methods to the debayer object","In-Reply-To":"<03c78fb7-7e2a-4071-8a36-843aceb3f4b3@linaro.org> (Bryan\n\tO'Donoghue's message of \"Wed, 10 Dec 2025 14:26:32 +0000\")","References":"<20251210003139.43606-1-bryan.odonoghue@linaro.org>\n\t<20251210003139.43606-16-bryan.odonoghue@linaro.org>\n\t<854ipy7l92.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>\n\t<03c78fb7-7e2a-4071-8a36-843aceb3f4b3@linaro.org>","Date":"Wed, 10 Dec 2025 16:25:23 +0100","Message-ID":"<85pl8m4avw.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"lFFqFr1phyEyukY_iU7Thelbnr2XoTyeIAfS1Sca5Dg_1765380325","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]