[{"id":16946,"web_url":"https://patchwork.libcamera.org/comment/16946/","msgid":"<0500ee30-7dc8-97a0-8e45-f718f7ca3d12@ideasonboard.com>","date":"2021-05-14T09:57:24","subject":"Re: [libcamera-devel] [PATCH v1 3/6] ipa: ipu3: Introduce\n\tIPAConfigInfo in IPC","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Umang,\n\nOn 14/05/2021 08:58, Umang Jain wrote:\n> IPAConfigInfo is a consolidated data structure passed from IPU3\n> pipeline-handler to IPU3 IPA. The structure can be extended with\n> additional parameiters to accommodate the requirements of multiple\n\ns/parameiters/parameters/\n\n> IPU3 IPA modules for e.g., an open-source IPA (in-tree) and a closed\n> source IPA (for ChromeOS).\n\n\nNo need to have the expression after the e.g. It's simply to accommodate\nthe parameters for the IPU3 IPAs.\n\nIn fact, this isn't related to supporting multiple IPAs at all. This\nsimply defines the interface. The fact that we have multiple\nimplementations is just a fact on top of that.\n\n\n\n> As usual, the documentation for .mojom files are kept in a separate\n> .cpp file. Hence, create and document IPAConfigInfo in\n> ipu3_ipa_interface.cpp.\n> \n> Finally, adapt the in-tree IPU3 IPA to use IPAConfigInfo as well.\n> \n> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> ---\n>  include/libcamera/ipa/ipu3.mojom             | 10 ++++-\n>  include/libcamera/ipa/ipu3_ipa_interface.cpp | 39 ++++++++++++++++++++\n\nOk, now I really think we need to have the ipa directory under\nsrc/libcamera/ipa as a prerequisite to this series.\n\nLet me know if you'll do that or if you prefer me to do it.\n\n\n\n>  include/libcamera/ipa/meson.build            |  1 +\n>  src/ipa/ipu3/ipu3.cpp                        | 14 +++----\n>  src/libcamera/pipeline/ipu3/ipu3.cpp         | 10 +++--\n>  5 files changed, 61 insertions(+), 13 deletions(-)\n>  create mode 100644 include/libcamera/ipa/ipu3_ipa_interface.cpp\n> \n> diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom\n> index a717b1e6..22c4ce1b 100644\n> --- a/include/libcamera/ipa/ipu3.mojom\n> +++ b/include/libcamera/ipa/ipu3.mojom\n> @@ -25,13 +25,19 @@ struct IPU3Action {\n>  \tlibcamera.ControlList controls;\n>  };\n>  \n> +struct IPAConfigInfo {\n> +\tlibcamera.IPACameraSensorInfo sensorInfo;\n> +\tmap<uint32, ControlInfoMap> entityControls;\n> +\tlibcamera.Size bdsOutputSize;\n> +\tlibcamera.Size iif;\n> +};\n> +\n>  interface IPAIPU3Interface {\n>  \tinit(libcamera.IPASettings settings) => (int32 ret);\n>  \tstart() => (int32 ret);\n>  \tstop();\n>  \n> -\tconfigure(map<uint32, libcamera.ControlInfoMap> entityControls,\n> -\t\t  libcamera.Size bdsOutputSize) => ();\n> +\tconfigure(IPAConfigInfo configInfo);\n>  \n>  \tmapBuffers(array<libcamera.IPABuffer> buffers);\n>  \tunmapBuffers(array<uint32> ids);\n> diff --git a/include/libcamera/ipa/ipu3_ipa_interface.cpp b/include/libcamera/ipa/ipu3_ipa_interface.cpp\n> new file mode 100644\n> index 00000000..9aafbcdb\n> --- /dev/null\n> +++ b/include/libcamera/ipa/ipu3_ipa_interface.cpp\n> @@ -0,0 +1,39 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +\n> +namespace libcamera {\n> +\n> +/**\n> + * \\struct IPAConfigInfo\n> + * \\brief Information to be passed from IPU3 pipeline-handler to the IPA\n> + *\n> + * The IPAConfigInfo structure stores the data passed from IPU3 pipeline-handler\n> + * to the IPAIPU3Interface::configure(). The structure provides extensibility\n> + * for additional configuration data as required, for closed-source or\n> + * open-source IPAs' configure() phases.\n> + */\n> +\n> +/**\n> + * \\var IPAConfigInfo::sensorInfo\n> + * \\brief Camera sensor information\n> + *\n> + * \\sa IPACameraSensorInfo\n> + */\n> +\n> +/**\n> + * \\var IPAConfigInfo::entityControls\n> + * \\brief Controls supported by the sensor\n> + *\n> + * A map of V4L2 controls supported by the sensor in order to read or write\n> + * control values to them.\n> + */\n> +\n> +/**\n> + * \\var IPAConfigInfo::bdsOutputSize\n> + * \\brief Size of ImgU BDS rectangle\n> + */\n> +\n> +/**\n> + * \\var IPAConfigInfo::iif\n> + * \\brief Size of ImgU input-feeder rectangle\n> + */\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build\n> index da60aa68..9684a34f 100644\n> --- a/include/libcamera/ipa/meson.build\n> +++ b/include/libcamera/ipa/meson.build\n> @@ -10,6 +10,7 @@ libcamera_ipa_headers = files([\n>  \n>  libcamera_ipa_docs = files([\n>      'core_ipa_interface.cpp',\n> +    'ipu3_ipa_interface.cpp',\n\nAnd of course then this would get added to the libcamera sources as part\nof src/libcamera/ipa/meson.build which would then get passed into doxygen.\n\n\n>  ])\n>  \n>  install_headers(libcamera_ipa_headers,\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index f5343547..769c24d3 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -43,8 +43,7 @@ public:\n>  \tint start() override;\n>  \tvoid stop() override {}\n>  \n> -\tvoid configure(const std::map<uint32_t, ControlInfoMap> &entityControls,\n> -\t\t       const Size &bdsOutputSize) override;\n> +\tvoid configure(const IPAConfigInfo &configInfo) override;\n>  \n>  \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override;\n>  \tvoid unmapBuffers(const std::vector<unsigned int> &ids) override;\n> @@ -139,13 +138,12 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize)\n>  \t\t\t    << (int)bdsGrid_.height << \" << \" << (int)bdsGrid_.block_height_log2 << \")\";\n>  }\n>  \n> -void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls,\n> -\t\t\tconst Size &bdsOutputSize)\n> +void IPAIPU3::configure(const IPAConfigInfo &configInfo)\n>  {\n> -\tif (entityControls.empty())\n> +\tif (configInfo.entityControls.empty())\n>  \t\treturn;\n>  \n> -\tctrls_ = entityControls.at(0);\n> +\tctrls_ = configInfo.entityControls.at(0);\n>  \n>  \tconst auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);\n>  \tif (itExp == ctrls_.end()) {\n> @@ -169,10 +167,10 @@ void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls\n>  \n>  \tparams_ = {};\n>  \n> -\tcalculateBdsGrid(bdsOutputSize);\n> +\tcalculateBdsGrid(configInfo.bdsOutputSize);\n>  \n>  \tawbAlgo_ = std::make_unique<IPU3Awb>();\n> -\tawbAlgo_->initialise(params_, bdsOutputSize, bdsGrid_);\n> +\tawbAlgo_->initialise(params_, configInfo.bdsOutputSize, bdsGrid_);\n>  \n>  \tagcAlgo_ = std::make_unique<IPU3Agc>();\n>  \tagcAlgo_->initialise(bdsGrid_);\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 98c6160f..5b15ca90 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -633,9 +633,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n>  \t\treturn ret;\n>  \t}\n>  \n> -\tstd::map<uint32_t, ControlInfoMap> entityControls;\n> -\tentityControls.emplace(0, data->cio2_.sensor()->controls());\n> -\tdata->ipa_->configure(entityControls, config->imguConfig().bds);\n> +\tipa::ipu3::IPAConfigInfo configInfo;\n> +\tconfigInfo.entityControls.emplace(0, data->cio2_.sensor()->controls());\n> +\tconfigInfo.sensorInfo = sensorInfo;\n> +\tconfigInfo.bdsOutputSize = config->imguConfig().bds;\n> +\tconfigInfo.iif = config->imguConfig().iif;\n> +\n> +\tdata->ipa_->configure(configInfo);\n\nOtherwise, the code move looks ok to me.\n\n>  \n>  \treturn 0;\n>  }\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 122A1C31F6\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 14 May 2021 09:57:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6FCD36891C;\n\tFri, 14 May 2021 11:57:30 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EFF2868911\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 14 May 2021 11:57:28 +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 AB8BE9F0;\n\tFri, 14 May 2021 11:57:27 +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=\"WRtdUFBD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1620986248;\n\tbh=r/dK7S7+ebivHUTLIlLUgVEPwYfhz52SHRkgczk1ZKs=;\n\th=Reply-To:To:References:From:Subject:Date:In-Reply-To:From;\n\tb=WRtdUFBD0tGKUit9YUbHprd5bMIxW4QsTizSVDSU+Fp6ssvV30gKE+9ValzzQgXip\n\tpzpfh+ZVs2TJYhEiOrp2fHRCsRNHzpK6r7RfhjZSkBMafBBND+FQ7U/CKh3GGjTl10\n\t5bztG8Sv7NcmUxV0kO3QJLJ4+tjl96Sv1Yz2xllA=","To":"Umang Jain <umang.jain@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210514075808.282479-1-umang.jain@ideasonboard.com>\n\t<20210514075808.282479-4-umang.jain@ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Organization":"Ideas on Board","Message-ID":"<0500ee30-7dc8-97a0-8e45-f718f7ca3d12@ideasonboard.com>","Date":"Fri, 14 May 2021 10:57:24 +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":"<20210514075808.282479-4-umang.jain@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v1 3/6] ipa: ipu3: Introduce\n\tIPAConfigInfo in IPC","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":16956,"web_url":"https://patchwork.libcamera.org/comment/16956/","msgid":"<YJ/3FXUplnr+51Zn@pendragon.ideasonboard.com>","date":"2021-05-15T16:30:13","subject":"Re: [libcamera-devel] [PATCH v1 3/6] ipa: ipu3: Introduce\n\tIPAConfigInfo in IPC","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Fri, May 14, 2021 at 10:57:24AM +0100, Kieran Bingham wrote:\n> On 14/05/2021 08:58, Umang Jain wrote:\n> > IPAConfigInfo is a consolidated data structure passed from IPU3\n> > pipeline-handler to IPU3 IPA. The structure can be extended with\n> > additional parameiters to accommodate the requirements of multiple\n> \n> s/parameiters/parameters/\n> \n> > IPU3 IPA modules for e.g., an open-source IPA (in-tree) and a closed\n> > source IPA (for ChromeOS).\n> \n> No need to have the expression after the e.g. It's simply to accommodate\n> the parameters for the IPU3 IPAs.\n> \n> In fact, this isn't related to supporting multiple IPAs at all. This\n> simply defines the interface. The fact that we have multiple\n> implementations is just a fact on top of that.\n> \n> > As usual, the documentation for .mojom files are kept in a separate\n> > .cpp file. Hence, create and document IPAConfigInfo in\n> > ipu3_ipa_interface.cpp.\n> > \n> > Finally, adapt the in-tree IPU3 IPA to use IPAConfigInfo as well.\n> > \n> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> > ---\n> >  include/libcamera/ipa/ipu3.mojom             | 10 ++++-\n> >  include/libcamera/ipa/ipu3_ipa_interface.cpp | 39 ++++++++++++++++++++\n> \n> Ok, now I really think we need to have the ipa directory under\n> src/libcamera/ipa as a prerequisite to this series.\n> \n> Let me know if you'll do that or if you prefer me to do it.\n\nI'm beginning to wonder if we should write a simple python script that\nwould copy comments from a mojom file to a cpp file, and keep the\ndocumentation in the .mojom file. What do you think ?\n\n> >  include/libcamera/ipa/meson.build            |  1 +\n> >  src/ipa/ipu3/ipu3.cpp                        | 14 +++----\n> >  src/libcamera/pipeline/ipu3/ipu3.cpp         | 10 +++--\n> >  5 files changed, 61 insertions(+), 13 deletions(-)\n> >  create mode 100644 include/libcamera/ipa/ipu3_ipa_interface.cpp\n> > \n> > diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom\n> > index a717b1e6..22c4ce1b 100644\n> > --- a/include/libcamera/ipa/ipu3.mojom\n> > +++ b/include/libcamera/ipa/ipu3.mojom\n> > @@ -25,13 +25,19 @@ struct IPU3Action {\n> >  \tlibcamera.ControlList controls;\n> >  };\n> >  \n> > +struct IPAConfigInfo {\n> > +\tlibcamera.IPACameraSensorInfo sensorInfo;\n> > +\tmap<uint32, ControlInfoMap> entityControls;\n> > +\tlibcamera.Size bdsOutputSize;\n> > +\tlibcamera.Size iif;\n> > +};\n> > +\n> >  interface IPAIPU3Interface {\n> >  \tinit(libcamera.IPASettings settings) => (int32 ret);\n> >  \tstart() => (int32 ret);\n> >  \tstop();\n> >  \n> > -\tconfigure(map<uint32, libcamera.ControlInfoMap> entityControls,\n> > -\t\t  libcamera.Size bdsOutputSize) => ();\n> > +\tconfigure(IPAConfigInfo configInfo);\n> >  \n> >  \tmapBuffers(array<libcamera.IPABuffer> buffers);\n> >  \tunmapBuffers(array<uint32> ids);\n> > diff --git a/include/libcamera/ipa/ipu3_ipa_interface.cpp b/include/libcamera/ipa/ipu3_ipa_interface.cpp\n> > new file mode 100644\n> > index 00000000..9aafbcdb\n> > --- /dev/null\n> > +++ b/include/libcamera/ipa/ipu3_ipa_interface.cpp\n> > @@ -0,0 +1,39 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +\n> > +namespace libcamera {\n> > +\n> > +/**\n> > + * \\struct IPAConfigInfo\n> > + * \\brief Information to be passed from IPU3 pipeline-handler to the IPA\n> > + *\n> > + * The IPAConfigInfo structure stores the data passed from IPU3 pipeline-handler\n> > + * to the IPAIPU3Interface::configure(). The structure provides extensibility\n> > + * for additional configuration data as required, for closed-source or\n> > + * open-source IPAs' configure() phases.\n> > + */\n> > +\n> > +/**\n> > + * \\var IPAConfigInfo::sensorInfo\n> > + * \\brief Camera sensor information\n> > + *\n> > + * \\sa IPACameraSensorInfo\n> > + */\n> > +\n> > +/**\n> > + * \\var IPAConfigInfo::entityControls\n> > + * \\brief Controls supported by the sensor\n> > + *\n> > + * A map of V4L2 controls supported by the sensor in order to read or write\n> > + * control values to them.\n> > + */\n> > +\n> > +/**\n> > + * \\var IPAConfigInfo::bdsOutputSize\n> > + * \\brief Size of ImgU BDS rectangle\n> > + */\n> > +\n> > +/**\n> > + * \\var IPAConfigInfo::iif\n> > + * \\brief Size of ImgU input-feeder rectangle\n> > + */\n> > +} /* namespace libcamera */\n> > diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build\n> > index da60aa68..9684a34f 100644\n> > --- a/include/libcamera/ipa/meson.build\n> > +++ b/include/libcamera/ipa/meson.build\n> > @@ -10,6 +10,7 @@ libcamera_ipa_headers = files([\n> >  \n> >  libcamera_ipa_docs = files([\n> >      'core_ipa_interface.cpp',\n> > +    'ipu3_ipa_interface.cpp',\n> \n> And of course then this would get added to the libcamera sources as part\n> of src/libcamera/ipa/meson.build which would then get passed into doxygen.\n> \n> >  ])\n> >  \n> >  install_headers(libcamera_ipa_headers,\n> > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > index f5343547..769c24d3 100644\n> > --- a/src/ipa/ipu3/ipu3.cpp\n> > +++ b/src/ipa/ipu3/ipu3.cpp\n> > @@ -43,8 +43,7 @@ public:\n> >  \tint start() override;\n> >  \tvoid stop() override {}\n> >  \n> > -\tvoid configure(const std::map<uint32_t, ControlInfoMap> &entityControls,\n> > -\t\t       const Size &bdsOutputSize) override;\n> > +\tvoid configure(const IPAConfigInfo &configInfo) override;\n> >  \n> >  \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override;\n> >  \tvoid unmapBuffers(const std::vector<unsigned int> &ids) override;\n> > @@ -139,13 +138,12 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize)\n> >  \t\t\t    << (int)bdsGrid_.height << \" << \" << (int)bdsGrid_.block_height_log2 << \")\";\n> >  }\n> >  \n> > -void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls,\n> > -\t\t\tconst Size &bdsOutputSize)\n> > +void IPAIPU3::configure(const IPAConfigInfo &configInfo)\n> >  {\n> > -\tif (entityControls.empty())\n> > +\tif (configInfo.entityControls.empty())\n> >  \t\treturn;\n> >  \n> > -\tctrls_ = entityControls.at(0);\n> > +\tctrls_ = configInfo.entityControls.at(0);\n> >  \n> >  \tconst auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);\n> >  \tif (itExp == ctrls_.end()) {\n> > @@ -169,10 +167,10 @@ void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls\n> >  \n> >  \tparams_ = {};\n> >  \n> > -\tcalculateBdsGrid(bdsOutputSize);\n> > +\tcalculateBdsGrid(configInfo.bdsOutputSize);\n> >  \n> >  \tawbAlgo_ = std::make_unique<IPU3Awb>();\n> > -\tawbAlgo_->initialise(params_, bdsOutputSize, bdsGrid_);\n> > +\tawbAlgo_->initialise(params_, configInfo.bdsOutputSize, bdsGrid_);\n> >  \n> >  \tagcAlgo_ = std::make_unique<IPU3Agc>();\n> >  \tagcAlgo_->initialise(bdsGrid_);\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 98c6160f..5b15ca90 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -633,9 +633,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> >  \t\treturn ret;\n> >  \t}\n> >  \n> > -\tstd::map<uint32_t, ControlInfoMap> entityControls;\n> > -\tentityControls.emplace(0, data->cio2_.sensor()->controls());\n> > -\tdata->ipa_->configure(entityControls, config->imguConfig().bds);\n> > +\tipa::ipu3::IPAConfigInfo configInfo;\n> > +\tconfigInfo.entityControls.emplace(0, data->cio2_.sensor()->controls());\n> > +\tconfigInfo.sensorInfo = sensorInfo;\n> > +\tconfigInfo.bdsOutputSize = config->imguConfig().bds;\n> > +\tconfigInfo.iif = config->imguConfig().iif;\n> > +\n> > +\tdata->ipa_->configure(configInfo);\n> \n> Otherwise, the code move looks ok to me.\n> \n> >  \n> >  \treturn 0;\n> >  }","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DED0FC31F6\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 15 May 2021 16:30:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 495DB6891D;\n\tSat, 15 May 2021 18:30:24 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5802B6891A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 15 May 2021 18:30:23 +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 C2EB9436;\n\tSat, 15 May 2021 18:30:22 +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=\"H/c+jXe6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621096223;\n\tbh=77EatoFAq5mUulz90cYfqJIq+d6UBrtokUi+bzh2ebg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=H/c+jXe69bFr0TimTf56sdkpKhPNTuLMeowoXH4WIAJ0ZGJfJQ7WnKub/D33IWaDA\n\t5MuWALhqF4enix6ApYwrAnlT8cOE+m+5MPqQitJijfStvbqh8+EY8Khpa1Zg8gZiT+\n\t6vorOZMGq7EYyRvmIQb4lNWjOv3UvMSFdXzH3O9U=","Date":"Sat, 15 May 2021 19:30:13 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<YJ/3FXUplnr+51Zn@pendragon.ideasonboard.com>","References":"<20210514075808.282479-1-umang.jain@ideasonboard.com>\n\t<20210514075808.282479-4-umang.jain@ideasonboard.com>\n\t<0500ee30-7dc8-97a0-8e45-f718f7ca3d12@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<0500ee30-7dc8-97a0-8e45-f718f7ca3d12@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v1 3/6] ipa: ipu3: Introduce\n\tIPAConfigInfo in IPC","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","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\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":16998,"web_url":"https://patchwork.libcamera.org/comment/16998/","msgid":"<20210518094332.GB195599@pyrite.rasen.tech>","date":"2021-05-18T09:43:32","subject":"Re: [libcamera-devel] [PATCH v1 3/6] ipa: ipu3: Introduce\n\tIPAConfigInfo in IPC","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Laurent,\n\nOn Sat, May 15, 2021 at 07:30:13PM +0300, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> On Fri, May 14, 2021 at 10:57:24AM +0100, Kieran Bingham wrote:\n> > On 14/05/2021 08:58, Umang Jain wrote:\n> > > IPAConfigInfo is a consolidated data structure passed from IPU3\n> > > pipeline-handler to IPU3 IPA. The structure can be extended with\n> > > additional parameiters to accommodate the requirements of multiple\n> > \n> > s/parameiters/parameters/\n> > \n> > > IPU3 IPA modules for e.g., an open-source IPA (in-tree) and a closed\n> > > source IPA (for ChromeOS).\n> > \n> > No need to have the expression after the e.g. It's simply to accommodate\n> > the parameters for the IPU3 IPAs.\n> > \n> > In fact, this isn't related to supporting multiple IPAs at all. This\n> > simply defines the interface. The fact that we have multiple\n> > implementations is just a fact on top of that.\n> > \n> > > As usual, the documentation for .mojom files are kept in a separate\n> > > .cpp file. Hence, create and document IPAConfigInfo in\n> > > ipu3_ipa_interface.cpp.\n> > > \n> > > Finally, adapt the in-tree IPU3 IPA to use IPAConfigInfo as well.\n> > > \n> > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> > > ---\n> > >  include/libcamera/ipa/ipu3.mojom             | 10 ++++-\n> > >  include/libcamera/ipa/ipu3_ipa_interface.cpp | 39 ++++++++++++++++++++\n> > \n> > Ok, now I really think we need to have the ipa directory under\n> > src/libcamera/ipa as a prerequisite to this series.\n> > \n> > Let me know if you'll do that or if you prefer me to do it.\n> \n> I'm beginning to wonder if we should write a simple python script that\n> would copy comments from a mojom file to a cpp file, and keep the\n> documentation in the .mojom file. What do you think ?\n\ntbh that's what I wanted to do, but I thought that was over-engineering\nit. Maybe it's worth it to stop the clutter from docs-only cpp files?\n\n\nPaul\n\n> \n> > >  include/libcamera/ipa/meson.build            |  1 +\n> > >  src/ipa/ipu3/ipu3.cpp                        | 14 +++----\n> > >  src/libcamera/pipeline/ipu3/ipu3.cpp         | 10 +++--\n> > >  5 files changed, 61 insertions(+), 13 deletions(-)\n> > >  create mode 100644 include/libcamera/ipa/ipu3_ipa_interface.cpp\n> > > \n> > > diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom\n> > > index a717b1e6..22c4ce1b 100644\n> > > --- a/include/libcamera/ipa/ipu3.mojom\n> > > +++ b/include/libcamera/ipa/ipu3.mojom\n> > > @@ -25,13 +25,19 @@ struct IPU3Action {\n> > >  \tlibcamera.ControlList controls;\n> > >  };\n> > >  \n> > > +struct IPAConfigInfo {\n> > > +\tlibcamera.IPACameraSensorInfo sensorInfo;\n> > > +\tmap<uint32, ControlInfoMap> entityControls;\n> > > +\tlibcamera.Size bdsOutputSize;\n> > > +\tlibcamera.Size iif;\n> > > +};\n> > > +\n> > >  interface IPAIPU3Interface {\n> > >  \tinit(libcamera.IPASettings settings) => (int32 ret);\n> > >  \tstart() => (int32 ret);\n> > >  \tstop();\n> > >  \n> > > -\tconfigure(map<uint32, libcamera.ControlInfoMap> entityControls,\n> > > -\t\t  libcamera.Size bdsOutputSize) => ();\n> > > +\tconfigure(IPAConfigInfo configInfo);\n> > >  \n> > >  \tmapBuffers(array<libcamera.IPABuffer> buffers);\n> > >  \tunmapBuffers(array<uint32> ids);\n> > > diff --git a/include/libcamera/ipa/ipu3_ipa_interface.cpp b/include/libcamera/ipa/ipu3_ipa_interface.cpp\n> > > new file mode 100644\n> > > index 00000000..9aafbcdb\n> > > --- /dev/null\n> > > +++ b/include/libcamera/ipa/ipu3_ipa_interface.cpp\n> > > @@ -0,0 +1,39 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +/**\n> > > + * \\struct IPAConfigInfo\n> > > + * \\brief Information to be passed from IPU3 pipeline-handler to the IPA\n> > > + *\n> > > + * The IPAConfigInfo structure stores the data passed from IPU3 pipeline-handler\n> > > + * to the IPAIPU3Interface::configure(). The structure provides extensibility\n> > > + * for additional configuration data as required, for closed-source or\n> > > + * open-source IPAs' configure() phases.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\var IPAConfigInfo::sensorInfo\n> > > + * \\brief Camera sensor information\n> > > + *\n> > > + * \\sa IPACameraSensorInfo\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\var IPAConfigInfo::entityControls\n> > > + * \\brief Controls supported by the sensor\n> > > + *\n> > > + * A map of V4L2 controls supported by the sensor in order to read or write\n> > > + * control values to them.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\var IPAConfigInfo::bdsOutputSize\n> > > + * \\brief Size of ImgU BDS rectangle\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\var IPAConfigInfo::iif\n> > > + * \\brief Size of ImgU input-feeder rectangle\n> > > + */\n> > > +} /* namespace libcamera */\n> > > diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build\n> > > index da60aa68..9684a34f 100644\n> > > --- a/include/libcamera/ipa/meson.build\n> > > +++ b/include/libcamera/ipa/meson.build\n> > > @@ -10,6 +10,7 @@ libcamera_ipa_headers = files([\n> > >  \n> > >  libcamera_ipa_docs = files([\n> > >      'core_ipa_interface.cpp',\n> > > +    'ipu3_ipa_interface.cpp',\n> > \n> > And of course then this would get added to the libcamera sources as part\n> > of src/libcamera/ipa/meson.build which would then get passed into doxygen.\n> > \n> > >  ])\n> > >  \n> > >  install_headers(libcamera_ipa_headers,\n> > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > > index f5343547..769c24d3 100644\n> > > --- a/src/ipa/ipu3/ipu3.cpp\n> > > +++ b/src/ipa/ipu3/ipu3.cpp\n> > > @@ -43,8 +43,7 @@ public:\n> > >  \tint start() override;\n> > >  \tvoid stop() override {}\n> > >  \n> > > -\tvoid configure(const std::map<uint32_t, ControlInfoMap> &entityControls,\n> > > -\t\t       const Size &bdsOutputSize) override;\n> > > +\tvoid configure(const IPAConfigInfo &configInfo) override;\n> > >  \n> > >  \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override;\n> > >  \tvoid unmapBuffers(const std::vector<unsigned int> &ids) override;\n> > > @@ -139,13 +138,12 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize)\n> > >  \t\t\t    << (int)bdsGrid_.height << \" << \" << (int)bdsGrid_.block_height_log2 << \")\";\n> > >  }\n> > >  \n> > > -void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls,\n> > > -\t\t\tconst Size &bdsOutputSize)\n> > > +void IPAIPU3::configure(const IPAConfigInfo &configInfo)\n> > >  {\n> > > -\tif (entityControls.empty())\n> > > +\tif (configInfo.entityControls.empty())\n> > >  \t\treturn;\n> > >  \n> > > -\tctrls_ = entityControls.at(0);\n> > > +\tctrls_ = configInfo.entityControls.at(0);\n> > >  \n> > >  \tconst auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);\n> > >  \tif (itExp == ctrls_.end()) {\n> > > @@ -169,10 +167,10 @@ void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls\n> > >  \n> > >  \tparams_ = {};\n> > >  \n> > > -\tcalculateBdsGrid(bdsOutputSize);\n> > > +\tcalculateBdsGrid(configInfo.bdsOutputSize);\n> > >  \n> > >  \tawbAlgo_ = std::make_unique<IPU3Awb>();\n> > > -\tawbAlgo_->initialise(params_, bdsOutputSize, bdsGrid_);\n> > > +\tawbAlgo_->initialise(params_, configInfo.bdsOutputSize, bdsGrid_);\n> > >  \n> > >  \tagcAlgo_ = std::make_unique<IPU3Agc>();\n> > >  \tagcAlgo_->initialise(bdsGrid_);\n> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > index 98c6160f..5b15ca90 100644\n> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > @@ -633,9 +633,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > >  \t\treturn ret;\n> > >  \t}\n> > >  \n> > > -\tstd::map<uint32_t, ControlInfoMap> entityControls;\n> > > -\tentityControls.emplace(0, data->cio2_.sensor()->controls());\n> > > -\tdata->ipa_->configure(entityControls, config->imguConfig().bds);\n> > > +\tipa::ipu3::IPAConfigInfo configInfo;\n> > > +\tconfigInfo.entityControls.emplace(0, data->cio2_.sensor()->controls());\n> > > +\tconfigInfo.sensorInfo = sensorInfo;\n> > > +\tconfigInfo.bdsOutputSize = config->imguConfig().bds;\n> > > +\tconfigInfo.iif = config->imguConfig().iif;\n> > > +\n> > > +\tdata->ipa_->configure(configInfo);\n> > \n> > Otherwise, the code move looks ok to me.\n> > \n> > >  \n> > >  \treturn 0;\n> > >  }","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C67C6C31FF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 18 May 2021 09:43:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3E2186891B;\n\tTue, 18 May 2021 11:43:42 +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 B5C79602B1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 18 May 2021 11:43:40 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D0D9E102;\n\tTue, 18 May 2021 11:43:38 +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=\"ufqGT5Kp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621331020;\n\tbh=/U7wfOKd4HAph4NYLPrkTeq/PmH1ja4y2hzfGLz6dWw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ufqGT5KpjWX7h8uudkTOrF68omZ9zbkszeKez0p60INIG0ZvYtLe6o+RxMWL/nFDU\n\tbRz1boQgCiPg/Xrgbd0bHw1FTUUUHKYLGSrS98imGB6t6mdN3dfSloFofzw9e/Ie/m\n\tcP6AcQU2j3Opv0DVlVAoFISXzb2pm6PqWMVe2UAU=","Date":"Tue, 18 May 2021 18:43:32 +0900","From":"paul.elder@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20210518094332.GB195599@pyrite.rasen.tech>","References":"<20210514075808.282479-1-umang.jain@ideasonboard.com>\n\t<20210514075808.282479-4-umang.jain@ideasonboard.com>\n\t<0500ee30-7dc8-97a0-8e45-f718f7ca3d12@ideasonboard.com>\n\t<YJ/3FXUplnr+51Zn@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<YJ/3FXUplnr+51Zn@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v1 3/6] ipa: ipu3: Introduce\n\tIPAConfigInfo in IPC","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":16999,"web_url":"https://patchwork.libcamera.org/comment/16999/","msgid":"<YKONLDYO+cdKfFIH@pendragon.ideasonboard.com>","date":"2021-05-18T09:47:24","subject":"Re: [libcamera-devel] [PATCH v1 3/6] ipa: ipu3: Introduce\n\tIPAConfigInfo in IPC","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nOn Tue, May 18, 2021 at 06:43:32PM +0900, paul.elder@ideasonboard.com wrote:\n> On Sat, May 15, 2021 at 07:30:13PM +0300, Laurent Pinchart wrote:\n> > On Fri, May 14, 2021 at 10:57:24AM +0100, Kieran Bingham wrote:\n> > > On 14/05/2021 08:58, Umang Jain wrote:\n> > > > IPAConfigInfo is a consolidated data structure passed from IPU3\n> > > > pipeline-handler to IPU3 IPA. The structure can be extended with\n> > > > additional parameiters to accommodate the requirements of multiple\n> > > \n> > > s/parameiters/parameters/\n> > > \n> > > > IPU3 IPA modules for e.g., an open-source IPA (in-tree) and a closed\n> > > > source IPA (for ChromeOS).\n> > > \n> > > No need to have the expression after the e.g. It's simply to accommodate\n> > > the parameters for the IPU3 IPAs.\n> > > \n> > > In fact, this isn't related to supporting multiple IPAs at all. This\n> > > simply defines the interface. The fact that we have multiple\n> > > implementations is just a fact on top of that.\n> > > \n> > > > As usual, the documentation for .mojom files are kept in a separate\n> > > > .cpp file. Hence, create and document IPAConfigInfo in\n> > > > ipu3_ipa_interface.cpp.\n> > > > \n> > > > Finally, adapt the in-tree IPU3 IPA to use IPAConfigInfo as well.\n> > > > \n> > > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> > > > ---\n> > > >  include/libcamera/ipa/ipu3.mojom             | 10 ++++-\n> > > >  include/libcamera/ipa/ipu3_ipa_interface.cpp | 39 ++++++++++++++++++++\n> > > \n> > > Ok, now I really think we need to have the ipa directory under\n> > > src/libcamera/ipa as a prerequisite to this series.\n> > > \n> > > Let me know if you'll do that or if you prefer me to do it.\n> > \n> > I'm beginning to wonder if we should write a simple python script that\n> > would copy comments from a mojom file to a cpp file, and keep the\n> > documentation in the .mojom file. What do you think ?\n> \n> tbh that's what I wanted to do, but I thought that was over-engineering\n> it. Maybe it's worth it to stop the clutter from docs-only cpp files?\n\nCan the mojo parser help there, could it be leveraged to extract\ncomments, or does it ignore them completely ?\n\n> > > >  include/libcamera/ipa/meson.build            |  1 +\n> > > >  src/ipa/ipu3/ipu3.cpp                        | 14 +++----\n> > > >  src/libcamera/pipeline/ipu3/ipu3.cpp         | 10 +++--\n> > > >  5 files changed, 61 insertions(+), 13 deletions(-)\n> > > >  create mode 100644 include/libcamera/ipa/ipu3_ipa_interface.cpp\n> > > > \n> > > > diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom\n> > > > index a717b1e6..22c4ce1b 100644\n> > > > --- a/include/libcamera/ipa/ipu3.mojom\n> > > > +++ b/include/libcamera/ipa/ipu3.mojom\n> > > > @@ -25,13 +25,19 @@ struct IPU3Action {\n> > > >  \tlibcamera.ControlList controls;\n> > > >  };\n> > > >  \n> > > > +struct IPAConfigInfo {\n> > > > +\tlibcamera.IPACameraSensorInfo sensorInfo;\n> > > > +\tmap<uint32, ControlInfoMap> entityControls;\n> > > > +\tlibcamera.Size bdsOutputSize;\n> > > > +\tlibcamera.Size iif;\n> > > > +};\n> > > > +\n> > > >  interface IPAIPU3Interface {\n> > > >  \tinit(libcamera.IPASettings settings) => (int32 ret);\n> > > >  \tstart() => (int32 ret);\n> > > >  \tstop();\n> > > >  \n> > > > -\tconfigure(map<uint32, libcamera.ControlInfoMap> entityControls,\n> > > > -\t\t  libcamera.Size bdsOutputSize) => ();\n> > > > +\tconfigure(IPAConfigInfo configInfo);\n> > > >  \n> > > >  \tmapBuffers(array<libcamera.IPABuffer> buffers);\n> > > >  \tunmapBuffers(array<uint32> ids);\n> > > > diff --git a/include/libcamera/ipa/ipu3_ipa_interface.cpp b/include/libcamera/ipa/ipu3_ipa_interface.cpp\n> > > > new file mode 100644\n> > > > index 00000000..9aafbcdb\n> > > > --- /dev/null\n> > > > +++ b/include/libcamera/ipa/ipu3_ipa_interface.cpp\n> > > > @@ -0,0 +1,39 @@\n> > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > > +\n> > > > +namespace libcamera {\n> > > > +\n> > > > +/**\n> > > > + * \\struct IPAConfigInfo\n> > > > + * \\brief Information to be passed from IPU3 pipeline-handler to the IPA\n> > > > + *\n> > > > + * The IPAConfigInfo structure stores the data passed from IPU3 pipeline-handler\n> > > > + * to the IPAIPU3Interface::configure(). The structure provides extensibility\n> > > > + * for additional configuration data as required, for closed-source or\n> > > > + * open-source IPAs' configure() phases.\n> > > > + */\n> > > > +\n> > > > +/**\n> > > > + * \\var IPAConfigInfo::sensorInfo\n> > > > + * \\brief Camera sensor information\n> > > > + *\n> > > > + * \\sa IPACameraSensorInfo\n> > > > + */\n> > > > +\n> > > > +/**\n> > > > + * \\var IPAConfigInfo::entityControls\n> > > > + * \\brief Controls supported by the sensor\n> > > > + *\n> > > > + * A map of V4L2 controls supported by the sensor in order to read or write\n> > > > + * control values to them.\n> > > > + */\n> > > > +\n> > > > +/**\n> > > > + * \\var IPAConfigInfo::bdsOutputSize\n> > > > + * \\brief Size of ImgU BDS rectangle\n> > > > + */\n> > > > +\n> > > > +/**\n> > > > + * \\var IPAConfigInfo::iif\n> > > > + * \\brief Size of ImgU input-feeder rectangle\n> > > > + */\n> > > > +} /* namespace libcamera */\n> > > > diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build\n> > > > index da60aa68..9684a34f 100644\n> > > > --- a/include/libcamera/ipa/meson.build\n> > > > +++ b/include/libcamera/ipa/meson.build\n> > > > @@ -10,6 +10,7 @@ libcamera_ipa_headers = files([\n> > > >  \n> > > >  libcamera_ipa_docs = files([\n> > > >      'core_ipa_interface.cpp',\n> > > > +    'ipu3_ipa_interface.cpp',\n> > > \n> > > And of course then this would get added to the libcamera sources as part\n> > > of src/libcamera/ipa/meson.build which would then get passed into doxygen.\n> > > \n> > > >  ])\n> > > >  \n> > > >  install_headers(libcamera_ipa_headers,\n> > > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> > > > index f5343547..769c24d3 100644\n> > > > --- a/src/ipa/ipu3/ipu3.cpp\n> > > > +++ b/src/ipa/ipu3/ipu3.cpp\n> > > > @@ -43,8 +43,7 @@ public:\n> > > >  \tint start() override;\n> > > >  \tvoid stop() override {}\n> > > >  \n> > > > -\tvoid configure(const std::map<uint32_t, ControlInfoMap> &entityControls,\n> > > > -\t\t       const Size &bdsOutputSize) override;\n> > > > +\tvoid configure(const IPAConfigInfo &configInfo) override;\n> > > >  \n> > > >  \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override;\n> > > >  \tvoid unmapBuffers(const std::vector<unsigned int> &ids) override;\n> > > > @@ -139,13 +138,12 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize)\n> > > >  \t\t\t    << (int)bdsGrid_.height << \" << \" << (int)bdsGrid_.block_height_log2 << \")\";\n> > > >  }\n> > > >  \n> > > > -void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls,\n> > > > -\t\t\tconst Size &bdsOutputSize)\n> > > > +void IPAIPU3::configure(const IPAConfigInfo &configInfo)\n> > > >  {\n> > > > -\tif (entityControls.empty())\n> > > > +\tif (configInfo.entityControls.empty())\n> > > >  \t\treturn;\n> > > >  \n> > > > -\tctrls_ = entityControls.at(0);\n> > > > +\tctrls_ = configInfo.entityControls.at(0);\n> > > >  \n> > > >  \tconst auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);\n> > > >  \tif (itExp == ctrls_.end()) {\n> > > > @@ -169,10 +167,10 @@ void IPAIPU3::configure(const std::map<uint32_t, ControlInfoMap> &entityControls\n> > > >  \n> > > >  \tparams_ = {};\n> > > >  \n> > > > -\tcalculateBdsGrid(bdsOutputSize);\n> > > > +\tcalculateBdsGrid(configInfo.bdsOutputSize);\n> > > >  \n> > > >  \tawbAlgo_ = std::make_unique<IPU3Awb>();\n> > > > -\tawbAlgo_->initialise(params_, bdsOutputSize, bdsGrid_);\n> > > > +\tawbAlgo_->initialise(params_, configInfo.bdsOutputSize, bdsGrid_);\n> > > >  \n> > > >  \tagcAlgo_ = std::make_unique<IPU3Agc>();\n> > > >  \tagcAlgo_->initialise(bdsGrid_);\n> > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > index 98c6160f..5b15ca90 100644\n> > > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > @@ -633,9 +633,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > > >  \t\treturn ret;\n> > > >  \t}\n> > > >  \n> > > > -\tstd::map<uint32_t, ControlInfoMap> entityControls;\n> > > > -\tentityControls.emplace(0, data->cio2_.sensor()->controls());\n> > > > -\tdata->ipa_->configure(entityControls, config->imguConfig().bds);\n> > > > +\tipa::ipu3::IPAConfigInfo configInfo;\n> > > > +\tconfigInfo.entityControls.emplace(0, data->cio2_.sensor()->controls());\n> > > > +\tconfigInfo.sensorInfo = sensorInfo;\n> > > > +\tconfigInfo.bdsOutputSize = config->imguConfig().bds;\n> > > > +\tconfigInfo.iif = config->imguConfig().iif;\n> > > > +\n> > > > +\tdata->ipa_->configure(configInfo);\n> > > \n> > > Otherwise, the code move looks ok to me.\n> > > \n> > > >  \n> > > >  \treturn 0;\n> > > >  }","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8885BC31FB\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 18 May 2021 09:47:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DA3E468920;\n\tTue, 18 May 2021 11:47:27 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 58179602B1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 18 May 2021 11:47:26 +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 CA090102;\n\tTue, 18 May 2021 11:47:25 +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=\"WCpBLEdw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621331246;\n\tbh=DNJHs0c7e4hMCeD85wR562HMhoQQL2Dkx78Zk+nbSvE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WCpBLEdwd7enKPy5wP8pT0LQOchXbxEf4R6bCNppgTXNctmIvC8P+X1Cq9NKtBfgm\n\ttIWQJi8MTXBo6Rr8a/3x7Jn8JBKKulicsyhqLrKpxj6GvBfNsn9wNqtELlEVoArGTJ\n\tlAYRrWSBAV6fM6egrMKzfvUJ6dcW3H+WZtLF+6pU=","Date":"Tue, 18 May 2021 12:47:24 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"paul.elder@ideasonboard.com","Message-ID":"<YKONLDYO+cdKfFIH@pendragon.ideasonboard.com>","References":"<20210514075808.282479-1-umang.jain@ideasonboard.com>\n\t<20210514075808.282479-4-umang.jain@ideasonboard.com>\n\t<0500ee30-7dc8-97a0-8e45-f718f7ca3d12@ideasonboard.com>\n\t<YJ/3FXUplnr+51Zn@pendragon.ideasonboard.com>\n\t<20210518094332.GB195599@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210518094332.GB195599@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [PATCH v1 3/6] ipa: ipu3: Introduce\n\tIPAConfigInfo in IPC","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":17000,"web_url":"https://patchwork.libcamera.org/comment/17000/","msgid":"<20210518100640.GC195599@pyrite.rasen.tech>","date":"2021-05-18T10:06:40","subject":"Re: [libcamera-devel] [PATCH v1 3/6] ipa: ipu3: Introduce\n\tIPAConfigInfo in IPC","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Laurent,\n\nOn Tue, May 18, 2021 at 12:47:24PM +0300, Laurent Pinchart wrote:\n> Hi Paul,\n> \n> On Tue, May 18, 2021 at 06:43:32PM +0900, paul.elder@ideasonboard.com wrote:\n> > On Sat, May 15, 2021 at 07:30:13PM +0300, Laurent Pinchart wrote:\n> > > On Fri, May 14, 2021 at 10:57:24AM +0100, Kieran Bingham wrote:\n> > > > On 14/05/2021 08:58, Umang Jain wrote:\n> > > > > IPAConfigInfo is a consolidated data structure passed from IPU3\n> > > > > pipeline-handler to IPU3 IPA. The structure can be extended with\n> > > > > additional parameiters to accommodate the requirements of multiple\n> > > > \n> > > > s/parameiters/parameters/\n> > > > \n> > > > > IPU3 IPA modules for e.g., an open-source IPA (in-tree) and a closed\n> > > > > source IPA (for ChromeOS).\n> > > > \n> > > > No need to have the expression after the e.g. It's simply to accommodate\n> > > > the parameters for the IPU3 IPAs.\n> > > > \n> > > > In fact, this isn't related to supporting multiple IPAs at all. This\n> > > > simply defines the interface. The fact that we have multiple\n> > > > implementations is just a fact on top of that.\n> > > > \n> > > > > As usual, the documentation for .mojom files are kept in a separate\n> > > > > .cpp file. Hence, create and document IPAConfigInfo in\n> > > > > ipu3_ipa_interface.cpp.\n> > > > > \n> > > > > Finally, adapt the in-tree IPU3 IPA to use IPAConfigInfo as well.\n> > > > > \n> > > > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> > > > > ---\n> > > > >  include/libcamera/ipa/ipu3.mojom             | 10 ++++-\n> > > > >  include/libcamera/ipa/ipu3_ipa_interface.cpp | 39 ++++++++++++++++++++\n> > > > \n> > > > Ok, now I really think we need to have the ipa directory under\n> > > > src/libcamera/ipa as a prerequisite to this series.\n> > > > \n> > > > Let me know if you'll do that or if you prefer me to do it.\n> > > \n> > > I'm beginning to wonder if we should write a simple python script that\n> > > would copy comments from a mojom file to a cpp file, and keep the\n> > > documentation in the .mojom file. What do you think ?\n> > \n> > tbh that's what I wanted to do, but I thought that was over-engineering\n> > it. Maybe it's worth it to stop the clutter from docs-only cpp files?\n> \n> Can the mojo parser help there, could it be leveraged to extract\n> comments, or does it ignore them completely ?\n\nThat's why I didn't pursue that route; the mojo parser ignores comments\ncompletely :/\n\nActually it's before that, they're dropped by the lexer.\n\n\nPaul\n\n<snip>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9F92EC31FF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 18 May 2021 10:06:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F0B846891C;\n\tTue, 18 May 2021 12:06:48 +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 14642602B2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 18 May 2021 12:06:48 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2F8E8102;\n\tTue, 18 May 2021 12:06:45 +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=\"nEIKPuH0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621332407;\n\tbh=GrFc03TxRhxJEVOHXo2FIyoOj0UYrOrrF1sS2r/WFTw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nEIKPuH0R7BbNAq1BMq7rPu276Rumj0YiUNLkkwCisZqFYLpY7wZo8bHwXcbz7F+Q\n\tvw2+7mBzBOkcA1OSCoKUzcpIGiaCkaLFHRrsGtbxmOCuCFsThhMlYxTWc92DbUrkTI\n\ttvY+ZYRmaVR28sakH+9J3/frf0H5PGqTEZfhT1yI=","Date":"Tue, 18 May 2021 19:06:40 +0900","From":"paul.elder@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20210518100640.GC195599@pyrite.rasen.tech>","References":"<20210514075808.282479-1-umang.jain@ideasonboard.com>\n\t<20210514075808.282479-4-umang.jain@ideasonboard.com>\n\t<0500ee30-7dc8-97a0-8e45-f718f7ca3d12@ideasonboard.com>\n\t<YJ/3FXUplnr+51Zn@pendragon.ideasonboard.com>\n\t<20210518094332.GB195599@pyrite.rasen.tech>\n\t<YKONLDYO+cdKfFIH@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<YKONLDYO+cdKfFIH@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v1 3/6] ipa: ipu3: Introduce\n\tIPAConfigInfo in IPC","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":17001,"web_url":"https://patchwork.libcamera.org/comment/17001/","msgid":"<YKOWdBq/YjBTbbty@pendragon.ideasonboard.com>","date":"2021-05-18T10:27:00","subject":"Re: [libcamera-devel] [PATCH v1 3/6] ipa: ipu3: Introduce\n\tIPAConfigInfo in IPC","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nOn Tue, May 18, 2021 at 07:06:40PM +0900, paul.elder@ideasonboard.com wrote:\n> On Tue, May 18, 2021 at 12:47:24PM +0300, Laurent Pinchart wrote:\n> > On Tue, May 18, 2021 at 06:43:32PM +0900, paul.elder@ideasonboard.com wrote:\n> > > On Sat, May 15, 2021 at 07:30:13PM +0300, Laurent Pinchart wrote:\n> > > > On Fri, May 14, 2021 at 10:57:24AM +0100, Kieran Bingham wrote:\n> > > > > On 14/05/2021 08:58, Umang Jain wrote:\n> > > > > > IPAConfigInfo is a consolidated data structure passed from IPU3\n> > > > > > pipeline-handler to IPU3 IPA. The structure can be extended with\n> > > > > > additional parameiters to accommodate the requirements of multiple\n> > > > > \n> > > > > s/parameiters/parameters/\n> > > > > \n> > > > > > IPU3 IPA modules for e.g., an open-source IPA (in-tree) and a closed\n> > > > > > source IPA (for ChromeOS).\n> > > > > \n> > > > > No need to have the expression after the e.g. It's simply to accommodate\n> > > > > the parameters for the IPU3 IPAs.\n> > > > > \n> > > > > In fact, this isn't related to supporting multiple IPAs at all. This\n> > > > > simply defines the interface. The fact that we have multiple\n> > > > > implementations is just a fact on top of that.\n> > > > > \n> > > > > > As usual, the documentation for .mojom files are kept in a separate\n> > > > > > .cpp file. Hence, create and document IPAConfigInfo in\n> > > > > > ipu3_ipa_interface.cpp.\n> > > > > > \n> > > > > > Finally, adapt the in-tree IPU3 IPA to use IPAConfigInfo as well.\n> > > > > > \n> > > > > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> > > > > > ---\n> > > > > >  include/libcamera/ipa/ipu3.mojom             | 10 ++++-\n> > > > > >  include/libcamera/ipa/ipu3_ipa_interface.cpp | 39 ++++++++++++++++++++\n> > > > > \n> > > > > Ok, now I really think we need to have the ipa directory under\n> > > > > src/libcamera/ipa as a prerequisite to this series.\n> > > > > \n> > > > > Let me know if you'll do that or if you prefer me to do it.\n> > > > \n> > > > I'm beginning to wonder if we should write a simple python script that\n> > > > would copy comments from a mojom file to a cpp file, and keep the\n> > > > documentation in the .mojom file. What do you think ?\n> > > \n> > > tbh that's what I wanted to do, but I thought that was over-engineering\n> > > it. Maybe it's worth it to stop the clutter from docs-only cpp files?\n> > \n> > Can the mojo parser help there, could it be leveraged to extract\n> > comments, or does it ignore them completely ?\n> \n> That's why I didn't pursue that route; the mojo parser ignores comments\n> completely :/\n> \n> Actually it's before that, they're dropped by the lexer.\n\nLet's not go down that rabbit hole then. Should you however file a bug\nreport against mojom to ask for this feature ? If you think it's a good\nidea, could you do so ?","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 230B9C31FF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 18 May 2021 10:27:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 816A76891C;\n\tTue, 18 May 2021 12:27:02 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 47AEB602B2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 18 May 2021 12:27:01 +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 B53A3102;\n\tTue, 18 May 2021 12:27:00 +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=\"WsBATpFl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621333620;\n\tbh=tCaAhg9ns8eQ02jgxmxW0SRTVFr8rtbAkamzyZjb5mw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WsBATpFlz0Pz00DSSYuKm3TqacWoPvM800PVxZI+1zTNM3Drc1zgxi+N9QELPa1gH\n\th57zHmMVxdI4fLzgDiJkUFLc67Eo+UP9R9Tx4DEoRI+2Ve3vgJRsg9lgFmRaJvZ3cy\n\tmMST3yWOXhXFMUST6NU8fB+VyV54kCIQbD0JHBqA=","Date":"Tue, 18 May 2021 13:27:00 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"paul.elder@ideasonboard.com","Message-ID":"<YKOWdBq/YjBTbbty@pendragon.ideasonboard.com>","References":"<20210514075808.282479-1-umang.jain@ideasonboard.com>\n\t<20210514075808.282479-4-umang.jain@ideasonboard.com>\n\t<0500ee30-7dc8-97a0-8e45-f718f7ca3d12@ideasonboard.com>\n\t<YJ/3FXUplnr+51Zn@pendragon.ideasonboard.com>\n\t<20210518094332.GB195599@pyrite.rasen.tech>\n\t<YKONLDYO+cdKfFIH@pendragon.ideasonboard.com>\n\t<20210518100640.GC195599@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210518100640.GC195599@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [PATCH v1 3/6] ipa: ipu3: Introduce\n\tIPAConfigInfo in IPC","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":17009,"web_url":"https://patchwork.libcamera.org/comment/17009/","msgid":"<20210519025113.GD195599@pyrite.rasen.tech>","date":"2021-05-19T02:51:13","subject":"Re: [libcamera-devel] [PATCH v1 3/6] ipa: ipu3: Introduce\n\tIPAConfigInfo in IPC","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Laurent,\n\nOn Tue, May 18, 2021 at 01:27:00PM +0300, Laurent Pinchart wrote:\n> Hi Paul,\n> \n> On Tue, May 18, 2021 at 07:06:40PM +0900, paul.elder@ideasonboard.com wrote:\n> > On Tue, May 18, 2021 at 12:47:24PM +0300, Laurent Pinchart wrote:\n> > > On Tue, May 18, 2021 at 06:43:32PM +0900, paul.elder@ideasonboard.com wrote:\n> > > > On Sat, May 15, 2021 at 07:30:13PM +0300, Laurent Pinchart wrote:\n> > > > > On Fri, May 14, 2021 at 10:57:24AM +0100, Kieran Bingham wrote:\n> > > > > > On 14/05/2021 08:58, Umang Jain wrote:\n> > > > > > > IPAConfigInfo is a consolidated data structure passed from IPU3\n> > > > > > > pipeline-handler to IPU3 IPA. The structure can be extended with\n> > > > > > > additional parameiters to accommodate the requirements of multiple\n> > > > > > \n> > > > > > s/parameiters/parameters/\n> > > > > > \n> > > > > > > IPU3 IPA modules for e.g., an open-source IPA (in-tree) and a closed\n> > > > > > > source IPA (for ChromeOS).\n> > > > > > \n> > > > > > No need to have the expression after the e.g. It's simply to accommodate\n> > > > > > the parameters for the IPU3 IPAs.\n> > > > > > \n> > > > > > In fact, this isn't related to supporting multiple IPAs at all. This\n> > > > > > simply defines the interface. The fact that we have multiple\n> > > > > > implementations is just a fact on top of that.\n> > > > > > \n> > > > > > > As usual, the documentation for .mojom files are kept in a separate\n> > > > > > > .cpp file. Hence, create and document IPAConfigInfo in\n> > > > > > > ipu3_ipa_interface.cpp.\n> > > > > > > \n> > > > > > > Finally, adapt the in-tree IPU3 IPA to use IPAConfigInfo as well.\n> > > > > > > \n> > > > > > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> > > > > > > ---\n> > > > > > >  include/libcamera/ipa/ipu3.mojom             | 10 ++++-\n> > > > > > >  include/libcamera/ipa/ipu3_ipa_interface.cpp | 39 ++++++++++++++++++++\n> > > > > > \n> > > > > > Ok, now I really think we need to have the ipa directory under\n> > > > > > src/libcamera/ipa as a prerequisite to this series.\n> > > > > > \n> > > > > > Let me know if you'll do that or if you prefer me to do it.\n> > > > > \n> > > > > I'm beginning to wonder if we should write a simple python script that\n> > > > > would copy comments from a mojom file to a cpp file, and keep the\n> > > > > documentation in the .mojom file. What do you think ?\n> > > > \n> > > > tbh that's what I wanted to do, but I thought that was over-engineering\n> > > > it. Maybe it's worth it to stop the clutter from docs-only cpp files?\n> > > \n> > > Can the mojo parser help there, could it be leveraged to extract\n> > > comments, or does it ignore them completely ?\n> > \n> > That's why I didn't pursue that route; the mojo parser ignores comments\n> > completely :/\n> > \n> > Actually it's before that, they're dropped by the lexer.\n> \n> Let's not go down that rabbit hole then. Should you however file a bug\n> report against mojom to ask for this feature ? If you think it's a good\n> idea, could you do so ?\n\nNo, I don't think it's a good idea. The job of the lexer is to turn the\nsequence of characters into a sequence of tokens to feed to the parser,\nand comments don't belong as a token.\n\nIt's probably better just to have a simple python script to extract the\ncomments.\n\n\nPaul","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 50B14C31FF\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 19 May 2021 02:51:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 799D06891B;\n\tWed, 19 May 2021 04:51:23 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4CAED602B0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 19 May 2021 04:51:21 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6612A45E;\n\tWed, 19 May 2021 04:51:19 +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=\"Kg4aKVXH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621392680;\n\tbh=ldYpZe9hRFrXaZI1eirLnRmULJ6JrPaZnhbmtBLcv9I=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Kg4aKVXH+vo6PhAg4n9dd10lJmmb1OqdVnsu1BUcagqOCHL59iU8np3RpSKAbvUGa\n\tdJCQjyjNzjcfbSTrMdT1TGtD30zQUklpQ3x6woD4co1Keatxkfosjy4xi2FiVqefQK\n\tEitNib1yW9YjeBBHnQjx6hL+pNbw7/EgDDI1vZsQ=","Date":"Wed, 19 May 2021 11:51:13 +0900","From":"paul.elder@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20210519025113.GD195599@pyrite.rasen.tech>","References":"<20210514075808.282479-1-umang.jain@ideasonboard.com>\n\t<20210514075808.282479-4-umang.jain@ideasonboard.com>\n\t<0500ee30-7dc8-97a0-8e45-f718f7ca3d12@ideasonboard.com>\n\t<YJ/3FXUplnr+51Zn@pendragon.ideasonboard.com>\n\t<20210518094332.GB195599@pyrite.rasen.tech>\n\t<YKONLDYO+cdKfFIH@pendragon.ideasonboard.com>\n\t<20210518100640.GC195599@pyrite.rasen.tech>\n\t<YKOWdBq/YjBTbbty@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<YKOWdBq/YjBTbbty@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v1 3/6] ipa: ipu3: Introduce\n\tIPAConfigInfo in IPC","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]