Patch Detail
Show a patch.
GET /api/1.1/patches/17165/?format=api
{ "id": 17165, "url": "https://patchwork.libcamera.org/api/1.1/patches/17165/?format=api", "web_url": "https://patchwork.libcamera.org/patch/17165/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20220818094410.1671-11-jacopo@jmondi.org>", "date": "2022-08-18T09:44:03", "name": "[libcamera-devel,v3,10/17] ipa: rkisp1: Align configure() with IPU3", "commit_ref": null, "pull_url": null, "state": "new", "archived": false, "hash": "c1ad6d4e447a91db4781076304eda9772adc94da", "submitter": { "id": 3, "url": "https://patchwork.libcamera.org/api/1.1/people/3/?format=api", "name": "Jacopo Mondi", "email": "jacopo@jmondi.org" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/17165/mbox/", "series": [ { "id": 3426, "url": "https://patchwork.libcamera.org/api/1.1/series/3426/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3426", "date": "2022-08-18T09:43:53", "name": "libcamera: Align IPU3 and RKISP1 interfaces", "version": 3, "mbox": "https://patchwork.libcamera.org/series/3426/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/17165/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/17165/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 30F81C3275\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 18 Aug 2022 09:44:38 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BA92861FDD;\n\tThu, 18 Aug 2022 11:44:37 +0200 (CEST)", "from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A88AF61FD9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 18 Aug 2022 11:44:32 +0200 (CEST)", "(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id B68CAFF80C;\n\tThu, 18 Aug 2022 09:44:31 +0000 (UTC)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660815877;\n\tbh=ydmiNplBrbkufWHt45m/xxp4OZlEQL6jepSEzxK6QHo=;\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=MKABOTMNULyK1jYbbcxTQT3OMpY0SIGi2/E4wBnDLH5cx+53ehiN1l3Km1gH8Kvs9\n\tisbga3am6VVoMFiI9+dGYbVq3rtcnaHSzk71NQuLKot6YYUogAGDaH98HIct1m765P\n\tX2zSnvcel4at2Jb2N+FSABUBy2aH/YWCytTQ7tdfVfFnqgRxbTS3h4zFnvq/TskHys\n\tzlvSeZNIPgjW2huv6yPDoWjfXkPrsH3ecjrkUyDcODo52wBrYZPbgF6nMwJ+UqUN0f\n\tSeNfEHOO3QaaeTS0vaZx14gZs4d2LO25O80PnNoOeKNkpleMDh6/hOvki2vIuuSWE7\n\tj0uy8CqQaWQ2w==", "To": "libcamera-devel@lists.libcamera.org", "Date": "Thu, 18 Aug 2022 11:44:03 +0200", "Message-Id": "<20220818094410.1671-11-jacopo@jmondi.org>", "X-Mailer": "git-send-email 2.37.2", "In-Reply-To": "<20220818094410.1671-1-jacopo@jmondi.org>", "References": "<20220818094410.1671-1-jacopo@jmondi.org>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v3 10/17] ipa: rkisp1: Align configure()\n\twith IPU3", "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": "The RkISP1 IPA::configure() function signature requires a map of\nentities controls and a map of stream configuration to be provided by\nthe pipeline handler to the IPA.\n\nThis design comes from the early days when the first IPA module was\nimplemented. With the introduction of mojom-defined IPA interfaces it's\nnow easier to define custom structures to group parameters together, as\nthe IPU3 IPA does.\n\nAlign the IPU3 and RkISP1 IPA interfaces to use the same function\nsignature and introduce rkisp1::IPAConfigInfo for the purpose of\ngrouping configuration parameters together.\n\nUpdate the implementation of the RkISP1 IPA to validate the supplied\nlist of controls and update the session configuration in separate\nfunctions.\n\nSigned-off-by: Jacopo Mondi <jacopo@jmondi.org>\n---\n include/libcamera/ipa/rkisp1.mojom | 10 ++-\n src/ipa/rkisp1/rkisp1.cpp | 98 ++++++++++++++----------\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 25 +++---\n 3 files changed, 74 insertions(+), 59 deletions(-)", "diff": "diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\nindex eaf3955e4096..7efe17746804 100644\n--- a/include/libcamera/ipa/rkisp1.mojom\n+++ b/include/libcamera/ipa/rkisp1.mojom\n@@ -8,6 +8,12 @@ module ipa.rkisp1;\n \n import \"include/libcamera/ipa/core.mojom\";\n \n+struct IPAConfigInfo {\n+\tlibcamera.IPACameraSensorInfo sensorInfo;\n+\tlibcamera.ControlInfoMap sensorControls;\n+\tlibcamera.ControlInfoMap lensControls;\n+};\n+\n interface IPARkISP1Interface {\n \tinit(libcamera.IPASettings settings,\n \t uint32 hwRevision)\n@@ -15,9 +21,7 @@ interface IPARkISP1Interface {\n \tstart() => (int32 ret);\n \tstop();\n \n-\tconfigure(libcamera.IPACameraSensorInfo sensorInfo,\n-\t\t map<uint32, libcamera.IPAStream> streamConfig,\n-\t\t map<uint32, libcamera.ControlInfoMap> entityControls)\n+\tconfigure(IPAConfigInfo configInfo)\n \t\t=> (int32 ret);\n \n \tmapBuffers(array<libcamera.IPABuffer> buffers);\ndiff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\nindex 3a98aaf75d98..f2075c893d29 100644\n--- a/src/ipa/rkisp1/rkisp1.cpp\n+++ b/src/ipa/rkisp1/rkisp1.cpp\n@@ -49,9 +49,7 @@ public:\n \tint start() override;\n \tvoid stop() override;\n \n-\tint configure(const IPACameraSensorInfo &info,\n-\t\t const std::map<uint32_t, IPAStream> &streamConfig,\n-\t\t const std::map<uint32_t, ControlInfoMap> &entityControls) override;\n+\tint configure(const IPAConfigInfo &configInfo) override;\n \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override;\n \tvoid unmapBuffers(const std::vector<unsigned int> &ids) override;\n \n@@ -64,6 +62,9 @@ protected:\n \tstd::string logPrefix() const override;\n \n private:\n+\tvoid updateSessionConfiguration(const IPAConfigInfo &configInfo);\n+\tbool validateSensorControls(const ControlInfoMap &sensorControls);\n+\n \tvoid setControls(unsigned int frame);\n \tvoid prepareMetadata(unsigned int frame, unsigned int aeState);\n \n@@ -193,44 +194,18 @@ void IPARkISP1::stop()\n \tcontext_.frameContexts.clear();\n }\n \n-/**\n- * \\todo The RkISP1 pipeline currently provides an empty IPACameraSensorInfo\n- * if the connected sensor does not provide enough information to properly\n- * assemble one. Make sure the reported sensor information are relevant\n- * before accessing them.\n- */\n-int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,\n-\t\t\t [[maybe_unused]] const std::map<uint32_t, IPAStream> &streamConfig,\n-\t\t\t const std::map<uint32_t, ControlInfoMap> &entityControls)\n+void IPARkISP1::updateSessionConfiguration(const IPAConfigInfo &configInfo)\n {\n-\tif (entityControls.empty())\n-\t\treturn -EINVAL;\n-\n-\tsensorCtrls_ = entityControls.at(0);\n-\n-\tauto lensControls = entityControls.find(1);\n-\tif (lensControls != entityControls.end())\n-\t\tlensCtrls_ = lensControls->second;\n+\tconst IPACameraSensorInfo &sensorInfo = configInfo.sensorInfo;\n+\tconst ControlInfoMap &sensorControls = configInfo.sensorControls;\n \n-\tconst auto itExp = sensorCtrls_.find(V4L2_CID_EXPOSURE);\n-\tif (itExp == sensorCtrls_.end()) {\n-\t\tLOG(IPARkISP1, Error) << \"Can't find exposure control\";\n-\t\treturn -EINVAL;\n-\t}\n-\n-\tconst auto itGain = sensorCtrls_.find(V4L2_CID_ANALOGUE_GAIN);\n-\tif (itGain == sensorCtrls_.end()) {\n-\t\tLOG(IPARkISP1, Error) << \"Can't find gain control\";\n-\t\treturn -EINVAL;\n-\t}\n+\tconst ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;\n+\tconst ControlInfo &v4l2Gain = sensorControls.find(V4L2_CID_ANALOGUE_GAIN)->second;\n+\tint32_t minExposure = v4l2Exposure.min().get<int32_t>();\n+\tint32_t maxExposure = v4l2Exposure.max().get<int32_t>();\n \n-\tautoExposure_ = true;\n-\n-\tint32_t minExposure = itExp->second.min().get<int32_t>();\n-\tint32_t maxExposure = itExp->second.max().get<int32_t>();\n-\n-\tint32_t minGain = itGain->second.min().get<int32_t>();\n-\tint32_t maxGain = itGain->second.max().get<int32_t>();\n+\tint32_t minGain = v4l2Gain.min().get<int32_t>();\n+\tint32_t maxGain = v4l2Gain.max().get<int32_t>();\n \n \tLOG(IPARkISP1, Info)\n \t\t<< \"Exposure: \" << minExposure << \"-\" << maxExposure\n@@ -243,8 +218,9 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,\n \t/* Set the hardware revision for the algorithms. */\n \tcontext_.configuration.hw.revision = hwRevision_;\n \n-\tcontext_.configuration.sensor.size = info.outputSize;\n-\tcontext_.configuration.sensor.lineDuration = info.lineLength * 1.0s / info.pixelRate;\n+\tcontext_.configuration.sensor.size = sensorInfo.outputSize;\n+\tcontext_.configuration.sensor.lineDuration = sensorInfo.lineLength * 1.0s\n+\t\t\t\t\t\t / sensorInfo.pixelRate;\n \n \t/*\n \t * When the AGC computes the new exposure values for a frame, it needs\n@@ -259,9 +235,49 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,\n \tcontext_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);\n \n \tcontext_.activeState.frameCount = 0;\n+}\n+\n+bool IPARkISP1::validateSensorControls(const ControlInfoMap &sensorControls)\n+{\n+\tstatic const uint32_t ctrls[] = {\n+\t\tV4L2_CID_ANALOGUE_GAIN,\n+\t\tV4L2_CID_EXPOSURE,\n+\t};\n+\n+\tfor (auto c : ctrls) {\n+\t\tif (sensorControls.find(c) == sensorControls.end()) {\n+\t\t\tLOG(IPARkISP1, Error) << \"Unable to find sensor control \"\n+\t\t\t\t\t << utils::hex(c);\n+\t\t\treturn false;\n+\t\t}\n+\t}\n+\n+\treturn true;\n+\n+}\n+\n+/**\n+ * \\todo The RkISP1 pipeline currently provides an empty IPACameraSensorInfo\n+ * if the connected sensor does not provide enough information to properly\n+ * assemble one. Make sure the reported sensor information are relevant\n+ * before accessing them.\n+ */\n+int IPARkISP1::configure(const IPAConfigInfo &configInfo)\n+{\n+\tif (!validateSensorControls(configInfo.sensorControls)) {\n+\t\tLOG(IPARkISP1, Error) << \"Sensor control validation failed.\";\n+\t\treturn -EINVAL;\n+\t}\n+\n+\tsensorCtrls_ = configInfo.sensorControls;\n+\tlensCtrls_ = configInfo.lensControls;\n+\n+\tautoExposure_ = true;\n+\n+\tupdateSessionConfiguration(configInfo);\n \n \tfor (auto const &algo : algorithms()) {\n-\t\tint ret = algo->configure(context_, info);\n+\t\tint ret = algo->configure(context_, configInfo.sensorInfo);\n \t\tif (ret)\n \t\t\treturn ret;\n \t}\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 5f10c26bcb4d..3b250b0ae346 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -651,20 +651,13 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n \t\t<< \"ISP output pad configured with \" << format\n \t\t<< \" crop \" << rect;\n \n-\tstd::map<unsigned int, IPAStream> streamConfig;\n-\n \tfor (const StreamConfiguration &cfg : *config) {\n-\t\tif (cfg.stream() == &data->mainPathStream_) {\n+\t\tif (cfg.stream() == &data->mainPathStream_)\n \t\t\tret = mainPath_.configure(cfg, format);\n-\t\t\tstreamConfig[0] = IPAStream(cfg.pixelFormat,\n-\t\t\t\t\t\t cfg.size);\n-\t\t} else if (hasSelfPath_) {\n+\t\telse if (hasSelfPath_)\n \t\t\tret = selfPath_.configure(cfg, format);\n-\t\t\tstreamConfig[1] = IPAStream(cfg.pixelFormat,\n-\t\t\t\t\t\t cfg.size);\n-\t\t} else {\n+\t\telse\n \t\t\treturn -ENODEV;\n-\t\t}\n \n \t\tif (ret)\n \t\t\treturn ret;\n@@ -682,7 +675,9 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n \tif (ret)\n \t\treturn ret;\n \n-\t/* Inform IPA of stream configuration and sensor controls. */\n+\t/* Configure the IPA module. */\n+\tipa::rkisp1::IPAConfigInfo configInfo;\n+\n \tIPACameraSensorInfo sensorInfo = {};\n \tret = data->sensor_->sensorInfo(&sensorInfo);\n \tif (ret) {\n@@ -692,14 +687,14 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n \t\tret = 0;\n \t}\n \n-\tstd::map<uint32_t, ControlInfoMap> entityControls;\n-\tentityControls.emplace(0, data->sensor_->controls());\n+\tconfigInfo.sensorInfo = sensorInfo;\n+\tconfigInfo.sensorControls = data->sensor_->controls();\n \n \tCameraLens *lens = data->sensor_->focusLens();\n \tif (lens)\n-\t\tentityControls.emplace(1, lens->controls());\n+\t\tconfigInfo.lensControls = lens->controls();\n \n-\tret = data->ipa_->configure(sensorInfo, streamConfig, entityControls);\n+\tret = data->ipa_->configure(configInfo);\n \tif (ret) {\n \t\tLOG(RkISP1, Error) << \"failed configuring IPA (\" << ret << \")\";\n \t\treturn ret;\n", "prefixes": [ "libcamera-devel", "v3", "10/17" ] }