[{"id":10964,"web_url":"https://patchwork.libcamera.org/comment/10964/","msgid":"<20200629143113.GC1852218@oden.dyn.berto.se>","date":"2020-06-29T14:31:13","subject":"Re: [libcamera-devel] [PATCH v1 3/9] libcamera: ipa_interface: Add\n\tsupport for custom IPA data to configure()","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2020-06-29 02:19:28 +0300, Laurent Pinchart wrote:\n> Add two new parameters, ipaConfig and result, to the\n> IPAInterface::configure() function to allow pipeline handlers to pass\n> custom data to their IPA, and receive data back. Wire this through the\n> code base. The C API interface will be addressed separately, likely\n> through automation of the C <-> C++ translation.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> include/libcamera/internal/ipa_context_wrapper.h | 4 +++-\n> include/libcamera/ipa/ipa_interface.h | 4 +++-\n> src/ipa/libipa/ipa_interface_wrapper.cpp | 5 ++++-\n> src/ipa/raspberrypi/raspberrypi.cpp | 8 ++++++--\n> src/ipa/rkisp1/rkisp1.cpp | 8 ++++++--\n> src/ipa/vimc/vimc.cpp | 4 +++-\n> src/libcamera/ipa_context_wrapper.cpp | 8 ++++++--\n> src/libcamera/ipa_interface.cpp | 7 +++++++\n> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 +++-\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +++-\n> src/libcamera/proxy/ipa_proxy_linux.cpp | 4 +++-\n> src/libcamera/proxy/ipa_proxy_thread.cpp | 11 ++++++++---\n> test/ipa/ipa_wrappers_test.cpp | 8 ++++++--\n> 13 files changed, 61 insertions(+), 18 deletions(-)\n> \n> diff --git a/include/libcamera/internal/ipa_context_wrapper.h b/include/libcamera/internal/ipa_context_wrapper.h\n> index 4e6f791d18e6..8f767e844221 100644\n> --- a/include/libcamera/internal/ipa_context_wrapper.h\n> +++ b/include/libcamera/internal/ipa_context_wrapper.h\n> @@ -24,7 +24,9 @@ public:\n> \tvoid stop() override;\n> \tvoid configure(const CameraSensorInfo &sensorInfo,\n> \t\t const std::map &streamConfig,\n> -\t\t const std::map &entityControls) override;\n> +\t\t const std::map &entityControls,\n> +\t\t const IPAOperationData &ipaConfig,\n> +\t\t IPAOperationData *result) override;\n> \n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n> diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h\n> index dc9fc714d564..5016ec25ea9c 100644\n> --- a/include/libcamera/ipa/ipa_interface.h\n> +++ b/include/libcamera/ipa/ipa_interface.h\n> @@ -158,7 +158,9 @@ public:\n> \n> \tvirtual void configure(const CameraSensorInfo &sensorInfo,\n> \t\t\t const std::map &streamConfig,\n> -\t\t\t const std::map &entityControls) = 0;\n> +\t\t\t const std::map &entityControls,\n> +\t\t\t const IPAOperationData &ipaConfig,\n> +\t\t\t IPAOperationData *result) = 0;\n> \n> \tvirtual void mapBuffers(const std::vector &buffers) = 0;\n> \tvirtual void unmapBuffers(const std::vector &ids) = 0;\n> diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp\n> index 2a2e43abc708..47ce5a704851 100644\n> --- a/src/ipa/libipa/ipa_interface_wrapper.cpp\n> +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp\n> @@ -166,7 +166,10 @@ void IPAInterfaceWrapper::configure(struct ipa_context *_ctx,\n> \t\tentityControls.emplace(id, infoMaps[id]);\n> \t}\n> \n> -\tctx->ipa_->configure(sensorInfo, ipaStreams, entityControls);\n> +\t/* \\todo Translate the ipaConfig and reponse */\n\nI wonder if we should add a FATAL LOG statement here to make it clear \nthat we break the wrapper and this issue should be solved before anyone \nattempts to use the feature, which we know won't work.\n\n> +\tIPAOperationData ipaConfig;\n> +\tctx->ipa_->configure(sensorInfo, ipaStreams, entityControls, ipaConfig,\n> +\t\t\t nullptr);\n> }\n> \n> void IPAInterfaceWrapper::map_buffers(struct ipa_context *_ctx,\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index bc89ab58d03a..860be22ddb5d 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -78,7 +78,9 @@ public:\n> \n> \tvoid configure(const CameraSensorInfo &sensorInfo,\n> \t\t const std::map &streamConfig,\n> -\t\t const std::map &entityControls) override;\n> +\t\t const std::map &entityControls,\n> +\t\t const IPAOperationData &data,\n> +\t\t IPAOperationData *response) override;\n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n> \tvoid processEvent(const IPAOperationData &event) override;\n> @@ -186,7 +188,9 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)\n> \n> void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> \t\t const std::map &streamConfig,\n> -\t\t const std::map &entityControls)\n> +\t\t const std::map &entityControls,\n> +\t\t const IPAOperationData &ipaConfig,\n> +\t\t IPAOperationData *result)\n> {\n> \tif (entityControls.empty())\n> \t\treturn;\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index fbdc908fc816..34d6f63a7ff4 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -39,7 +39,9 @@ public:\n> \n> \tvoid configure(const CameraSensorInfo &info,\n> \t\t const std::map &streamConfig,\n> -\t\t const std::map &entityControls) override;\n> +\t\t const std::map &entityControls,\n> +\t\t const IPAOperationData &ipaConfig,\n> +\t\t IPAOperationData *response) override;\n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n> \tvoid processEvent(const IPAOperationData &event) override;\n> @@ -76,7 +78,9 @@ private:\n> */\n> void IPARkISP1::configure(const CameraSensorInfo &info,\n> \t\t\t const std::map &streamConfig,\n> -\t\t\t const std::map &entityControls)\n> +\t\t\t const std::map &entityControls,\n> +\t\t\t const IPAOperationData &ipaConfig,\n> +\t\t\t IPAOperationData *result)\n> {\n> \tif (entityControls.empty())\n> \t\treturn;\n> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\n> index af278a482b8a..1593c92d80f3 100644\n> --- a/src/ipa/vimc/vimc.cpp\n> +++ b/src/ipa/vimc/vimc.cpp\n> @@ -39,7 +39,9 @@ public:\n> \n> \tvoid configure(const CameraSensorInfo &sensorInfo,\n> \t\t const std::map &streamConfig,\n> -\t\t const std::map &entityControls) override {}\n> +\t\t const std::map &entityControls,\n> +\t\t const IPAOperationData &ipaConfig,\n> +\t\t IPAOperationData *result) override {}\n> \tvoid mapBuffers(const std::vector &buffers) override {}\n> \tvoid unmapBuffers(const std::vector &ids) override {}\n> \tvoid processEvent(const IPAOperationData &event) override {}\n> diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp\n> index 471118f57cdc..231300ce0bec 100644\n> --- a/src/libcamera/ipa_context_wrapper.cpp\n> +++ b/src/libcamera/ipa_context_wrapper.cpp\n> @@ -110,10 +110,13 @@ void IPAContextWrapper::stop()\n> \n> void IPAContextWrapper::configure(const CameraSensorInfo &sensorInfo,\n> \t\t\t\t const std::map &streamConfig,\n> -\t\t\t\t const std::map &entityControls)\n> +\t\t\t\t const std::map &entityControls,\n> +\t\t\t\t const IPAOperationData &ipaConfig,\n> +\t\t\t\t IPAOperationData *result)\n> {\n> \tif (intf_)\n> -\t\treturn intf_->configure(sensorInfo, streamConfig, entityControls);\n> +\t\treturn intf_->configure(sensorInfo, streamConfig,\n> +\t\t\t\t\tentityControls, ipaConfig, result);\n> \n> \tif (!ctx_)\n> \t\treturn;\n> @@ -174,6 +177,7 @@ void IPAContextWrapper::configure(const CameraSensorInfo &sensorInfo,\n> \t\t++i;\n> \t}\n> \n> +\t/* \\todo Translate the ipaConfig and reponse */\n> \tctx_->ops->configure(ctx_, &sensor_info, c_streams, streamConfig.size(),\n> \t\t\t c_info_maps, entityControls.size());\n> }\n> diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp\n> index ebe47e1233a5..23fc56d7d48e 100644\n> --- a/src/libcamera/ipa_interface.cpp\n> +++ b/src/libcamera/ipa_interface.cpp\n> @@ -557,6 +557,8 @@ namespace libcamera {\n> * \\param[in] sensorInfo Camera sensor information\n> * \\param[in] streamConfig Configuration of all active streams\n> * \\param[in] entityControls Controls provided by the pipeline entities\n> + * \\param[in] ipaConfig Pipeline-handler-specific configuration data\n> + * \\param[out] result Pipeline-handler-specific configuration result\n> *\n> * This method shall be called when the camera is started to inform the IPA of\n> * the camera's streams and the sensor settings. The meaning of the numerical\n> @@ -566,6 +568,11 @@ namespace libcamera {\n> * The \\a sensorInfo conveys information about the camera sensor settings that\n> * the pipeline handler has selected for the configuration. The IPA may use\n> * that information to tune its algorithms.\n> + *\n> + * The \\a ipaConfig and \\a result parameters carry custom data passed by the\n> + * pipeline handler to the IPA and back. The pipeline handler may set the \\a\n> + * result parameter to null if the IPA protocol doesn't need to pass a result\n> + * back through the configure() function.\n> */\n> \n> /**\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index dcd737a1d1a0..3b5cdf1e1849 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1017,7 +1017,9 @@ int PipelineHandlerRPi::configureIPA(Camera *camera)\n> \t}\n> \n> \t/* Ready the IPA - it must know about the sensor resolution. */\n> -\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls);\n> +\tIPAOperationData ipaConfig;\n> +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> +\t\t\t ipaConfig, nullptr);\n> \n> \treturn 0;\n> }\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 3c01821135f8..4fde5539e667 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -843,7 +843,9 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n> \tstd::map entityControls;\n> \tentityControls.emplace(0, data->sensor_->controls());\n> \n> -\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls);\n> +\tIPAOperationData ipaConfig;\n> +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> +\t\t\t ipaConfig, nullptr);\n> \n> \treturn ret;\n> }\n> diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> index be34f20aa857..68eafb307b2a 100644\n> --- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n> +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> @@ -31,7 +31,9 @@ public:\n> \tvoid stop() override {}\n> \tvoid configure(const CameraSensorInfo &sensorInfo,\n> \t\t const std::map &streamConfig,\n> -\t\t const std::map &entityControls) override {}\n> +\t\t const std::map &entityControls,\n> +\t\t const IPAOperationData &ipaConfig,\n> +\t\t IPAOperationData *result) override {}\n> \tvoid mapBuffers(const std::vector &buffers) override {}\n> \tvoid unmapBuffers(const std::vector &ids) override {}\n> \tvoid processEvent(const IPAOperationData &event) override {}\n> diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> index 6fbebed2ba72..aa403e22f297 100644\n> --- a/src/libcamera/proxy/ipa_proxy_thread.cpp\n> +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> @@ -31,7 +31,9 @@ public:\n> \n> \tvoid configure(const CameraSensorInfo &sensorInfo,\n> \t\t const std::map &streamConfig,\n> -\t\t const std::map &entityControls) override;\n> +\t\t const std::map &entityControls,\n> +\t\t const IPAOperationData &ipaConfig,\n> +\t\t IPAOperationData *result) override;\n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n> \tvoid processEvent(const IPAOperationData &event) override;\n> @@ -129,9 +131,12 @@ void IPAProxyThread::stop()\n> \n> void IPAProxyThread::configure(const CameraSensorInfo &sensorInfo,\n> \t\t\t const std::map &streamConfig,\n> -\t\t\t const std::map &entityControls)\n> +\t\t\t const std::map &entityControls,\n> +\t\t\t const IPAOperationData &ipaConfig,\n> +\t\t\t IPAOperationData *result)\n> {\n> -\tipa_->configure(sensorInfo, streamConfig, entityControls);\n> +\tipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig,\n> +\t\t\tresult);\n> }\n> \n> void IPAProxyThread::mapBuffers(const std::vector &buffers)\n> diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp\n> index aa7a9dcc6050..23c799da0663 100644\n> --- a/test/ipa/ipa_wrappers_test.cpp\n> +++ b/test/ipa/ipa_wrappers_test.cpp\n> @@ -69,7 +69,9 @@ public:\n> \n> \tvoid configure(const CameraSensorInfo &sensorInfo,\n> \t\t const std::map &streamConfig,\n> -\t\t const std::map &entityControls) override\n> +\t\t const std::map &entityControls,\n> +\t\t const IPAOperationData &ipaConfig,\n> +\t\t IPAOperationData *result) override\n> \t{\n> \t\t/* Verify sensorInfo. */\n> \t\tif (sensorInfo.outputSize.width != 2560 ||\n> @@ -317,7 +319,9 @@ protected:\n> \t\t};\n> \t\tstd::map controlInfo;\n> \t\tcontrolInfo.emplace(42, subdev_->controls());\n> -\t\tret = INVOKE(configure, sensorInfo, config, controlInfo);\n> +\t\tIPAOperationData ipaConfig;\n> +\t\tret = INVOKE(configure, sensorInfo, config, controlInfo,\n> +\t\t\t ipaConfig, nullptr);\n> \t\tif (ret == TestFail)\n> \t\t\treturn TestFail;\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 57936BFFE2\n\tfor ;\n\tMon, 29 Jun 2020 14:31:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D7BFF609D6;\n\tMon, 29 Jun 2020 16:31:16 +0200 (CEST)","from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0D53C603BB\n\tfor ;\n\tMon, 29 Jun 2020 16:31:16 +0200 (CEST)","by mail-lj1-x243.google.com with SMTP id d17so3586266ljl.3\n\tfor ;\n\tMon, 29 Jun 2020 07:31:15 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id g5sm14302ljk.93.2020.06.29.07.31.14\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 29 Jun 2020 07:31:14 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"K4RAU8xl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=m8fssFS3kZxQjWO7gURykE6gciwLOUmVqNM/WodZIsY=;\n\tb=K4RAU8xlvp4mvKOzMBSYMY18CYla+hAFmp8tDBbEw64Fls6k6BPUwgYhNBmiZrqhxL\n\t7ShaXf7i9Pj8Cytpy+scU2r5+guS4vgP4N3S/FJMfwlaiONnTB4sVnmcJ+PTTVbUf3Zb\n\thlKeAvyf1ibyPW2MCrLD3Qy5YGRxuD9rkkaTGQKCyNtJAHrDP8HZek7jsrctAOQ6Ud1v\n\tENJi23FqpHQN2WjKlHkAmutGhVCui7kmz0K3fGed4e3O5qYu5+Mrnbjig0bITck7LaCT\n\tf01AEYBBjhcRqmM/XgfIktIMndOrTo+HZaMC5Q6LdEQaCOdCfOUTfjKoNkCEWa/+PNJa\n\tEi1g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=m8fssFS3kZxQjWO7gURykE6gciwLOUmVqNM/WodZIsY=;\n\tb=PVuIuhf+6H+3b9UjoyEBSLI2F+Ebv4dQhY9ywkYDINVOdenCxOvfc0oPw795RBXXA/\n\tgaqNFqmZdQttdsnlxlxUA6QkcE35wCCIt3yjCvTTAI6cpRvcnRBf7o7oOjOZXmtW0DaN\n\thfDezF2mi33TJbPe91dkIdyqq2oSXu0M8ZWd+GgUpx111nb1xdAfYixpXdMFy0K7io0K\n\tNuruwzgh/Y8SpY4hbuHrTPnhFNpbMdji69DyE9MKsGPnF0Ds28jYdb0aXNLgB3OEJvn5\n\t0a5xSKxLlt06rnQRERIMgluoa02xTrz1L4yYM+OJQTpdA/gxPBTmgy4Ha6Itu6cnd+R2\n\tNVXw==","X-Gm-Message-State":"AOAM533Opxay7ecq2PxtsecmEdz9zjXWQWfQr2ERRms5lGosULcWmAt+\n\toGkl9ugOdMk2wVzRAFvHRHl36A==","X-Google-Smtp-Source":"ABdhPJwWgzww5wT8rKsb3DWBoivjrvY/fuxVqoUMyjL4o7cHQcmVw0caekO2+TeM84MgI8RA1t7BNg==","X-Received":"by 2002:a2e:9885:: with SMTP id b5mr5926041ljj.125.1593441075117;\n\tMon, 29 Jun 2020 07:31:15 -0700 (PDT)","Date":"Mon, 29 Jun 2020 16:31:13 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"<20200629143113.GC1852218@oden.dyn.berto.se>","References":"<20200628231934.29025-1-laurent.pinchart@ideasonboard.com>\n\t<20200628231934.29025-4-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200628231934.29025-4-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v1 3/9] libcamera: ipa_interface: Add\n\tsupport for custom IPA data to configure()","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":10970,"web_url":"https://patchwork.libcamera.org/comment/10970/","msgid":"<20200629145743.GG10681@pendragon.ideasonboard.com>","date":"2020-06-29T14:57:43","subject":"Re: [libcamera-devel] [PATCH v1 3/9] libcamera: ipa_interface: Add\n\tsupport for custom IPA data to configure()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Mon, Jun 29, 2020 at 04:31:13PM +0200, Niklas Söderlund wrote:\n> On 2020-06-29 02:19:28 +0300, Laurent Pinchart wrote:\n> > Add two new parameters, ipaConfig and result, to the\n> > IPAInterface::configure() function to allow pipeline handlers to pass\n> > custom data to their IPA, and receive data back. Wire this through the\n> > code base. The C API interface will be addressed separately, likely\n> > through automation of the C <-> C++ translation.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > include/libcamera/internal/ipa_context_wrapper.h | 4 +++-\n> > include/libcamera/ipa/ipa_interface.h | 4 +++-\n> > src/ipa/libipa/ipa_interface_wrapper.cpp | 5 ++++-\n> > src/ipa/raspberrypi/raspberrypi.cpp | 8 ++++++--\n> > src/ipa/rkisp1/rkisp1.cpp | 8 ++++++--\n> > src/ipa/vimc/vimc.cpp | 4 +++-\n> > src/libcamera/ipa_context_wrapper.cpp | 8 ++++++--\n> > src/libcamera/ipa_interface.cpp | 7 +++++++\n> > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 +++-\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +++-\n> > src/libcamera/proxy/ipa_proxy_linux.cpp | 4 +++-\n> > src/libcamera/proxy/ipa_proxy_thread.cpp | 11 ++++++++---\n> > test/ipa/ipa_wrappers_test.cpp | 8 ++++++--\n> > 13 files changed, 61 insertions(+), 18 deletions(-)\n> > \n> > diff --git a/include/libcamera/internal/ipa_context_wrapper.h b/include/libcamera/internal/ipa_context_wrapper.h\n> > index 4e6f791d18e6..8f767e844221 100644\n> > --- a/include/libcamera/internal/ipa_context_wrapper.h\n> > +++ b/include/libcamera/internal/ipa_context_wrapper.h\n> > @@ -24,7 +24,9 @@ public:\n> > \tvoid stop() override;\n> > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > \t\t const std::map &streamConfig,\n> > -\t\t const std::map &entityControls) override;\n> > +\t\t const std::map &entityControls,\n> > +\t\t const IPAOperationData &ipaConfig,\n> > +\t\t IPAOperationData *result) override;\n> > \n> > \tvoid mapBuffers(const std::vector &buffers) override;\n> > \tvoid unmapBuffers(const std::vector &ids) override;\n> > diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h\n> > index dc9fc714d564..5016ec25ea9c 100644\n> > --- a/include/libcamera/ipa/ipa_interface.h\n> > +++ b/include/libcamera/ipa/ipa_interface.h\n> > @@ -158,7 +158,9 @@ public:\n> > \n> > \tvirtual void configure(const CameraSensorInfo &sensorInfo,\n> > \t\t\t const std::map &streamConfig,\n> > -\t\t\t const std::map &entityControls) = 0;\n> > +\t\t\t const std::map &entityControls,\n> > +\t\t\t const IPAOperationData &ipaConfig,\n> > +\t\t\t IPAOperationData *result) = 0;\n> > \n> > \tvirtual void mapBuffers(const std::vector &buffers) = 0;\n> > \tvirtual void unmapBuffers(const std::vector &ids) = 0;\n> > diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp\n> > index 2a2e43abc708..47ce5a704851 100644\n> > --- a/src/ipa/libipa/ipa_interface_wrapper.cpp\n> > +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp\n> > @@ -166,7 +166,10 @@ void IPAInterfaceWrapper::configure(struct ipa_context *_ctx,\n> > \t\tentityControls.emplace(id, infoMaps[id]);\n> > \t}\n> > \n> > -\tctx->ipa_->configure(sensorInfo, ipaStreams, entityControls);\n> > +\t/* \\todo Translate the ipaConfig and reponse */\n> \n> I wonder if we should add a FATAL LOG statement here to make it clear \n> that we break the wrapper and this issue should be solved before anyone \n> attempts to use the feature, which we know won't work.\n\nThat would break existing tests though, as they use this function.\n\n> > +\tIPAOperationData ipaConfig;\n> > +\tctx->ipa_->configure(sensorInfo, ipaStreams, entityControls, ipaConfig,\n> > +\t\t\t nullptr);\n> > }\n> > \n> > void IPAInterfaceWrapper::map_buffers(struct ipa_context *_ctx,\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index bc89ab58d03a..860be22ddb5d 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -78,7 +78,9 @@ public:\n> > \n> > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > \t\t const std::map &streamConfig,\n> > -\t\t const std::map &entityControls) override;\n> > +\t\t const std::map &entityControls,\n> > +\t\t const IPAOperationData &data,\n> > +\t\t IPAOperationData *response) override;\n> > \tvoid mapBuffers(const std::vector &buffers) override;\n> > \tvoid unmapBuffers(const std::vector &ids) override;\n> > \tvoid processEvent(const IPAOperationData &event) override;\n> > @@ -186,7 +188,9 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)\n> > \n> > void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> > \t\t const std::map &streamConfig,\n> > -\t\t const std::map &entityControls)\n> > +\t\t const std::map &entityControls,\n> > +\t\t const IPAOperationData &ipaConfig,\n> > +\t\t IPAOperationData *result)\n> > {\n> > \tif (entityControls.empty())\n> > \t\treturn;\n> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > index fbdc908fc816..34d6f63a7ff4 100644\n> > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > @@ -39,7 +39,9 @@ public:\n> > \n> > \tvoid configure(const CameraSensorInfo &info,\n> > \t\t const std::map &streamConfig,\n> > -\t\t const std::map &entityControls) override;\n> > +\t\t const std::map &entityControls,\n> > +\t\t const IPAOperationData &ipaConfig,\n> > +\t\t IPAOperationData *response) override;\n> > \tvoid mapBuffers(const std::vector &buffers) override;\n> > \tvoid unmapBuffers(const std::vector &ids) override;\n> > \tvoid processEvent(const IPAOperationData &event) override;\n> > @@ -76,7 +78,9 @@ private:\n> > */\n> > void IPARkISP1::configure(const CameraSensorInfo &info,\n> > \t\t\t const std::map &streamConfig,\n> > -\t\t\t const std::map &entityControls)\n> > +\t\t\t const std::map &entityControls,\n> > +\t\t\t const IPAOperationData &ipaConfig,\n> > +\t\t\t IPAOperationData *result)\n> > {\n> > \tif (entityControls.empty())\n> > \t\treturn;\n> > diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\n> > index af278a482b8a..1593c92d80f3 100644\n> > --- a/src/ipa/vimc/vimc.cpp\n> > +++ b/src/ipa/vimc/vimc.cpp\n> > @@ -39,7 +39,9 @@ public:\n> > \n> > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > \t\t const std::map &streamConfig,\n> > -\t\t const std::map &entityControls) override {}\n> > +\t\t const std::map &entityControls,\n> > +\t\t const IPAOperationData &ipaConfig,\n> > +\t\t IPAOperationData *result) override {}\n> > \tvoid mapBuffers(const std::vector &buffers) override {}\n> > \tvoid unmapBuffers(const std::vector &ids) override {}\n> > \tvoid processEvent(const IPAOperationData &event) override {}\n> > diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp\n> > index 471118f57cdc..231300ce0bec 100644\n> > --- a/src/libcamera/ipa_context_wrapper.cpp\n> > +++ b/src/libcamera/ipa_context_wrapper.cpp\n> > @@ -110,10 +110,13 @@ void IPAContextWrapper::stop()\n> > \n> > void IPAContextWrapper::configure(const CameraSensorInfo &sensorInfo,\n> > \t\t\t\t const std::map &streamConfig,\n> > -\t\t\t\t const std::map &entityControls)\n> > +\t\t\t\t const std::map &entityControls,\n> > +\t\t\t\t const IPAOperationData &ipaConfig,\n> > +\t\t\t\t IPAOperationData *result)\n> > {\n> > \tif (intf_)\n> > -\t\treturn intf_->configure(sensorInfo, streamConfig, entityControls);\n> > +\t\treturn intf_->configure(sensorInfo, streamConfig,\n> > +\t\t\t\t\tentityControls, ipaConfig, result);\n> > \n> > \tif (!ctx_)\n> > \t\treturn;\n> > @@ -174,6 +177,7 @@ void IPAContextWrapper::configure(const CameraSensorInfo &sensorInfo,\n> > \t\t++i;\n> > \t}\n> > \n> > +\t/* \\todo Translate the ipaConfig and reponse */\n> > \tctx_->ops->configure(ctx_, &sensor_info, c_streams, streamConfig.size(),\n> > \t\t\t c_info_maps, entityControls.size());\n> > }\n> > diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp\n> > index ebe47e1233a5..23fc56d7d48e 100644\n> > --- a/src/libcamera/ipa_interface.cpp\n> > +++ b/src/libcamera/ipa_interface.cpp\n> > @@ -557,6 +557,8 @@ namespace libcamera {\n> > * \\param[in] sensorInfo Camera sensor information\n> > * \\param[in] streamConfig Configuration of all active streams\n> > * \\param[in] entityControls Controls provided by the pipeline entities\n> > + * \\param[in] ipaConfig Pipeline-handler-specific configuration data\n> > + * \\param[out] result Pipeline-handler-specific configuration result\n> > *\n> > * This method shall be called when the camera is started to inform the IPA of\n> > * the camera's streams and the sensor settings. The meaning of the numerical\n> > @@ -566,6 +568,11 @@ namespace libcamera {\n> > * The \\a sensorInfo conveys information about the camera sensor settings that\n> > * the pipeline handler has selected for the configuration. The IPA may use\n> > * that information to tune its algorithms.\n> > + *\n> > + * The \\a ipaConfig and \\a result parameters carry custom data passed by the\n> > + * pipeline handler to the IPA and back. The pipeline handler may set the \\a\n> > + * result parameter to null if the IPA protocol doesn't need to pass a result\n> > + * back through the configure() function.\n> > */\n> > \n> > /**\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index dcd737a1d1a0..3b5cdf1e1849 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -1017,7 +1017,9 @@ int PipelineHandlerRPi::configureIPA(Camera *camera)\n> > \t}\n> > \n> > \t/* Ready the IPA - it must know about the sensor resolution. */\n> > -\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls);\n> > +\tIPAOperationData ipaConfig;\n> > +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> > +\t\t\t ipaConfig, nullptr);\n> > \n> > \treturn 0;\n> > }\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 3c01821135f8..4fde5539e667 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -843,7 +843,9 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n> > \tstd::map entityControls;\n> > \tentityControls.emplace(0, data->sensor_->controls());\n> > \n> > -\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls);\n> > +\tIPAOperationData ipaConfig;\n> > +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> > +\t\t\t ipaConfig, nullptr);\n> > \n> > \treturn ret;\n> > }\n> > diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > index be34f20aa857..68eafb307b2a 100644\n> > --- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > @@ -31,7 +31,9 @@ public:\n> > \tvoid stop() override {}\n> > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > \t\t const std::map &streamConfig,\n> > -\t\t const std::map &entityControls) override {}\n> > +\t\t const std::map &entityControls,\n> > +\t\t const IPAOperationData &ipaConfig,\n> > +\t\t IPAOperationData *result) override {}\n> > \tvoid mapBuffers(const std::vector &buffers) override {}\n> > \tvoid unmapBuffers(const std::vector &ids) override {}\n> > \tvoid processEvent(const IPAOperationData &event) override {}\n> > diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > index 6fbebed2ba72..aa403e22f297 100644\n> > --- a/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > @@ -31,7 +31,9 @@ public:\n> > \n> > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > \t\t const std::map &streamConfig,\n> > -\t\t const std::map &entityControls) override;\n> > +\t\t const std::map &entityControls,\n> > +\t\t const IPAOperationData &ipaConfig,\n> > +\t\t IPAOperationData *result) override;\n> > \tvoid mapBuffers(const std::vector &buffers) override;\n> > \tvoid unmapBuffers(const std::vector &ids) override;\n> > \tvoid processEvent(const IPAOperationData &event) override;\n> > @@ -129,9 +131,12 @@ void IPAProxyThread::stop()\n> > \n> > void IPAProxyThread::configure(const CameraSensorInfo &sensorInfo,\n> > \t\t\t const std::map &streamConfig,\n> > -\t\t\t const std::map &entityControls)\n> > +\t\t\t const std::map &entityControls,\n> > +\t\t\t const IPAOperationData &ipaConfig,\n> > +\t\t\t IPAOperationData *result)\n> > {\n> > -\tipa_->configure(sensorInfo, streamConfig, entityControls);\n> > +\tipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig,\n> > +\t\t\tresult);\n> > }\n> > \n> > void IPAProxyThread::mapBuffers(const std::vector &buffers)\n> > diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp\n> > index aa7a9dcc6050..23c799da0663 100644\n> > --- a/test/ipa/ipa_wrappers_test.cpp\n> > +++ b/test/ipa/ipa_wrappers_test.cpp\n> > @@ -69,7 +69,9 @@ public:\n> > \n> > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > \t\t const std::map &streamConfig,\n> > -\t\t const std::map &entityControls) override\n> > +\t\t const std::map &entityControls,\n> > +\t\t const IPAOperationData &ipaConfig,\n> > +\t\t IPAOperationData *result) override\n> > \t{\n> > \t\t/* Verify sensorInfo. */\n> > \t\tif (sensorInfo.outputSize.width != 2560 ||\n> > @@ -317,7 +319,9 @@ protected:\n> > \t\t};\n> > \t\tstd::map controlInfo;\n> > \t\tcontrolInfo.emplace(42, subdev_->controls());\n> > -\t\tret = INVOKE(configure, sensorInfo, config, controlInfo);\n> > +\t\tIPAOperationData ipaConfig;\n> > +\t\tret = INVOKE(configure, sensorInfo, config, controlInfo,\n> > +\t\t\t ipaConfig, nullptr);\n> > \t\tif (ret == TestFail)\n> > \t\t\treturn TestFail;\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 814B5BFFE2\n\tfor ;\n\tMon, 29 Jun 2020 14:57:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 50572609DD;\n\tMon, 29 Jun 2020 16:57:49 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2A082603B4\n\tfor ;\n\tMon, 29 Jun 2020 16:57:48 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 433B9299;\n\tMon, 29 Jun 2020 16:57:47 +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=\"gQSxeNun\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593442667;\n\tbh=lE5lSo5LmEy5M+55NmqvHBt2a5gxOiwbF0iZopRNLJk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gQSxeNunU3nzyksDXayAXdAN1WIo6iQreldhxKCvbPcbn2IohiD7TXsan7BTkhHdE\n\tqU6llsQULP8tRxQX1aHaQKSthde9wr6Qd7ebs9SWF5jpD5cAECtbkN2XuqrSxe3mU0\n\t7fid60xYK45NrLfNdebNo3P5LKRRaLK9DFsL+H6Y=","Date":"Mon, 29 Jun 2020 17:57:43 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20200629145743.GG10681@pendragon.ideasonboard.com>","References":"<20200628231934.29025-1-laurent.pinchart@ideasonboard.com>\n\t<20200628231934.29025-4-laurent.pinchart@ideasonboard.com>\n\t<20200629143113.GC1852218@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200629143113.GC1852218@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH v1 3/9] libcamera: ipa_interface: Add\n\tsupport for custom IPA data to configure()","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":10972,"web_url":"https://patchwork.libcamera.org/comment/10972/","msgid":"<20200629162537.GB1881444@oden.dyn.berto.se>","date":"2020-06-29T16:25:37","subject":"Re: [libcamera-devel] [PATCH v1 3/9] libcamera: ipa_interface: Add\n\tsupport for custom IPA data to configure()","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nOn 2020-06-29 17:57:43 +0300, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> On Mon, Jun 29, 2020 at 04:31:13PM +0200, Niklas Söderlund wrote:\n> > On 2020-06-29 02:19:28 +0300, Laurent Pinchart wrote:\n> > > Add two new parameters, ipaConfig and result, to the\n> > > IPAInterface::configure() function to allow pipeline handlers to pass\n> > > custom data to their IPA, and receive data back. Wire this through the\n> > > code base. The C API interface will be addressed separately, likely\n> > > through automation of the C <-> C++ translation.\n> > > \n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > include/libcamera/internal/ipa_context_wrapper.h | 4 +++-\n> > > include/libcamera/ipa/ipa_interface.h | 4 +++-\n> > > src/ipa/libipa/ipa_interface_wrapper.cpp | 5 ++++-\n> > > src/ipa/raspberrypi/raspberrypi.cpp | 8 ++++++--\n> > > src/ipa/rkisp1/rkisp1.cpp | 8 ++++++--\n> > > src/ipa/vimc/vimc.cpp | 4 +++-\n> > > src/libcamera/ipa_context_wrapper.cpp | 8 ++++++--\n> > > src/libcamera/ipa_interface.cpp | 7 +++++++\n> > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 +++-\n> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +++-\n> > > src/libcamera/proxy/ipa_proxy_linux.cpp | 4 +++-\n> > > src/libcamera/proxy/ipa_proxy_thread.cpp | 11 ++++++++---\n> > > test/ipa/ipa_wrappers_test.cpp | 8 ++++++--\n> > > 13 files changed, 61 insertions(+), 18 deletions(-)\n> > > \n> > > diff --git a/include/libcamera/internal/ipa_context_wrapper.h b/include/libcamera/internal/ipa_context_wrapper.h\n> > > index 4e6f791d18e6..8f767e844221 100644\n> > > --- a/include/libcamera/internal/ipa_context_wrapper.h\n> > > +++ b/include/libcamera/internal/ipa_context_wrapper.h\n> > > @@ -24,7 +24,9 @@ public:\n> > > \tvoid stop() override;\n> > > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > > \t\t const std::map &streamConfig,\n> > > -\t\t const std::map &entityControls) override;\n> > > +\t\t const std::map &entityControls,\n> > > +\t\t const IPAOperationData &ipaConfig,\n> > > +\t\t IPAOperationData *result) override;\n> > > \n> > > \tvoid mapBuffers(const std::vector &buffers) override;\n> > > \tvoid unmapBuffers(const std::vector &ids) override;\n> > > diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h\n> > > index dc9fc714d564..5016ec25ea9c 100644\n> > > --- a/include/libcamera/ipa/ipa_interface.h\n> > > +++ b/include/libcamera/ipa/ipa_interface.h\n> > > @@ -158,7 +158,9 @@ public:\n> > > \n> > > \tvirtual void configure(const CameraSensorInfo &sensorInfo,\n> > > \t\t\t const std::map &streamConfig,\n> > > -\t\t\t const std::map &entityControls) = 0;\n> > > +\t\t\t const std::map &entityControls,\n> > > +\t\t\t const IPAOperationData &ipaConfig,\n> > > +\t\t\t IPAOperationData *result) = 0;\n> > > \n> > > \tvirtual void mapBuffers(const std::vector &buffers) = 0;\n> > > \tvirtual void unmapBuffers(const std::vector &ids) = 0;\n> > > diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp\n> > > index 2a2e43abc708..47ce5a704851 100644\n> > > --- a/src/ipa/libipa/ipa_interface_wrapper.cpp\n> > > +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp\n> > > @@ -166,7 +166,10 @@ void IPAInterfaceWrapper::configure(struct ipa_context *_ctx,\n> > > \t\tentityControls.emplace(id, infoMaps[id]);\n> > > \t}\n> > > \n> > > -\tctx->ipa_->configure(sensorInfo, ipaStreams, entityControls);\n> > > +\t/* \\todo Translate the ipaConfig and reponse */\n> > \n> > I wonder if we should add a FATAL LOG statement here to make it clear \n> > that we break the wrapper and this issue should be solved before anyone \n> > attempts to use the feature, which we know won't work.\n> \n> That would break existing tests though, as they use this function.\n\nSince all this is going away I wont stress it :-)\n\n> \n> > > +\tIPAOperationData ipaConfig;\n> > > +\tctx->ipa_->configure(sensorInfo, ipaStreams, entityControls, ipaConfig,\n> > > +\t\t\t nullptr);\n> > > }\n> > > \n> > > void IPAInterfaceWrapper::map_buffers(struct ipa_context *_ctx,\n> > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > index bc89ab58d03a..860be22ddb5d 100644\n> > > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > @@ -78,7 +78,9 @@ public:\n> > > \n> > > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > > \t\t const std::map &streamConfig,\n> > > -\t\t const std::map &entityControls) override;\n> > > +\t\t const std::map &entityControls,\n> > > +\t\t const IPAOperationData &data,\n> > > +\t\t IPAOperationData *response) override;\n> > > \tvoid mapBuffers(const std::vector &buffers) override;\n> > > \tvoid unmapBuffers(const std::vector &ids) override;\n> > > \tvoid processEvent(const IPAOperationData &event) override;\n> > > @@ -186,7 +188,9 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)\n> > > \n> > > void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> > > \t\t const std::map &streamConfig,\n> > > -\t\t const std::map &entityControls)\n> > > +\t\t const std::map &entityControls,\n> > > +\t\t const IPAOperationData &ipaConfig,\n> > > +\t\t IPAOperationData *result)\n> > > {\n> > > \tif (entityControls.empty())\n> > > \t\treturn;\n> > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > index fbdc908fc816..34d6f63a7ff4 100644\n> > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > @@ -39,7 +39,9 @@ public:\n> > > \n> > > \tvoid configure(const CameraSensorInfo &info,\n> > > \t\t const std::map &streamConfig,\n> > > -\t\t const std::map &entityControls) override;\n> > > +\t\t const std::map &entityControls,\n> > > +\t\t const IPAOperationData &ipaConfig,\n> > > +\t\t IPAOperationData *response) override;\n> > > \tvoid mapBuffers(const std::vector &buffers) override;\n> > > \tvoid unmapBuffers(const std::vector &ids) override;\n> > > \tvoid processEvent(const IPAOperationData &event) override;\n> > > @@ -76,7 +78,9 @@ private:\n> > > */\n> > > void IPARkISP1::configure(const CameraSensorInfo &info,\n> > > \t\t\t const std::map &streamConfig,\n> > > -\t\t\t const std::map &entityControls)\n> > > +\t\t\t const std::map &entityControls,\n> > > +\t\t\t const IPAOperationData &ipaConfig,\n> > > +\t\t\t IPAOperationData *result)\n> > > {\n> > > \tif (entityControls.empty())\n> > > \t\treturn;\n> > > diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\n> > > index af278a482b8a..1593c92d80f3 100644\n> > > --- a/src/ipa/vimc/vimc.cpp\n> > > +++ b/src/ipa/vimc/vimc.cpp\n> > > @@ -39,7 +39,9 @@ public:\n> > > \n> > > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > > \t\t const std::map &streamConfig,\n> > > -\t\t const std::map &entityControls) override {}\n> > > +\t\t const std::map &entityControls,\n> > > +\t\t const IPAOperationData &ipaConfig,\n> > > +\t\t IPAOperationData *result) override {}\n> > > \tvoid mapBuffers(const std::vector &buffers) override {}\n> > > \tvoid unmapBuffers(const std::vector &ids) override {}\n> > > \tvoid processEvent(const IPAOperationData &event) override {}\n> > > diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp\n> > > index 471118f57cdc..231300ce0bec 100644\n> > > --- a/src/libcamera/ipa_context_wrapper.cpp\n> > > +++ b/src/libcamera/ipa_context_wrapper.cpp\n> > > @@ -110,10 +110,13 @@ void IPAContextWrapper::stop()\n> > > \n> > > void IPAContextWrapper::configure(const CameraSensorInfo &sensorInfo,\n> > > \t\t\t\t const std::map &streamConfig,\n> > > -\t\t\t\t const std::map &entityControls)\n> > > +\t\t\t\t const std::map &entityControls,\n> > > +\t\t\t\t const IPAOperationData &ipaConfig,\n> > > +\t\t\t\t IPAOperationData *result)\n> > > {\n> > > \tif (intf_)\n> > > -\t\treturn intf_->configure(sensorInfo, streamConfig, entityControls);\n> > > +\t\treturn intf_->configure(sensorInfo, streamConfig,\n> > > +\t\t\t\t\tentityControls, ipaConfig, result);\n> > > \n> > > \tif (!ctx_)\n> > > \t\treturn;\n> > > @@ -174,6 +177,7 @@ void IPAContextWrapper::configure(const CameraSensorInfo &sensorInfo,\n> > > \t\t++i;\n> > > \t}\n> > > \n> > > +\t/* \\todo Translate the ipaConfig and reponse */\n> > > \tctx_->ops->configure(ctx_, &sensor_info, c_streams, streamConfig.size(),\n> > > \t\t\t c_info_maps, entityControls.size());\n> > > }\n> > > diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp\n> > > index ebe47e1233a5..23fc56d7d48e 100644\n> > > --- a/src/libcamera/ipa_interface.cpp\n> > > +++ b/src/libcamera/ipa_interface.cpp\n> > > @@ -557,6 +557,8 @@ namespace libcamera {\n> > > * \\param[in] sensorInfo Camera sensor information\n> > > * \\param[in] streamConfig Configuration of all active streams\n> > > * \\param[in] entityControls Controls provided by the pipeline entities\n> > > + * \\param[in] ipaConfig Pipeline-handler-specific configuration data\n> > > + * \\param[out] result Pipeline-handler-specific configuration result\n> > > *\n> > > * This method shall be called when the camera is started to inform the IPA of\n> > > * the camera's streams and the sensor settings. The meaning of the numerical\n> > > @@ -566,6 +568,11 @@ namespace libcamera {\n> > > * The \\a sensorInfo conveys information about the camera sensor settings that\n> > > * the pipeline handler has selected for the configuration. The IPA may use\n> > > * that information to tune its algorithms.\n> > > + *\n> > > + * The \\a ipaConfig and \\a result parameters carry custom data passed by the\n> > > + * pipeline handler to the IPA and back. The pipeline handler may set the \\a\n> > > + * result parameter to null if the IPA protocol doesn't need to pass a result\n> > > + * back through the configure() function.\n> > > */\n> > > \n> > > /**\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index dcd737a1d1a0..3b5cdf1e1849 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -1017,7 +1017,9 @@ int PipelineHandlerRPi::configureIPA(Camera *camera)\n> > > \t}\n> > > \n> > > \t/* Ready the IPA - it must know about the sensor resolution. */\n> > > -\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls);\n> > > +\tIPAOperationData ipaConfig;\n> > > +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> > > +\t\t\t ipaConfig, nullptr);\n> > > \n> > > \treturn 0;\n> > > }\n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > index 3c01821135f8..4fde5539e667 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > @@ -843,7 +843,9 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n> > > \tstd::map entityControls;\n> > > \tentityControls.emplace(0, data->sensor_->controls());\n> > > \n> > > -\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls);\n> > > +\tIPAOperationData ipaConfig;\n> > > +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> > > +\t\t\t ipaConfig, nullptr);\n> > > \n> > > \treturn ret;\n> > > }\n> > > diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > > index be34f20aa857..68eafb307b2a 100644\n> > > --- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > > +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > > @@ -31,7 +31,9 @@ public:\n> > > \tvoid stop() override {}\n> > > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > > \t\t const std::map &streamConfig,\n> > > -\t\t const std::map &entityControls) override {}\n> > > +\t\t const std::map &entityControls,\n> > > +\t\t const IPAOperationData &ipaConfig,\n> > > +\t\t IPAOperationData *result) override {}\n> > > \tvoid mapBuffers(const std::vector &buffers) override {}\n> > > \tvoid unmapBuffers(const std::vector &ids) override {}\n> > > \tvoid processEvent(const IPAOperationData &event) override {}\n> > > diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > > index 6fbebed2ba72..aa403e22f297 100644\n> > > --- a/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > > +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > > @@ -31,7 +31,9 @@ public:\n> > > \n> > > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > > \t\t const std::map &streamConfig,\n> > > -\t\t const std::map &entityControls) override;\n> > > +\t\t const std::map &entityControls,\n> > > +\t\t const IPAOperationData &ipaConfig,\n> > > +\t\t IPAOperationData *result) override;\n> > > \tvoid mapBuffers(const std::vector &buffers) override;\n> > > \tvoid unmapBuffers(const std::vector &ids) override;\n> > > \tvoid processEvent(const IPAOperationData &event) override;\n> > > @@ -129,9 +131,12 @@ void IPAProxyThread::stop()\n> > > \n> > > void IPAProxyThread::configure(const CameraSensorInfo &sensorInfo,\n> > > \t\t\t const std::map &streamConfig,\n> > > -\t\t\t const std::map &entityControls)\n> > > +\t\t\t const std::map &entityControls,\n> > > +\t\t\t const IPAOperationData &ipaConfig,\n> > > +\t\t\t IPAOperationData *result)\n> > > {\n> > > -\tipa_->configure(sensorInfo, streamConfig, entityControls);\n> > > +\tipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig,\n> > > +\t\t\tresult);\n> > > }\n> > > \n> > > void IPAProxyThread::mapBuffers(const std::vector &buffers)\n> > > diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp\n> > > index aa7a9dcc6050..23c799da0663 100644\n> > > --- a/test/ipa/ipa_wrappers_test.cpp\n> > > +++ b/test/ipa/ipa_wrappers_test.cpp\n> > > @@ -69,7 +69,9 @@ public:\n> > > \n> > > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > > \t\t const std::map &streamConfig,\n> > > -\t\t const std::map &entityControls) override\n> > > +\t\t const std::map &entityControls,\n> > > +\t\t const IPAOperationData &ipaConfig,\n> > > +\t\t IPAOperationData *result) override\n> > > \t{\n> > > \t\t/* Verify sensorInfo. */\n> > > \t\tif (sensorInfo.outputSize.width != 2560 ||\n> > > @@ -317,7 +319,9 @@ protected:\n> > > \t\t};\n> > > \t\tstd::map controlInfo;\n> > > \t\tcontrolInfo.emplace(42, subdev_->controls());\n> > > -\t\tret = INVOKE(configure, sensorInfo, config, controlInfo);\n> > > +\t\tIPAOperationData ipaConfig;\n> > > +\t\tret = INVOKE(configure, sensorInfo, config, controlInfo,\n> > > +\t\t\t ipaConfig, nullptr);\n> > > \t\tif (ret == TestFail)\n> > > \t\t\treturn TestFail;\n> > > \n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 C3098BFFE2\n\tfor ;\n\tMon, 29 Jun 2020 16:25:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3DBCB609C9;\n\tMon, 29 Jun 2020 18:25:41 +0200 (CEST)","from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5E107603B1\n\tfor ;\n\tMon, 29 Jun 2020 18:25:39 +0200 (CEST)","by mail-lj1-x243.google.com with SMTP id t25so14131382lji.12\n\tfor ;\n\tMon, 29 Jun 2020 09:25:39 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id v5sm74708ljd.10.2020.06.29.09.25.37\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 29 Jun 2020 09:25:37 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"ALJo8v1v\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=DxgRuC0MYCWGWiCORMKHqSS/SUccFKm6xs+q2ugl3Lc=;\n\tb=ALJo8v1vGVPi1MhnZdTe5Q7Gv5UpBryAryOp4TATi1yZg5VEUWoogkGIRsDfWfbV7o\n\teBqy8UcMyYSjjE+JQ7qFw/qRoCxYnErz3+JeeoA7jiZZfris+J0k5BeS7QBOjWI8vzNu\n\tvQhBPVG/DJc4mGE5LsLqM2STfJ4gut9eiqWOcfiHGSBxlIOtCEFTeHG63m/TeqLyMlkp\n\tamUh0o+u6QLmPN4jaBmPeQt2BA+1euTK4oE/sazYwgCVGtsXdyhAGtZl4gqmRs/1QMTJ\n\tySbXnC2TNGssUUsnak24GrSOWckl3JXTnHW1TtmqR1Kzudq50cmeD1gdRW1qHLPwqA6y\n\tD5Hg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=DxgRuC0MYCWGWiCORMKHqSS/SUccFKm6xs+q2ugl3Lc=;\n\tb=dd71ra9++AWqrvVKL2mI8tPaSwjRjNU4rZHvMDiZLXT9AsjEsSMexnXP4RJQ5BFIk5\n\t8WoFuaGn1HxN/7quNtW3/Dsl0Kp5b7fy6EFmbwgwVPsOpAnztYk/BDROAyb7CxDfBAf4\n\t4DGVdfaVHL7WGk+xMc9yYSkDSa9YFIypUoUSN0S9lRJEUNwAGLwNXovKaWMOv8DQHEgZ\n\tbLZCwYnOr3Nszu9zTPL8hk1w2jKhQG6IsoYKyVseFpb6IVcdXtO5D7tplNyJmhQ2hLjr\n\t7hauN9CbSmqY1ypWMGWlXYC14qW5LFtCrCbwN5U3PHhM78YXU0N+S/jjoYNjNL+2Fz1g\n\tUtIw==","X-Gm-Message-State":"AOAM532WmUJS1J+w+7xPbTTF5Hwu155llv3bYR9rL6gFtk9XMbkEm4l2\n\t7d1BKtO+YZWEXGMNKtO3OSpp/ujmLtU=","X-Google-Smtp-Source":"ABdhPJwXF5Kl2yMnFssv726cy/Ebja5Dshe2mZtSSdiyys/xN+cy+BI7f7nlODlo8RPaa9DymNDm7w==","X-Received":"by 2002:a2e:730c:: with SMTP id\n\to12mr2723302ljc.165.1593447938541; \n\tMon, 29 Jun 2020 09:25:38 -0700 (PDT)","Date":"Mon, 29 Jun 2020 18:25:37 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"<20200629162537.GB1881444@oden.dyn.berto.se>","References":"<20200628231934.29025-1-laurent.pinchart@ideasonboard.com>\n\t<20200628231934.29025-4-laurent.pinchart@ideasonboard.com>\n\t<20200629143113.GC1852218@oden.dyn.berto.se>\n\t<20200629145743.GG10681@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200629145743.GG10681@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v1 3/9] libcamera: ipa_interface: Add\n\tsupport for custom IPA data to configure()","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":10988,"web_url":"https://patchwork.libcamera.org/comment/10988/","msgid":"<20200629205143.GA1893170@oden.dyn.berto.se>","date":"2020-06-29T20:51:43","subject":"Re: [libcamera-devel] [PATCH v1 3/9] libcamera: ipa_interface: Add\n\tsupport for custom IPA data to configure()","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nOn 2020-06-29 18:25:38 +0200, Niklas Söderlund wrote:\n> Hi Laurent,\n> \n> On 2020-06-29 17:57:43 +0300, Laurent Pinchart wrote:\n> > Hi Niklas,\n> > \n> > On Mon, Jun 29, 2020 at 04:31:13PM +0200, Niklas Söderlund wrote:\n> > > On 2020-06-29 02:19:28 +0300, Laurent Pinchart wrote:\n> > > > Add two new parameters, ipaConfig and result, to the\n> > > > IPAInterface::configure() function to allow pipeline handlers to pass\n> > > > custom data to their IPA, and receive data back. Wire this through the\n> > > > code base. The C API interface will be addressed separately, likely\n> > > > through automation of the C <-> C++ translation.\n> > > > \n> > > > Signed-off-by: Laurent Pinchart \n> > > > ---\n> > > > include/libcamera/internal/ipa_context_wrapper.h | 4 +++-\n> > > > include/libcamera/ipa/ipa_interface.h | 4 +++-\n> > > > src/ipa/libipa/ipa_interface_wrapper.cpp | 5 ++++-\n> > > > src/ipa/raspberrypi/raspberrypi.cpp | 8 ++++++--\n> > > > src/ipa/rkisp1/rkisp1.cpp | 8 ++++++--\n> > > > src/ipa/vimc/vimc.cpp | 4 +++-\n> > > > src/libcamera/ipa_context_wrapper.cpp | 8 ++++++--\n> > > > src/libcamera/ipa_interface.cpp | 7 +++++++\n> > > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 +++-\n> > > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +++-\n> > > > src/libcamera/proxy/ipa_proxy_linux.cpp | 4 +++-\n> > > > src/libcamera/proxy/ipa_proxy_thread.cpp | 11 ++++++++---\n> > > > test/ipa/ipa_wrappers_test.cpp | 8 ++++++--\n> > > > 13 files changed, 61 insertions(+), 18 deletions(-)\n> > > > \n> > > > diff --git a/include/libcamera/internal/ipa_context_wrapper.h b/include/libcamera/internal/ipa_context_wrapper.h\n> > > > index 4e6f791d18e6..8f767e844221 100644\n> > > > --- a/include/libcamera/internal/ipa_context_wrapper.h\n> > > > +++ b/include/libcamera/internal/ipa_context_wrapper.h\n> > > > @@ -24,7 +24,9 @@ public:\n> > > > \tvoid stop() override;\n> > > > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > > > \t\t const std::map &streamConfig,\n> > > > -\t\t const std::map &entityControls) override;\n> > > > +\t\t const std::map &entityControls,\n> > > > +\t\t const IPAOperationData &ipaConfig,\n> > > > +\t\t IPAOperationData *result) override;\n> > > > \n> > > > \tvoid mapBuffers(const std::vector &buffers) override;\n> > > > \tvoid unmapBuffers(const std::vector &ids) override;\n> > > > diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h\n> > > > index dc9fc714d564..5016ec25ea9c 100644\n> > > > --- a/include/libcamera/ipa/ipa_interface.h\n> > > > +++ b/include/libcamera/ipa/ipa_interface.h\n> > > > @@ -158,7 +158,9 @@ public:\n> > > > \n> > > > \tvirtual void configure(const CameraSensorInfo &sensorInfo,\n> > > > \t\t\t const std::map &streamConfig,\n> > > > -\t\t\t const std::map &entityControls) = 0;\n> > > > +\t\t\t const std::map &entityControls,\n> > > > +\t\t\t const IPAOperationData &ipaConfig,\n> > > > +\t\t\t IPAOperationData *result) = 0;\n> > > > \n> > > > \tvirtual void mapBuffers(const std::vector &buffers) = 0;\n> > > > \tvirtual void unmapBuffers(const std::vector &ids) = 0;\n> > > > diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp\n> > > > index 2a2e43abc708..47ce5a704851 100644\n> > > > --- a/src/ipa/libipa/ipa_interface_wrapper.cpp\n> > > > +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp\n> > > > @@ -166,7 +166,10 @@ void IPAInterfaceWrapper::configure(struct ipa_context *_ctx,\n> > > > \t\tentityControls.emplace(id, infoMaps[id]);\n> > > > \t}\n> > > > \n> > > > -\tctx->ipa_->configure(sensorInfo, ipaStreams, entityControls);\n> > > > +\t/* \\todo Translate the ipaConfig and reponse */\n> > > \n> > > I wonder if we should add a FATAL LOG statement here to make it clear \n> > > that we break the wrapper and this issue should be solved before anyone \n> > > attempts to use the feature, which we know won't work.\n> > \n> > That would break existing tests though, as they use this function.\n> \n> Since all this is going away I wont stress it :-)\n\nI forgot to add,\n\nReviewed-by: Niklas Söderlund \n\n> \n> > \n> > > > +\tIPAOperationData ipaConfig;\n> > > > +\tctx->ipa_->configure(sensorInfo, ipaStreams, entityControls, ipaConfig,\n> > > > +\t\t\t nullptr);\n> > > > }\n> > > > \n> > > > void IPAInterfaceWrapper::map_buffers(struct ipa_context *_ctx,\n> > > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > > index bc89ab58d03a..860be22ddb5d 100644\n> > > > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > > > @@ -78,7 +78,9 @@ public:\n> > > > \n> > > > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > > > \t\t const std::map &streamConfig,\n> > > > -\t\t const std::map &entityControls) override;\n> > > > +\t\t const std::map &entityControls,\n> > > > +\t\t const IPAOperationData &data,\n> > > > +\t\t IPAOperationData *response) override;\n> > > > \tvoid mapBuffers(const std::vector &buffers) override;\n> > > > \tvoid unmapBuffers(const std::vector &ids) override;\n> > > > \tvoid processEvent(const IPAOperationData &event) override;\n> > > > @@ -186,7 +188,9 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)\n> > > > \n> > > > void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> > > > \t\t const std::map &streamConfig,\n> > > > -\t\t const std::map &entityControls)\n> > > > +\t\t const std::map &entityControls,\n> > > > +\t\t const IPAOperationData &ipaConfig,\n> > > > +\t\t IPAOperationData *result)\n> > > > {\n> > > > \tif (entityControls.empty())\n> > > > \t\treturn;\n> > > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > > index fbdc908fc816..34d6f63a7ff4 100644\n> > > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > > @@ -39,7 +39,9 @@ public:\n> > > > \n> > > > \tvoid configure(const CameraSensorInfo &info,\n> > > > \t\t const std::map &streamConfig,\n> > > > -\t\t const std::map &entityControls) override;\n> > > > +\t\t const std::map &entityControls,\n> > > > +\t\t const IPAOperationData &ipaConfig,\n> > > > +\t\t IPAOperationData *response) override;\n> > > > \tvoid mapBuffers(const std::vector &buffers) override;\n> > > > \tvoid unmapBuffers(const std::vector &ids) override;\n> > > > \tvoid processEvent(const IPAOperationData &event) override;\n> > > > @@ -76,7 +78,9 @@ private:\n> > > > */\n> > > > void IPARkISP1::configure(const CameraSensorInfo &info,\n> > > > \t\t\t const std::map &streamConfig,\n> > > > -\t\t\t const std::map &entityControls)\n> > > > +\t\t\t const std::map &entityControls,\n> > > > +\t\t\t const IPAOperationData &ipaConfig,\n> > > > +\t\t\t IPAOperationData *result)\n> > > > {\n> > > > \tif (entityControls.empty())\n> > > > \t\treturn;\n> > > > diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\n> > > > index af278a482b8a..1593c92d80f3 100644\n> > > > --- a/src/ipa/vimc/vimc.cpp\n> > > > +++ b/src/ipa/vimc/vimc.cpp\n> > > > @@ -39,7 +39,9 @@ public:\n> > > > \n> > > > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > > > \t\t const std::map &streamConfig,\n> > > > -\t\t const std::map &entityControls) override {}\n> > > > +\t\t const std::map &entityControls,\n> > > > +\t\t const IPAOperationData &ipaConfig,\n> > > > +\t\t IPAOperationData *result) override {}\n> > > > \tvoid mapBuffers(const std::vector &buffers) override {}\n> > > > \tvoid unmapBuffers(const std::vector &ids) override {}\n> > > > \tvoid processEvent(const IPAOperationData &event) override {}\n> > > > diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp\n> > > > index 471118f57cdc..231300ce0bec 100644\n> > > > --- a/src/libcamera/ipa_context_wrapper.cpp\n> > > > +++ b/src/libcamera/ipa_context_wrapper.cpp\n> > > > @@ -110,10 +110,13 @@ void IPAContextWrapper::stop()\n> > > > \n> > > > void IPAContextWrapper::configure(const CameraSensorInfo &sensorInfo,\n> > > > \t\t\t\t const std::map &streamConfig,\n> > > > -\t\t\t\t const std::map &entityControls)\n> > > > +\t\t\t\t const std::map &entityControls,\n> > > > +\t\t\t\t const IPAOperationData &ipaConfig,\n> > > > +\t\t\t\t IPAOperationData *result)\n> > > > {\n> > > > \tif (intf_)\n> > > > -\t\treturn intf_->configure(sensorInfo, streamConfig, entityControls);\n> > > > +\t\treturn intf_->configure(sensorInfo, streamConfig,\n> > > > +\t\t\t\t\tentityControls, ipaConfig, result);\n> > > > \n> > > > \tif (!ctx_)\n> > > > \t\treturn;\n> > > > @@ -174,6 +177,7 @@ void IPAContextWrapper::configure(const CameraSensorInfo &sensorInfo,\n> > > > \t\t++i;\n> > > > \t}\n> > > > \n> > > > +\t/* \\todo Translate the ipaConfig and reponse */\n> > > > \tctx_->ops->configure(ctx_, &sensor_info, c_streams, streamConfig.size(),\n> > > > \t\t\t c_info_maps, entityControls.size());\n> > > > }\n> > > > diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp\n> > > > index ebe47e1233a5..23fc56d7d48e 100644\n> > > > --- a/src/libcamera/ipa_interface.cpp\n> > > > +++ b/src/libcamera/ipa_interface.cpp\n> > > > @@ -557,6 +557,8 @@ namespace libcamera {\n> > > > * \\param[in] sensorInfo Camera sensor information\n> > > > * \\param[in] streamConfig Configuration of all active streams\n> > > > * \\param[in] entityControls Controls provided by the pipeline entities\n> > > > + * \\param[in] ipaConfig Pipeline-handler-specific configuration data\n> > > > + * \\param[out] result Pipeline-handler-specific configuration result\n> > > > *\n> > > > * This method shall be called when the camera is started to inform the IPA of\n> > > > * the camera's streams and the sensor settings. The meaning of the numerical\n> > > > @@ -566,6 +568,11 @@ namespace libcamera {\n> > > > * The \\a sensorInfo conveys information about the camera sensor settings that\n> > > > * the pipeline handler has selected for the configuration. The IPA may use\n> > > > * that information to tune its algorithms.\n> > > > + *\n> > > > + * The \\a ipaConfig and \\a result parameters carry custom data passed by the\n> > > > + * pipeline handler to the IPA and back. The pipeline handler may set the \\a\n> > > > + * result parameter to null if the IPA protocol doesn't need to pass a result\n> > > > + * back through the configure() function.\n> > > > */\n> > > > \n> > > > /**\n> > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > index dcd737a1d1a0..3b5cdf1e1849 100644\n> > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > @@ -1017,7 +1017,9 @@ int PipelineHandlerRPi::configureIPA(Camera *camera)\n> > > > \t}\n> > > > \n> > > > \t/* Ready the IPA - it must know about the sensor resolution. */\n> > > > -\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls);\n> > > > +\tIPAOperationData ipaConfig;\n> > > > +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> > > > +\t\t\t ipaConfig, nullptr);\n> > > > \n> > > > \treturn 0;\n> > > > }\n> > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > index 3c01821135f8..4fde5539e667 100644\n> > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > @@ -843,7 +843,9 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n> > > > \tstd::map entityControls;\n> > > > \tentityControls.emplace(0, data->sensor_->controls());\n> > > > \n> > > > -\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls);\n> > > > +\tIPAOperationData ipaConfig;\n> > > > +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> > > > +\t\t\t ipaConfig, nullptr);\n> > > > \n> > > > \treturn ret;\n> > > > }\n> > > > diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > > > index be34f20aa857..68eafb307b2a 100644\n> > > > --- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > > > +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > > > @@ -31,7 +31,9 @@ public:\n> > > > \tvoid stop() override {}\n> > > > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > > > \t\t const std::map &streamConfig,\n> > > > -\t\t const std::map &entityControls) override {}\n> > > > +\t\t const std::map &entityControls,\n> > > > +\t\t const IPAOperationData &ipaConfig,\n> > > > +\t\t IPAOperationData *result) override {}\n> > > > \tvoid mapBuffers(const std::vector &buffers) override {}\n> > > > \tvoid unmapBuffers(const std::vector &ids) override {}\n> > > > \tvoid processEvent(const IPAOperationData &event) override {}\n> > > > diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > > > index 6fbebed2ba72..aa403e22f297 100644\n> > > > --- a/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > > > +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > > > @@ -31,7 +31,9 @@ public:\n> > > > \n> > > > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > > > \t\t const std::map &streamConfig,\n> > > > -\t\t const std::map &entityControls) override;\n> > > > +\t\t const std::map &entityControls,\n> > > > +\t\t const IPAOperationData &ipaConfig,\n> > > > +\t\t IPAOperationData *result) override;\n> > > > \tvoid mapBuffers(const std::vector &buffers) override;\n> > > > \tvoid unmapBuffers(const std::vector &ids) override;\n> > > > \tvoid processEvent(const IPAOperationData &event) override;\n> > > > @@ -129,9 +131,12 @@ void IPAProxyThread::stop()\n> > > > \n> > > > void IPAProxyThread::configure(const CameraSensorInfo &sensorInfo,\n> > > > \t\t\t const std::map &streamConfig,\n> > > > -\t\t\t const std::map &entityControls)\n> > > > +\t\t\t const std::map &entityControls,\n> > > > +\t\t\t const IPAOperationData &ipaConfig,\n> > > > +\t\t\t IPAOperationData *result)\n> > > > {\n> > > > -\tipa_->configure(sensorInfo, streamConfig, entityControls);\n> > > > +\tipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig,\n> > > > +\t\t\tresult);\n> > > > }\n> > > > \n> > > > void IPAProxyThread::mapBuffers(const std::vector &buffers)\n> > > > diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp\n> > > > index aa7a9dcc6050..23c799da0663 100644\n> > > > --- a/test/ipa/ipa_wrappers_test.cpp\n> > > > +++ b/test/ipa/ipa_wrappers_test.cpp\n> > > > @@ -69,7 +69,9 @@ public:\n> > > > \n> > > > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > > > \t\t const std::map &streamConfig,\n> > > > -\t\t const std::map &entityControls) override\n> > > > +\t\t const std::map &entityControls,\n> > > > +\t\t const IPAOperationData &ipaConfig,\n> > > > +\t\t IPAOperationData *result) override\n> > > > \t{\n> > > > \t\t/* Verify sensorInfo. */\n> > > > \t\tif (sensorInfo.outputSize.width != 2560 ||\n> > > > @@ -317,7 +319,9 @@ protected:\n> > > > \t\t};\n> > > > \t\tstd::map controlInfo;\n> > > > \t\tcontrolInfo.emplace(42, subdev_->controls());\n> > > > -\t\tret = INVOKE(configure, sensorInfo, config, controlInfo);\n> > > > +\t\tIPAOperationData ipaConfig;\n> > > > +\t\tret = INVOKE(configure, sensorInfo, config, controlInfo,\n> > > > +\t\t\t ipaConfig, nullptr);\n> > > > \t\tif (ret == TestFail)\n> > > > \t\t\treturn TestFail;\n> > > > \n> > \n> > -- \n> > Regards,\n> > \n> > Laurent Pinchart\n> \n> -- \n> Regards,\n> Niklas Söderlund","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 E24FFBFFE2\n\tfor ;\n\tMon, 29 Jun 2020 20:51:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 72507609DB;\n\tMon, 29 Jun 2020 22:51:47 +0200 (CEST)","from mail-lj1-x244.google.com (mail-lj1-x244.google.com\n\t[IPv6:2a00:1450:4864:20::244])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5E24F603B2\n\tfor ;\n\tMon, 29 Jun 2020 22:51:45 +0200 (CEST)","by mail-lj1-x244.google.com with SMTP id d17so5163988ljl.3\n\tfor ;\n\tMon, 29 Jun 2020 13:51:45 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\ta20sm173282lfl.20.2020.06.29.13.51.43\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 29 Jun 2020 13:51:43 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"OSiOyIOF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=ZpFWGmRMyf2IWk2Iu0/TVwL1EzUVRMSPt1xtB2PJLvw=;\n\tb=OSiOyIOF4GdxCoqSZ4Wl4FEsv0F2+mrVk2FGp/xL2QV+RdPEkVZsZJ6j7iM45BKFcU\n\t7yJaXSiwxE5mzOI0yV5NyCcp3Lpu9U5VrTuAOV0pnYHm7Rt5OLJXB66kYvaOCuNFHLmf\n\tKhLQmMVqhnMEeIt9PFUZePkzWGpoV3PD53KpLPfRLwqLOFJLZg4fDhpYQqKUl13gV5aZ\n\tsQ44Py0MZTEpQNRpCwRsnsQQKna61gbw6xFMwGXnGVUoH7gTP4Io59xYkqc1hfWvkxwI\n\trQg5K/JKl5pzOXD8Fg4vJk1PQzomO8rO0Z5GghZAoVWPdaZe2QCNapicZMDx5DLeLuI5\n\tXtXw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=ZpFWGmRMyf2IWk2Iu0/TVwL1EzUVRMSPt1xtB2PJLvw=;\n\tb=iMeLQkzdSE92XXzzlNWTT77rj0COGiEo9B1eTZVlX58jZtTt22gCUHseVBCB+epJP2\n\tphOhoTngPk4lqk0MMGdA88x6AANza9GH1UBDDHNk+RvZhSbgSfvz5rgvBzxci8cw3B12\n\t4XQvHJwaJfjtQZtBiq6Z5UwDo16MGjJsc8wKyH9AEFB+Z7RrJ0yvQgD/n8t/v1zskkGG\n\tGdlFCSX9xWWF2aBLpA2BtAaBPBpa4MB4YHad6ufv0uAjGtmD8pmoZRNnZqW71LJmOBlc\n\t5TVrgSu07CDN6HemsqmWXkEQ0JbatB9F5FHJTNBVEnNadjPi1jMF1jFH5TiZU2OPP582\n\tqdLg==","X-Gm-Message-State":"AOAM530My4YkVEkPvPTTVg08vVA4lOiWH3yDTbz6tQi1AwnupsdsPPN0\n\tDXJOw0w8G5eBZaxvNpDDXEGHrxitHdE=","X-Google-Smtp-Source":"ABdhPJy262XDI7h76m2/NduT8rgig27Tc/DsySyI8qciQscJPuGIdhZkxxp5ple+2goqjh86DV0Dfg==","X-Received":"by 2002:a2e:9f13:: with SMTP id\n\tu19mr6736544ljk.427.1593463904372; \n\tMon, 29 Jun 2020 13:51:44 -0700 (PDT)","Date":"Mon, 29 Jun 2020 22:51:43 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"<20200629205143.GA1893170@oden.dyn.berto.se>","References":"<20200628231934.29025-1-laurent.pinchart@ideasonboard.com>\n\t<20200628231934.29025-4-laurent.pinchart@ideasonboard.com>\n\t<20200629143113.GC1852218@oden.dyn.berto.se>\n\t<20200629145743.GG10681@pendragon.ideasonboard.com>\n\t<20200629162537.GB1881444@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200629162537.GB1881444@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH v1 3/9] libcamera: ipa_interface: Add\n\tsupport for custom IPA data to configure()","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11006,"web_url":"https://patchwork.libcamera.org/comment/11006/","msgid":"<20200630103459.5bknmttl4iixhkql@uno.localdomain>","date":"2020-06-30T10:34:59","subject":"Re: [libcamera-devel] [PATCH v1 3/9] libcamera: ipa_interface: Add\n\tsupport for custom IPA data to configure()","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Mon, Jun 29, 2020 at 02:19:28AM +0300, Laurent Pinchart wrote:\n> Add two new parameters, ipaConfig and result, to the\n> IPAInterface::configure() function to allow pipeline handlers to pass\n> custom data to their IPA, and receive data back. Wire this through the\n> code base. The C API interface will be addressed separately, likely\n> through automation of the C <-> C++ translation.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> include/libcamera/internal/ipa_context_wrapper.h | 4 +++-\n> include/libcamera/ipa/ipa_interface.h | 4 +++-\n> src/ipa/libipa/ipa_interface_wrapper.cpp | 5 ++++-\n> src/ipa/raspberrypi/raspberrypi.cpp | 8 ++++++--\n> src/ipa/rkisp1/rkisp1.cpp | 8 ++++++--\n> src/ipa/vimc/vimc.cpp | 4 +++-\n> src/libcamera/ipa_context_wrapper.cpp | 8 ++++++--\n> src/libcamera/ipa_interface.cpp | 7 +++++++\n> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 +++-\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +++-\n> src/libcamera/proxy/ipa_proxy_linux.cpp | 4 +++-\n> src/libcamera/proxy/ipa_proxy_thread.cpp | 11 ++++++++---\n> test/ipa/ipa_wrappers_test.cpp | 8 ++++++--\n> 13 files changed, 61 insertions(+), 18 deletions(-)\n>\n> diff --git a/include/libcamera/internal/ipa_context_wrapper.h b/include/libcamera/internal/ipa_context_wrapper.h\n> index 4e6f791d18e6..8f767e844221 100644\n> --- a/include/libcamera/internal/ipa_context_wrapper.h\n> +++ b/include/libcamera/internal/ipa_context_wrapper.h\n> @@ -24,7 +24,9 @@ public:\n> \tvoid stop() override;\n> \tvoid configure(const CameraSensorInfo &sensorInfo,\n> \t\t const std::map &streamConfig,\n> -\t\t const std::map &entityControls) override;\n> +\t\t const std::map &entityControls,\n> +\t\t const IPAOperationData &ipaConfig,\n> +\t\t IPAOperationData *result) override;\n\nThis, and all the other places where this is changed in the same way,\nfit in one line if I'm not mistaken.\n\n>\n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n> diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h\n> index dc9fc714d564..5016ec25ea9c 100644\n> --- a/include/libcamera/ipa/ipa_interface.h\n> +++ b/include/libcamera/ipa/ipa_interface.h\n> @@ -158,7 +158,9 @@ public:\n>\n> \tvirtual void configure(const CameraSensorInfo &sensorInfo,\n> \t\t\t const std::map &streamConfig,\n> -\t\t\t const std::map &entityControls) = 0;\n> +\t\t\t const std::map &entityControls,\n> +\t\t\t const IPAOperationData &ipaConfig,\n> +\t\t\t IPAOperationData *result) = 0;\n>\n> \tvirtual void mapBuffers(const std::vector &buffers) = 0;\n> \tvirtual void unmapBuffers(const std::vector &ids) = 0;\n> diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp\n> index 2a2e43abc708..47ce5a704851 100644\n> --- a/src/ipa/libipa/ipa_interface_wrapper.cpp\n> +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp\n> @@ -166,7 +166,10 @@ void IPAInterfaceWrapper::configure(struct ipa_context *_ctx,\n> \t\tentityControls.emplace(id, infoMaps[id]);\n> \t}\n>\n> -\tctx->ipa_->configure(sensorInfo, ipaStreams, entityControls);\n> +\t/* \\todo Translate the ipaConfig and reponse */\n\nif the usage of response in place of result is not intentional, please\nfix. Also missing . at the end, not sure if should be used in todo\nentries though.\n\n> +\tIPAOperationData ipaConfig;\n> +\tctx->ipa_->configure(sensorInfo, ipaStreams, entityControls, ipaConfig,\n> +\t\t\t nullptr);\n> }\n>\n> void IPAInterfaceWrapper::map_buffers(struct ipa_context *_ctx,\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index bc89ab58d03a..860be22ddb5d 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -78,7 +78,9 @@ public:\n>\n> \tvoid configure(const CameraSensorInfo &sensorInfo,\n> \t\t const std::map &streamConfig,\n> -\t\t const std::map &entityControls) override;\n> +\t\t const std::map &entityControls,\n> +\t\t const IPAOperationData &data,\n> +\t\t IPAOperationData *response) override;\n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n> \tvoid processEvent(const IPAOperationData &event) override;\n> @@ -186,7 +188,9 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)\n>\n> void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> \t\t const std::map &streamConfig,\n> -\t\t const std::map &entityControls)\n> +\t\t const std::map &entityControls,\n> +\t\t const IPAOperationData &ipaConfig,\n> +\t\t IPAOperationData *result)\n> {\n> \tif (entityControls.empty())\n> \t\treturn;\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index fbdc908fc816..34d6f63a7ff4 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -39,7 +39,9 @@ public:\n>\n> \tvoid configure(const CameraSensorInfo &info,\n> \t\t const std::map &streamConfig,\n> -\t\t const std::map &entityControls) override;\n> +\t\t const std::map &entityControls,\n> +\t\t const IPAOperationData &ipaConfig,\n> +\t\t IPAOperationData *response) override;\n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n> \tvoid processEvent(const IPAOperationData &event) override;\n> @@ -76,7 +78,9 @@ private:\n> */\n> void IPARkISP1::configure(const CameraSensorInfo &info,\n> \t\t\t const std::map &streamConfig,\n> -\t\t\t const std::map &entityControls)\n> +\t\t\t const std::map &entityControls,\n> +\t\t\t const IPAOperationData &ipaConfig,\n> +\t\t\t IPAOperationData *result)\n> {\n> \tif (entityControls.empty())\n> \t\treturn;\n> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\n> index af278a482b8a..1593c92d80f3 100644\n> --- a/src/ipa/vimc/vimc.cpp\n> +++ b/src/ipa/vimc/vimc.cpp\n> @@ -39,7 +39,9 @@ public:\n>\n> \tvoid configure(const CameraSensorInfo &sensorInfo,\n> \t\t const std::map &streamConfig,\n> -\t\t const std::map &entityControls) override {}\n> +\t\t const std::map &entityControls,\n> +\t\t const IPAOperationData &ipaConfig,\n> +\t\t IPAOperationData *result) override {}\n> \tvoid mapBuffers(const std::vector &buffers) override {}\n> \tvoid unmapBuffers(const std::vector &ids) override {}\n> \tvoid processEvent(const IPAOperationData &event) override {}\n> diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp\n> index 471118f57cdc..231300ce0bec 100644\n> --- a/src/libcamera/ipa_context_wrapper.cpp\n> +++ b/src/libcamera/ipa_context_wrapper.cpp\n> @@ -110,10 +110,13 @@ void IPAContextWrapper::stop()\n>\n> void IPAContextWrapper::configure(const CameraSensorInfo &sensorInfo,\n> \t\t\t\t const std::map &streamConfig,\n> -\t\t\t\t const std::map &entityControls)\n> +\t\t\t\t const std::map &entityControls,\n> +\t\t\t\t const IPAOperationData &ipaConfig,\n> +\t\t\t\t IPAOperationData *result)\n> {\n> \tif (intf_)\n> -\t\treturn intf_->configure(sensorInfo, streamConfig, entityControls);\n> +\t\treturn intf_->configure(sensorInfo, streamConfig,\n> +\t\t\t\t\tentityControls, ipaConfig, result);\n>\n> \tif (!ctx_)\n> \t\treturn;\n> @@ -174,6 +177,7 @@ void IPAContextWrapper::configure(const CameraSensorInfo &sensorInfo,\n> \t\t++i;\n> \t}\n>\n> +\t/* \\todo Translate the ipaConfig and reponse */\n> \tctx_->ops->configure(ctx_, &sensor_info, c_streams, streamConfig.size(),\n> \t\t\t c_info_maps, entityControls.size());\n> }\n> diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp\n> index ebe47e1233a5..23fc56d7d48e 100644\n> --- a/src/libcamera/ipa_interface.cpp\n> +++ b/src/libcamera/ipa_interface.cpp\n> @@ -557,6 +557,8 @@ namespace libcamera {\n> * \\param[in] sensorInfo Camera sensor information\n> * \\param[in] streamConfig Configuration of all active streams\n> * \\param[in] entityControls Controls provided by the pipeline entities\n> + * \\param[in] ipaConfig Pipeline-handler-specific configuration data\n> + * \\param[out] result Pipeline-handler-specific configuration result\n> *\n> * This method shall be called when the camera is started to inform the IPA of\n> * the camera's streams and the sensor settings. The meaning of the numerical\n> @@ -566,6 +568,11 @@ namespace libcamera {\n> * The \\a sensorInfo conveys information about the camera sensor settings that\n> * the pipeline handler has selected for the configuration. The IPA may use\n> * that information to tune its algorithms.\n> + *\n> + * The \\a ipaConfig and \\a result parameters carry custom data passed by the\n> + * pipeline handler to the IPA and back. The pipeline handler may set the \\a\n> + * result parameter to null if the IPA protocol doesn't need to pass a result\n> + * back through the configure() function.\n\nI would like to have a bit more details here. Like in example, how\ndoes this differ from entityControls ? The way controls are passed\nthere and associated to an arbitrary entity shouldn't (in the long\nrun, provided all required controls are defined) serve the same\npurpose ? Equally, if 'result' aims to carry sensor configurations,\nshouldn't it be a control list ?\n\n> */\n>\n> /**\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index dcd737a1d1a0..3b5cdf1e1849 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1017,7 +1017,9 @@ int PipelineHandlerRPi::configureIPA(Camera *camera)\n> \t}\n>\n> \t/* Ready the IPA - it must know about the sensor resolution. */\n> -\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls);\n> +\tIPAOperationData ipaConfig;\n> +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> +\t\t\t ipaConfig, nullptr);\n>\n> \treturn 0;\n> }\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 3c01821135f8..4fde5539e667 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -843,7 +843,9 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n> \tstd::map entityControls;\n> \tentityControls.emplace(0, data->sensor_->controls());\n>\n> -\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls);\n> +\tIPAOperationData ipaConfig;\n> +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> +\t\t\t ipaConfig, nullptr);\n>\n> \treturn ret;\n> }\n> diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> index be34f20aa857..68eafb307b2a 100644\n> --- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n> +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> @@ -31,7 +31,9 @@ public:\n> \tvoid stop() override {}\n> \tvoid configure(const CameraSensorInfo &sensorInfo,\n> \t\t const std::map &streamConfig,\n> -\t\t const std::map &entityControls) override {}\n> +\t\t const std::map &entityControls,\n> +\t\t const IPAOperationData &ipaConfig,\n> +\t\t IPAOperationData *result) override {}\n> \tvoid mapBuffers(const std::vector &buffers) override {}\n> \tvoid unmapBuffers(const std::vector &ids) override {}\n> \tvoid processEvent(const IPAOperationData &event) override {}\n> diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> index 6fbebed2ba72..aa403e22f297 100644\n> --- a/src/libcamera/proxy/ipa_proxy_thread.cpp\n> +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> @@ -31,7 +31,9 @@ public:\n>\n> \tvoid configure(const CameraSensorInfo &sensorInfo,\n> \t\t const std::map &streamConfig,\n> -\t\t const std::map &entityControls) override;\n> +\t\t const std::map &entityControls,\n> +\t\t const IPAOperationData &ipaConfig,\n> +\t\t IPAOperationData *result) override;\n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n> \tvoid processEvent(const IPAOperationData &event) override;\n> @@ -129,9 +131,12 @@ void IPAProxyThread::stop()\n>\n> void IPAProxyThread::configure(const CameraSensorInfo &sensorInfo,\n> \t\t\t const std::map &streamConfig,\n> -\t\t\t const std::map &entityControls)\n> +\t\t\t const std::map &entityControls,\n> +\t\t\t const IPAOperationData &ipaConfig,\n> +\t\t\t IPAOperationData *result)\n> {\n> -\tipa_->configure(sensorInfo, streamConfig, entityControls);\n> +\tipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig,\n> +\t\t\tresult);\n> }\n>\n> void IPAProxyThread::mapBuffers(const std::vector &buffers)\n> diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp\n> index aa7a9dcc6050..23c799da0663 100644\n> --- a/test/ipa/ipa_wrappers_test.cpp\n> +++ b/test/ipa/ipa_wrappers_test.cpp\n> @@ -69,7 +69,9 @@ public:\n>\n> \tvoid configure(const CameraSensorInfo &sensorInfo,\n> \t\t const std::map &streamConfig,\n> -\t\t const std::map &entityControls) override\n> +\t\t const std::map &entityControls,\n> +\t\t const IPAOperationData &ipaConfig,\n> +\t\t IPAOperationData *result) override\n> \t{\n> \t\t/* Verify sensorInfo. */\n> \t\tif (sensorInfo.outputSize.width != 2560 ||\n> @@ -317,7 +319,9 @@ protected:\n> \t\t};\n> \t\tstd::map controlInfo;\n> \t\tcontrolInfo.emplace(42, subdev_->controls());\n> -\t\tret = INVOKE(configure, sensorInfo, config, controlInfo);\n> +\t\tIPAOperationData ipaConfig;\n> +\t\tret = INVOKE(configure, sensorInfo, config, controlInfo,\n> +\t\t\t ipaConfig, nullptr);\n> \t\tif (ret == TestFail)\n> \t\t\treturn TestFail;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0F8FFBFFE2\n\tfor ;\n\tTue, 30 Jun 2020 10:31:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8509860C58;\n\tTue, 30 Jun 2020 12:31:31 +0200 (CEST)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5282F603B4\n\tfor ;\n\tTue, 30 Jun 2020 12:31:30 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id C4BE310000B;\n\tTue, 30 Jun 2020 10:31:29 +0000 (UTC)"],"Date":"Tue, 30 Jun 2020 12:34:59 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Message-ID":"<20200630103459.5bknmttl4iixhkql@uno.localdomain>","References":"<20200628231934.29025-1-laurent.pinchart@ideasonboard.com>\n\t<20200628231934.29025-4-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200628231934.29025-4-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v1 3/9] libcamera: ipa_interface: Add\n\tsupport for custom IPA data to configure()","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":11083,"web_url":"https://patchwork.libcamera.org/comment/11083/","msgid":"<20200702212301.GR12562@pendragon.ideasonboard.com>","date":"2020-07-02T21:23:01","subject":"Re: [libcamera-devel] [PATCH v1 3/9] libcamera: ipa_interface: Add\n\tsupport for custom IPA data to configure()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tue, Jun 30, 2020 at 12:34:59PM +0200, Jacopo Mondi wrote:\n> On Mon, Jun 29, 2020 at 02:19:28AM +0300, Laurent Pinchart wrote:\n> > Add two new parameters, ipaConfig and result, to the\n> > IPAInterface::configure() function to allow pipeline handlers to pass\n> > custom data to their IPA, and receive data back. Wire this through the\n> > code base. The C API interface will be addressed separately, likely\n> > through automation of the C <-> C++ translation.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > include/libcamera/internal/ipa_context_wrapper.h | 4 +++-\n> > include/libcamera/ipa/ipa_interface.h | 4 +++-\n> > src/ipa/libipa/ipa_interface_wrapper.cpp | 5 ++++-\n> > src/ipa/raspberrypi/raspberrypi.cpp | 8 ++++++--\n> > src/ipa/rkisp1/rkisp1.cpp | 8 ++++++--\n> > src/ipa/vimc/vimc.cpp | 4 +++-\n> > src/libcamera/ipa_context_wrapper.cpp | 8 ++++++--\n> > src/libcamera/ipa_interface.cpp | 7 +++++++\n> > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 +++-\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +++-\n> > src/libcamera/proxy/ipa_proxy_linux.cpp | 4 +++-\n> > src/libcamera/proxy/ipa_proxy_thread.cpp | 11 ++++++++---\n> > test/ipa/ipa_wrappers_test.cpp | 8 ++++++--\n> > 13 files changed, 61 insertions(+), 18 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/ipa_context_wrapper.h b/include/libcamera/internal/ipa_context_wrapper.h\n> > index 4e6f791d18e6..8f767e844221 100644\n> > --- a/include/libcamera/internal/ipa_context_wrapper.h\n> > +++ b/include/libcamera/internal/ipa_context_wrapper.h\n> > @@ -24,7 +24,9 @@ public:\n> > \tvoid stop() override;\n> > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > \t\t const std::map &streamConfig,\n> > -\t\t const std::map &entityControls) override;\n> > +\t\t const std::map &entityControls,\n> > +\t\t const IPAOperationData &ipaConfig,\n> > +\t\t IPAOperationData *result) override;\n> \n> This, and all the other places where this is changed in the same way,\n> fit in one line if I'm not mistaken.\n\n\t\t const IPAOperationData &ipaConfig, IPAOperationData *result) override;\n\nis 93 characters long, and we try to keep lines at most 80 characters\nlong when it doesn't hinder readability.\n\n> >\n> > \tvoid mapBuffers(const std::vector &buffers) override;\n> > \tvoid unmapBuffers(const std::vector &ids) override;\n> > diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h\n> > index dc9fc714d564..5016ec25ea9c 100644\n> > --- a/include/libcamera/ipa/ipa_interface.h\n> > +++ b/include/libcamera/ipa/ipa_interface.h\n> > @@ -158,7 +158,9 @@ public:\n> >\n> > \tvirtual void configure(const CameraSensorInfo &sensorInfo,\n> > \t\t\t const std::map &streamConfig,\n> > -\t\t\t const std::map &entityControls) = 0;\n> > +\t\t\t const std::map &entityControls,\n> > +\t\t\t const IPAOperationData &ipaConfig,\n> > +\t\t\t IPAOperationData *result) = 0;\n> >\n> > \tvirtual void mapBuffers(const std::vector &buffers) = 0;\n> > \tvirtual void unmapBuffers(const std::vector &ids) = 0;\n> > diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp\n> > index 2a2e43abc708..47ce5a704851 100644\n> > --- a/src/ipa/libipa/ipa_interface_wrapper.cpp\n> > +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp\n> > @@ -166,7 +166,10 @@ void IPAInterfaceWrapper::configure(struct ipa_context *_ctx,\n> > \t\tentityControls.emplace(id, infoMaps[id]);\n> > \t}\n> >\n> > -\tctx->ipa_->configure(sensorInfo, ipaStreams, entityControls);\n> > +\t/* \\todo Translate the ipaConfig and reponse */\n> \n> if the usage of response in place of result is not intentional, please\n> fix.\n\nGood catch. I'll fix that.\n\n> Also missing . at the end, not sure if should be used in todo\n> entries though.\n\nI would have sworn we don't add periods at the end of todo entries, but\namong the 33 entries that fit on a single line, 17 have a period. I'll\nthus follow the majority for once :-)\n\n> > +\tIPAOperationData ipaConfig;\n> > +\tctx->ipa_->configure(sensorInfo, ipaStreams, entityControls, ipaConfig,\n> > +\t\t\t nullptr);\n> > }\n> >\n> > void IPAInterfaceWrapper::map_buffers(struct ipa_context *_ctx,\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index bc89ab58d03a..860be22ddb5d 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -78,7 +78,9 @@ public:\n> >\n> > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > \t\t const std::map &streamConfig,\n> > -\t\t const std::map &entityControls) override;\n> > +\t\t const std::map &entityControls,\n> > +\t\t const IPAOperationData &data,\n> > +\t\t IPAOperationData *response) override;\n> > \tvoid mapBuffers(const std::vector &buffers) override;\n> > \tvoid unmapBuffers(const std::vector &ids) override;\n> > \tvoid processEvent(const IPAOperationData &event) override;\n> > @@ -186,7 +188,9 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)\n> >\n> > void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n> > \t\t const std::map &streamConfig,\n> > -\t\t const std::map &entityControls)\n> > +\t\t const std::map &entityControls,\n> > +\t\t const IPAOperationData &ipaConfig,\n> > +\t\t IPAOperationData *result)\n> > {\n> > \tif (entityControls.empty())\n> > \t\treturn;\n> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > index fbdc908fc816..34d6f63a7ff4 100644\n> > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > @@ -39,7 +39,9 @@ public:\n> >\n> > \tvoid configure(const CameraSensorInfo &info,\n> > \t\t const std::map &streamConfig,\n> > -\t\t const std::map &entityControls) override;\n> > +\t\t const std::map &entityControls,\n> > +\t\t const IPAOperationData &ipaConfig,\n> > +\t\t IPAOperationData *response) override;\n> > \tvoid mapBuffers(const std::vector &buffers) override;\n> > \tvoid unmapBuffers(const std::vector &ids) override;\n> > \tvoid processEvent(const IPAOperationData &event) override;\n> > @@ -76,7 +78,9 @@ private:\n> > */\n> > void IPARkISP1::configure(const CameraSensorInfo &info,\n> > \t\t\t const std::map &streamConfig,\n> > -\t\t\t const std::map &entityControls)\n> > +\t\t\t const std::map &entityControls,\n> > +\t\t\t const IPAOperationData &ipaConfig,\n> > +\t\t\t IPAOperationData *result)\n> > {\n> > \tif (entityControls.empty())\n> > \t\treturn;\n> > diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\n> > index af278a482b8a..1593c92d80f3 100644\n> > --- a/src/ipa/vimc/vimc.cpp\n> > +++ b/src/ipa/vimc/vimc.cpp\n> > @@ -39,7 +39,9 @@ public:\n> >\n> > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > \t\t const std::map &streamConfig,\n> > -\t\t const std::map &entityControls) override {}\n> > +\t\t const std::map &entityControls,\n> > +\t\t const IPAOperationData &ipaConfig,\n> > +\t\t IPAOperationData *result) override {}\n> > \tvoid mapBuffers(const std::vector &buffers) override {}\n> > \tvoid unmapBuffers(const std::vector &ids) override {}\n> > \tvoid processEvent(const IPAOperationData &event) override {}\n> > diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp\n> > index 471118f57cdc..231300ce0bec 100644\n> > --- a/src/libcamera/ipa_context_wrapper.cpp\n> > +++ b/src/libcamera/ipa_context_wrapper.cpp\n> > @@ -110,10 +110,13 @@ void IPAContextWrapper::stop()\n> >\n> > void IPAContextWrapper::configure(const CameraSensorInfo &sensorInfo,\n> > \t\t\t\t const std::map &streamConfig,\n> > -\t\t\t\t const std::map &entityControls)\n> > +\t\t\t\t const std::map &entityControls,\n> > +\t\t\t\t const IPAOperationData &ipaConfig,\n> > +\t\t\t\t IPAOperationData *result)\n> > {\n> > \tif (intf_)\n> > -\t\treturn intf_->configure(sensorInfo, streamConfig, entityControls);\n> > +\t\treturn intf_->configure(sensorInfo, streamConfig,\n> > +\t\t\t\t\tentityControls, ipaConfig, result);\n> >\n> > \tif (!ctx_)\n> > \t\treturn;\n> > @@ -174,6 +177,7 @@ void IPAContextWrapper::configure(const CameraSensorInfo &sensorInfo,\n> > \t\t++i;\n> > \t}\n> >\n> > +\t/* \\todo Translate the ipaConfig and reponse */\n> > \tctx_->ops->configure(ctx_, &sensor_info, c_streams, streamConfig.size(),\n> > \t\t\t c_info_maps, entityControls.size());\n> > }\n> > diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp\n> > index ebe47e1233a5..23fc56d7d48e 100644\n> > --- a/src/libcamera/ipa_interface.cpp\n> > +++ b/src/libcamera/ipa_interface.cpp\n> > @@ -557,6 +557,8 @@ namespace libcamera {\n> > * \\param[in] sensorInfo Camera sensor information\n> > * \\param[in] streamConfig Configuration of all active streams\n> > * \\param[in] entityControls Controls provided by the pipeline entities\n> > + * \\param[in] ipaConfig Pipeline-handler-specific configuration data\n> > + * \\param[out] result Pipeline-handler-specific configuration result\n> > *\n> > * This method shall be called when the camera is started to inform the IPA of\n> > * the camera's streams and the sensor settings. The meaning of the numerical\n> > @@ -566,6 +568,11 @@ namespace libcamera {\n> > * The \\a sensorInfo conveys information about the camera sensor settings that\n> > * the pipeline handler has selected for the configuration. The IPA may use\n> > * that information to tune its algorithms.\n> > + *\n> > + * The \\a ipaConfig and \\a result parameters carry custom data passed by the\n> > + * pipeline handler to the IPA and back. The pipeline handler may set the \\a\n> > + * result parameter to null if the IPA protocol doesn't need to pass a result\n> > + * back through the configure() function.\n> \n> I would like to have a bit more details here. Like in example, how\n> does this differ from entityControls ? The way controls are passed\n> there and associated to an arbitrary entity shouldn't (in the long\n> run, provided all required controls are defined) serve the same\n> purpose ? Equally, if 'result' aims to carry sensor configurations,\n> shouldn't it be a control list ?\n\nAs we've discussed offline, entityControls is a map of entity ID to\nControlMapInfo, not ControlList. The two are thus different. Both\nipaConfig and result (which I'll rename to ipaResult for symmetry) carry\na vector of ControlList.\n\n> > */\n> >\n> > /**\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index dcd737a1d1a0..3b5cdf1e1849 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -1017,7 +1017,9 @@ int PipelineHandlerRPi::configureIPA(Camera *camera)\n> > \t}\n> >\n> > \t/* Ready the IPA - it must know about the sensor resolution. */\n> > -\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls);\n> > +\tIPAOperationData ipaConfig;\n> > +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> > +\t\t\t ipaConfig, nullptr);\n> >\n> > \treturn 0;\n> > }\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 3c01821135f8..4fde5539e667 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -843,7 +843,9 @@ int PipelineHandlerRkISP1::start(Camera *camera)\n> > \tstd::map entityControls;\n> > \tentityControls.emplace(0, data->sensor_->controls());\n> >\n> > -\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls);\n> > +\tIPAOperationData ipaConfig;\n> > +\tdata->ipa_->configure(sensorInfo, streamConfig, entityControls,\n> > +\t\t\t ipaConfig, nullptr);\n> >\n> > \treturn ret;\n> > }\n> > diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > index be34f20aa857..68eafb307b2a 100644\n> > --- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > @@ -31,7 +31,9 @@ public:\n> > \tvoid stop() override {}\n> > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > \t\t const std::map &streamConfig,\n> > -\t\t const std::map &entityControls) override {}\n> > +\t\t const std::map &entityControls,\n> > +\t\t const IPAOperationData &ipaConfig,\n> > +\t\t IPAOperationData *result) override {}\n> > \tvoid mapBuffers(const std::vector &buffers) override {}\n> > \tvoid unmapBuffers(const std::vector &ids) override {}\n> > \tvoid processEvent(const IPAOperationData &event) override {}\n> > diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > index 6fbebed2ba72..aa403e22f297 100644\n> > --- a/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > @@ -31,7 +31,9 @@ public:\n> >\n> > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > \t\t const std::map &streamConfig,\n> > -\t\t const std::map &entityControls) override;\n> > +\t\t const std::map &entityControls,\n> > +\t\t const IPAOperationData &ipaConfig,\n> > +\t\t IPAOperationData *result) override;\n> > \tvoid mapBuffers(const std::vector &buffers) override;\n> > \tvoid unmapBuffers(const std::vector &ids) override;\n> > \tvoid processEvent(const IPAOperationData &event) override;\n> > @@ -129,9 +131,12 @@ void IPAProxyThread::stop()\n> >\n> > void IPAProxyThread::configure(const CameraSensorInfo &sensorInfo,\n> > \t\t\t const std::map &streamConfig,\n> > -\t\t\t const std::map &entityControls)\n> > +\t\t\t const std::map &entityControls,\n> > +\t\t\t const IPAOperationData &ipaConfig,\n> > +\t\t\t IPAOperationData *result)\n> > {\n> > -\tipa_->configure(sensorInfo, streamConfig, entityControls);\n> > +\tipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig,\n> > +\t\t\tresult);\n> > }\n> >\n> > void IPAProxyThread::mapBuffers(const std::vector &buffers)\n> > diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp\n> > index aa7a9dcc6050..23c799da0663 100644\n> > --- a/test/ipa/ipa_wrappers_test.cpp\n> > +++ b/test/ipa/ipa_wrappers_test.cpp\n> > @@ -69,7 +69,9 @@ public:\n> >\n> > \tvoid configure(const CameraSensorInfo &sensorInfo,\n> > \t\t const std::map &streamConfig,\n> > -\t\t const std::map &entityControls) override\n> > +\t\t const std::map &entityControls,\n> > +\t\t const IPAOperationData &ipaConfig,\n> > +\t\t IPAOperationData *result) override\n> > \t{\n> > \t\t/* Verify sensorInfo. */\n> > \t\tif (sensorInfo.outputSize.width != 2560 ||\n> > @@ -317,7 +319,9 @@ protected:\n> > \t\t};\n> > \t\tstd::map controlInfo;\n> > \t\tcontrolInfo.emplace(42, subdev_->controls());\n> > -\t\tret = INVOKE(configure, sensorInfo, config, controlInfo);\n> > +\t\tIPAOperationData ipaConfig;\n> > +\t\tret = INVOKE(configure, sensorInfo, config, controlInfo,\n> > +\t\t\t ipaConfig, nullptr);\n> > \t\tif (ret == TestFail)\n> > \t\t\treturn TestFail;\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 6DD76BE905\n\tfor ;\n\tThu, 2 Jul 2020 21:23:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 042AD60C58;\n\tThu, 2 Jul 2020 23:23:07 +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 865E5603B4\n\tfor ;\n\tThu, 2 Jul 2020 23:23:05 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E6C769CB;\n\tThu, 2 Jul 2020 23:23:04 +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=\"vpjj/0oE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593724985;\n\tbh=sN/kp/gkITucbNFmq+E3zdzh/d+nvAUtd0ijHsA2s4A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=vpjj/0oEmCT099S3LkUWjjYADfKKmD+SDFwgKDfUpC53gcgiwsMVbNm1cHRnzYjAr\n\t5qjrXF8yJrATcn1Zosf0ithpAOyYWz9SNX0nKUJmSMeyWQ/TnEBk0S51P73QF6E/QS\n\tSdLFavWxdv2kBY7OIQ5qQv3U16U4YqJt3cEoQoq4=","Date":"Fri, 3 Jul 2020 00:23:01 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"<20200702212301.GR12562@pendragon.ideasonboard.com>","References":"<20200628231934.29025-1-laurent.pinchart@ideasonboard.com>\n\t<20200628231934.29025-4-laurent.pinchart@ideasonboard.com>\n\t<20200630103459.5bknmttl4iixhkql@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200630103459.5bknmttl4iixhkql@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v1 3/9] libcamera: ipa_interface: Add\n\tsupport for custom IPA data to configure()","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\" "}}]