[{"id":15800,"web_url":"https://patchwork.libcamera.org/comment/15800/","msgid":"<YFkEfo+0xgTzqmYt@pendragon.ideasonboard.com>","date":"2021-03-22T20:56:30","subject":"Re: [libcamera-devel] [PATCH 1/7] ipa: Add sensor model string to\n\tIPASettings","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Wed, Mar 17, 2021 at 10:02:05AM +0000, Naushir Patuck wrote:\n> Pass the sensor model string to the IPA init() method through the\n> IPASettings structure.\n> \n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> ---\n>  include/libcamera/ipa/core.mojom                   | 8 ++++++++\n>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 3 ++-\n>  src/libcamera/pipeline/vimc/vimc.cpp               | 2 +-\n>  test/ipa/ipa_interface_test.cpp                    | 2 +-\n>  4 files changed, 12 insertions(+), 3 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom\n> index 5236a672461c..5363f1c5b48b 100644\n> --- a/include/libcamera/ipa/core.mojom\n> +++ b/include/libcamera/ipa/core.mojom\n> @@ -145,8 +145,16 @@ struct IPABuffer {\n>   * This field may be an empty string if the IPA doesn't require a configuration\n>   * file.\n>   */\n> +\n> + /**\n> + * \\var IPASettings::sensorModel\n> + * \\brief The sensor model name\n> + *\n> + * Provides the sensor model name to the IPA.\n> + */\n>  struct IPASettings {\n>  \tstring configurationFile;\n> +\tstring sensorModel;\n\nThis makes complete sense for all IPAs. I wonder if we should remove it\nfrom CameraSensorInfo then, but that can be done on top of the series.\n\n>  };\n>  \n>  /**\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 00fa62c972ed..2c8ae31a28ad 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1227,7 +1227,8 @@ int RPiCameraData::loadIPA()\n>  \tipa_->setIspControls.connect(this, &RPiCameraData::setIspControls);\n>  \tipa_->setDelayedControls.connect(this, &RPiCameraData::setDelayedControls);\n>  \n> -\tIPASettings settings(ipa_->configurationFile(sensor_->model() + \".json\"));\n> +\tIPASettings settings(ipa_->configurationFile(sensor_->model() + \".json\"),\n> +\t\t\t     sensor_->model());\n>  \n>  \treturn ipa_->init(settings);\n>  }\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 57c65b021c46..2dfb63ecf2ef 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -425,7 +425,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n>  \tdata->ipa_ = IPAManager::createIPA<ipa::vimc::IPAProxyVimc>(this, 0, 0);\n>  \tif (data->ipa_ != nullptr) {\n>  \t\tstd::string conf = data->ipa_->configurationFile(\"vimc.conf\");\n> -\t\tdata->ipa_->init(IPASettings{ conf });\n> +\t\tdata->ipa_->init(IPASettings{ conf, data->sensor_->model() });\n>  \t} else {\n>  \t\tLOG(VIMC, Warning) << \"no matching IPA found\";\n>  \t}\n> diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\n> index 4b88806f8f67..d6ca6f5137b0 100644\n> --- a/test/ipa/ipa_interface_test.cpp\n> +++ b/test/ipa/ipa_interface_test.cpp\n> @@ -104,7 +104,7 @@ protected:\n>  \n>  \t\t/* Test initialization of IPA module. */\n>  \t\tstd::string conf = ipa_->configurationFile(\"vimc.conf\");\n> -\t\tint ret = ipa_->init(IPASettings{ conf });\n> +\t\tint ret = ipa_->init(IPASettings{ conf, \"vimc\" });\n>  \t\tif (ret < 0) {\n>  \t\t\tcerr << \"IPA interface init() failed\" << endl;\n>  \t\t\treturn TestFail;\n\nCould you please also update the ipu3 and rkisp1 pipeline handlers ?","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 CF827C32E1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 22 Mar 2021 20:57:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2D38C68D62;\n\tMon, 22 Mar 2021 21:57:13 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0470D68D50\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 22 Mar 2021 21:57:12 +0100 (CET)","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 6DF6AED;\n\tMon, 22 Mar 2021 21:57:11 +0100 (CET)"],"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=\"q+D/cTmR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616446631;\n\tbh=S7m7OhgDghsf4w6/Y1n+Lgnz2E2XBhgS+oruOI9Ovfk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=q+D/cTmR+MOLB1Jd8qrOogFAa4IzgkuzsERyuinLYnteHbabasMwrvcB4mM35rzBC\n\tC0znu+cV7g7jd9tDAIBsfRqsSdHEiryrYslXwgSz6T+s9+LrINu9ggPbKbhZlBckAD\n\tw87oNNZv4ypyKGdKm3DCg7DuGMxKRQpToD/pkE/o=","Date":"Mon, 22 Mar 2021 22:56:30 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<YFkEfo+0xgTzqmYt@pendragon.ideasonboard.com>","References":"<20210317100211.1067585-1-naush@raspberrypi.com>\n\t<20210317100211.1067585-2-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210317100211.1067585-2-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 1/7] ipa: Add sensor model string to\n\tIPASettings","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":15824,"web_url":"https://patchwork.libcamera.org/comment/15824/","msgid":"<CAEmqJPpkUcsR5Xkh1C3mk63aGB+7DRDf0s3anmU88VF_=Y4=wQ@mail.gmail.com>","date":"2021-03-23T09:52:17","subject":"Re: [libcamera-devel] [PATCH 1/7] ipa: Add sensor model string to\n\tIPASettings","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThank you for your review feedb\n\nOn Mon, 22 Mar 2021 at 20:57, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Wed, Mar 17, 2021 at 10:02:05AM +0000, Naushir Patuck wrote:\n> > Pass the sensor model string to the IPA init() method through the\n> > IPASettings structure.\n> >\n> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > ---\n> >  include/libcamera/ipa/core.mojom                   | 8 ++++++++\n> >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 3 ++-\n> >  src/libcamera/pipeline/vimc/vimc.cpp               | 2 +-\n> >  test/ipa/ipa_interface_test.cpp                    | 2 +-\n> >  4 files changed, 12 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/include/libcamera/ipa/core.mojom\n> b/include/libcamera/ipa/core.mojom\n> > index 5236a672461c..5363f1c5b48b 100644\n> > --- a/include/libcamera/ipa/core.mojom\n> > +++ b/include/libcamera/ipa/core.mojom\n> > @@ -145,8 +145,16 @@ struct IPABuffer {\n> >   * This field may be an empty string if the IPA doesn't require a\n> configuration\n> >   * file.\n> >   */\n> > +\n> > + /**\n> > + * \\var IPASettings::sensorModel\n> > + * \\brief The sensor model name\n> > + *\n> > + * Provides the sensor model name to the IPA.\n> > + */\n> >  struct IPASettings {\n> >       string configurationFile;\n> > +     string sensorModel;\n>\n> This makes complete sense for all IPAs. I wonder if we should remove it\n> from CameraSensorInfo then, but that can be done on top of the series.\n>\n> >  };\n> >\n> >  /**\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 00fa62c972ed..2c8ae31a28ad 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -1227,7 +1227,8 @@ int RPiCameraData::loadIPA()\n> >       ipa_->setIspControls.connect(this, &RPiCameraData::setIspControls);\n> >       ipa_->setDelayedControls.connect(this,\n> &RPiCameraData::setDelayedControls);\n> >\n> > -     IPASettings settings(ipa_->configurationFile(sensor_->model() +\n> \".json\"));\n> > +     IPASettings settings(ipa_->configurationFile(sensor_->model() +\n> \".json\"),\n> > +                          sensor_->model());\n> >\n> >       return ipa_->init(settings);\n> >  }\n> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp\n> b/src/libcamera/pipeline/vimc/vimc.cpp\n> > index 57c65b021c46..2dfb63ecf2ef 100644\n> > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > @@ -425,7 +425,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator\n> *enumerator)\n> >       data->ipa_ = IPAManager::createIPA<ipa::vimc::IPAProxyVimc>(this,\n> 0, 0);\n> >       if (data->ipa_ != nullptr) {\n> >               std::string conf =\n> data->ipa_->configurationFile(\"vimc.conf\");\n> > -             data->ipa_->init(IPASettings{ conf });\n> > +             data->ipa_->init(IPASettings{ conf, data->sensor_->model()\n> });\n> >       } else {\n> >               LOG(VIMC, Warning) << \"no matching IPA found\";\n> >       }\n> > diff --git a/test/ipa/ipa_interface_test.cpp\n> b/test/ipa/ipa_interface_test.cpp\n> > index 4b88806f8f67..d6ca6f5137b0 100644\n> > --- a/test/ipa/ipa_interface_test.cpp\n> > +++ b/test/ipa/ipa_interface_test.cpp\n> > @@ -104,7 +104,7 @@ protected:\n> >\n> >               /* Test initialization of IPA module. */\n> >               std::string conf = ipa_->configurationFile(\"vimc.conf\");\n> > -             int ret = ipa_->init(IPASettings{ conf });\n> > +             int ret = ipa_->init(IPASettings{ conf, \"vimc\" });\n> >               if (ret < 0) {\n> >                       cerr << \"IPA interface init() failed\" << endl;\n> >                       return TestFail;\n>\n> Could you please also update the ipu3 and rkisp1 pipeline handlers ?\n>\n\nI did intend on modifying them, but they call init() in slightly different\nways:\n\nipu3: ipa_->init(IPASettings{});\nrkisp1: ipa_->init(hwRevision);\n\nrkisp1 does not use IPASettings at all, so I left it out, and ipu3\ninitialises an\nempty IPASettings object.  I could initialise ipu3 with the something like\nthe\nfollowing:\n\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp\nb/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex 2ea13ec9e1b9..0e9b05f2b771 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -1144,7 +1144,8 @@ int IPU3CameraData::loadIPA()\n\n        ipa_->queueFrameAction.connect(this,\n&IPU3CameraData::queueFrameAction);\n\n-       ipa_->init(IPASettings{});\n+       CameraSensor *sensor = cio2_.sensor();\n+       ipa_->init(IPASettings{\"\", sensor->model()});\n\nLet me know what you think is appropriate.\n\nRegards,\nNaush\n\n\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 65E09C32E5\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 23 Mar 2021 09:52:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D6D8A68D65;\n\tTue, 23 Mar 2021 10:52:37 +0100 (CET)","from mail-lj1-x22e.google.com (mail-lj1-x22e.google.com\n\t[IPv6:2a00:1450:4864:20::22e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1F75F602E3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 23 Mar 2021 10:52:36 +0100 (CET)","by mail-lj1-x22e.google.com with SMTP id 15so24840179ljj.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 23 Mar 2021 02:52:36 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"HM6XLo/b\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=KpdEDtShRkurDTq6hB3Hwt62zgaegOzc+H54XbezF5A=;\n\tb=HM6XLo/bl7DKfI0j7Ig9Fh/w2bXYj8UHwjLv9ipxkvzw5XoYNdAaf0EZROYhtT8u5I\n\t/wrklfhZx6mJnKkGjCS2aq7ge/GB9V8Yl3rAGw5A4bZjt4yYFbMwUfYf3Y/c0ieNtQ8M\n\t7Y05SNW8hwOfxkjQcN0+DkGtSEpR18hB2yPEu7htenlH1CyhxiEFQ9+AXeHOUp4CWpF7\n\tqtqLmZQ7KSo/vCa1eDhb7mDMNKPGXXriqZLLqkq0veg+rsSZ5yOaIwn8e6Uiriky6Qgj\n\tV63z2LT+kGCmN8sA0EqexQ6vQm1sTb/p0jC/pQbQsG+DsCQ3s00+RJEns8KHF6KQOo+h\n\tTpTg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=KpdEDtShRkurDTq6hB3Hwt62zgaegOzc+H54XbezF5A=;\n\tb=j868uBusGFCKcGzKXGinOHLeLgb+DSTu4V/t2iFQ2XmQpxfUyIPh8UGhj/UgmAY0oe\n\tD8sriYMpqfznwP7F/u2IR+26C1trk6xs3QRL28FdtHYzVrIuZ7t2wc9DVm3w0boIuAXe\n\tQQUpmiGGKV2Evb0cnV9Vl2eqIYxAs5MWIaSDyT3I55yoxAuvisKLHeEm5R7y9WI8Kx0C\n\tXwU7wwPsHt2t+/WOzu3nOD/uaz99C8SXo16JPdainFVSSujSpzRt0h0WH+pSk8rC8lve\n\tIhuKmNLgCOM9J57YSVfPedzJkc5E5iNIIip5QYGAVK0wD6AQy2gErS40fxVp+yoO9jLV\n\tiUNQ==","X-Gm-Message-State":"AOAM530X71tjeEmz+ByCka991CTshel11j7iLjqhG+IhI/hR+l3HwS3X\n\tcD5oQNl/UYiW0gSnMVwfHk1tfh7WbkWKlPPArTO/9DQftVCSxg==","X-Google-Smtp-Source":"ABdhPJxp3SNIYU5rwjJYp17TV5HXcW87z805uGqK/Ia/yrSGf897a3tqxVbgBrA6VukVo26lQtpV3Y3rWQuKlokl++M=","X-Received":"by 2002:a2e:9b10:: with SMTP id\n\tu16mr2444602lji.253.1616493155296; \n\tTue, 23 Mar 2021 02:52:35 -0700 (PDT)","MIME-Version":"1.0","References":"<20210317100211.1067585-1-naush@raspberrypi.com>\n\t<20210317100211.1067585-2-naush@raspberrypi.com>\n\t<YFkEfo+0xgTzqmYt@pendragon.ideasonboard.com>","In-Reply-To":"<YFkEfo+0xgTzqmYt@pendragon.ideasonboard.com>","From":"Naushir Patuck <naush@raspberrypi.com>","Date":"Tue, 23 Mar 2021 09:52:17 +0000","Message-ID":"<CAEmqJPpkUcsR5Xkh1C3mk63aGB+7DRDf0s3anmU88VF_=Y4=wQ@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/7] ipa: Add sensor model string to\n\tIPASettings","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 <libcamera-devel@lists.libcamera.org>","Content-Type":"multipart/mixed;\n\tboundary=\"===============2025795443442515090==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":15825,"web_url":"https://patchwork.libcamera.org/comment/15825/","msgid":"<YFm/iJLsZDT6yevL@pendragon.ideasonboard.com>","date":"2021-03-23T10:14:32","subject":"Re: [libcamera-devel] [PATCH 1/7] ipa: Add sensor model string to\n\tIPASettings","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Tue, Mar 23, 2021 at 09:52:17AM +0000, Naushir Patuck wrote:\n> On Mon, 22 Mar 2021 at 20:57, Laurent Pinchart wrote:\n> > On Wed, Mar 17, 2021 at 10:02:05AM +0000, Naushir Patuck wrote:\n> > > Pass the sensor model string to the IPA init() method through the\n> > > IPASettings structure.\n> > >\n> > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > > ---\n> > >  include/libcamera/ipa/core.mojom                   | 8 ++++++++\n> > >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 3 ++-\n> > >  src/libcamera/pipeline/vimc/vimc.cpp               | 2 +-\n> > >  test/ipa/ipa_interface_test.cpp                    | 2 +-\n> > >  4 files changed, 12 insertions(+), 3 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom\n> > > index 5236a672461c..5363f1c5b48b 100644\n> > > --- a/include/libcamera/ipa/core.mojom\n> > > +++ b/include/libcamera/ipa/core.mojom\n> > > @@ -145,8 +145,16 @@ struct IPABuffer {\n> > >   * This field may be an empty string if the IPA doesn't require a configuration\n> > >   * file.\n> > >   */\n> > > +\n> > > + /**\n> > > + * \\var IPASettings::sensorModel\n> > > + * \\brief The sensor model name\n> > > + *\n> > > + * Provides the sensor model name to the IPA.\n> > > + */\n> > >  struct IPASettings {\n> > >       string configurationFile;\n> > > +     string sensorModel;\n> > This makes complete sense for all IPAs. I wonder if we should remove it from CameraSensorInfo then, but that can be done on top of the series.\n> >\n> > >  };\n> > >\n> > >  /**\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index 00fa62c972ed..2c8ae31a28ad 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -1227,7 +1227,8 @@ int RPiCameraData::loadIPA()\n> > >       ipa_->setIspControls.connect(this, &RPiCameraData::setIspControls);\n> > >       ipa_->setDelayedControls.connect(this, &RPiCameraData::setDelayedControls);\n> > >\n> > > -     IPASettings settings(ipa_->configurationFile(sensor_->model() + \".json\"));\n> > > +     IPASettings settings(ipa_->configurationFile(sensor_->model() + \".json\"),\n> > > +                          sensor_->model());\n> > >\n> > >       return ipa_->init(settings);\n> > >  }\n> > > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> > > index 57c65b021c46..2dfb63ecf2ef 100644\n> > > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > > @@ -425,7 +425,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> > >       data->ipa_ = IPAManager::createIPA<ipa::vimc::IPAProxyVimc>(this, 0, 0);\n> > >       if (data->ipa_ != nullptr) {\n> > >               std::string conf = data->ipa_->configurationFile(\"vimc.conf\");\n> > > -             data->ipa_->init(IPASettings{ conf });\n> > > +             data->ipa_->init(IPASettings{ conf, data->sensor_->model() });\n> > >       } else {\n> > >               LOG(VIMC, Warning) << \"no matching IPA found\";\n> > >       }\n> > > diff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\n> > > index 4b88806f8f67..d6ca6f5137b0 100644\n> > > --- a/test/ipa/ipa_interface_test.cpp\n> > > +++ b/test/ipa/ipa_interface_test.cpp\n> > > @@ -104,7 +104,7 @@ protected:\n> > >\n> > >               /* Test initialization of IPA module. */\n> > >               std::string conf = ipa_->configurationFile(\"vimc.conf\");\n> > > -             int ret = ipa_->init(IPASettings{ conf });\n> > > +             int ret = ipa_->init(IPASettings{ conf, \"vimc\" });\n> > >               if (ret < 0) {\n> > >                       cerr << \"IPA interface init() failed\" << endl;\n> > >                       return TestFail;\n> >\n> > Could you please also update the ipu3 and rkisp1 pipeline handlers ?\n> \n> I did intend on modifying them, but they call init() in slightly different\n> ways:\n> \n> ipu3: ipa_->init(IPASettings{});\n> rkisp1: ipa_->init(hwRevision);\n> \n> rkisp1 does not use IPASettings at all, so I left it out,\n\nFair eough :-)\n\n> and ipu3 initialises an\n> empty IPASettings object.  I could initialise ipu3 with the something like the\n> following:\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 2ea13ec9e1b9..0e9b05f2b771 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -1144,7 +1144,8 @@ int IPU3CameraData::loadIPA()\n> \n>         ipa_->queueFrameAction.connect(this, &IPU3CameraData::queueFrameAction);\n> \n> -       ipa_->init(IPASettings{});\n> +       CameraSensor *sensor = cio2_.sensor();\n> +       ipa_->init(IPASettings{\"\", sensor->model()});\n> \n> Let me know what you think is appropriate.\n\nThat seems good to me. Thanks.","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 6D4B7C32E5\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 23 Mar 2021 10:15:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AF1C368D69;\n\tTue, 23 Mar 2021 11:15:16 +0100 (CET)","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 BACE4602E3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 23 Mar 2021 11:15:15 +0100 (CET)","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 24686885;\n\tTue, 23 Mar 2021 11:15:15 +0100 (CET)"],"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=\"cMFlM1sP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616494515;\n\tbh=HoPiIRB5okP/HqAzOugRyQKc1e8GKy32ZtmuXlOdBhM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cMFlM1sPVUAAmSCnPJl+ScEDU5JxeatK4JhRrA+6bruyX4l2JWGKTulQsY1mhJaDb\n\tHDY3aJnEET2yFU5cLIu5dF/wsJYGrFeLzI7iFSRd6jbcqM8gLVsLW/74ukNf1kF2BY\n\tQPKMGo9mjiggOWSK/ecwGdWAROAm5u35eex1hW3U=","Date":"Tue, 23 Mar 2021 12:14:32 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<YFm/iJLsZDT6yevL@pendragon.ideasonboard.com>","References":"<20210317100211.1067585-1-naush@raspberrypi.com>\n\t<20210317100211.1067585-2-naush@raspberrypi.com>\n\t<YFkEfo+0xgTzqmYt@pendragon.ideasonboard.com>\n\t<CAEmqJPpkUcsR5Xkh1C3mk63aGB+7DRDf0s3anmU88VF_=Y4=wQ@mail.gmail.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<CAEmqJPpkUcsR5Xkh1C3mk63aGB+7DRDf0s3anmU88VF_=Y4=wQ@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH 1/7] ipa: Add sensor model string to\n\tIPASettings","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 <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>"}}]