{"id":16473,"url":"https://patchwork.libcamera.org/api/1.1/patches/16473/?format=json","web_url":"https://patchwork.libcamera.org/patch/16473/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20220630133902.321099-16-jacopo@jmondi.org>","date":"2022-06-30T13:38:54","name":"[libcamera-devel,v3,15/23] libcamera: ipu3: Initialize controls in the pipeline","commit_ref":null,"pull_url":null,"state":"not-applicable","archived":true,"hash":"077aafb82a94c9bdd42c754e90a096f2a3302dcc","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/1.1/people/3/?format=json","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/16473/mbox/","series":[{"id":3238,"url":"https://patchwork.libcamera.org/api/1.1/series/3238/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=3238","date":"2022-06-30T13:38:39","name":"Internal controls, sensor delays and IPA rework","version":3,"mbox":"https://patchwork.libcamera.org/series/3238/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/16473/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/16473/checks/","tags":{},"headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5520AC3273\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 30 Jun 2022 13:39:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E068E656AC;\n\tThu, 30 Jun 2022 15:39:44 +0200 (CEST)","from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 21AAC65656\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 30 Jun 2022 15:39:38 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 10A0024000A;\n\tThu, 30 Jun 2022 13:39:36 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656596385;\n\tbh=jpm1bAu1PHHZy4fnkMag/Lb87tUq5kBmNOIsvRnDuMg=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=OEW+q2Qrm38sR5rhoAVPdDy33r+gd11tEhcmVD0AIPXXh18Ix7QhQCnOyzGJsuKqQ\n\t9TI7vrhT3SMGyYF3xxcKBAjp8afxGSDqm6/+wo0r0cMOp0TsTi9XI42wG8X/lXwqsu\n\tlhYHYi2aXw5FVMLkBSOmE7sWM8tTHtG3IbB3dxe7l3GNzMtI9kdQxM/PoBLzYPerwJ\n\t4iEopbYXh4LSuBEDtwdIsujD89vY3oTColNVCV8zyILJbd5wBeeyursgLUslxJO4A5\n\tw1Lh1cIsT6cB9v3utcIwjGURjFAyBQzE5+MzGdjAE1VuQsrHfbrjUQRnjQeOB/KlkC\n\tkiiqqZy6j4EFQ==","To":"libcamera-devel@lists.libcamera.org","Date":"Thu, 30 Jun 2022 15:38:54 +0200","Message-Id":"<20220630133902.321099-16-jacopo@jmondi.org>","X-Mailer":"git-send-email 2.36.1","In-Reply-To":"<20220630133902.321099-1-jacopo@jmondi.org>","References":"<20220630133902.321099-1-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v3 15/23] libcamera: ipu3: Initialize\n\tcontrols in the pipeline","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"},"content":"Currently the map of V4L2 controls collected from the sensor is passed to\nthe IPA module which initializes Camera controls by inspecting the\nsensor control values.\n\nAs the CameraSensor class translates V4L2 controls into\nlibcamera::internal controls, the Camera controls can be initialized\ndirectly in the pipeline handler, without sending them to the IPA which\nreceives all the information it needs using the CameraSensorInfo.\n\nRemove from the IPA interface the list of V4L2 controls from the sensor\nsent to the IPA as it is not used anymore and initialize Camera controls\nin the pipeline directly.\n\nSigned-off-by: Jacopo Mondi <jacopo@jmondi.org>\n---\n include/libcamera/ipa/ipu3.mojom     |  7 +--\n src/ipa/ipu3/ipu3.cpp                | 86 ++--------------------------\n src/libcamera/pipeline/ipu3/ipu3.cpp | 32 ++++++-----\n 3 files changed, 25 insertions(+), 100 deletions(-)","diff":"diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom\nindex d1b1c6b867da..5818cd3a6846 100644\n--- a/include/libcamera/ipa/ipu3.mojom\n+++ b/include/libcamera/ipa/ipu3.mojom\n@@ -18,14 +18,13 @@ struct IPAConfigInfo {\n \n interface IPAIPU3Interface {\n \tinit(libcamera.IPASettings settings,\n-\t     libcamera.IPACameraSensorInfo sensorInfo,\n-\t     libcamera.ControlInfoMap sensorControls)\n-\t\t=> (int32 ret, libcamera.ControlInfoMap ipaControls);\n+\t     libcamera.IPACameraSensorInfo sensorInfo)\n+\t\t=> (int32 ret);\n \tstart() => (int32 ret);\n \tstop();\n \n \tconfigure(IPAConfigInfo configInfo)\n-\t\t=> (int32 ret, libcamera.ControlInfoMap ipaControls);\n+\t\t=> (int32 ret);\n \n \tmapBuffers(array<libcamera.IPABuffer> buffers);\n \tunmapBuffers(array<uint32> ids);\ndiff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\nindex 2f6bb672f7bb..a6e5dcbaada9 100644\n--- a/src/ipa/ipu3/ipu3.cpp\n+++ b/src/ipa/ipu3/ipu3.cpp\n@@ -132,15 +132,12 @@ class IPAIPU3 : public IPAIPU3Interface\n {\n public:\n \tint init(const IPASettings &settings,\n-\t\t const IPACameraSensorInfo &sensorInfo,\n-\t\t const ControlInfoMap &sensorControls,\n-\t\t ControlInfoMap *ipaControls) override;\n+\t\t const IPACameraSensorInfo &sensorInfo) override;\n \n \tint start() override;\n \tvoid stop() override;\n \n-\tint configure(const IPAConfigInfo &configInfo,\n-\t\t      ControlInfoMap *ipaControls) override;\n+\tint configure(const IPAConfigInfo &configInfo) override;\n \n \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override;\n \tvoid unmapBuffers(const std::vector<unsigned int> &ids) override;\n@@ -151,9 +148,6 @@ public:\n \t\t\t\tconst uint32_t bufferId,\n \t\t\t\tconst ControlList &sensorControls) override;\n private:\n-\tvoid updateControls(const IPACameraSensorInfo &sensorInfo,\n-\t\t\t    const ControlInfoMap &sensorControls,\n-\t\t\t    ControlInfoMap *ipaControls);\n \tvoid updateSessionConfiguration(const ControlInfoMap &sensorControls);\n \n \tbool validateSensorControls();\n@@ -208,68 +202,6 @@ void IPAIPU3::updateSessionConfiguration(const ControlInfoMap &sensorControls)\n \tcontext_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);\n }\n \n-/**\n- * \\brief Compute camera controls using the sensor information and the sensor\n- * V4L2 controls\n- *\n- * Some of the camera controls are computed by the pipeline handler, some others\n- * by the IPA module which is in charge of handling, for example, the exposure\n- * time and the frame duration.\n- *\n- * This function computes:\n- * - controls::ExposureTime\n- * - controls::FrameDurationLimits\n- */\n-void IPAIPU3::updateControls(const IPACameraSensorInfo &sensorInfo,\n-\t\t\t     const ControlInfoMap &sensorControls,\n-\t\t\t     ControlInfoMap *ipaControls)\n-{\n-\tControlInfoMap::Map controls{};\n-\tdouble lineDuration = context_.configuration.sensor.lineDuration.get<std::micro>();\n-\n-\t/*\n-\t * Compute exposure time limits by using line length and pixel rate\n-\t * converted to microseconds. Use the V4L2_CID_EXPOSURE control to get\n-\t * exposure min, max and default and convert it from lines to\n-\t * microseconds.\n-\t */\n-\tconst ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;\n-\tint32_t minExposure = v4l2Exposure.min().get<int32_t>() * lineDuration;\n-\tint32_t maxExposure = v4l2Exposure.max().get<int32_t>() * lineDuration;\n-\tint32_t defExposure = v4l2Exposure.def().get<int32_t>() * lineDuration;\n-\tcontrols[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure,\n-\t\t\t\t\t\t\tdefExposure);\n-\n-\t/*\n-\t * Compute the frame duration limits.\n-\t *\n-\t * The frame length is computed assuming a fixed line length combined\n-\t * with the vertical frame sizes.\n-\t */\n-\tconst ControlInfo &v4l2HBlank = sensorControls.find(V4L2_CID_HBLANK)->second;\n-\tuint32_t hblank = v4l2HBlank.def().get<int32_t>();\n-\tuint32_t lineLength = sensorInfo.outputSize.width + hblank;\n-\n-\tconst ControlInfo &v4l2VBlank = sensorControls.find(V4L2_CID_VBLANK)->second;\n-\tstd::array<uint32_t, 3> frameHeights{\n-\t\tv4l2VBlank.min().get<int32_t>() + sensorInfo.outputSize.height,\n-\t\tv4l2VBlank.max().get<int32_t>() + sensorInfo.outputSize.height,\n-\t\tv4l2VBlank.def().get<int32_t>() + sensorInfo.outputSize.height,\n-\t};\n-\n-\tstd::array<int64_t, 3> frameDurations;\n-\tfor (unsigned int i = 0; i < frameHeights.size(); ++i) {\n-\t\tuint64_t frameSize = lineLength * frameHeights[i];\n-\t\tframeDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);\n-\t}\n-\n-\tcontrols[&controls::FrameDurationLimits] = ControlInfo(frameDurations[0],\n-\t\t\t\t\t\t\t       frameDurations[1],\n-\t\t\t\t\t\t\t       frameDurations[2]);\n-\n-\t*ipaControls = ControlInfoMap(std::move(controls), controls::controls);\n-}\n-\n /**\n  * \\brief Validate that the sensor controls mandatory for the IPA exists\n  */\n@@ -300,9 +232,7 @@ bool IPAIPU3::validateSensorControls()\n  * them in the \\a ipaControls output parameter.\n  */\n int IPAIPU3::init(const IPASettings &settings,\n-\t\t  const IPACameraSensorInfo &sensorInfo,\n-\t\t  const ControlInfoMap &sensorControls,\n-\t\t  ControlInfoMap *ipaControls)\n+\t\t  const IPACameraSensorInfo &sensorInfo)\n {\n \tcamHelper_ = CameraSensorHelperFactory::create(settings.sensorModel);\n \tif (camHelper_ == nullptr) {\n@@ -323,9 +253,6 @@ int IPAIPU3::init(const IPASettings &settings,\n \talgorithms_.push_back(std::make_unique<algorithms::BlackLevelCorrection>());\n \talgorithms_.push_back(std::make_unique<algorithms::ToneMapping>());\n \n-\t/* Initialize controls. */\n-\tupdateControls(sensorInfo, sensorControls, ipaControls);\n-\n \treturn 0;\n }\n \n@@ -423,7 +350,6 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize)\n  * \\brief Configure the IPU3 IPA\n  * \\param[in] configInfo The IPA configuration data, received from the pipeline\n  * handler\n- * \\param[in] ipaControls The IPA controls to update\n  *\n  * Calculate the best grid for the statistics based on the pipeline handler BDS\n  * output, and parse the minimum and maximum exposure and analogue gain control\n@@ -434,8 +360,7 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize)\n  * All algorithm modules are called to allow them to prepare the\n  * \\a IPASessionConfiguration structure for the \\a IPAContext.\n  */\n-int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n-\t\t       ControlInfoMap *ipaControls)\n+int IPAIPU3::configure(const IPAConfigInfo &configInfo)\n {\n \tif (configInfo.sensorControls.empty()) {\n \t\tLOG(IPAIPU3, Error) << \"No sensor controls provided\";\n@@ -464,9 +389,6 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n \t\treturn -EINVAL;\n \t}\n \n-\t/* Update the camera controls using the new sensor settings. */\n-\tupdateControls(sensorInfo_, sensorCtrls_, ipaControls);\n-\n \t/* Update the IPASessionConfiguration using the sensor settings. */\n \tupdateSessionConfiguration(sensorCtrls_);\n \ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex a5a35b6585c6..8c5b6c36ae0b 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -26,6 +26,7 @@\n #include \"libcamera/internal/camera.h\"\n #include \"libcamera/internal/camera_lens.h\"\n #include \"libcamera/internal/camera_sensor.h\"\n+#include \"libcamera/internal/control_ids.h\"\n #include \"libcamera/internal/device_enumerator.h\"\n #include \"libcamera/internal/framebuffer.h\"\n #include \"libcamera/internal/ipa_manager.h\"\n@@ -82,8 +83,6 @@ public:\n \t/* Requests queued to the CIO2 device but not yet processed by the ImgU. */\n \tstd::queue<Request *> processingRequests_;\n \n-\tControlInfoMap ipaControls_;\n-\n private:\n \tvoid metadataReady(unsigned int id, const ControlList &metadata);\n \tvoid paramsBufferReady(unsigned int id);\n@@ -670,7 +669,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n \tconfigInfo.bdsOutputSize = config->imguConfig().bds;\n \tconfigInfo.iif = config->imguConfig().iif;\n \n-\tret = data->ipa_->configure(configInfo, &data->ipaControls_);\n+\tret = data->ipa_->configure(configInfo);\n \tif (ret) {\n \t\tLOG(IPU3, Error) << \"Failed to configure IPA: \"\n \t\t\t\t << strerror(-ret);\n@@ -946,9 +945,6 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n  * Initialize the camera controls by calculating controls which the pipeline\n  * is reponsible for and merge them with the controls computed by the IPA.\n  *\n- * This function needs data->ipaControls_ to be initialized by the IPA init()\n- * function at camera creation time. Always call this function after IPA init().\n- *\n  * \\return 0 on success or a negative error code otherwise\n  */\n int PipelineHandlerIPU3::initControls(IPU3CameraData *data)\n@@ -978,9 +974,6 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data)\n  * Compute the camera controls by calculating controls which the pipeline\n  * is reponsible for and merge them with the controls computed by the IPA.\n  *\n- * This function needs data->ipaControls_ to be refreshed when a new\n- * configuration is applied to the camera by the IPA configure() function.\n- *\n  * Always call this function after IPA configure() to make sure to have a\n  * properly refreshed IPA controls list.\n  *\n@@ -1059,9 +1052,21 @@ int PipelineHandlerIPU3::updateControls(IPU3CameraData *data)\n \n \tcontrols[&controls::ScalerCrop] = ControlInfo(minCrop, maxCrop, maxCrop);\n \n-\t/* Add the IPA registered controls to list of camera controls. */\n-\tfor (const auto &ipaControl : data->ipaControls_)\n-\t\tcontrols[ipaControl.first] = ipaControl.second;\n+\t/*\n+\t * Translate the sensor controls to Camera controls.\n+\t *\n+\t * Exposure time and the frame duration limits are reported directly\n+\t * from the sensor.\n+\t */\n+\tconst ControlInfoMap &sensorControls = sensor->controls();\n+\n+\tif (sensorControls.count(&controls::internal::ExposureTime))\n+\t\tcontrols[&controls::ExposureTime] =\n+\t\t\tsensorControls.at(&controls::internal::ExposureTime);\n+\n+\tif (sensorControls.count(&controls::internal::FrameDuration))\n+\t\tcontrols[&controls::FrameDurationLimits] =\n+\t\t\tsensorControls.at(&controls::internal::FrameDuration);\n \n \tdata->controlInfo_ = ControlInfoMap(std::move(controls),\n \t\t\t\t\t    controls::controls);\n@@ -1237,8 +1242,7 @@ int IPU3CameraData::loadIPA()\n \tif (ret)\n \t\treturn ret;\n \n-\tret = ipa_->init(IPASettings{ \"\", sensor->model() }, sensorInfo,\n-\t\t\t sensor->v4l2Controls(), &ipaControls_);\n+\tret = ipa_->init(IPASettings{ \"\", sensor->model() }, sensorInfo);\n \tif (ret) {\n \t\tLOG(IPU3, Error) << \"Failed to initialise the IPU3 IPA\";\n \t\treturn ret;\n","prefixes":["libcamera-devel","v3","15/23"]}