[{"id":22670,"web_url":"https://patchwork.libcamera.org/comment/22670/","msgid":"<164941819412.3968198.18148847026865390158@Monstersaurus>","date":"2022-04-08T11:43:14","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: vimc: Synchronise parameter\n\tbuffer ops naming","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Umang Jain via libcamera-devel (2022-04-08 11:54:39)\n> Synchronise the names of the operations with respect to parameters\n> buffer with the names used in other IPA interfaces.\n> \n\nThe VIMC pipeline handler doesn't yet use an ISP, so I was curious at\nwhat buffers this would represent - but actually I think it's fine to\nkeep it aligned at least.\n\nI expect we could also use the VIMC pipeline handler with any GPU based\nISP we might develop so I expect it will get more alignment with that\ndevelopment too.\n\nSo for this - I think it's fine.\n\n\nReviewed-by: Kieran Bingham \n\n> Signed-off-by: Umang Jain \n> ---\n> include/libcamera/ipa/vimc.mojom | 4 ++--\n> src/ipa/vimc/vimc.cpp | 6 +++---\n> src/libcamera/pipeline/vimc/vimc.cpp | 8 ++++----\n> 3 files changed, 9 insertions(+), 9 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom\n> index cdc03ffb..718b9674 100644\n> --- a/include/libcamera/ipa/vimc.mojom\n> +++ b/include/libcamera/ipa/vimc.mojom\n> @@ -37,9 +37,9 @@ interface IPAVimcInterface {\n> * interface functions that mimick how other pipeline handlers typically\n> * handle parameters at runtime.\n> */\n> - [async] fillParams(uint32 frame, uint32 bufferId);\n> + [async] fillParamsBuffer(uint32 frame, uint32 bufferId);\n> };\n> \n> interface IPAVimcEventInterface {\n> - paramsFilled(uint32 bufferId);\n> + paramsBufferReady(uint32 bufferId);\n> };\n> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\n> index a62e72b0..85afb279 100644\n> --- a/src/ipa/vimc/vimc.cpp\n> +++ b/src/ipa/vimc/vimc.cpp\n> @@ -44,7 +44,7 @@ public:\n> void unmapBuffers(const std::vector &ids) override;\n> \n> void queueRequest(uint32_t frame, const ControlList &controls) override;\n> - void fillParams(uint32_t frame, uint32_t bufferId) override;\n> + void fillParamsBuffer(uint32_t frame, uint32_t bufferId) override;\n> \n> private:\n> void initTrace();\n> @@ -134,7 +134,7 @@ void IPAVimc::queueRequest([[maybe_unused]] uint32_t frame,\n> {\n> }\n> \n> -void IPAVimc::fillParams([[maybe_unused]] uint32_t frame, uint32_t bufferId)\n> +void IPAVimc::fillParamsBuffer([[maybe_unused]] uint32_t frame, uint32_t bufferId)\n> {\n> auto it = buffers_.find(bufferId);\n> if (it == buffers_.end()) {\n> @@ -142,7 +142,7 @@ void IPAVimc::fillParams([[maybe_unused]] uint32_t frame, uint32_t bufferId)\n> return;\n> }\n> \n> - paramsFilled.emit(bufferId);\n> + paramsBufferReady.emit(bufferId);\n> }\n> \n> void IPAVimc::initTrace()\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 69b83d07..fff95a34 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -53,7 +53,7 @@ public:\n> int init();\n> int allocateMockIPABuffers();\n> void bufferReady(FrameBuffer *buffer);\n> - void paramsFilled(unsigned int id);\n> + void paramsBufferReady(unsigned int id);\n> \n> MediaDevice *media_;\n> std::unique_ptr sensor_;\n> @@ -467,7 +467,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> return false;\n> }\n> \n> - data->ipa_->paramsFilled.connect(data.get(), &VimcCameraData::paramsFilled);\n> + data->ipa_->paramsBufferReady.connect(data.get(), &VimcCameraData::paramsBufferReady);\n> \n> std::string conf = data->ipa_->configurationFile(\"vimc.conf\");\n> data->ipa_->init(IPASettings{ conf, data->sensor_->model() });\n> @@ -589,7 +589,7 @@ void VimcCameraData::bufferReady(FrameBuffer *buffer)\n> pipe->completeBuffer(request, buffer);\n> pipe->completeRequest(request);\n> \n> - ipa_->fillParams(request->sequence(), mockIPABufs_[0]->cookie());\n> + ipa_->fillParamsBuffer(request->sequence(), mockIPABufs_[0]->cookie());\n> }\n> \n> int VimcCameraData::allocateMockIPABuffers()\n> @@ -607,7 +607,7 @@ int VimcCameraData::allocateMockIPABuffers()\n> return video_->exportBuffers(kBufCount, &mockIPABufs_);\n> }\n> \n> -void VimcCameraData::paramsFilled([[maybe_unused]] unsigned int id)\n> +void VimcCameraData::paramsBufferReady([[maybe_unused]] unsigned int id)\n> {\n> }\n> \n> -- \n> 2.31.0\n>","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 BF97DC3256\n\tfor ;\n\tFri, 8 Apr 2022 11:43:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C232665642;\n\tFri, 8 Apr 2022 13:43:18 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 497E8604BA\n\tfor ;\n\tFri, 8 Apr 2022 13:43:17 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CCA74486;\n\tFri, 8 Apr 2022 13:43:16 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649418198;\n\tbh=PyOV3OW5kXmW+X/ugZ+Equ69hZGcjzwWGedKwEUhSos=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=RkcKwIVdL9V3H+tBYHONFaQEPV2GlqOjI1U0qvqUj37J2eYFQMo/iBDqxC6MN15Fk\n\tVDYStNSsM8BMfhM+5irkqMnKaUalynsOmRB57alW4cnndfQckvI2mQKA7u7HCkyiIa\n\tH1jrjIpkgCnA5t0l2EL/9g7VmZjYXD1g0gIsm2s3uAZjAt9GkbSp0gMRRt5cBYuSMx\n\tU/pMcYZIOPwu/s5hwKcAr+H1d3i0M18ipbOGGMWJYbFkK4MEFg7jtLoRV28qxwWm9W\n\t4r2deOz6fo5D21CibN/ZtB1ud+8J3HdkILSqYs1W30tizFDtqJivOadMbX23emFyFN\n\tH7GeChesBeBKg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1649418196;\n\tbh=PyOV3OW5kXmW+X/ugZ+Equ69hZGcjzwWGedKwEUhSos=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=hV37iIG2ppwPU0s8L+wkxtRi/PrqCaL/IoAhjTkqxSjUD8NJqrAgaCA2sYZ2CGWGr\n\t44G1pPGcQGzDU8+0YqXGau/PSnNTROWMdbDHIxct3QdSGZXIAIX0jnTsJlcWdnLCxu\n\tX4Z7U8KVgSfHQlZEhaBbNbCR/xiuhkH81Dxd+Bo0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"hV37iIG2\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220408105439.144182-3-umang.jain@ideasonboard.com>","References":"<20220408105439.144182-1-umang.jain@ideasonboard.com>\n\t<20220408105439.144182-3-umang.jain@ideasonboard.com>","To":"Umang Jain ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 08 Apr 2022 12:43:14 +0100","Message-ID":"<164941819412.3968198.18148847026865390158@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: vimc: Synchronise parameter\n\tbuffer ops naming","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":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":22673,"web_url":"https://patchwork.libcamera.org/comment/22673/","msgid":"<17b76c96-ab86-2f8a-b219-4af43a5438ad@ideasonboard.com>","date":"2022-04-08T12:20:01","subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: vimc: Synchronise parameter\n\tbuffer ops naming","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Kieran,\n\nOn 4/8/22 17:13, Kieran Bingham wrote:\n> Quoting Umang Jain via libcamera-devel (2022-04-08 11:54:39)\n>> Synchronise the names of the operations with respect to parameters\n>> buffer with the names used in other IPA interfaces.\n>>\n> The VIMC pipeline handler doesn't yet use an ISP, so I was curious at\n> what buffers this would represent - but actually I think it's fine to\n> keep it aligned at least.\n\n\nThese are mock buffers. They enable IPA IPC tests we have in tests/.\n\nAlso look at,\n\nhttps://git.linuxtv.org/libcamera.git/commit/?id=3c5732d04a879d71d928e200d399c8d61b0da27a\n\nhttps://git.linuxtv.org/libcamera.git/commit/?id=c2437e8cdefc7ae7efbfbd8662d822a02133084d\n\n>\n> I expect we could also use the VIMC pipeline handler with any GPU based\n> ISP we might develop so I expect it will get more alignment with that\n> development too.\n\n\nYes.\n\n>\n> So for this - I think it's fine.\n>\n>\n> Reviewed-by: Kieran Bingham \n\n\nThanks!\n\n>\n>> Signed-off-by: Umang Jain \n>> ---\n>> include/libcamera/ipa/vimc.mojom | 4 ++--\n>> src/ipa/vimc/vimc.cpp | 6 +++---\n>> src/libcamera/pipeline/vimc/vimc.cpp | 8 ++++----\n>> 3 files changed, 9 insertions(+), 9 deletions(-)\n>>\n>> diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom\n>> index cdc03ffb..718b9674 100644\n>> --- a/include/libcamera/ipa/vimc.mojom\n>> +++ b/include/libcamera/ipa/vimc.mojom\n>> @@ -37,9 +37,9 @@ interface IPAVimcInterface {\n>> * interface functions that mimick how other pipeline handlers typically\n>> * handle parameters at runtime.\n>> */\n>> - [async] fillParams(uint32 frame, uint32 bufferId);\n>> + [async] fillParamsBuffer(uint32 frame, uint32 bufferId);\n>> };\n>> \n>> interface IPAVimcEventInterface {\n>> - paramsFilled(uint32 bufferId);\n>> + paramsBufferReady(uint32 bufferId);\n>> };\n>> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\n>> index a62e72b0..85afb279 100644\n>> --- a/src/ipa/vimc/vimc.cpp\n>> +++ b/src/ipa/vimc/vimc.cpp\n>> @@ -44,7 +44,7 @@ public:\n>> void unmapBuffers(const std::vector &ids) override;\n>> \n>> void queueRequest(uint32_t frame, const ControlList &controls) override;\n>> - void fillParams(uint32_t frame, uint32_t bufferId) override;\n>> + void fillParamsBuffer(uint32_t frame, uint32_t bufferId) override;\n>> \n>> private:\n>> void initTrace();\n>> @@ -134,7 +134,7 @@ void IPAVimc::queueRequest([[maybe_unused]] uint32_t frame,\n>> {\n>> }\n>> \n>> -void IPAVimc::fillParams([[maybe_unused]] uint32_t frame, uint32_t bufferId)\n>> +void IPAVimc::fillParamsBuffer([[maybe_unused]] uint32_t frame, uint32_t bufferId)\n>> {\n>> auto it = buffers_.find(bufferId);\n>> if (it == buffers_.end()) {\n>> @@ -142,7 +142,7 @@ void IPAVimc::fillParams([[maybe_unused]] uint32_t frame, uint32_t bufferId)\n>> return;\n>> }\n>> \n>> - paramsFilled.emit(bufferId);\n>> + paramsBufferReady.emit(bufferId);\n>> }\n>> \n>> void IPAVimc::initTrace()\n>> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n>> index 69b83d07..fff95a34 100644\n>> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n>> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n>> @@ -53,7 +53,7 @@ public:\n>> int init();\n>> int allocateMockIPABuffers();\n>> void bufferReady(FrameBuffer *buffer);\n>> - void paramsFilled(unsigned int id);\n>> + void paramsBufferReady(unsigned int id);\n>> \n>> MediaDevice *media_;\n>> std::unique_ptr sensor_;\n>> @@ -467,7 +467,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n>> return false;\n>> }\n>> \n>> - data->ipa_->paramsFilled.connect(data.get(), &VimcCameraData::paramsFilled);\n>> + data->ipa_->paramsBufferReady.connect(data.get(), &VimcCameraData::paramsBufferReady);\n>> \n>> std::string conf = data->ipa_->configurationFile(\"vimc.conf\");\n>> data->ipa_->init(IPASettings{ conf, data->sensor_->model() });\n>> @@ -589,7 +589,7 @@ void VimcCameraData::bufferReady(FrameBuffer *buffer)\n>> pipe->completeBuffer(request, buffer);\n>> pipe->completeRequest(request);\n>> \n>> - ipa_->fillParams(request->sequence(), mockIPABufs_[0]->cookie());\n>> + ipa_->fillParamsBuffer(request->sequence(), mockIPABufs_[0]->cookie());\n>> }\n>> \n>> int VimcCameraData::allocateMockIPABuffers()\n>> @@ -607,7 +607,7 @@ int VimcCameraData::allocateMockIPABuffers()\n>> return video_->exportBuffers(kBufCount, &mockIPABufs_);\n>> }\n>> \n>> -void VimcCameraData::paramsFilled([[maybe_unused]] unsigned int id)\n>> +void VimcCameraData::paramsBufferReady([[maybe_unused]] unsigned int id)\n>> {\n>> }\n>> \n>> -- \n>> 2.31.0\n>>","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 12393C3256\n\tfor ;\n\tFri, 8 Apr 2022 12:20:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5FD8F65642;\n\tFri, 8 Apr 2022 14:20:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3FECD604BA\n\tfor ;\n\tFri, 8 Apr 2022 14:20:08 +0200 (CEST)","from [192.168.1.110] (unknown [27.57.186.178])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 37C8B486;\n\tFri, 8 Apr 2022 14:20:07 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1649420409;\n\tbh=EPetqMQ3S5POT1LvpKro/AzsGUHOXIKFr61AeycVyvY=;\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:\n\tFrom;\n\tb=x8RuOzsczwS50XPkxLVLBMUx55FDP4mUrjbGxg7FJSjZqTzJ57iudMIa+egOQ+IDO\n\t8Ax1lDW5Aw07ZmXqPsCpFfM9PuyYV2Wxvm0dmv0odQz6rZ8YviHHKQAKQHN9DF+wAA\n\te+jIhrzEqSbKSB9HjUvukFsLf7BDqxafwLUH+sGt04NmmXN7PKpBZ3rH3GW1HkDoBl\n\t4VSEjH1lSRNRj0Cf/1Pldq9xC5B+DB5gBux9oieGV4BVOH9fU0UK69svnQsa/Ix5Wo\n\tFdRg/lhHoGWTnjcpdAC2WxusuXjDVPp5yvoxBMs+ucV4DIOSryOyAEdGHQGIa0ddjt\n\tLQ9lmiICFcnrA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1649420407;\n\tbh=EPetqMQ3S5POT1LvpKro/AzsGUHOXIKFr61AeycVyvY=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=LqUQG/xg291mS+U/67YhdUnR7fMAiN+N6xi+sXBfsiGbYIDpuvVeGMFghtzUrZEqL\n\t6hlm4o4e1nXtnvoHN62DE+HmzWUsnR8bUI+ovQ2ing9VREjsqEFzVGmNwdP0EjaDwy\n\tOqK1NZpOn34NpkjFUaPN6dadbVfxh4VcB+voDXi0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"LqUQG/xg\"; dkim-atps=neutral","Message-ID":"<17b76c96-ab86-2f8a-b219-4af43a5438ad@ideasonboard.com>","Date":"Fri, 8 Apr 2022 17:50:01 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220408105439.144182-1-umang.jain@ideasonboard.com>\n\t<20220408105439.144182-3-umang.jain@ideasonboard.com>\n\t<164941819412.3968198.18148847026865390158@Monstersaurus>","In-Reply-To":"<164941819412.3968198.18148847026865390158@Monstersaurus>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH 2/2] ipa: vimc: Synchronise parameter\n\tbuffer ops naming","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":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]