[{"id":15501,"web_url":"https://patchwork.libcamera.org/comment/15501/","msgid":"","date":"2021-03-08T08:47:03","subject":"Re: [libcamera-devel] [PATCH v2 3/3] DEMO: raspberrypi: Use custom\n\tparameters to init()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Mon, Mar 08, 2021 at 04:48:28PM +0900, Paul Elder wrote:\n> This is just a demo to show custom parameters to init() with the\n> raspberrypi IPA interface.\n> \n> Signed-off-by: Paul Elder \n> ---\n> include/libcamera/ipa/raspberrypi.mojom | 5 ++-\n> src/ipa/raspberrypi/raspberrypi.cpp | 44 ++++++++++---------\n> .../pipeline/raspberrypi/raspberrypi.cpp | 10 +++--\n> 3 files changed, 32 insertions(+), 27 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> index f733a2cd..99a62c02 100644\n> --- a/include/libcamera/ipa/raspberrypi.mojom\n> +++ b/include/libcamera/ipa/raspberrypi.mojom\n> @@ -51,7 +51,8 @@ struct StartControls {\n> };\n> \n> interface IPARPiInterface {\n> -\tinit(IPASettings settings) => (int32 ret);\n> +\tinit(IPASettings settings, string sensorName)\n> +\t\t=> (int32 ret, bool metadataSupport);\n> \tstart(StartControls controls) => (StartControls result);\n> \tstop();\n> \n> @@ -77,7 +78,7 @@ interface IPARPiInterface {\n> \t\t map streamConfig,\n> \t\t map entityControls,\n> \t\t ConfigInput ipaConfig)\n> -\t\t=> (ConfigOutput results, int32 ret);\n> +\t\t=> (int32 ret, ConfigOutput results);\n\nI wonder if it would make sense to split those two changes. The change\nto configure() could be reviewed and merged already, and Naush could\ntake this DEMO patch as an example to move data->sensorMetadata_\nhandling to match() and IPA init() time.\n\nFor the configure() part of this patch,\n\nReviewed-by: Laurent Pinchart \n\nYou don't have to include an init() demo in v3 of the series (or just\nv2.1 of this patch actually), this is enough.\n\n> \n> \t/**\n> \t * \\fn mapBuffers()\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 6348d071..ac18dcbd 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -79,16 +79,17 @@ public:\n> \t\t\tmunmap(lsTable_, ipa::RPi::MaxLsGridSize);\n> \t}\n> \n> -\tint init(const IPASettings &settings) override;\n> +\tint init(const IPASettings &settings, const std::string &sensorName,\n> +\t\t bool *metadataSupport) override;\n> \tvoid start(const ipa::RPi::StartControls &data,\n> \t\t ipa::RPi::StartControls *result) override;\n> \tvoid stop() override {}\n> \n> -\tvoid configure(const CameraSensorInfo &sensorInfo,\n> -\t\t const std::map &streamConfig,\n> -\t\t const std::map &entityControls,\n> -\t\t const ipa::RPi::ConfigInput &data,\n> -\t\t ipa::RPi::ConfigOutput *response, int32_t *ret) override;\n> +\tint configure(const CameraSensorInfo &sensorInfo,\n> +\t\t const std::map &streamConfig,\n> +\t\t const std::map &entityControls,\n> +\t\t const ipa::RPi::ConfigInput &data,\n> +\t\t ipa::RPi::ConfigOutput *response) override;\n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n> \tvoid signalStatReady(const uint32_t bufferId) override;\n> @@ -164,9 +165,14 @@ private:\n> \tdouble maxFrameDuration_;\n> };\n> \n> -int IPARPi::init(const IPASettings &settings)\n> +int IPARPi::init(const IPASettings &settings, const std::string &sensorName,\n> +\t\t bool *metadataSupport)\n> {\n> +\tLOG(IPARPI, Debug) << \"sensor name is \" << sensorName;\n> +\n> \ttuningFile_ = settings.configurationFile;\n> +\n> +\t*metadataSupport = true;\n> \treturn 0;\n> }\n> \n> @@ -290,16 +296,15 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)\n> \tmode_.max_frame_length = sensorInfo.maxFrameLength;\n> }\n> \n> -void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> -\t\t [[maybe_unused]] const std::map &streamConfig,\n> -\t\t const std::map &entityControls,\n> -\t\t const ipa::RPi::ConfigInput &ipaConfig,\n> -\t\t ipa::RPi::ConfigOutput *result, int32_t *ret)\n> +int IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> +\t\t [[maybe_unused]] const std::map &streamConfig,\n> +\t\t const std::map &entityControls,\n> +\t\t const ipa::RPi::ConfigInput &ipaConfig,\n> +\t\t ipa::RPi::ConfigOutput *result)\n> {\n> \tif (entityControls.size() != 2) {\n> \t\tLOG(IPARPI, Error) << \"No ISP or sensor controls found.\";\n> -\t\t*ret = -1;\n> -\t\treturn;\n> +\t\treturn -1;\n> \t}\n> \n> \tresult->params = 0;\n> @@ -309,14 +314,12 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> \n> \tif (!validateSensorControls()) {\n> \t\tLOG(IPARPI, Error) << \"Sensor control validation failed.\";\n> -\t\t*ret = -1;\n> -\t\treturn;\n> +\t\treturn -1;\n> \t}\n> \n> \tif (!validateIspControls()) {\n> \t\tLOG(IPARPI, Error) << \"ISP control validation failed.\";\n> -\t\t*ret = -1;\n> -\t\treturn;\n> +\t\treturn -1;\n> \t}\n> \n> \t/* Setup a metadata ControlList to output metadata. */\n> @@ -334,8 +337,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> \t\tif (!helper_) {\n> \t\t\tLOG(IPARPI, Error) << \"Could not create camera helper for \"\n> \t\t\t\t\t << cameraName;\n> -\t\t\t*ret = -1;\n> -\t\t\treturn;\n> +\t\t\treturn -1;\n> \t\t}\n> \n> \t\t/*\n> @@ -403,7 +405,7 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> \n> \tlastMode_ = mode_;\n> \n> -\t*ret = 0;\n> +\treturn 0;\n> }\n> \n> void IPARPi::mapBuffers(const std::vector &buffers)\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index db91f1b5..3ff0f1cd 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1194,7 +1194,10 @@ int RPiCameraData::loadIPA()\n> \n> \tIPASettings settings(ipa_->configurationFile(sensor_->model() + \".json\"));\n> \n> -\treturn ipa_->init(settings);\n> +\tbool metadataSupport;\n> +\tint ret = ipa_->init(settings, \"sensor name\", &metadataSupport);\n> +\tLOG(RPI, Debug) << \"metadata support \" << (metadataSupport ? \"yes\" : \"no\");\n> +\treturn ret;\n> }\n> \n> int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> @@ -1250,9 +1253,8 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> \t/* Ready the IPA - it must know about the sensor resolution. */\n> \tipa::RPi::ConfigOutput result;\n> \n> -\tipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig,\n> -\t\t\t&result, &ret);\n> -\n> +\tret = ipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig,\n> +\t\t\t &result);\n> \tif (ret < 0) {\n> \t\tLOG(RPI, Error) << \"IPA configuration failed!\";\n> \t\treturn -EPIPE;","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 B8636BD1F1\n\tfor ;\n\tMon, 8 Mar 2021 08:47:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3239268A91;\n\tMon, 8 Mar 2021 09:47:36 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E43FC602E6\n\tfor ;\n\tMon, 8 Mar 2021 09:47:34 +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 52D4F8A3;\n\tMon, 8 Mar 2021 09:47:34 +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=\"fYaLakTV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615193254;\n\tbh=YsK7Y6WnsNvoSiGiiaaFJdot9+NKlA9h2PDPbuXECrI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=fYaLakTVYhoQaM6iRvqVoc78XjxCH4Q5NPNIJb3bne4ae8LLR7x2/Mmmor5uMtv40\n\tBKNbb/RqZFQIEFHvCKxSGRB7J1vSyTnTF3VTCqWfDxX2Khoz/q8WuSYv1LFCqUxQOK\n\t9cbJuE1mlzHbiLSdm0+zwWCotIGkr5+pzu6TIFHI=","Date":"Mon, 8 Mar 2021 10:47:03 +0200","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"","References":"<20210308074828.13228-1-paul.elder@ideasonboard.com>\n\t<20210308074828.13228-4-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210308074828.13228-4-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] DEMO: raspberrypi: Use custom\n\tparameters to init()","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":15524,"web_url":"https://patchwork.libcamera.org/comment/15524/","msgid":"","date":"2021-03-08T15:31:38","subject":"Re: [libcamera-devel] [PATCH v2 3/3] DEMO: raspberrypi: Use custom\n\tparameters to init()","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Paul and Laurent,\n\nOn Mon, 8 Mar 2021 at 08:47, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Paul,\n>\n> Thank you for the patch.\n>\n> On Mon, Mar 08, 2021 at 04:48:28PM +0900, Paul Elder wrote:\n> > This is just a demo to show custom parameters to init() with the\n> > raspberrypi IPA interface.\n> >\n> > Signed-off-by: Paul Elder \n> > ---\n> > include/libcamera/ipa/raspberrypi.mojom | 5 ++-\n> > src/ipa/raspberrypi/raspberrypi.cpp | 44 ++++++++++---------\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 10 +++--\n> > 3 files changed, 32 insertions(+), 27 deletions(-)\n> >\n> > diff --git a/include/libcamera/ipa/raspberrypi.mojom\n> b/include/libcamera/ipa/raspberrypi.mojom\n> > index f733a2cd..99a62c02 100644\n> > --- a/include/libcamera/ipa/raspberrypi.mojom\n> > +++ b/include/libcamera/ipa/raspberrypi.mojom\n> > @@ -51,7 +51,8 @@ struct StartControls {\n> > };\n> >\n> > interface IPARPiInterface {\n> > - init(IPASettings settings) => (int32 ret);\n> > + init(IPASettings settings, string sensorName)\n> > + => (int32 ret, bool metadataSupport);\n> > start(StartControls controls) => (StartControls result);\n> > stop();\n> >\n> > @@ -77,7 +78,7 @@ interface IPARPiInterface {\n> > map streamConfig,\n> > map entityControls,\n> > ConfigInput ipaConfig)\n> > - => (ConfigOutput results, int32 ret);\n> > + => (int32 ret, ConfigOutput results);\n>\n> I wonder if it would make sense to split those two changes. The change\n> to configure() could be reviewed and merged already, and Naush could\n> take this DEMO patch as an example to move data->sensorMetadata_\n> handling to match() and IPA init() time.\n>\n\nThat is a reasonable approach. I got a few things on my list to clear off\nbefore I get to this task, so separating it to allow you to get it merged\nearlier would make sense.\n\nRegards,\nNaush\n\n\n>\n> For the configure() part of this patch,\n>\n> Reviewed-by: Laurent Pinchart \n>\n> You don't have to include an init() demo in v3 of the series (or just\n> v2.1 of this patch actually), this is enough.\n>\n> >\n> > /**\n> > * \\fn mapBuffers()\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp\n> b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index 6348d071..ac18dcbd 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -79,16 +79,17 @@ public:\n> > munmap(lsTable_, ipa::RPi::MaxLsGridSize);\n> > }\n> >\n> > - int init(const IPASettings &settings) override;\n> > + int init(const IPASettings &settings, const std::string\n> &sensorName,\n> > + bool *metadataSupport) override;\n> > void start(const ipa::RPi::StartControls &data,\n> > ipa::RPi::StartControls *result) override;\n> > void stop() override {}\n> >\n> > - void configure(const CameraSensorInfo &sensorInfo,\n> > - const std::map\n> &streamConfig,\n> > - const std::map\n> &entityControls,\n> > - const ipa::RPi::ConfigInput &data,\n> > - ipa::RPi::ConfigOutput *response, int32_t *ret)\n> override;\n> > + int configure(const CameraSensorInfo &sensorInfo,\n> > + const std::map\n> &streamConfig,\n> > + const std::map\n> &entityControls,\n> > + const ipa::RPi::ConfigInput &data,\n> > + ipa::RPi::ConfigOutput *response) override;\n> > void mapBuffers(const std::vector &buffers) override;\n> > void unmapBuffers(const std::vector &ids) override;\n> > void signalStatReady(const uint32_t bufferId) override;\n> > @@ -164,9 +165,14 @@ private:\n> > double maxFrameDuration_;\n> > };\n> >\n> > -int IPARPi::init(const IPASettings &settings)\n> > +int IPARPi::init(const IPASettings &settings, const std::string\n> &sensorName,\n> > + bool *metadataSupport)\n> > {\n> > + LOG(IPARPI, Debug) << \"sensor name is \" << sensorName;\n> > +\n> > tuningFile_ = settings.configurationFile;\n> > +\n> > + *metadataSupport = true;\n> > return 0;\n> > }\n> >\n> > @@ -290,16 +296,15 @@ void IPARPi::setMode(const CameraSensorInfo\n> &sensorInfo)\n> > mode_.max_frame_length = sensorInfo.maxFrameLength;\n> > }\n> >\n> > -void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> > - [[maybe_unused]] const std::map IPAStream> &streamConfig,\n> > - const std::map\n> &entityControls,\n> > - const ipa::RPi::ConfigInput &ipaConfig,\n> > - ipa::RPi::ConfigOutput *result, int32_t *ret)\n> > +int IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> > + [[maybe_unused]] const std::map IPAStream> &streamConfig,\n> > + const std::map\n> &entityControls,\n> > + const ipa::RPi::ConfigInput &ipaConfig,\n> > + ipa::RPi::ConfigOutput *result)\n> > {\n> > if (entityControls.size() != 2) {\n> > LOG(IPARPI, Error) << \"No ISP or sensor controls found.\";\n> > - *ret = -1;\n> > - return;\n> > + return -1;\n> > }\n> >\n> > result->params = 0;\n> > @@ -309,14 +314,12 @@ void IPARPi::configure(const CameraSensorInfo\n> &sensorInfo,\n> >\n> > if (!validateSensorControls()) {\n> > LOG(IPARPI, Error) << \"Sensor control validation failed.\";\n> > - *ret = -1;\n> > - return;\n> > + return -1;\n> > }\n> >\n> > if (!validateIspControls()) {\n> > LOG(IPARPI, Error) << \"ISP control validation failed.\";\n> > - *ret = -1;\n> > - return;\n> > + return -1;\n> > }\n> >\n> > /* Setup a metadata ControlList to output metadata. */\n> > @@ -334,8 +337,7 @@ void IPARPi::configure(const CameraSensorInfo\n> &sensorInfo,\n> > if (!helper_) {\n> > LOG(IPARPI, Error) << \"Could not create camera\n> helper for \"\n> > << cameraName;\n> > - *ret = -1;\n> > - return;\n> > + return -1;\n> > }\n> >\n> > /*\n> > @@ -403,7 +405,7 @@ void IPARPi::configure(const CameraSensorInfo\n> &sensorInfo,\n> >\n> > lastMode_ = mode_;\n> >\n> > - *ret = 0;\n> > + return 0;\n> > }\n> >\n> > void IPARPi::mapBuffers(const std::vector &buffers)\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index db91f1b5..3ff0f1cd 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -1194,7 +1194,10 @@ int RPiCameraData::loadIPA()\n> >\n> > IPASettings settings(ipa_->configurationFile(sensor_->model() +\n> \".json\"));\n> >\n> > - return ipa_->init(settings);\n> > + bool metadataSupport;\n> > + int ret = ipa_->init(settings, \"sensor name\", &metadataSupport);\n> > + LOG(RPI, Debug) << \"metadata support \" << (metadataSupport ? \"yes\"\n> : \"no\");\n> > + return ret;\n> > }\n> >\n> > int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> > @@ -1250,9 +1253,8 @@ int RPiCameraData::configureIPA(const\n> CameraConfiguration *config)\n> > /* Ready the IPA - it must know about the sensor resolution. */\n> > ipa::RPi::ConfigOutput result;\n> >\n> > - ipa_->configure(sensorInfo_, streamConfig, entityControls,\n> ipaConfig,\n> > - &result, &ret);\n> > -\n> > + ret = ipa_->configure(sensorInfo_, streamConfig, entityControls,\n> ipaConfig,\n> > + &result);\n> > if (ret < 0) {\n> > LOG(RPI, Error) << \"IPA configuration failed!\";\n> > return -EPIPE;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel\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 33A20BD80C\n\tfor ;\n\tMon, 8 Mar 2021 15:31:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6ED2F68AA0;\n\tMon, 8 Mar 2021 16:31:58 +0100 (CET)","from mail-lf1-x130.google.com (mail-lf1-x130.google.com\n\t[IPv6:2a00:1450:4864:20::130])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EF0A568A92\n\tfor ;\n\tMon, 8 Mar 2021 16:31:55 +0100 (CET)","by mail-lf1-x130.google.com with SMTP id u4so21675658lfs.0\n\tfor ;\n\tMon, 08 Mar 2021 07:31:55 -0800 (PST)"],"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=\"HGaVPWlj\"; 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=r2/b/NQ7S+6x4wU+XDKCBcYKBMNDwsNxvpnho2l1vM8=;\n\tb=HGaVPWljDElzO27V+EjCwhQeeS+fGW9r4VNoh2W1+6PdZqsYeVcaHm2v3Rt7OSBnTi\n\tzagpR0cwLU79ZzK/3kPxASWD3GSlG7pIvmSx11+pZKtQ5qSSm1Tj1sCm7sdiZ5YkAs0R\n\tnOtgZXz9H77X4ZbLVd1B4OiixlgsWwBrgcx+szgbZNRXS4OvEYC+ydWKtETWRE+YuI4n\n\teu5/Smvz07ulURLv/Z7myMv8V0MMW4nYA/1ke/S8GgxSjc0L1Mx8QOcAEhJxT9kbmAw7\n\tVPL0UDdWPXbgydR2TXvqRkgMAwbzSTNn9SuSRin6l5gdPn4wPcmNL29UKaei3pIk6tgx\n\taOeQ==","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=r2/b/NQ7S+6x4wU+XDKCBcYKBMNDwsNxvpnho2l1vM8=;\n\tb=bRCbis3quepZs4DvbltX4wiEdFPWJq/ONV9UUHvQZ0kP/LjZp3nsj77XhypEqsndJh\n\tYSCpzqs62k1trS48JbcfVEzzX4eY/eEwQsd6GcRGFcZ1ndD4Zz8IWDycANnldBaZ65Kg\n\te+GJlIwVTbncppqHeu0kyaPVimTcvCL++MTppmOz4Eb5DjEVx05FLyWnf5WxbuvIje5b\n\t7+wejRfvsdj8Lo/AVu7/N5nY1eBXwMxXnuzISORuziFk32sIHt8yEBEodP8gPCgSN7kX\n\tNCAu6p2wUAn7FMfWAvkNsFhzOVTUflr3D1OvWYkRtLKn1JOhIUp11VSrSY3tjs9iaZmi\n\tEpyA==","X-Gm-Message-State":"AOAM532D7IT8OryW8NcHYkXUSKNFhE4x3S23e5gYGnmNeD+VCc7oXMYz\n\tnsps4fxcQhRA8Ivzbc+z62i2ty+/pLizV8n4i0KtFA==","X-Google-Smtp-Source":"ABdhPJzTocDOoDlp2guiF1hlesKt/r3TE9rpnLrPMtytRlqTAHLWJ0noPoUCrEloM9jtU5COut3N4PATxI21QumSc4I=","X-Received":"by 2002:a05:6512:3226:: with SMTP id\n\tf6mr14220987lfe.171.1615217515079; \n\tMon, 08 Mar 2021 07:31:55 -0800 (PST)","MIME-Version":"1.0","References":"<20210308074828.13228-1-paul.elder@ideasonboard.com>\n\t<20210308074828.13228-4-paul.elder@ideasonboard.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Mon, 8 Mar 2021 15:31:38 +0000","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] DEMO: raspberrypi: Use custom\n\tparameters to init()","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=\"===============3828806492610502263==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15550,"web_url":"https://patchwork.libcamera.org/comment/15550/","msgid":"<20210309015606.GA15203@pyrite.rasen.tech>","date":"2021-03-09T01:56:06","subject":"Re: [libcamera-devel] [PATCH v2 3/3] DEMO: raspberrypi: Use custom\n\tparameters to init()","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Naush and Laurent,\n\nOn Mon, Mar 08, 2021 at 03:31:38PM +0000, Naushir Patuck wrote:\n> Hi Paul and Laurent,\n> \n> On Mon, 8 Mar 2021 at 08:47, Laurent Pinchart <\n> laurent.pinchart@ideasonboard.com> wrote:\n> \n> Hi Paul,\n> \n> Thank you for the patch.\n> \n> On Mon, Mar 08, 2021 at 04:48:28PM +0900, Paul Elder wrote:\n> > This is just a demo to show custom parameters to init() with the\n> > raspberrypi IPA interface.\n> >\n> > Signed-off-by: Paul Elder \n> > ---\n> >  include/libcamera/ipa/raspberrypi.mojom       |  5 ++-\n> >  src/ipa/raspberrypi/raspberrypi.cpp           | 44 ++++++++++---------\n> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 10 +++--\n> >  3 files changed, 32 insertions(+), 27 deletions(-)\n> >\n> > diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/\n> ipa/raspberrypi.mojom\n> > index f733a2cd..99a62c02 100644\n> > --- a/include/libcamera/ipa/raspberrypi.mojom\n> > +++ b/include/libcamera/ipa/raspberrypi.mojom\n> > @@ -51,7 +51,8 @@ struct StartControls {\n> >  };\n> > \n> >  interface IPARPiInterface {\n> > -     init(IPASettings settings) => (int32 ret);\n> > +     init(IPASettings settings, string sensorName)\n> > +             => (int32 ret, bool metadataSupport);\n> >       start(StartControls controls) => (StartControls result);\n> >       stop();\n> > \n> > @@ -77,7 +78,7 @@ interface IPARPiInterface {\n> >                 map streamConfig,\n> >                 map entityControls,\n> >                 ConfigInput ipaConfig)\n> > -             => (ConfigOutput results, int32 ret);\n> > +             => (int32 ret, ConfigOutput results);\n> \n> I wonder if it would make sense to split those two changes. The change\n> to configure() could be reviewed and merged already, and Naush could\n> take this DEMO patch as an example to move data->sensorMetadata_\n> handling to match() and IPA init() time.\n> \n> \n> That is a reasonable approach.  I got a few things on my list to clear off\n> before I get to this task, so separating it to allow you to get it merged\n> earlier would make sense.\n\nI didn't intend for this patch to be merged at all; it was just to show\nhow I intended it to be used.\n\n\nThanks,\n\nPaul\n\n>  \n> \n> \n> For the configure() part of this patch,\n> \n> Reviewed-by: Laurent Pinchart \n> \n> You don't have to include an init() demo in v3 of the series (or just\n> v2.1 of this patch actually), this is enough.\n> \n> > \n> >       /**\n> >        * \\fn mapBuffers()\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/\n> raspberrypi.cpp\n> > index 6348d071..ac18dcbd 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -79,16 +79,17 @@ public:\n> >                       munmap(lsTable_, ipa::RPi::MaxLsGridSize);\n> >       }\n> > \n> > -     int init(const IPASettings &settings) override;\n> > +     int init(const IPASettings &settings, const std::string &\n> sensorName,\n> > +              bool *metadataSupport) override;\n> >       void start(const ipa::RPi::StartControls &data,\n> >                  ipa::RPi::StartControls *result) override;\n> >       void stop() override {}\n> > \n> > -     void configure(const CameraSensorInfo &sensorInfo,\n> > -                    const std::map &\n> streamConfig,\n> > -                    const std::map &\n> entityControls,\n> > -                    const ipa::RPi::ConfigInput &data,\n> > -                    ipa::RPi::ConfigOutput *response, int32_t *ret)\n> override;\n> > +     int configure(const CameraSensorInfo &sensorInfo,\n> > +                   const std::map &\n> streamConfig,\n> > +                   const std::map &\n> entityControls,\n> > +                   const ipa::RPi::ConfigInput &data,\n> > +                   ipa::RPi::ConfigOutput *response) override;\n> >       void mapBuffers(const std::vector &buffers) override;\n> >       void unmapBuffers(const std::vector &ids) override;\n> >       void signalStatReady(const uint32_t bufferId) override;\n> > @@ -164,9 +165,14 @@ private:\n> >       double maxFrameDuration_;\n> >  };\n> > \n> > -int IPARPi::init(const IPASettings &settings)\n> > +int IPARPi::init(const IPASettings &settings, const std::string &\n> sensorName,\n> > +              bool *metadataSupport)\n> >  {\n> > +     LOG(IPARPI, Debug) << \"sensor name is \" << sensorName;\n> > +\n> >       tuningFile_ = settings.configurationFile;\n> > +\n> > +     *metadataSupport = true;\n> >       return 0;\n> >  }\n> > \n> > @@ -290,16 +296,15 @@ void IPARPi::setMode(const CameraSensorInfo &\n> sensorInfo)\n> >       mode_.max_frame_length = sensorInfo.maxFrameLength;\n> >  }\n> > \n> > -void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> > -                    [[maybe_unused]] const std::map IPAStream> &streamConfig,\n> > -                    const std::map &\n> entityControls,\n> > -                    const ipa::RPi::ConfigInput &ipaConfig,\n> > -                    ipa::RPi::ConfigOutput *result, int32_t *ret)\n> > +int IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> > +                   [[maybe_unused]] const std::map IPAStream> &streamConfig,\n> > +                   const std::map &\n> entityControls,\n> > +                   const ipa::RPi::ConfigInput &ipaConfig,\n> > +                   ipa::RPi::ConfigOutput *result)\n> >  {\n> >       if (entityControls.size() != 2) {\n> >               LOG(IPARPI, Error) << \"No ISP or sensor controls found.\";\n> > -             *ret = -1;\n> > -             return;\n> > +             return -1;\n> >       }\n> > \n> >       result->params = 0;\n> > @@ -309,14 +314,12 @@ void IPARPi::configure(const CameraSensorInfo &\n> sensorInfo,\n> > \n> >       if (!validateSensorControls()) {\n> >               LOG(IPARPI, Error) << \"Sensor control validation failed.\";\n> > -             *ret = -1;\n> > -             return;\n> > +             return -1;\n> >       }\n> > \n> >       if (!validateIspControls()) {\n> >               LOG(IPARPI, Error) << \"ISP control validation failed.\";\n> > -             *ret = -1;\n> > -             return;\n> > +             return -1;\n> >       }\n> > \n> >       /* Setup a metadata ControlList to output metadata. */\n> > @@ -334,8 +337,7 @@ void IPARPi::configure(const CameraSensorInfo &\n> sensorInfo,\n> >               if (!helper_) {\n> >                       LOG(IPARPI, Error) << \"Could not create camera\n> helper for \"\n> >                                          << cameraName;\n> > -                     *ret = -1;\n> > -                     return;\n> > +                     return -1;\n> >               }\n> > \n> >               /*\n> > @@ -403,7 +405,7 @@ void IPARPi::configure(const CameraSensorInfo &\n> sensorInfo,\n> > \n> >       lastMode_ = mode_;\n> > \n> > -     *ret = 0;\n> > +     return 0;\n> >  }\n> > \n> >  void IPARPi::mapBuffers(const std::vector &buffers)\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/\n> libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index db91f1b5..3ff0f1cd 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -1194,7 +1194,10 @@ int RPiCameraData::loadIPA()\n> > \n> >       IPASettings settings(ipa_->configurationFile(sensor_->model() +\n> \".json\"));\n> > \n> > -     return ipa_->init(settings);\n> > +     bool metadataSupport;\n> > +     int ret = ipa_->init(settings, \"sensor name\", &metadataSupport);\n> > +     LOG(RPI, Debug) << \"metadata support \" << (metadataSupport ? \"yes\"\n> : \"no\");\n> > +     return ret;\n> >  }\n> > \n> >  int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> > @@ -1250,9 +1253,8 @@ int RPiCameraData::configureIPA(const\n> CameraConfiguration *config)\n> >       /* Ready the IPA - it must know about the sensor resolution. */\n> >       ipa::RPi::ConfigOutput result;\n> > \n> > -     ipa_->configure(sensorInfo_, streamConfig, entityControls,\n> ipaConfig,\n> > -                     &result, &ret);\n> > -\n> > +     ret = ipa_->configure(sensorInfo_, streamConfig, entityControls,\n> ipaConfig,\n> > +                           &result);\n> >       if (ret < 0) {\n> >               LOG(RPI, Error) << \"IPA configuration failed!\";\n> >               return -EPIPE;","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 F0F28BD1F1\n\tfor ;\n\tTue, 9 Mar 2021 01:56:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3F2C968A9C;\n\tTue, 9 Mar 2021 02:56: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 6FDA7602E4\n\tfor ;\n\tTue, 9 Mar 2021 02:56:14 +0100 (CET)","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 863FE527;\n\tTue, 9 Mar 2021 02:56:12 +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=\"PBGUcsyL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615254974;\n\tbh=4jO060CIZyM6QYlE0l36N+w+N6K/G8706Mwnt0xWIdA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PBGUcsyL1MBlfaQqjBpr6EG/fz3opYz3o53tS0o8eBDUYPWs+4lfVVnJexlUyu8YD\n\te4SctDjIzAe86LJnahc7xddM2wlHMq93St06z4R//sksLi+24B6BBOP6rada5aqONY\n\to5lH9Jhp14bAJt1aSazwlLohx9bvRn2vlFXgBhMQ=","Date":"Tue, 9 Mar 2021 10:56:06 +0900","From":"paul.elder@ideasonboard.com","To":"Naushir Patuck ","Message-ID":"<20210309015606.GA15203@pyrite.rasen.tech>","References":"<20210308074828.13228-1-paul.elder@ideasonboard.com>\n\t<20210308074828.13228-4-paul.elder@ideasonboard.com>\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] DEMO: raspberrypi: Use custom\n\tparameters to init()","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]