[{"id":16529,"web_url":"https://patchwork.libcamera.org/comment/16529/","msgid":"<6c07405a-b23e-c9db-c730-85589a520ce4@ideasonboard.com>","date":"2021-04-23T09:02:15","subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi JM,\n\nOn 23/04/2021 07:39, Jean-Michel Hautbois wrote:\n> Simplify name-spacing of the RKISP1 components by placing it in the\n> ipa::rkisp1 namespace directly.\n> \n\nGiven that I did the same for the IPU3 - Perhaps I'm biased, but I think\nthis is better ;-D\n\n> Signed-off-by: Jean-Michel Hautbois \n\nReviewed-by: Kieran Bingham \n\n\n\n> ---\n> src/ipa/rkisp1/rkisp1.cpp | 28 ++++++++++++++++------------\n> 1 file changed, 16 insertions(+), 12 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 8a57b080..6d45673c 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -28,7 +28,9 @@ namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(IPARkISP1)\n> \n> -class IPARkISP1 : public ipa::rkisp1::IPARkISP1Interface\n> +namespace ipa::rkisp1 {\n> +\n> +class IPARkISP1 : public IPARkISP1Interface\n> {\n> public:\n> \tint init(unsigned int hwRevision) override;\n> @@ -40,7 +42,7 @@ public:\n> \t\t const std::map &entityControls) override;\n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n> -\tvoid processEvent(const ipa::rkisp1::RkISP1Event &event) override;\n> +\tvoid processEvent(const RkISP1Event &event) override;\n> \n> private:\n> \tvoid queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n> @@ -171,10 +173,10 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n> \t}\n> }\n> \n> -void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event &event)\n> +void IPARkISP1::processEvent(const RkISP1Event &event)\n> {\n> \tswitch (event.op) {\n> -\tcase ipa::rkisp1::EventSignalStatBuffer: {\n> +\tcase EventSignalStatBuffer: {\n> \t\tunsigned int frame = event.frame;\n> \t\tunsigned int bufferId = event.bufferId;\n> \n> @@ -184,7 +186,7 @@ void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event &event)\n> \t\tupdateStatistics(frame, stats);\n> \t\tbreak;\n> \t}\n> -\tcase ipa::rkisp1::EventQueueRequest: {\n> +\tcase EventQueueRequest: {\n> \t\tunsigned int frame = event.frame;\n> \t\tunsigned int bufferId = event.bufferId;\n> \n> @@ -215,8 +217,8 @@ void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n> \t\tparams->module_en_update = RKISP1_CIF_ISP_MODULE_AEC;\n> \t}\n> \n> -\tipa::rkisp1::RkISP1Action op;\n> -\top.op = ipa::rkisp1::ActionParamFilled;\n> +\tRkISP1Action op;\n> +\top.op = ActionParamFilled;\n> \n> \tqueueFrameAction.emit(frame, op);\n> }\n> @@ -268,8 +270,8 @@ void IPARkISP1::updateStatistics(unsigned int frame,\n> \n> void IPARkISP1::setControls(unsigned int frame)\n> {\n> -\tipa::rkisp1::RkISP1Action op;\n> -\top.op = ipa::rkisp1::ActionV4L2Set;\n> +\tRkISP1Action op;\n> +\top.op = ActionV4L2Set;\n> \n> \tControlList ctrls(ctrls_);\n> \tctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure_));\n> @@ -286,13 +288,15 @@ void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)\n> \tif (aeState)\n> \t\tctrls.set(controls::AeLocked, aeState == 2);\n> \n> -\tipa::rkisp1::RkISP1Action op;\n> -\top.op = ipa::rkisp1::ActionMetadata;\n> +\tRkISP1Action op;\n> +\top.op = ActionMetadata;\n> \top.controls = ctrls;\n> \n> \tqueueFrameAction.emit(frame, op);\n> }\n> \n> +} /* namespace ipa::rkisp1 */\n> +\n> /*\n> * External IPA module interface\n> */\n> @@ -307,7 +311,7 @@ const struct IPAModuleInfo ipaModuleInfo = {\n> \n> IPAInterface *ipaCreate()\n> {\n> -\treturn new IPARkISP1();\n> +\treturn new ipa::rkisp1::IPARkISP1();\n> }\n> }\n> \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 4F18DBDB1A\n\tfor ;\n\tFri, 23 Apr 2021 09:02:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8A28B68879;\n\tFri, 23 Apr 2021 11:02:20 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9523C68875\n\tfor ;\n\tFri, 23 Apr 2021 11:02:19 +0200 (CEST)","from [192.168.0.20]\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 7736E332;\n\tFri, 23 Apr 2021 11:02:18 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"tCP1Zc/v\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1619168538;\n\tbh=RjiZIpGvGqQVrcfddj+2CgzcTQAzAdRkhezMi1iNXwA=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=tCP1Zc/vtcS+jzNCn0RJIk1FZ/LgDYt5f93DlG8eToWm/pbGxo+Mw2puKew6Xhxf4\n\tewACY65e1hRBZY7JloHUmhtxJGhqDPxf1fMUIF4mmMxuOLlzoLI/GauciA+7NxvbI2\n\tDD7QE4eIORC1vAy2PJPyEOwLQ4jZ7fXcXPshgHE4=","To":"Jean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210423063919.26273-1-jeanmichel.hautbois@ideasonboard.com>","From":"Kieran Bingham ","Organization":"Ideas on Board","Message-ID":"<6c07405a-b23e-c9db-c730-85589a520ce4@ideasonboard.com>","Date":"Fri, 23 Apr 2021 10:02:15 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"<20210423063919.26273-1-jeanmichel.hautbois@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","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","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16530,"web_url":"https://patchwork.libcamera.org/comment/16530/","msgid":"","date":"2021-04-23T09:07:29","subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","submitter":{"id":74,"url":"https://patchwork.libcamera.org/api/people/74/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@gmail.com"},"content":"Hi Kieran,\n\nOn 23/04/2021 11:02, Kieran Bingham wrote:\n> Hi JM,\n> \n> On 23/04/2021 07:39, Jean-Michel Hautbois wrote:\n>> Simplify name-spacing of the RKISP1 components by placing it in the\n>> ipa::rkisp1 namespace directly.\n>>\n> \n> Given that I did the same for the IPU3 - Perhaps I'm biased, but I think\n> this is better ;-D\n\nNot that I copy/pasted your commit ^_^\n\n>> Signed-off-by: Jean-Michel Hautbois \n> \n> Reviewed-by: Kieran Bingham \n> \n> \n> \n>> ---\n>> src/ipa/rkisp1/rkisp1.cpp | 28 ++++++++++++++++------------\n>> 1 file changed, 16 insertions(+), 12 deletions(-)\n>>\n>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>> index 8a57b080..6d45673c 100644\n>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>> @@ -28,7 +28,9 @@ namespace libcamera {\n>> \n>> LOG_DEFINE_CATEGORY(IPARkISP1)\n>> \n>> -class IPARkISP1 : public ipa::rkisp1::IPARkISP1Interface\n>> +namespace ipa::rkisp1 {\n>> +\n>> +class IPARkISP1 : public IPARkISP1Interface\n>> {\n>> public:\n>> \tint init(unsigned int hwRevision) override;\n>> @@ -40,7 +42,7 @@ public:\n>> \t\t const std::map &entityControls) override;\n>> \tvoid mapBuffers(const std::vector &buffers) override;\n>> \tvoid unmapBuffers(const std::vector &ids) override;\n>> -\tvoid processEvent(const ipa::rkisp1::RkISP1Event &event) override;\n>> +\tvoid processEvent(const RkISP1Event &event) override;\n>> \n>> private:\n>> \tvoid queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n>> @@ -171,10 +173,10 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n>> \t}\n>> }\n>> \n>> -void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event &event)\n>> +void IPARkISP1::processEvent(const RkISP1Event &event)\n>> {\n>> \tswitch (event.op) {\n>> -\tcase ipa::rkisp1::EventSignalStatBuffer: {\n>> +\tcase EventSignalStatBuffer: {\n>> \t\tunsigned int frame = event.frame;\n>> \t\tunsigned int bufferId = event.bufferId;\n>> \n>> @@ -184,7 +186,7 @@ void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event &event)\n>> \t\tupdateStatistics(frame, stats);\n>> \t\tbreak;\n>> \t}\n>> -\tcase ipa::rkisp1::EventQueueRequest: {\n>> +\tcase EventQueueRequest: {\n>> \t\tunsigned int frame = event.frame;\n>> \t\tunsigned int bufferId = event.bufferId;\n>> \n>> @@ -215,8 +217,8 @@ void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n>> \t\tparams->module_en_update = RKISP1_CIF_ISP_MODULE_AEC;\n>> \t}\n>> \n>> -\tipa::rkisp1::RkISP1Action op;\n>> -\top.op = ipa::rkisp1::ActionParamFilled;\n>> +\tRkISP1Action op;\n>> +\top.op = ActionParamFilled;\n>> \n>> \tqueueFrameAction.emit(frame, op);\n>> }\n>> @@ -268,8 +270,8 @@ void IPARkISP1::updateStatistics(unsigned int frame,\n>> \n>> void IPARkISP1::setControls(unsigned int frame)\n>> {\n>> -\tipa::rkisp1::RkISP1Action op;\n>> -\top.op = ipa::rkisp1::ActionV4L2Set;\n>> +\tRkISP1Action op;\n>> +\top.op = ActionV4L2Set;\n>> \n>> \tControlList ctrls(ctrls_);\n>> \tctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure_));\n>> @@ -286,13 +288,15 @@ void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)\n>> \tif (aeState)\n>> \t\tctrls.set(controls::AeLocked, aeState == 2);\n>> \n>> -\tipa::rkisp1::RkISP1Action op;\n>> -\top.op = ipa::rkisp1::ActionMetadata;\n>> +\tRkISP1Action op;\n>> +\top.op = ActionMetadata;\n>> \top.controls = ctrls;\n>> \n>> \tqueueFrameAction.emit(frame, op);\n>> }\n>> \n>> +} /* namespace ipa::rkisp1 */\n>> +\n>> /*\n>> * External IPA module interface\n>> */\n>> @@ -307,7 +311,7 @@ const struct IPAModuleInfo ipaModuleInfo = {\n>> \n>> IPAInterface *ipaCreate()\n>> {\n>> -\treturn new IPARkISP1();\n>> +\treturn new ipa::rkisp1::IPARkISP1();\n>> }\n>> }\n>> \n>>\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 66D0FBDB15\n\tfor ;\n\tFri, 23 Apr 2021 09:07:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B520068879;\n\tFri, 23 Apr 2021 11:07:31 +0200 (CEST)","from mail-wm1-x330.google.com (mail-wm1-x330.google.com\n\t[IPv6:2a00:1450:4864:20::330])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CDFB568875\n\tfor ;\n\tFri, 23 Apr 2021 11:07:30 +0200 (CEST)","by mail-wm1-x330.google.com with SMTP id u187so6093039wmb.0\n\tfor ;\n\tFri, 23 Apr 2021 02:07:30 -0700 (PDT)","from ?IPv6:2a01:e0a:169:7140:f55f:87fa:7523:c557?\n\t([2a01:e0a:169:7140:f55f:87fa:7523:c557])\n\tby smtp.gmail.com with ESMTPSA id\n\th10sm7986132wrt.40.2021.04.23.02.07.29\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tFri, 23 Apr 2021 02:07:29 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"H7EGKaPU\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=subject:to:references:from:message-id:date:user-agent:mime-version\n\t:in-reply-to:content-language:content-transfer-encoding;\n\tbh=6tEOiiuiKsBiYTSOwf1mPhKGBq+m9oZijIqFELryfMs=;\n\tb=H7EGKaPU7b5YClaLgWdWlad7AfQTI8s/DU9J0TXQCcwLhxlRT8ko56c1nsfK1XmzE4\n\tWZ3ePGCcdQCtgaezdxAFHQf6RkrB+mGvdDs+8g22wAoyP/MsjM5lQjFdJcyGQeR4RRwA\n\tCo4RRYPnaPwg5wmHi1KcDP4wJBuIL3OdXQ2CZcI4jtfgt8eKRJjRhLgJXUrMCajVH9EB\n\ttqyGIgJVDqltviCdkQ3Lt8tGMUUApqlTRJ2iztBSNAKoJNqA6ZJKFhJ+ppkdcLPQ1zcr\n\tEGayyHc45uOExqRB6a9Dldw3tZPXqCKcxIqPRs062w8FJQdYKWGedAUJdZt5lnt12pbP\n\tY9vg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:to:references:from:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=6tEOiiuiKsBiYTSOwf1mPhKGBq+m9oZijIqFELryfMs=;\n\tb=Al12fUuY9jftItfKhqXqIQ93jGRIzjBX89lrnWgrd9tIcZiK5oT7jmbkYy4h+6xwHg\n\tSkrxjOO29PgiWz4aaIcAHgFHureQdGQhStCPgxoSGEQ5tqVT3KykzWzXHEDzJ//E+CFt\n\t+jqQW3Ijd15Bb7m3UoYvjx4W66VtFu26JlXZBwDxkfrhe9UUXL1OeG2Zjnh1J217kpD4\n\tQQDD366TtAOG+AqmbNjrsBs/mXxP153KLjz7NsemCj3wQizVh1sOhsLi/aWuCn53yGyV\n\tUE59IViriLvjkr6S8yH55UkF8+XBPgI3mCPxmtnY86QxJF3Yq3khz+qAYCRgYrfeXYQb\n\tbuGg==","X-Gm-Message-State":"AOAM531RSqEELzbE5JafipcZusTfQMo6whOoVikbhoTIzY3/OQGYXpni\n\t2Oj70RUOXggmAVgO5Km27dYxhaRNmXj2sw==","X-Google-Smtp-Source":"ABdhPJyYOVb2LJzv43Q1XPn9K6iYW6JTJ/vjzp648R3YY6yknmkrMypHZZ0EPvOaP/BlEcqlby8UNw==","X-Received":"by 2002:a1c:dc41:: with SMTP id t62mr4566806wmg.14.1619168850098;\n\tFri, 23 Apr 2021 02:07:30 -0700 (PDT)","To":"kieran.bingham@ideasonboard.com,\n\tJean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210423063919.26273-1-jeanmichel.hautbois@ideasonboard.com>\n\t<6c07405a-b23e-c9db-c730-85589a520ce4@ideasonboard.com>","From":"Jean-Michel Hautbois ","Message-ID":"","Date":"Fri, 23 Apr 2021 11:07:29 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"<6c07405a-b23e-c9db-c730-85589a520ce4@ideasonboard.com>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16534,"web_url":"https://patchwork.libcamera.org/comment/16534/","msgid":"","date":"2021-04-25T22:03:36","subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Apr 23, 2021 at 10:02:15AM +0100, Kieran Bingham wrote:\n> Hi JM,\n> \n> On 23/04/2021 07:39, Jean-Michel Hautbois wrote:\n> > Simplify name-spacing of the RKISP1 components by placing it in the\n> > ipa::rkisp1 namespace directly.\n> > \n> \n> Given that I did the same for the IPU3 - Perhaps I'm biased, but I think\n> this is better ;-D\n> \n> > Signed-off-by: Jean-Michel Hautbois \n> \n> Reviewed-by: Kieran Bingham \n\nReviewed-by: Laurent Pinchart \n\nWe should consider renaming IPARkISP1Interface to Interface though, and\nIPARkISP1 to... Implementation ?\n\n> > ---\n> > src/ipa/rkisp1/rkisp1.cpp | 28 ++++++++++++++++------------\n> > 1 file changed, 16 insertions(+), 12 deletions(-)\n> > \n> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > index 8a57b080..6d45673c 100644\n> > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > @@ -28,7 +28,9 @@ namespace libcamera {\n> > \n> > LOG_DEFINE_CATEGORY(IPARkISP1)\n> > \n> > -class IPARkISP1 : public ipa::rkisp1::IPARkISP1Interface\n> > +namespace ipa::rkisp1 {\n> > +\n> > +class IPARkISP1 : public IPARkISP1Interface\n> > {\n> > public:\n> > \tint init(unsigned int hwRevision) override;\n> > @@ -40,7 +42,7 @@ public:\n> > \t\t const std::map &entityControls) override;\n> > \tvoid mapBuffers(const std::vector &buffers) override;\n> > \tvoid unmapBuffers(const std::vector &ids) override;\n> > -\tvoid processEvent(const ipa::rkisp1::RkISP1Event &event) override;\n> > +\tvoid processEvent(const RkISP1Event &event) override;\n> > \n> > private:\n> > \tvoid queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n> > @@ -171,10 +173,10 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n> > \t}\n> > }\n> > \n> > -void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event &event)\n> > +void IPARkISP1::processEvent(const RkISP1Event &event)\n> > {\n> > \tswitch (event.op) {\n> > -\tcase ipa::rkisp1::EventSignalStatBuffer: {\n> > +\tcase EventSignalStatBuffer: {\n> > \t\tunsigned int frame = event.frame;\n> > \t\tunsigned int bufferId = event.bufferId;\n> > \n> > @@ -184,7 +186,7 @@ void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event &event)\n> > \t\tupdateStatistics(frame, stats);\n> > \t\tbreak;\n> > \t}\n> > -\tcase ipa::rkisp1::EventQueueRequest: {\n> > +\tcase EventQueueRequest: {\n> > \t\tunsigned int frame = event.frame;\n> > \t\tunsigned int bufferId = event.bufferId;\n> > \n> > @@ -215,8 +217,8 @@ void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n> > \t\tparams->module_en_update = RKISP1_CIF_ISP_MODULE_AEC;\n> > \t}\n> > \n> > -\tipa::rkisp1::RkISP1Action op;\n> > -\top.op = ipa::rkisp1::ActionParamFilled;\n> > +\tRkISP1Action op;\n> > +\top.op = ActionParamFilled;\n> > \n> > \tqueueFrameAction.emit(frame, op);\n> > }\n> > @@ -268,8 +270,8 @@ void IPARkISP1::updateStatistics(unsigned int frame,\n> > \n> > void IPARkISP1::setControls(unsigned int frame)\n> > {\n> > -\tipa::rkisp1::RkISP1Action op;\n> > -\top.op = ipa::rkisp1::ActionV4L2Set;\n> > +\tRkISP1Action op;\n> > +\top.op = ActionV4L2Set;\n> > \n> > \tControlList ctrls(ctrls_);\n> > \tctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure_));\n> > @@ -286,13 +288,15 @@ void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)\n> > \tif (aeState)\n> > \t\tctrls.set(controls::AeLocked, aeState == 2);\n> > \n> > -\tipa::rkisp1::RkISP1Action op;\n> > -\top.op = ipa::rkisp1::ActionMetadata;\n> > +\tRkISP1Action op;\n> > +\top.op = ActionMetadata;\n> > \top.controls = ctrls;\n> > \n> > \tqueueFrameAction.emit(frame, op);\n> > }\n> > \n> > +} /* namespace ipa::rkisp1 */\n> > +\n> > /*\n> > * External IPA module interface\n> > */\n> > @@ -307,7 +311,7 @@ const struct IPAModuleInfo ipaModuleInfo = {\n> > \n> > IPAInterface *ipaCreate()\n> > {\n> > -\treturn new IPARkISP1();\n> > +\treturn new ipa::rkisp1::IPARkISP1();\n> > }\n> > }\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 D9F77BDB97\n\tfor ;\n\tSun, 25 Apr 2021 22:03:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 203B76887C;\n\tMon, 26 Apr 2021 00:03:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C658F602D1\n\tfor ;\n\tMon, 26 Apr 2021 00:03:41 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 380244FB;\n\tMon, 26 Apr 2021 00:03:41 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"bTGp/+5v\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1619388221;\n\tbh=/TTZhvvlZFSr9MouUCCU5M+rTExF9CBnlKIshajPncE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bTGp/+5v5FzuVJZkQd827i4I5ekKBPS3h6EyWfmqn+lmYtTwZBVtmVEpILhUhcx4+\n\t0LLwCrWgA46FSUSenks9sSwPBSsE6elOkDxSoPuHI+jZVMtWbF/ew6+yoNYE5BzNXd\n\tkMpAWOuQyou4zH9quq7GMUIFVoT+sP/I1bx9FSe4=","Date":"Mon, 26 Apr 2021 01:03:36 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210423063919.26273-1-jeanmichel.hautbois@ideasonboard.com>\n\t<6c07405a-b23e-c9db-c730-85589a520ce4@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<6c07405a-b23e-c9db-c730-85589a520ce4@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16566,"web_url":"https://patchwork.libcamera.org/comment/16566/","msgid":"<583696f1-7d40-04ee-2662-f2461d3cb458@ideasonboard.com>","date":"2021-04-26T05:49:11","subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 26/04/2021 00:03, Laurent Pinchart wrote:\n> On Fri, Apr 23, 2021 at 10:02:15AM +0100, Kieran Bingham wrote:\n>> Hi JM,\n>>\n>> On 23/04/2021 07:39, Jean-Michel Hautbois wrote:\n>>> Simplify name-spacing of the RKISP1 components by placing it in the\n>>> ipa::rkisp1 namespace directly.\n>>>\n>>\n>> Given that I did the same for the IPU3 - Perhaps I'm biased, but I think\n>> this is better ;-D\n>>\n>>> Signed-off-by: Jean-Michel Hautbois \n>>\n>> Reviewed-by: Kieran Bingham \n> \n> Reviewed-by: Laurent Pinchart \n> \n> We should consider renaming IPARkISP1Interface to Interface though, and\n> IPARkISP1 to... Implementation ?\n\nThanks.\nDo you mean having the following ?\n\n-class IPARkISP1 : public IPARkISP1Interface\n+class Implementation : public Interface\n\nIt would be the same for all IPAs then ?\n\nJM\n>>> ---\n>>> src/ipa/rkisp1/rkisp1.cpp | 28 ++++++++++++++++------------\n>>> 1 file changed, 16 insertions(+), 12 deletions(-)\n>>>\n>>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>>> index 8a57b080..6d45673c 100644\n>>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>>> @@ -28,7 +28,9 @@ namespace libcamera {\n>>> \n>>> LOG_DEFINE_CATEGORY(IPARkISP1)\n>>> \n>>> -class IPARkISP1 : public ipa::rkisp1::IPARkISP1Interface\n>>> +namespace ipa::rkisp1 {\n>>> +\n>>> +class IPARkISP1 : public IPARkISP1Interface\n>>> {\n>>> public:\n>>> \tint init(unsigned int hwRevision) override;\n>>> @@ -40,7 +42,7 @@ public:\n>>> \t\t const std::map &entityControls) override;\n>>> \tvoid mapBuffers(const std::vector &buffers) override;\n>>> \tvoid unmapBuffers(const std::vector &ids) override;\n>>> -\tvoid processEvent(const ipa::rkisp1::RkISP1Event &event) override;\n>>> +\tvoid processEvent(const RkISP1Event &event) override;\n>>> \n>>> private:\n>>> \tvoid queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n>>> @@ -171,10 +173,10 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n>>> \t}\n>>> }\n>>> \n>>> -void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event &event)\n>>> +void IPARkISP1::processEvent(const RkISP1Event &event)\n>>> {\n>>> \tswitch (event.op) {\n>>> -\tcase ipa::rkisp1::EventSignalStatBuffer: {\n>>> +\tcase EventSignalStatBuffer: {\n>>> \t\tunsigned int frame = event.frame;\n>>> \t\tunsigned int bufferId = event.bufferId;\n>>> \n>>> @@ -184,7 +186,7 @@ void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event &event)\n>>> \t\tupdateStatistics(frame, stats);\n>>> \t\tbreak;\n>>> \t}\n>>> -\tcase ipa::rkisp1::EventQueueRequest: {\n>>> +\tcase EventQueueRequest: {\n>>> \t\tunsigned int frame = event.frame;\n>>> \t\tunsigned int bufferId = event.bufferId;\n>>> \n>>> @@ -215,8 +217,8 @@ void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n>>> \t\tparams->module_en_update = RKISP1_CIF_ISP_MODULE_AEC;\n>>> \t}\n>>> \n>>> -\tipa::rkisp1::RkISP1Action op;\n>>> -\top.op = ipa::rkisp1::ActionParamFilled;\n>>> +\tRkISP1Action op;\n>>> +\top.op = ActionParamFilled;\n>>> \n>>> \tqueueFrameAction.emit(frame, op);\n>>> }\n>>> @@ -268,8 +270,8 @@ void IPARkISP1::updateStatistics(unsigned int frame,\n>>> \n>>> void IPARkISP1::setControls(unsigned int frame)\n>>> {\n>>> -\tipa::rkisp1::RkISP1Action op;\n>>> -\top.op = ipa::rkisp1::ActionV4L2Set;\n>>> +\tRkISP1Action op;\n>>> +\top.op = ActionV4L2Set;\n>>> \n>>> \tControlList ctrls(ctrls_);\n>>> \tctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure_));\n>>> @@ -286,13 +288,15 @@ void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)\n>>> \tif (aeState)\n>>> \t\tctrls.set(controls::AeLocked, aeState == 2);\n>>> \n>>> -\tipa::rkisp1::RkISP1Action op;\n>>> -\top.op = ipa::rkisp1::ActionMetadata;\n>>> +\tRkISP1Action op;\n>>> +\top.op = ActionMetadata;\n>>> \top.controls = ctrls;\n>>> \n>>> \tqueueFrameAction.emit(frame, op);\n>>> }\n>>> \n>>> +} /* namespace ipa::rkisp1 */\n>>> +\n>>> /*\n>>> * External IPA module interface\n>>> */\n>>> @@ -307,7 +311,7 @@ const struct IPAModuleInfo ipaModuleInfo = {\n>>> \n>>> IPAInterface *ipaCreate()\n>>> {\n>>> -\treturn new IPARkISP1();\n>>> +\treturn new ipa::rkisp1::IPARkISP1();\n>>> }\n>>> }\n>>> \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 36C69BDC95\n\tfor ;\n\tMon, 26 Apr 2021 05:49:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7C8E6688AF;\n\tMon, 26 Apr 2021 07:49:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8E0E9688A7\n\tfor ;\n\tMon, 26 Apr 2021 07:49:12 +0200 (CEST)","from [IPv6:2a01:e0a:169:7140:5ef:55ab:b54c:2213] (unknown\n\t[IPv6:2a01:e0a:169:7140:5ef:55ab:b54c:2213])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E7DEF4FB;\n\tMon, 26 Apr 2021 07:49:11 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"muPnm0ig\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1619416152;\n\tbh=hPNZDyHlZ22kagPpWOV4dhryC9q58cnFSO0NB00YiUk=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=muPnm0igeUcmcYAUYaNbsCDkBOy5SABmDo2WZvsB0H8W4SF7kMeqhZ3UXY5kInnYQ\n\tmI9hOc0IMeuVus1e41u8HoqzOcOKaeakusCBzxtnyteDT/MrWaTqn8R7XpqXc1+uEu\n\tJSTVtnoz79vEyfMX7NQDvxOJkDnAebEs+ZCYohA8=","To":"Laurent Pinchart ,\n\tlibcamera devel ","References":"<20210423063919.26273-1-jeanmichel.hautbois@ideasonboard.com>\n\t<6c07405a-b23e-c9db-c730-85589a520ce4@ideasonboard.com>\n\t","From":"Jean-Michel Hautbois ","Message-ID":"<583696f1-7d40-04ee-2662-f2461d3cb458@ideasonboard.com>","Date":"Mon, 26 Apr 2021 07:49:11 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16567,"web_url":"https://patchwork.libcamera.org/comment/16567/","msgid":"","date":"2021-04-26T05:57:45","subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jean-Michel,\n\nOn Mon, Apr 26, 2021 at 07:49:11AM +0200, Jean-Michel Hautbois wrote:\n> On 26/04/2021 00:03, Laurent Pinchart wrote:\n> > On Fri, Apr 23, 2021 at 10:02:15AM +0100, Kieran Bingham wrote:\n> >> On 23/04/2021 07:39, Jean-Michel Hautbois wrote:\n> >>> Simplify name-spacing of the RKISP1 components by placing it in the\n> >>> ipa::rkisp1 namespace directly.\n> >>\n> >> Given that I did the same for the IPU3 - Perhaps I'm biased, but I think\n> >> this is better ;-D\n> >>\n> >>> Signed-off-by: Jean-Michel Hautbois \n> >>\n> >> Reviewed-by: Kieran Bingham \n> > \n> > Reviewed-by: Laurent Pinchart \n> > \n> > We should consider renaming IPARkISP1Interface to Interface though, and\n> > IPARkISP1 to... Implementation ?\n> \n> Thanks.\n> Do you mean having the following ?\n> \n> -class IPARkISP1 : public IPARkISP1Interface\n> +class Implementation : public Interface\n> \n> It would be the same for all IPAs then ?\n\nThat's the idea, yes. It's just an idea though :-) It looks a bit...\nbare maybe ? But if we keep IPA and RkISP1 in the name of the classes,\nit defeats the purpose of namespaces a little bit.\n\n> >>> ---\n> >>> src/ipa/rkisp1/rkisp1.cpp | 28 ++++++++++++++++------------\n> >>> 1 file changed, 16 insertions(+), 12 deletions(-)\n> >>>\n> >>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> >>> index 8a57b080..6d45673c 100644\n> >>> --- a/src/ipa/rkisp1/rkisp1.cpp\n> >>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> >>> @@ -28,7 +28,9 @@ namespace libcamera {\n> >>> \n> >>> LOG_DEFINE_CATEGORY(IPARkISP1)\n> >>> \n> >>> -class IPARkISP1 : public ipa::rkisp1::IPARkISP1Interface\n> >>> +namespace ipa::rkisp1 {\n> >>> +\n> >>> +class IPARkISP1 : public IPARkISP1Interface\n> >>> {\n> >>> public:\n> >>> \tint init(unsigned int hwRevision) override;\n> >>> @@ -40,7 +42,7 @@ public:\n> >>> \t\t const std::map &entityControls) override;\n> >>> \tvoid mapBuffers(const std::vector &buffers) override;\n> >>> \tvoid unmapBuffers(const std::vector &ids) override;\n> >>> -\tvoid processEvent(const ipa::rkisp1::RkISP1Event &event) override;\n> >>> +\tvoid processEvent(const RkISP1Event &event) override;\n> >>> \n> >>> private:\n> >>> \tvoid queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n> >>> @@ -171,10 +173,10 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n> >>> \t}\n> >>> }\n> >>> \n> >>> -void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event &event)\n> >>> +void IPARkISP1::processEvent(const RkISP1Event &event)\n> >>> {\n> >>> \tswitch (event.op) {\n> >>> -\tcase ipa::rkisp1::EventSignalStatBuffer: {\n> >>> +\tcase EventSignalStatBuffer: {\n> >>> \t\tunsigned int frame = event.frame;\n> >>> \t\tunsigned int bufferId = event.bufferId;\n> >>> \n> >>> @@ -184,7 +186,7 @@ void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event &event)\n> >>> \t\tupdateStatistics(frame, stats);\n> >>> \t\tbreak;\n> >>> \t}\n> >>> -\tcase ipa::rkisp1::EventQueueRequest: {\n> >>> +\tcase EventQueueRequest: {\n> >>> \t\tunsigned int frame = event.frame;\n> >>> \t\tunsigned int bufferId = event.bufferId;\n> >>> \n> >>> @@ -215,8 +217,8 @@ void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n> >>> \t\tparams->module_en_update = RKISP1_CIF_ISP_MODULE_AEC;\n> >>> \t}\n> >>> \n> >>> -\tipa::rkisp1::RkISP1Action op;\n> >>> -\top.op = ipa::rkisp1::ActionParamFilled;\n> >>> +\tRkISP1Action op;\n> >>> +\top.op = ActionParamFilled;\n> >>> \n> >>> \tqueueFrameAction.emit(frame, op);\n> >>> }\n> >>> @@ -268,8 +270,8 @@ void IPARkISP1::updateStatistics(unsigned int frame,\n> >>> \n> >>> void IPARkISP1::setControls(unsigned int frame)\n> >>> {\n> >>> -\tipa::rkisp1::RkISP1Action op;\n> >>> -\top.op = ipa::rkisp1::ActionV4L2Set;\n> >>> +\tRkISP1Action op;\n> >>> +\top.op = ActionV4L2Set;\n> >>> \n> >>> \tControlList ctrls(ctrls_);\n> >>> \tctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure_));\n> >>> @@ -286,13 +288,15 @@ void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)\n> >>> \tif (aeState)\n> >>> \t\tctrls.set(controls::AeLocked, aeState == 2);\n> >>> \n> >>> -\tipa::rkisp1::RkISP1Action op;\n> >>> -\top.op = ipa::rkisp1::ActionMetadata;\n> >>> +\tRkISP1Action op;\n> >>> +\top.op = ActionMetadata;\n> >>> \top.controls = ctrls;\n> >>> \n> >>> \tqueueFrameAction.emit(frame, op);\n> >>> }\n> >>> \n> >>> +} /* namespace ipa::rkisp1 */\n> >>> +\n> >>> /*\n> >>> * External IPA module interface\n> >>> */\n> >>> @@ -307,7 +311,7 @@ const struct IPAModuleInfo ipaModuleInfo = {\n> >>> \n> >>> IPAInterface *ipaCreate()\n> >>> {\n> >>> -\treturn new IPARkISP1();\n> >>> +\treturn new ipa::rkisp1::IPARkISP1();\n> >>> }\n> >>> }\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 85B4DBDC92\n\tfor ;\n\tMon, 26 Apr 2021 05:57:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EAD56688AE;\n\tMon, 26 Apr 2021 07:57:51 +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 E8AE4602D1\n\tfor ;\n\tMon, 26 Apr 2021 07:57:50 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7103D4FB;\n\tMon, 26 Apr 2021 07:57:50 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"voE7ZcYF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1619416670;\n\tbh=MBnfcAOjsQ7J98Ys+SKObZbOGrT62zm7RPvSllXRfgA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=voE7ZcYFDr4QVjRl3Qi9BBa3RGz6cPAh2Jzmb+yDeZwbS2PGZsbPiQqlFdQpOQDnZ\n\tNi4aQmnAywQGfJs59Tp1rWaQyKGhDErnp9tOPC8m6yFjcVRI/U4h0RD9lKXHVctlwO\n\tYNjQ9OcynDNU+RJ/N/8DtYojVgdvwIeIjpJ3LYlo=","Date":"Mon, 26 Apr 2021 08:57:45 +0300","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20210423063919.26273-1-jeanmichel.hautbois@ideasonboard.com>\n\t<6c07405a-b23e-c9db-c730-85589a520ce4@ideasonboard.com>\n\t\n\t<583696f1-7d40-04ee-2662-f2461d3cb458@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<583696f1-7d40-04ee-2662-f2461d3cb458@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","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","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16568,"web_url":"https://patchwork.libcamera.org/comment/16568/","msgid":"","date":"2021-04-26T06:01:11","subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"On 26/04/2021 07:57, Laurent Pinchart wrote:\n> Hi Jean-Michel,\n> \n> On Mon, Apr 26, 2021 at 07:49:11AM +0200, Jean-Michel Hautbois wrote:\n>> On 26/04/2021 00:03, Laurent Pinchart wrote:\n>>> On Fri, Apr 23, 2021 at 10:02:15AM +0100, Kieran Bingham wrote:\n>>>> On 23/04/2021 07:39, Jean-Michel Hautbois wrote:\n>>>>> Simplify name-spacing of the RKISP1 components by placing it in the\n>>>>> ipa::rkisp1 namespace directly.\n>>>>\n>>>> Given that I did the same for the IPU3 - Perhaps I'm biased, but I think\n>>>> this is better ;-D\n>>>>\n>>>>> Signed-off-by: Jean-Michel Hautbois \n>>>>\n>>>> Reviewed-by: Kieran Bingham \n>>>\n>>> Reviewed-by: Laurent Pinchart \n>>>\n>>> We should consider renaming IPARkISP1Interface to Interface though, and\n>>> IPARkISP1 to... Implementation ?\n>>\n>> Thanks.\n>> Do you mean having the following ?\n>>\n>> -class IPARkISP1 : public IPARkISP1Interface\n>> +class Implementation : public Interface\n>>\n>> It would be the same for all IPAs then ?\n> \n> That's the idea, yes. It's just an idea though :-) It looks a bit...\n> bare maybe ? But if we keep IPA and RkISP1 in the name of the classes,\n> it defeats the purpose of namespaces a little bit.\nFor reading ease couldn't we have:\nclass RkISP1Implementation: public RkISP1Interface\n\nThat would keep the naming without the IPA :-).\nIt is \"just\" a mojom patching, right :-p ?\n\n>>>>> ---\n>>>>> src/ipa/rkisp1/rkisp1.cpp | 28 ++++++++++++++++------------\n>>>>> 1 file changed, 16 insertions(+), 12 deletions(-)\n>>>>>\n>>>>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>>>>> index 8a57b080..6d45673c 100644\n>>>>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>>>>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>>>>> @@ -28,7 +28,9 @@ namespace libcamera {\n>>>>> \n>>>>> LOG_DEFINE_CATEGORY(IPARkISP1)\n>>>>> \n>>>>> -class IPARkISP1 : public ipa::rkisp1::IPARkISP1Interface\n>>>>> +namespace ipa::rkisp1 {\n>>>>> +\n>>>>> +class IPARkISP1 : public IPARkISP1Interface\n>>>>> {\n>>>>> public:\n>>>>> \tint init(unsigned int hwRevision) override;\n>>>>> @@ -40,7 +42,7 @@ public:\n>>>>> \t\t const std::map &entityControls) override;\n>>>>> \tvoid mapBuffers(const std::vector &buffers) override;\n>>>>> \tvoid unmapBuffers(const std::vector &ids) override;\n>>>>> -\tvoid processEvent(const ipa::rkisp1::RkISP1Event &event) override;\n>>>>> +\tvoid processEvent(const RkISP1Event &event) override;\n>>>>> \n>>>>> private:\n>>>>> \tvoid queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n>>>>> @@ -171,10 +173,10 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n>>>>> \t}\n>>>>> }\n>>>>> \n>>>>> -void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event &event)\n>>>>> +void IPARkISP1::processEvent(const RkISP1Event &event)\n>>>>> {\n>>>>> \tswitch (event.op) {\n>>>>> -\tcase ipa::rkisp1::EventSignalStatBuffer: {\n>>>>> +\tcase EventSignalStatBuffer: {\n>>>>> \t\tunsigned int frame = event.frame;\n>>>>> \t\tunsigned int bufferId = event.bufferId;\n>>>>> \n>>>>> @@ -184,7 +186,7 @@ void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event &event)\n>>>>> \t\tupdateStatistics(frame, stats);\n>>>>> \t\tbreak;\n>>>>> \t}\n>>>>> -\tcase ipa::rkisp1::EventQueueRequest: {\n>>>>> +\tcase EventQueueRequest: {\n>>>>> \t\tunsigned int frame = event.frame;\n>>>>> \t\tunsigned int bufferId = event.bufferId;\n>>>>> \n>>>>> @@ -215,8 +217,8 @@ void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n>>>>> \t\tparams->module_en_update = RKISP1_CIF_ISP_MODULE_AEC;\n>>>>> \t}\n>>>>> \n>>>>> -\tipa::rkisp1::RkISP1Action op;\n>>>>> -\top.op = ipa::rkisp1::ActionParamFilled;\n>>>>> +\tRkISP1Action op;\n>>>>> +\top.op = ActionParamFilled;\n>>>>> \n>>>>> \tqueueFrameAction.emit(frame, op);\n>>>>> }\n>>>>> @@ -268,8 +270,8 @@ void IPARkISP1::updateStatistics(unsigned int frame,\n>>>>> \n>>>>> void IPARkISP1::setControls(unsigned int frame)\n>>>>> {\n>>>>> -\tipa::rkisp1::RkISP1Action op;\n>>>>> -\top.op = ipa::rkisp1::ActionV4L2Set;\n>>>>> +\tRkISP1Action op;\n>>>>> +\top.op = ActionV4L2Set;\n>>>>> \n>>>>> \tControlList ctrls(ctrls_);\n>>>>> \tctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure_));\n>>>>> @@ -286,13 +288,15 @@ void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)\n>>>>> \tif (aeState)\n>>>>> \t\tctrls.set(controls::AeLocked, aeState == 2);\n>>>>> \n>>>>> -\tipa::rkisp1::RkISP1Action op;\n>>>>> -\top.op = ipa::rkisp1::ActionMetadata;\n>>>>> +\tRkISP1Action op;\n>>>>> +\top.op = ActionMetadata;\n>>>>> \top.controls = ctrls;\n>>>>> \n>>>>> \tqueueFrameAction.emit(frame, op);\n>>>>> }\n>>>>> \n>>>>> +} /* namespace ipa::rkisp1 */\n>>>>> +\n>>>>> /*\n>>>>> * External IPA module interface\n>>>>> */\n>>>>> @@ -307,7 +311,7 @@ const struct IPAModuleInfo ipaModuleInfo = {\n>>>>> \n>>>>> IPAInterface *ipaCreate()\n>>>>> {\n>>>>> -\treturn new IPARkISP1();\n>>>>> +\treturn new ipa::rkisp1::IPARkISP1();\n>>>>> }\n>>>>> }\n>>>>> \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 96E5ABDC95\n\tfor ;\n\tMon, 26 Apr 2021 06:01:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E30F7688B4;\n\tMon, 26 Apr 2021 08:01:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BE49E688AB\n\tfor ;\n\tMon, 26 Apr 2021 08:01:12 +0200 (CEST)","from [IPv6:2a01:e0a:169:7140:5ef:55ab:b54c:2213] (unknown\n\t[IPv6:2a01:e0a:169:7140:5ef:55ab:b54c:2213])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 14E11D8B;\n\tMon, 26 Apr 2021 08:01:12 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"qdbNGZ6a\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1619416872;\n\tbh=gqgGWJYfBVf+ITz9G9GNjrULfCNZmxBuhH8GiwlSir4=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=qdbNGZ6a7neb9O6C86KCmVqIvKjwc5q1vPNgm/dq4RrhSGQkPsZ96wnzuPy8E/fOz\n\tSDkEnp0xKa7zzblXQauFven9MudZQIVo7M2nffU8jjzV7D4c3d5m6QAXcmPMMPp9C+\n\tuMwJbVnIrU/dNOBRQtEk94k0MdoVDB2FKkiPRjrQ=","To":"Laurent Pinchart ,\n\tJean-Michel Hautbois ","References":"<20210423063919.26273-1-jeanmichel.hautbois@ideasonboard.com>\n\t<6c07405a-b23e-c9db-c730-85589a520ce4@ideasonboard.com>\n\t\n\t<583696f1-7d40-04ee-2662-f2461d3cb458@ideasonboard.com>\n\t","From":"Jean-Michel Hautbois ","Message-ID":"","Date":"Mon, 26 Apr 2021 08:01:11 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","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","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16569,"web_url":"https://patchwork.libcamera.org/comment/16569/","msgid":"","date":"2021-04-26T06:42:35","subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jean-Michel,\n\nOn Mon, Apr 26, 2021 at 08:01:11AM +0200, Jean-Michel Hautbois wrote:\n> On 26/04/2021 07:57, Laurent Pinchart wrote:\n> > On Mon, Apr 26, 2021 at 07:49:11AM +0200, Jean-Michel Hautbois wrote:\n> >> On 26/04/2021 00:03, Laurent Pinchart wrote:\n> >>> On Fri, Apr 23, 2021 at 10:02:15AM +0100, Kieran Bingham wrote:\n> >>>> On 23/04/2021 07:39, Jean-Michel Hautbois wrote:\n> >>>>> Simplify name-spacing of the RKISP1 components by placing it in the\n> >>>>> ipa::rkisp1 namespace directly.\n> >>>>\n> >>>> Given that I did the same for the IPU3 - Perhaps I'm biased, but I think\n> >>>> this is better ;-D\n> >>>>\n> >>>>> Signed-off-by: Jean-Michel Hautbois \n> >>>>\n> >>>> Reviewed-by: Kieran Bingham \n> >>>\n> >>> Reviewed-by: Laurent Pinchart \n> >>>\n> >>> We should consider renaming IPARkISP1Interface to Interface though, and\n> >>> IPARkISP1 to... Implementation ?\n> >>\n> >> Thanks.\n> >> Do you mean having the following ?\n> >>\n> >> -class IPARkISP1 : public IPARkISP1Interface\n> >> +class Implementation : public Interface\n> >>\n> >> It would be the same for all IPAs then ?\n> > \n> > That's the idea, yes. It's just an idea though :-) It looks a bit...\n> > bare maybe ? But if we keep IPA and RkISP1 in the name of the classes,\n> > it defeats the purpose of namespaces a little bit.\n>\n> For reading ease couldn't we have:\n> class RkISP1Implementation: public RkISP1Interface\n> \n> That would keep the naming without the IPA :-).\n> It is \"just\" a mojom patching, right :-p ?\n\nOr\n\nclass IPAImplementation : public IPAInterface\n\n? :-)\n\n> >>>>> ---\n> >>>>> src/ipa/rkisp1/rkisp1.cpp | 28 ++++++++++++++++------------\n> >>>>> 1 file changed, 16 insertions(+), 12 deletions(-)\n> >>>>>\n> >>>>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> >>>>> index 8a57b080..6d45673c 100644\n> >>>>> --- a/src/ipa/rkisp1/rkisp1.cpp\n> >>>>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> >>>>> @@ -28,7 +28,9 @@ namespace libcamera {\n> >>>>> \n> >>>>> LOG_DEFINE_CATEGORY(IPARkISP1)\n> >>>>> \n> >>>>> -class IPARkISP1 : public ipa::rkisp1::IPARkISP1Interface\n> >>>>> +namespace ipa::rkisp1 {\n> >>>>> +\n> >>>>> +class IPARkISP1 : public IPARkISP1Interface\n> >>>>> {\n> >>>>> public:\n> >>>>> \tint init(unsigned int hwRevision) override;\n> >>>>> @@ -40,7 +42,7 @@ public:\n> >>>>> \t\t const std::map &entityControls) override;\n> >>>>> \tvoid mapBuffers(const std::vector &buffers) override;\n> >>>>> \tvoid unmapBuffers(const std::vector &ids) override;\n> >>>>> -\tvoid processEvent(const ipa::rkisp1::RkISP1Event &event) override;\n> >>>>> +\tvoid processEvent(const RkISP1Event &event) override;\n> >>>>> \n> >>>>> private:\n> >>>>> \tvoid queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n> >>>>> @@ -171,10 +173,10 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n> >>>>> \t}\n> >>>>> }\n> >>>>> \n> >>>>> -void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event &event)\n> >>>>> +void IPARkISP1::processEvent(const RkISP1Event &event)\n> >>>>> {\n> >>>>> \tswitch (event.op) {\n> >>>>> -\tcase ipa::rkisp1::EventSignalStatBuffer: {\n> >>>>> +\tcase EventSignalStatBuffer: {\n> >>>>> \t\tunsigned int frame = event.frame;\n> >>>>> \t\tunsigned int bufferId = event.bufferId;\n> >>>>> \n> >>>>> @@ -184,7 +186,7 @@ void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event &event)\n> >>>>> \t\tupdateStatistics(frame, stats);\n> >>>>> \t\tbreak;\n> >>>>> \t}\n> >>>>> -\tcase ipa::rkisp1::EventQueueRequest: {\n> >>>>> +\tcase EventQueueRequest: {\n> >>>>> \t\tunsigned int frame = event.frame;\n> >>>>> \t\tunsigned int bufferId = event.bufferId;\n> >>>>> \n> >>>>> @@ -215,8 +217,8 @@ void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n> >>>>> \t\tparams->module_en_update = RKISP1_CIF_ISP_MODULE_AEC;\n> >>>>> \t}\n> >>>>> \n> >>>>> -\tipa::rkisp1::RkISP1Action op;\n> >>>>> -\top.op = ipa::rkisp1::ActionParamFilled;\n> >>>>> +\tRkISP1Action op;\n> >>>>> +\top.op = ActionParamFilled;\n> >>>>> \n> >>>>> \tqueueFrameAction.emit(frame, op);\n> >>>>> }\n> >>>>> @@ -268,8 +270,8 @@ void IPARkISP1::updateStatistics(unsigned int frame,\n> >>>>> \n> >>>>> void IPARkISP1::setControls(unsigned int frame)\n> >>>>> {\n> >>>>> -\tipa::rkisp1::RkISP1Action op;\n> >>>>> -\top.op = ipa::rkisp1::ActionV4L2Set;\n> >>>>> +\tRkISP1Action op;\n> >>>>> +\top.op = ActionV4L2Set;\n> >>>>> \n> >>>>> \tControlList ctrls(ctrls_);\n> >>>>> \tctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure_));\n> >>>>> @@ -286,13 +288,15 @@ void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)\n> >>>>> \tif (aeState)\n> >>>>> \t\tctrls.set(controls::AeLocked, aeState == 2);\n> >>>>> \n> >>>>> -\tipa::rkisp1::RkISP1Action op;\n> >>>>> -\top.op = ipa::rkisp1::ActionMetadata;\n> >>>>> +\tRkISP1Action op;\n> >>>>> +\top.op = ActionMetadata;\n> >>>>> \top.controls = ctrls;\n> >>>>> \n> >>>>> \tqueueFrameAction.emit(frame, op);\n> >>>>> }\n> >>>>> \n> >>>>> +} /* namespace ipa::rkisp1 */\n> >>>>> +\n> >>>>> /*\n> >>>>> * External IPA module interface\n> >>>>> */\n> >>>>> @@ -307,7 +311,7 @@ const struct IPAModuleInfo ipaModuleInfo = {\n> >>>>> \n> >>>>> IPAInterface *ipaCreate()\n> >>>>> {\n> >>>>> -\treturn new IPARkISP1();\n> >>>>> +\treturn new ipa::rkisp1::IPARkISP1();\n> >>>>> }\n> >>>>> }\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 8E776BDC98\n\tfor ;\n\tMon, 26 Apr 2021 06:42:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D56B7688AC;\n\tMon, 26 Apr 2021 08:42:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BF9BD688AB\n\tfor ;\n\tMon, 26 Apr 2021 08:42:40 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 460F0D8B;\n\tMon, 26 Apr 2021 08:42:40 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"TuUfbLgC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1619419360;\n\tbh=JNrkbHlpoQ6pBQEk3WLbGKsTy0Fkfoeu9vL8XT+2dXs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TuUfbLgCmRhYrDypjY2KUTHgsRjwMFYD1loYCt2hdoG/8M0iE8xswANESfXnQZXQL\n\tCnBaQCw+Z85kasLoqcCWx9sni7v+F/2QjO9G7T/QT57KbfgVawJ5OIX5TxbL1k1ByB\n\t2ogMimWpkNSlqQ8fvORAH0AAn/YRYWyQ17+pbua0=","Date":"Mon, 26 Apr 2021 09:42:35 +0300","From":"Laurent Pinchart ","To":"Jean-Michel Hautbois ","Message-ID":"","References":"<20210423063919.26273-1-jeanmichel.hautbois@ideasonboard.com>\n\t<6c07405a-b23e-c9db-c730-85589a520ce4@ideasonboard.com>\n\t\n\t<583696f1-7d40-04ee-2662-f2461d3cb458@ideasonboard.com>\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","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","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16571,"web_url":"https://patchwork.libcamera.org/comment/16571/","msgid":"<20210426070838.wzvpwdvizdh5j2xf@basti-TUXEDO-Book-XA1510>","date":"2021-04-26T07:08:38","subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","submitter":{"id":78,"url":"https://patchwork.libcamera.org/api/people/78/","name":"Sebastian Fricke","email":"sebastian.fricke@posteo.net"},"content":"Hey Laurent and Jean-Michel,\n\nOn 26.04.2021 09:42, Laurent Pinchart wrote:\n>Hi Jean-Michel,\n>\n>On Mon, Apr 26, 2021 at 08:01:11AM +0200, Jean-Michel Hautbois wrote:\n>> On 26/04/2021 07:57, Laurent Pinchart wrote:\n>> > On Mon, Apr 26, 2021 at 07:49:11AM +0200, Jean-Michel Hautbois wrote:\n>> >> On 26/04/2021 00:03, Laurent Pinchart wrote:\n>> >>> On Fri, Apr 23, 2021 at 10:02:15AM +0100, Kieran Bingham wrote:\n>> >>>> On 23/04/2021 07:39, Jean-Michel Hautbois wrote:\n>> >>>>> Simplify name-spacing of the RKISP1 components by placing it in the\n>> >>>>> ipa::rkisp1 namespace directly.\n>> >>>>\n>> >>>> Given that I did the same for the IPU3 - Perhaps I'm biased, but I think\n>> >>>> this is better ;-D\n>> >>>>\n>> >>>>> Signed-off-by: Jean-Michel Hautbois \n>> >>>>\n>> >>>> Reviewed-by: Kieran Bingham \n>> >>>\n>> >>> Reviewed-by: Laurent Pinchart \n>> >>>\n>> >>> We should consider renaming IPARkISP1Interface to Interface though, and\n>> >>> IPARkISP1 to... Implementation ?\n>> >>\n>> >> Thanks.\n>> >> Do you mean having the following ?\n>> >>\n>> >> -class IPARkISP1 : public IPARkISP1Interface\n>> >> +class Implementation : public Interface\n>> >>\n>> >> It would be the same for all IPAs then ?\n>> >\n>> > That's the idea, yes. It's just an idea though :-) It looks a bit...\n>> > bare maybe ? But if we keep IPA and RkISP1 in the name of the classes,\n>> > it defeats the purpose of namespaces a little bit.\n>>\n>> For reading ease couldn't we have:\n>> class RkISP1Implementation: public RkISP1Interface\n>>\n>> That would keep the naming without the IPA :-).\n>> It is \"just\" a mojom patching, right :-p ?\n>\n>Or\n>\n>class IPAImplementation : public IPAInterface\n\n\n>\n>? :-)\n\nI like that the most as it still highlights that we talk about IPAs,\nwhile being general enough to act as a namespace for different\nimplementations.\n\nGreetings,\nSebastian\n\n>\n>> >>>>> ---\n>> >>>>> src/ipa/rkisp1/rkisp1.cpp | 28 ++++++++++++++++------------\n>> >>>>> 1 file changed, 16 insertions(+), 12 deletions(-)\n>> >>>>>\n>> >>>>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>> >>>>> index 8a57b080..6d45673c 100644\n>> >>>>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>> >>>>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>> >>>>> @@ -28,7 +28,9 @@ namespace libcamera {\n>> >>>>>\n>> >>>>> LOG_DEFINE_CATEGORY(IPARkISP1)\n>> >>>>>\n>> >>>>> -class IPARkISP1 : public ipa::rkisp1::IPARkISP1Interface\n>> >>>>> +namespace ipa::rkisp1 {\n>> >>>>> +\n>> >>>>> +class IPARkISP1 : public IPARkISP1Interface\n>> >>>>> {\n>> >>>>> public:\n>> >>>>> \tint init(unsigned int hwRevision) override;\n>> >>>>> @@ -40,7 +42,7 @@ public:\n>> >>>>> \t\t const std::map &entityControls) override;\n>> >>>>> \tvoid mapBuffers(const std::vector &buffers) override;\n>> >>>>> \tvoid unmapBuffers(const std::vector &ids) override;\n>> >>>>> -\tvoid processEvent(const ipa::rkisp1::RkISP1Event &event) override;\n>> >>>>> +\tvoid processEvent(const RkISP1Event &event) override;\n>> >>>>>\n>> >>>>> private:\n>> >>>>> \tvoid queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n>> >>>>> @@ -171,10 +173,10 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n>> >>>>> \t}\n>> >>>>> }\n>> >>>>>\n>> >>>>> -void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event &event)\n>> >>>>> +void IPARkISP1::processEvent(const RkISP1Event &event)\n>> >>>>> {\n>> >>>>> \tswitch (event.op) {\n>> >>>>> -\tcase ipa::rkisp1::EventSignalStatBuffer: {\n>> >>>>> +\tcase EventSignalStatBuffer: {\n>> >>>>> \t\tunsigned int frame = event.frame;\n>> >>>>> \t\tunsigned int bufferId = event.bufferId;\n>> >>>>>\n>> >>>>> @@ -184,7 +186,7 @@ void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event &event)\n>> >>>>> \t\tupdateStatistics(frame, stats);\n>> >>>>> \t\tbreak;\n>> >>>>> \t}\n>> >>>>> -\tcase ipa::rkisp1::EventQueueRequest: {\n>> >>>>> +\tcase EventQueueRequest: {\n>> >>>>> \t\tunsigned int frame = event.frame;\n>> >>>>> \t\tunsigned int bufferId = event.bufferId;\n>> >>>>>\n>> >>>>> @@ -215,8 +217,8 @@ void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,\n>> >>>>> \t\tparams->module_en_update = RKISP1_CIF_ISP_MODULE_AEC;\n>> >>>>> \t}\n>> >>>>>\n>> >>>>> -\tipa::rkisp1::RkISP1Action op;\n>> >>>>> -\top.op = ipa::rkisp1::ActionParamFilled;\n>> >>>>> +\tRkISP1Action op;\n>> >>>>> +\top.op = ActionParamFilled;\n>> >>>>>\n>> >>>>> \tqueueFrameAction.emit(frame, op);\n>> >>>>> }\n>> >>>>> @@ -268,8 +270,8 @@ void IPARkISP1::updateStatistics(unsigned int frame,\n>> >>>>>\n>> >>>>> void IPARkISP1::setControls(unsigned int frame)\n>> >>>>> {\n>> >>>>> -\tipa::rkisp1::RkISP1Action op;\n>> >>>>> -\top.op = ipa::rkisp1::ActionV4L2Set;\n>> >>>>> +\tRkISP1Action op;\n>> >>>>> +\top.op = ActionV4L2Set;\n>> >>>>>\n>> >>>>> \tControlList ctrls(ctrls_);\n>> >>>>> \tctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure_));\n>> >>>>> @@ -286,13 +288,15 @@ void IPARkISP1::metadataReady(unsigned int frame, unsigned int aeState)\n>> >>>>> \tif (aeState)\n>> >>>>> \t\tctrls.set(controls::AeLocked, aeState == 2);\n>> >>>>>\n>> >>>>> -\tipa::rkisp1::RkISP1Action op;\n>> >>>>> -\top.op = ipa::rkisp1::ActionMetadata;\n>> >>>>> +\tRkISP1Action op;\n>> >>>>> +\top.op = ActionMetadata;\n>> >>>>> \top.controls = ctrls;\n>> >>>>>\n>> >>>>> \tqueueFrameAction.emit(frame, op);\n>> >>>>> }\n>> >>>>>\n>> >>>>> +} /* namespace ipa::rkisp1 */\n>> >>>>> +\n>> >>>>> /*\n>> >>>>> * External IPA module interface\n>> >>>>> */\n>> >>>>> @@ -307,7 +311,7 @@ const struct IPAModuleInfo ipaModuleInfo = {\n>> >>>>>\n>> >>>>> IPAInterface *ipaCreate()\n>> >>>>> {\n>> >>>>> -\treturn new IPARkISP1();\n>> >>>>> +\treturn new ipa::rkisp1::IPARkISP1();\n>> >>>>> }\n>> >>>>> }\n>> >>>>>\n>\n>-- \n>Regards,\n>\n>Laurent Pinchart\n>_______________________________________________\n>libcamera-devel mailing list\n>libcamera-devel@lists.libcamera.org\n>https://lists.libcamera.org/listinfo/libcamera-devel","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 277F3BDC99\n\tfor ;\n\tMon, 26 Apr 2021 07:08:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 80633688B3;\n\tMon, 26 Apr 2021 09:08:42 +0200 (CEST)","from mout02.posteo.de (mout02.posteo.de [185.67.36.66])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 89D32688A7\n\tfor ;\n\tMon, 26 Apr 2021 09:08:40 +0200 (CEST)","from submission (posteo.de [89.146.220.130]) \n\tby mout02.posteo.de (Postfix) with ESMTPS id F301D2400FD\n\tfor ;\n\tMon, 26 Apr 2021 09:08:39 +0200 (CEST)","from customer (localhost [127.0.0.1])\n\tby submission (posteo.de) with ESMTPSA id 4FTGFz2GgBz6tmD;\n\tMon, 26 Apr 2021 09:08:39 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=posteo.net header.i=@posteo.net\n\theader.b=\"i7m2KtoL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017;\n\tt=1619420920; bh=HjfVNAzhMwpzXHmSHSm+5WRTgoehqrrzREvB83RwAZg=;\n\th=Date:From:To:Cc:Subject:From;\n\tb=i7m2KtoL6jtEn7tU/igAL1I18VipKqZ60OIIz7PL/IGFhJLlvVujyAFNkHMVp9iVM\n\t/5daKHN26dysbvD7zZ30bOhbPOJVqfC1EhuadUvSRiCyd6jetIKJJYgs+bqMPxVCvy\n\tU9EByaIce6ife+Wps0Y44W5dkuhQvbRiueTcuy+zD2lnV9of+ORd1362X7LLPEOlaW\n\t7XZ+TGNDfPfDRkxVnypEOWglENB1HnbQ0bw1f6c73+iCpe6WQK37ZkMuwWWQoU74X3\n\tsoQsPT6OQSHtxp1mSm1smmp3YsV9G6QRPfho5s0yXqXPGCrpbmme//HWedoWCgP5iR\n\tvYqpxYOB527+g==","Date":"Mon, 26 Apr 2021 07:08:38 +0000","From":"Sebastian Fricke ","To":"Laurent Pinchart ","Message-ID":"<20210426070838.wzvpwdvizdh5j2xf@basti-TUXEDO-Book-XA1510>","References":"<20210423063919.26273-1-jeanmichel.hautbois@ideasonboard.com>\n\t<6c07405a-b23e-c9db-c730-85589a520ce4@ideasonboard.com>\n\t\n\t<583696f1-7d40-04ee-2662-f2461d3cb458@ideasonboard.com>\n\t\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","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","Cc":"libcamera devel ","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16576,"web_url":"https://patchwork.libcamera.org/comment/16576/","msgid":"<4ae20825-da12-ae08-6aa8-e48f7fd78ea5@ideasonboard.com>","date":"2021-04-26T08:59:58","subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"On 26/04/2021 09:08, Sebastian Fricke wrote:\n> Hey Laurent and Jean-Michel,\n> \n> On 26.04.2021 09:42, Laurent Pinchart wrote:\n>> Hi Jean-Michel,\n>>\n>> On Mon, Apr 26, 2021 at 08:01:11AM +0200, Jean-Michel Hautbois wrote:\n>>> On 26/04/2021 07:57, Laurent Pinchart wrote:\n>>> > On Mon, Apr 26, 2021 at 07:49:11AM +0200, Jean-Michel Hautbois wrote:\n>>> >> On 26/04/2021 00:03, Laurent Pinchart wrote:\n>>> >>> On Fri, Apr 23, 2021 at 10:02:15AM +0100, Kieran Bingham wrote:\n>>> >>>> On 23/04/2021 07:39, Jean-Michel Hautbois wrote:\n>>> >>>>> Simplify name-spacing of the RKISP1 components by placing it in\n>>> the\n>>> >>>>> ipa::rkisp1 namespace directly.\n>>> >>>>\n>>> >>>> Given that I did the same for the IPU3 - Perhaps I'm biased, but\n>>> I think\n>>> >>>> this is better ;-D\n>>> >>>>\n>>> >>>>> Signed-off-by: Jean-Michel Hautbois\n>>> \n>>> >>>>\n>>> >>>> Reviewed-by: Kieran Bingham \n>>> >>>\n>>> >>> Reviewed-by: Laurent Pinchart \n>>> >>>\n>>> >>> We should consider renaming IPARkISP1Interface to Interface\n>>> though, and\n>>> >>> IPARkISP1 to... Implementation ?\n>>> >>\n>>> >> Thanks.\n>>> >> Do you mean having the following ?\n>>> >>\n>>> >> -class IPARkISP1 : public IPARkISP1Interface\n>>> >> +class Implementation : public Interface\n>>> >>\n>>> >> It would be the same for all IPAs then ?\n>>> >\n>>> > That's the idea, yes. It's just an idea though :-) It looks a bit...\n>>> > bare maybe ? But if we keep IPA and RkISP1 in the name of the classes,\n>>> > it defeats the purpose of namespaces a little bit.\n>>>\n>>> For reading ease couldn't we have:\n>>> class RkISP1Implementation: public RkISP1Interface\n>>>\n>>> That would keep the naming without the IPA :-).\n>>> It is \"just\" a mojom patching, right :-p ?\n>>\n>> Or\n>>\n>> class IPAImplementation : public IPAInterface\n\nBut there is already a base class IPAInterface in\ninclude/libcamera/ipa/ipa_interface.h !\nAccording to me, RkISP1Interface inherits from the IPAInterface right now...\n\nOr did I miss something here... (I probably have)\n> \n>>\n>> ? :-)\n> \n> I like that the most as it still highlights that we talk about IPAs,\n> while being general enough to act as a namespace for different\n> implementations.\n> \n> Greetings,\n> Sebastian\n> \n>>\n>>> >>>>> ---\n>>> >>>>>  src/ipa/rkisp1/rkisp1.cpp | 28 ++++++++++++++++------------\n>>> >>>>>  1 file changed, 16 insertions(+), 12 deletions(-)\n>>> >>>>>\n>>> >>>>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>>> >>>>> index 8a57b080..6d45673c 100644\n>>> >>>>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>>> >>>>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>>> >>>>> @@ -28,7 +28,9 @@ namespace libcamera {\n>>> >>>>>\n>>> >>>>>  LOG_DEFINE_CATEGORY(IPARkISP1)\n>>> >>>>>\n>>> >>>>> -class IPARkISP1 : public ipa::rkisp1::IPARkISP1Interface\n>>> >>>>> +namespace ipa::rkisp1 {\n>>> >>>>> +\n>>> >>>>> +class IPARkISP1 : public IPARkISP1Interface\n>>> >>>>>  {\n>>> >>>>>  public:\n>>> >>>>>      int init(unsigned int hwRevision) override;\n>>> >>>>> @@ -40,7 +42,7 @@ public:\n>>> >>>>>                const std::map\n>>> &entityControls) override;\n>>> >>>>>      void mapBuffers(const std::vector &buffers)\n>>> override;\n>>> >>>>>      void unmapBuffers(const std::vector &ids)\n>>> override;\n>>> >>>>> -    void processEvent(const ipa::rkisp1::RkISP1Event &event)\n>>> override;\n>>> >>>>> +    void processEvent(const RkISP1Event &event) override;\n>>> >>>>>\n>>> >>>>>  private:\n>>> >>>>>      void queueRequest(unsigned int frame, rkisp1_params_cfg\n>>> *params,\n>>> >>>>> @@ -171,10 +173,10 @@ void IPARkISP1::unmapBuffers(const\n>>> std::vector &ids)\n>>> >>>>>      }\n>>> >>>>>  }\n>>> >>>>>\n>>> >>>>> -void IPARkISP1::processEvent(const ipa::rkisp1::RkISP1Event\n>>> &event)\n>>> >>>>> +void IPARkISP1::processEvent(const RkISP1Event &event)\n>>> >>>>>  {\n>>> >>>>>      switch (event.op) {\n>>> >>>>> -    case ipa::rkisp1::EventSignalStatBuffer: {\n>>> >>>>> +    case EventSignalStatBuffer: {\n>>> >>>>>          unsigned int frame = event.frame;\n>>> >>>>>          unsigned int bufferId = event.bufferId;\n>>> >>>>>\n>>> >>>>> @@ -184,7 +186,7 @@ void IPARkISP1::processEvent(const\n>>> ipa::rkisp1::RkISP1Event &event)\n>>> >>>>>          updateStatistics(frame, stats);\n>>> >>>>>          break;\n>>> >>>>>      }\n>>> >>>>> -    case ipa::rkisp1::EventQueueRequest: {\n>>> >>>>> +    case EventQueueRequest: {\n>>> >>>>>          unsigned int frame = event.frame;\n>>> >>>>>          unsigned int bufferId = event.bufferId;\n>>> >>>>>\n>>> >>>>> @@ -215,8 +217,8 @@ void IPARkISP1::queueRequest(unsigned int\n>>> frame, rkisp1_params_cfg *params,\n>>> >>>>>          params->module_en_update = RKISP1_CIF_ISP_MODULE_AEC;\n>>> >>>>>      }\n>>> >>>>>\n>>> >>>>> -    ipa::rkisp1::RkISP1Action op;\n>>> >>>>> -    op.op = ipa::rkisp1::ActionParamFilled;\n>>> >>>>> +    RkISP1Action op;\n>>> >>>>> +    op.op = ActionParamFilled;\n>>> >>>>>\n>>> >>>>>      queueFrameAction.emit(frame, op);\n>>> >>>>>  }\n>>> >>>>> @@ -268,8 +270,8 @@ void IPARkISP1::updateStatistics(unsigned\n>>> int frame,\n>>> >>>>>\n>>> >>>>>  void IPARkISP1::setControls(unsigned int frame)\n>>> >>>>>  {\n>>> >>>>> -    ipa::rkisp1::RkISP1Action op;\n>>> >>>>> -    op.op = ipa::rkisp1::ActionV4L2Set;\n>>> >>>>> +    RkISP1Action op;\n>>> >>>>> +    op.op = ActionV4L2Set;\n>>> >>>>>\n>>> >>>>>      ControlList ctrls(ctrls_);\n>>> >>>>>      ctrls.set(V4L2_CID_EXPOSURE,\n>>> static_cast(exposure_));\n>>> >>>>> @@ -286,13 +288,15 @@ void IPARkISP1::metadataReady(unsigned\n>>> int frame, unsigned int aeState)\n>>> >>>>>      if (aeState)\n>>> >>>>>          ctrls.set(controls::AeLocked, aeState == 2);\n>>> >>>>>\n>>> >>>>> -    ipa::rkisp1::RkISP1Action op;\n>>> >>>>> -    op.op = ipa::rkisp1::ActionMetadata;\n>>> >>>>> +    RkISP1Action op;\n>>> >>>>> +    op.op = ActionMetadata;\n>>> >>>>>      op.controls = ctrls;\n>>> >>>>>\n>>> >>>>>      queueFrameAction.emit(frame, op);\n>>> >>>>>  }\n>>> >>>>>\n>>> >>>>> +} /* namespace ipa::rkisp1 */\n>>> >>>>> +\n>>> >>>>>  /*\n>>> >>>>>   * External IPA module interface\n>>> >>>>>   */\n>>> >>>>> @@ -307,7 +311,7 @@ const struct IPAModuleInfo ipaModuleInfo = {\n>>> >>>>>\n>>> >>>>>  IPAInterface *ipaCreate()\n>>> >>>>>  {\n>>> >>>>> -    return new IPARkISP1();\n>>> >>>>> +    return new ipa::rkisp1::IPARkISP1();\n>>> >>>>>  }\n>>> >>>>>  }\n>>> >>>>>\n>>\n>> -- \n>> Regards,\n>>\n>> Laurent Pinchart\n>> _______________________________________________\n>> libcamera-devel mailing list\n>> libcamera-devel@lists.libcamera.org\n>> https://lists.libcamera.org/listinfo/libcamera-devel","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 AEC00BDC9B\n\tfor ;\n\tMon, 26 Apr 2021 09:00:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 05898688B4;\n\tMon, 26 Apr 2021 11:00:01 +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 362DC60516\n\tfor ;\n\tMon, 26 Apr 2021 10:59:59 +0200 (CEST)","from [IPv6:2a01:e0a:169:7140:5ef:55ab:b54c:2213] (unknown\n\t[IPv6:2a01:e0a:169:7140:5ef:55ab:b54c:2213])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C1784D8B;\n\tMon, 26 Apr 2021 10:59:58 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"B2+KKMSF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1619427598;\n\tbh=y4EzXe5eL0+PnD7A5FmFzhoInNaaWmrQHARdHqbUgmM=;\n\th=Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=B2+KKMSFKeSMnZTx8FE93KYOO9GtktFMUdV6bm/F2DMHvbE+sYQzL2nUtYsLtLQhm\n\t8oTZgr13PGM53X+OdAknqoeHhgQinbmcx5muCxUvlkC1CDhCuistJxAmRP9PyegAA0\n\tAjXn8wafLJh0V4yDI6Yg517HFk8HeGcmWP1mUTY8=","To":"Sebastian Fricke ,\n\tLaurent Pinchart ","References":"<20210423063919.26273-1-jeanmichel.hautbois@ideasonboard.com>\n\t<6c07405a-b23e-c9db-c730-85589a520ce4@ideasonboard.com>\n\t\n\t<583696f1-7d40-04ee-2662-f2461d3cb458@ideasonboard.com>\n\t\n\t\n\t\n\t<20210426070838.wzvpwdvizdh5j2xf@basti-TUXEDO-Book-XA1510>","From":"Jean-Michel Hautbois ","Message-ID":"<4ae20825-da12-ae08-6aa8-e48f7fd78ea5@ideasonboard.com>","Date":"Mon, 26 Apr 2021 10:59:58 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"<20210426070838.wzvpwdvizdh5j2xf@basti-TUXEDO-Book-XA1510>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","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","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16578,"web_url":"https://patchwork.libcamera.org/comment/16578/","msgid":"<55c323a5-dc3e-d565-a8c1-c07eba429fec@ideasonboard.com>","date":"2021-04-26T09:09:39","subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi JM,\n\nOn 26/04/2021 09:59, Jean-Michel Hautbois wrote:\n> \n> \n> On 26/04/2021 09:08, Sebastian Fricke wrote:\n>> Hey Laurent and Jean-Michel,\n>>\n>> On 26.04.2021 09:42, Laurent Pinchart wrote:\n>>> Hi Jean-Michel,\n>>>\n>>> On Mon, Apr 26, 2021 at 08:01:11AM +0200, Jean-Michel Hautbois wrote:\n>>>> On 26/04/2021 07:57, Laurent Pinchart wrote:\n>>>>> On Mon, Apr 26, 2021 at 07:49:11AM +0200, Jean-Michel Hautbois wrote:\n>>>>>> On 26/04/2021 00:03, Laurent Pinchart wrote:\n>>>>>>> On Fri, Apr 23, 2021 at 10:02:15AM +0100, Kieran Bingham wrote:\n>>>>>>>> On 23/04/2021 07:39, Jean-Michel Hautbois wrote:\n>>>>>>>>> Simplify name-spacing of the RKISP1 components by placing it in\n>>>> the\n>>>>>>>>> ipa::rkisp1 namespace directly.\n>>>>>>>>\n>>>>>>>> Given that I did the same for the IPU3 - Perhaps I'm biased, but\n>>>> I think\n>>>>>>>> this is better ;-D\n>>>>>>>>\n>>>>>>>>> Signed-off-by: Jean-Michel Hautbois\n>>>> \n>>>>>>>>\n>>>>>>>> Reviewed-by: Kieran Bingham \n>>>>>>>\n>>>>>>> Reviewed-by: Laurent Pinchart \n>>>>>>>\n>>>>>>> We should consider renaming IPARkISP1Interface to Interface\n>>>> though, and\n>>>>>>> IPARkISP1 to... Implementation ?\n>>>>>>\n>>>>>> Thanks.\n>>>>>> Do you mean having the following ?\n>>>>>>\n>>>>>> -class IPARkISP1 : public IPARkISP1Interface\n>>>>>> +class Implementation : public Interface\n>>>>>>\n>>>>>> It would be the same for all IPAs then ?\n>>>>>\n>>>>> That's the idea, yes. It's just an idea though :-) It looks a bit...\n>>>>> bare maybe ? But if we keep IPA and RkISP1 in the name of the classes,\n>>>>> it defeats the purpose of namespaces a little bit.\n>>>>\n>>>> For reading ease couldn't we have:\n>>>> class RkISP1Implementation: public RkISP1Interface\n>>>>\n>>>> That would keep the naming without the IPA :-).\n>>>> It is \"just\" a mojom patching, right :-p ?\n>>>\n>>> Or\n>>>\n>>> class IPAImplementation : public IPAInterface\n> \n> But there is already a base class IPAInterface in\n> include/libcamera/ipa/ipa_interface.h !\n> According to me, RkISP1Interface inherits from the IPAInterface right now...\n> \n> Or did I miss something here... (I probably have)\n\nIsn't it the namespacing?\n\nI.e.\n\nlibcamera::ipa::IPAInterface\n\nis a different class to\n\nlibcamera::ipa::rkisp1::IPAInterface\n\n\nThus the namespace specifies the ... specification of the interface ...\n\nRegards\n--\nKieran","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 C1DE6BDC9A\n\tfor ;\n\tMon, 26 Apr 2021 09:09:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 84C24688AF;\n\tMon, 26 Apr 2021 11:09:43 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 38D5660516\n\tfor ;\n\tMon, 26 Apr 2021 11:09:42 +0200 (CEST)","from [192.168.0.20]\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 ACBB5D8B;\n\tMon, 26 Apr 2021 11:09:41 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"wnMeDb4N\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1619428181;\n\tbh=e93dI3puTynN4+MYssukqzeivw6/dBkyw8HUMNwAWC8=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=wnMeDb4NyZEkeKgfmLvsSXtnq1+93KtRxt9Wxm/Iqg4N8xmS/i6anrFNrtLlnMttj\n\t1UQv+TzNK2GgfGYmm49n7dYXep+0AbKf+JdwNdF9/3qUdqLU618lXcI4GH97RA32gf\n\tvaELqYwJQY18CTwxlbbD0zMJJPzX/HM/l/H6Ih/8=","To":"Jean-Michel Hautbois ,\n\tSebastian Fricke ,\n\tLaurent Pinchart ","References":"<20210423063919.26273-1-jeanmichel.hautbois@ideasonboard.com>\n\t<6c07405a-b23e-c9db-c730-85589a520ce4@ideasonboard.com>\n\t\n\t<583696f1-7d40-04ee-2662-f2461d3cb458@ideasonboard.com>\n\t\n\t\n\t\n\t<20210426070838.wzvpwdvizdh5j2xf@basti-TUXEDO-Book-XA1510>\n\t<4ae20825-da12-ae08-6aa8-e48f7fd78ea5@ideasonboard.com>","From":"Kieran Bingham ","Organization":"Ideas on Board","Message-ID":"<55c323a5-dc3e-d565-a8c1-c07eba429fec@ideasonboard.com>","Date":"Mon, 26 Apr 2021 10:09:39 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"<4ae20825-da12-ae08-6aa8-e48f7fd78ea5@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH] ipa: rkisp1: Move the IPA to the\n\tipa::rkisp1 namespace","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]