[{"id":15841,"web_url":"https://patchwork.libcamera.org/comment/15841/","msgid":"","date":"2021-03-23T16:35:38","subject":"Re: [libcamera-devel] [PATCH v3 1/7] ipa: Add sensor model string\n\tto IPASettings","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 02:36:04PM +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 \n> Tested-by: David Plowman \n> Reviewed-by: Laurent Pinchart \n> ---\n> include/libcamera/ipa/core.mojom | 8 ++++++++\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ++-\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> 5 files changed, 14 insertions(+), 4 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> };\n> \n> /**\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 2ea13ec9e1b9..c27120710323 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> \tipa_->queueFrameAction.connect(this, &IPU3CameraData::queueFrameAction);\n> \n> -\tipa_->init(IPASettings{});\n> +\tCameraSensor *sensor = cio2_.sensor();\n> +\tipa_->init(IPASettings{ \"\", sensor->model() });\n> \n> \treturn 0;\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(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\nThis crashes, as data->sensor_ is null here :-S This fixes it.\n\ndiff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\nindex 2dfb63ecf2ef..8f5f4ba30953 100644\n--- a/src/libcamera/pipeline/vimc/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc/vimc.cpp\n@@ -422,6 +422,10 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n\n \tstd::unique_ptr data = std::make_unique(this, media);\n\n+\t/* Locate and open the capture video node. */\n+\tif (data->init())\n+\t\treturn false;\n+\n \tdata->ipa_ = IPAManager::createIPA(this, 0, 0);\n \tif (data->ipa_ != nullptr) {\n \t\tstd::string conf = data->ipa_->configurationFile(\"vimc.conf\");\n@@ -430,10 +434,6 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n \t\tLOG(VIMC, Warning) << \"no matching IPA found\";\n \t}\n\n-\t/* Locate and open the capture video node. */\n-\tif (data->init())\n-\t\treturn false;\n-\n \t/* Create and register the camera. */\n \tstd::set streams{ &data->stream_ };\n \tstd::shared_ptr camera =\n\nCould you run \"ninja test\" on future patch series ? It requires the\nvimc, vivid and vim2m modules to be loaded, and you can do so on an x86\nmachine.\n\nI'll fold this fix in when merging.\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> \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;","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 12B63C32E5\n\tfor ;\n\tTue, 23 Mar 2021 16:36:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4D74568D63;\n\tTue, 23 Mar 2021 17:36:22 +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 9674D602D7\n\tfor ;\n\tTue, 23 Mar 2021 17:36:20 +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 0ACF2885;\n\tTue, 23 Mar 2021 17:36:19 +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=\"nM50VzsP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616517380;\n\tbh=U+BqVFO++zcolmkYV1vfSb7jUm/uXMEn4U0TwgLh2UM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nM50VzsPGD/l87MTvE1aemd7IHOusgjO6DFq6wN56Kq1bPIeToCIXGKv4a1cs6//v\n\tZcImU1z7uNPMyUuoijgA0pITgweAkZe8yUjGG/P4Gj31g848GVVnT7U32Fyzj72oJ6\n\tRtWHKMLfZla0fVic2PeN4FwvH+SJ1LqnRHg/Ga5A=","Date":"Tue, 23 Mar 2021 18:35:38 +0200","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210323143610.787760-1-naush@raspberrypi.com>\n\t<20210323143610.787760-2-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210323143610.787760-2-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v3 1/7] ipa: Add sensor model string\n\tto IPASettings","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":15842,"web_url":"https://patchwork.libcamera.org/comment/15842/","msgid":"","date":"2021-03-23T16:44:35","subject":"Re: [libcamera-devel] [PATCH v3 1/7] ipa: Add sensor model string\n\tto IPASettings","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nOn Tue, 23 Mar 2021 at 16:36, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> On Tue, Mar 23, 2021 at 02:36:04PM +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 \n> > Tested-by: David Plowman \n> > Reviewed-by: Laurent Pinchart \n> > ---\n> > include/libcamera/ipa/core.mojom | 8 ++++++++\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ++-\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> > 5 files changed, 14 insertions(+), 4 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> >\n> > /**\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 2ea13ec9e1b9..c27120710323 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> >\n> > return 0;\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(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>\n> This crashes, as data->sensor_ is null here :-S This fixes it.\n>\n\nSorry for breaking that :(\n\n\n>\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp\n> b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 2dfb63ecf2ef..8f5f4ba30953 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -422,6 +422,10 @@ bool PipelineHandlerVimc::match(DeviceEnumerator\n> *enumerator)\n>\n> std::unique_ptr data =\n> std::make_unique(this, media);\n>\n> + /* Locate and open the capture video node. */\n> + if (data->init())\n> + return false;\n> +\n> data->ipa_ = IPAManager::createIPA(this,\n> 0, 0);\n> if (data->ipa_ != nullptr) {\n> std::string conf =\n> data->ipa_->configurationFile(\"vimc.conf\");\n> @@ -430,10 +434,6 @@ bool PipelineHandlerVimc::match(DeviceEnumerator\n> *enumerator)\n> LOG(VIMC, Warning) << \"no matching IPA found\";\n> }\n>\n> - /* Locate and open the capture video node. */\n> - if (data->init())\n> - return false;\n> -\n> /* Create and register the camera. */\n> std::set streams{ &data->stream_ };\n> std::shared_ptr camera =\n>\n> Could you run \"ninja test\" on future patch series ? It requires the\n> vimc, vivid and vim2m modules to be loaded, and you can do so on an x86\n> machine.\n>\n\nI'll be sure to run through these tests before submitting next time.\n\nRegards,\nNaush\n\n\n>\n> I'll fold this fix in when merging.\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> --\n> Regards,\n>\n> Laurent Pinchart\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 0E2FBC32E4\n\tfor ;\n\tTue, 23 Mar 2021 16:44:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 69BFE68D6A;\n\tTue, 23 Mar 2021 17:44:54 +0100 (CET)","from mail-lj1-x236.google.com (mail-lj1-x236.google.com\n\t[IPv6:2a00:1450:4864:20::236])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3F784602D7\n\tfor ;\n\tTue, 23 Mar 2021 17:44:53 +0100 (CET)","by mail-lj1-x236.google.com with SMTP id s17so26495111ljc.5\n\tfor ;\n\tTue, 23 Mar 2021 09:44:53 -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=\"Xr/KnqKP\"; 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=+P+Y+nA4qw+9KnSWPXBk9gNHKQHQu1+KQPD0l+j8TCk=;\n\tb=Xr/KnqKPF0RTIIG1RO1AdrjYTFM+5LWp8uqVnHDH8R5bE6AJTcUjgOGfO320oGTH/0\n\t44jBuJC/3G7ZpMLE76WkHGFR5ZI3PCprBp826GFnTuS7kt9eRXoyBzyXoLgT1+3RFVb+\n\t2aa4ywUdyQzmMG5Acq/ElAgkxii44CNG/4D9zFGABh3VQQMdkloNd7nBU3wti3sa+WyM\n\t/pgtcZw+5XYv/7tVn8/jNwCydQ4oZkpHn8wBg5g8cqd7cqrLMKSMU1Gsjxv8u+CVYYiZ\n\tA54QVfL/U+JiKx+S9Vr7fVwBcIHbk1uR1Bc5uttYhGRNtaRKQDeIDKb7B7UvWLs+/6eW\n\tOw5g==","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=+P+Y+nA4qw+9KnSWPXBk9gNHKQHQu1+KQPD0l+j8TCk=;\n\tb=PXtrEzlT6kUN04ZHSPbnvPjx/055NwY3eWO535Zt+RFRhXYka4rvKaulR7fVPVG/wM\n\tXpSSA4Ad5ix1LEYfkE4kyMMiadEkUJSYQtP0TPsMmuCsKU2Fk3r23sBFZY8FJMv3JWwu\n\ttz+fFC/+W3viKjE7y3uDqSDRL15hddJVUYSVppBH016wk0ssscn23COiQCg4EO0NN0ER\n\tvSZDj3Dt9tnz1ng3IeCXG11lOhG+zjVYKrkM9i5dAgleVOHcjA6lQCI8OIBHMaLCi1RV\n\t3HGaCFvKxJLIikbPrR9y/eBg8qS1euZxpAt5OjUv1dGjq9sb06p6BwwLnRZUM1/U78zb\n\tRfTg==","X-Gm-Message-State":"AOAM530uGC2/ExQNIdBo3ElTaRzibYGEmg5Ili6nZvUyCDrIJmCljr1f\n\tEhdiggzb5SiPx3p9ZhhrSsCuNw+LYtIcWuBtAXCcGA==","X-Google-Smtp-Source":"ABdhPJxR1oO/uNRlygJAO2vNLlHUPAPtp9QvR76wXdnk3GHAIN0G9c0OetpT6CcigxZ+EYaNv6y2WW5+Q3rWMRd+Kmg=","X-Received":"by 2002:a2e:9b10:: with SMTP id\n\tu16mr3442814lji.253.1616517892366; \n\tTue, 23 Mar 2021 09:44:52 -0700 (PDT)","MIME-Version":"1.0","References":"<20210323143610.787760-1-naush@raspberrypi.com>\n\t<20210323143610.787760-2-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Tue, 23 Mar 2021 16:44:35 +0000","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH v3 1/7] ipa: Add sensor model string\n\tto IPASettings","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":"multipart/mixed;\n\tboundary=\"===============8882937779734059106==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15843,"web_url":"https://patchwork.libcamera.org/comment/15843/","msgid":"","date":"2021-03-23T16:51:01","subject":"Re: [libcamera-devel] [PATCH v3 1/7] ipa: Add sensor model string\n\tto IPASettings","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 04:44:35PM +0000, Naushir Patuck wrote:\n> On Tue, 23 Mar 2021 at 16:36, Laurent Pinchart wrote:\n> > On Tue, Mar 23, 2021 at 02:36:04PM +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 \n> > > Tested-by: David Plowman \n> > > Reviewed-by: Laurent Pinchart \n> > > ---\n> > > include/libcamera/ipa/core.mojom | 8 ++++++++\n> > > src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ++-\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> > > 5 files changed, 14 insertions(+), 4 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> > > };\n> > >\n> > > /**\n> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > index 2ea13ec9e1b9..c27120710323 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> > > return 0;\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(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> >\n> > This crashes, as data->sensor_ is null here :-S This fixes it.\n> \n> Sorry for breaking that :(\n\nNo worries, the breakage was caught before it got upstream :-)\n\n> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp\n> > b/src/libcamera/pipeline/vimc/vimc.cpp\n> > index 2dfb63ecf2ef..8f5f4ba30953 100644\n> > --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> > @@ -422,6 +422,10 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> >\n> > std::unique_ptr data = std::make_unique(this, media);\n> >\n> > + /* Locate and open the capture video node. */\n> > + if (data->init())\n> > + return false;\n> > +\n> > data->ipa_ = IPAManager::createIPA(this, 0, 0);\n> > if (data->ipa_ != nullptr) {\n> > std::string conf = data->ipa_->configurationFile(\"vimc.conf\");\n> > @@ -430,10 +434,6 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> > LOG(VIMC, Warning) << \"no matching IPA found\";\n> > }\n> >\n> > - /* Locate and open the capture video node. */\n> > - if (data->init())\n> > - return false;\n> > -\n> > /* Create and register the camera. */\n> > std::set streams{ &data->stream_ };\n> > std::shared_ptr camera =\n> >\n> > Could you run \"ninja test\" on future patch series ? It requires the\n> > vimc, vivid and vim2m modules to be loaded, and you can do so on an x86\n> > machine.\n> \n> I'll be sure to run through these tests before submitting next time.\n\nEventually we'll have automated tests that will do this all for you. In\nthe meantime, thanks for your help.\n\n> > I'll fold this fix in when merging.\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;","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 3A409C32E5\n\tfor ;\n\tTue, 23 Mar 2021 16:51:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 842EB68D6A;\n\tTue, 23 Mar 2021 17:51:45 +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 D3AE668D5E\n\tfor ;\n\tTue, 23 Mar 2021 17:51:43 +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 46773885;\n\tTue, 23 Mar 2021 17:51:43 +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=\"i8wd9ski\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616518303;\n\tbh=JYiFmcayubE2SKU5MStZdUdESIxAioRhm2D7Nmcfgho=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=i8wd9skie6n0X+dcp35fxr7eVbWCidscM/SRY5feE+gcQXDhJom7dNwU3V0Hep12b\n\tNsOkJuTdpCrSYH0izmpgrxI3+F+dngcawPezQbhrqkIBJhjIHeZW95ZrZtON7bm7FT\n\tRmeHZ4vt8N9LBqF3GQdVDjkZv7AH12AeHKdbOy3g=","Date":"Tue, 23 Mar 2021 18:51:01 +0200","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210323143610.787760-1-naush@raspberrypi.com>\n\t<20210323143610.787760-2-naush@raspberrypi.com>\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v3 1/7] ipa: Add sensor model string\n\tto IPASettings","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\" "}}]