[{"id":25694,"web_url":"https://patchwork.libcamera.org/comment/25694/","msgid":"<20221031114233.qe74tdcdqauqilnu@uno.localdomain>","date":"2022-10-31T11:42:33","subject":"Re: [libcamera-devel] [PATCH v6 2/5] ipa: rkisp1: add\n\tFrameDurationLimits control","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Nicholas\n\nOn Sun, Oct 30, 2022 at 06:04:57PM -0500, Nicholas Roth via libcamera-devel wrote:\n> Currently, the Android HAL does not work on rkisp1-based devices because\n> required FrameDurationLimits metadata is missing from the IPA\n> implementation.\n>\n> This change sets FrameDurationLimits for rkisp1 based on the existing\n> ipu3 implementation, using the sensor's reported range of vertical\n> blanking intervals with the minimum reported horizontal blanking\n> interval.\n>\n> Signed-off-by: Nicholas Roth \n> ---\n> include/libcamera/ipa/rkisp1.mojom | 6 ++-\n> src/ipa/rkisp1/rkisp1.cpp | 57 +++++++++++++++++++++---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 +++--\n> 3 files changed, 64 insertions(+), 11 deletions(-)\n>\n> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\n> index eaf3955e..86ff6d0e 100644\n> --- a/include/libcamera/ipa/rkisp1.mojom\n> +++ b/include/libcamera/ipa/rkisp1.mojom\n> @@ -10,7 +10,9 @@ import \"include/libcamera/ipa/core.mojom\";\n>\n> interface IPARkISP1Interface {\n> \tinit(libcamera.IPASettings settings,\n> -\t uint32 hwRevision)\n> +\t uint32 hwRevision,\n> +\t libcamera.IPACameraSensorInfo sensorInfo,\n> +\t libcamera.ControlInfoMap sensorControls)\n> \t\t=> (int32 ret, libcamera.ControlInfoMap ipaControls);\n> \tstart() => (int32 ret);\n> \tstop();\n> @@ -18,7 +20,7 @@ interface IPARkISP1Interface {\n> \tconfigure(libcamera.IPACameraSensorInfo sensorInfo,\n> \t\t map streamConfig,\n> \t\t map entityControls)\n> -\t\t=> (int32 ret);\n> +\t\t=> (int32 ret, libcamera.ControlInfoMap ipaControls);\n>\n> \tmapBuffers(array buffers);\n> \tunmapBuffers(array ids);\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 069c901b..49239a87 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -49,13 +49,16 @@ public:\n> \tIPARkISP1();\n>\n> \tint init(const IPASettings &settings, unsigned int hwRevision,\n> +\t\t const IPACameraSensorInfo &sensorInfo,\n> +\t\t const ControlInfoMap &sensorControls,\n> \t\t ControlInfoMap *ipaControls) override;\n> \tint start() override;\n> \tvoid stop() override;\n>\n> \tint configure(const IPACameraSensorInfo &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 ControlInfoMap *ipaControls) override;\n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n>\n> @@ -68,6 +71,9 @@ protected:\n> \tstd::string logPrefix() const override;\n>\n> private:\n> +\tvoid updateControls(const IPACameraSensorInfo &sensorInfo,\n> +\t\t\t const ControlInfoMap &sensorControls,\n> +\t\t\t ControlInfoMap *ipaControls);\n> \tvoid setControls(unsigned int frame);\n>\n> \tstd::map buffers_;\n> @@ -115,6 +121,8 @@ std::string IPARkISP1::logPrefix() const\n> }\n>\n> int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> +\t\t const IPACameraSensorInfo &sensorInfo,\n> +\t\t const ControlInfoMap &sensorControls,\n> \t\t ControlInfoMap *ipaControls)\n> {\n> \t/* \\todo Add support for other revisions */\n> @@ -180,9 +188,8 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> \tif (ret)\n> \t\treturn ret;\n>\n> -\t/* Return the controls handled by the IPA. */\n> -\tControlInfoMap::Map ctrlMap = rkisp1Controls;\n> -\t*ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);\n> +\t/* Initialize controls. */\n> +\tupdateControls(sensorInfo, sensorControls, ipaControls);\n\nNot your fault. The IPU3 IPA does the same here: calls\nupdateControls() without validating them first in init(), while in\nconfigure it first validates all the required sensor controls are there and\nthen update the camera controls. RkISP1 equally validates the required\ncontrols are there only in configure().\n\nIPU3 has to be fixed anyhow, but since this code introduces a possible\nregression...\n\n\n>\n> \treturn 0;\n> }\n> @@ -207,7 +214,8 @@ void IPARkISP1::stop()\n> */\n> int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,\n> \t\t\t [[maybe_unused]] const std::map &streamConfig,\n> -\t\t\t const std::map &entityControls)\n> +\t\t\t const std::map &entityControls,\n> +\t\t\t ControlInfoMap *ipaControls)\n> {\n> \tif (entityControls.empty())\n> \t\treturn -EINVAL;\n> @@ -249,6 +257,9 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,\n> \tcontext_.configuration.sensor.size = info.outputSize;\n> \tcontext_.configuration.sensor.lineDuration = info.minLineLength * 1.0s / info.pixelRate;\n>\n> +\t/* Update the camera controls using the new sensor settings. */\n> +\tupdateControls(info, ctrls_, ipaControls);\n> +\n> \t/*\n> \t * When the AGC computes the new exposure values for a frame, it needs\n> \t * to know the limits for shutter speed and analogue gain.\n> @@ -349,6 +360,42 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId\n> \tmetadataReady.emit(frame, metadata);\n> }\n>\n> +void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo,\n> +\t\t\t const ControlInfoMap &sensorControls,\n> +\t\t\t ControlInfoMap *ipaControls)\n> +{\n> +\tControlInfoMap::Map ctrlMap = rkisp1Controls;\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();\n\n... as there you might end up accessing an unexisting control in your\ninit() call path I would at least copy the sensor controls validation\nwe have in configure()\n\n\tconst auto itExp = ctrls_.find(V4L2_CID_EXPOSURE);\n\tif (itExp == ctrls_.end()) {\n\t\tLOG(IPARkISP1, Error) << \"Can't find exposure control\";\n\t\treturn -EINVAL;\n\t}\n\n\tconst auto itGain = ctrls_.find(V4L2_CID_ANALOGUE_GAIN);\n\tif (itGain == ctrls_.end()) {\n\t\tLOG(IPARkISP1, Error) << \"Can't find gain control\";\n\t\treturn -EINVAL;\n\t}\n\nto init() by making sure HBLANK/VBLANK are there.\n\nTo make it more messy, we have this validation in CameraSensor class\n\n\tstatic constexpr uint32_t mandatoryControls[] = {\n\t\tV4L2_CID_EXPOSURE,\n\t\tV4L2_CID_HBLANK,\n\t\tV4L2_CID_PIXEL_RATE,\n\t\tV4L2_CID_VBLANK,\n\t};\n\nThe optimal path would be:\n1) Decide how much to validate on the pipeline handler side vs IPA\n side. If have to validate on IPA side for $reasons:\n 2) Move validateSensorControls() to IPU3 init()\n 3) Copy validateSensorControls in RkISP1\n 4) Call validateSensorControls() in RkISP1::init()\n 5) Drop custom validation code from RkISP1::configure()\n\nAs this is more work for you, and we need to finally make a call on\nwhere sensor control validation should happen, I would be fine merging\nthis as it is or, if you feel particularly motived, with a check\nfor HBLANK/VBLANK before calling updateControls() in init() as a\nsafety measure, with a pinky promise from our side we fix this mess on\nboth platforms as soon as these patches go in. (not something I'm\ngenerally confortable with as accepting technical debt with the\npromise that we'll eventually fix it is usually a recipe for disaster,\nbut as we're keeping a close eye on the series and we want to progress\nit as soon as possible, I think we can take the risk here)\n\n\n> +\tuint32_t lineLength = sensorInfo.outputSize.width + hblank;\n> +\n> +\tconst ControlInfo &v4l2VBlank = sensorControls.find(V4L2_CID_VBLANK)->second;\n> +\tstd::array frameHeights{\n> +\t\tv4l2VBlank.min().get() + sensorInfo.outputSize.height,\n> +\t\tv4l2VBlank.max().get() + sensorInfo.outputSize.height,\n> +\t\tv4l2VBlank.def().get() + sensorInfo.outputSize.height,\n> +\t};\n> +\n> +\tstd::array 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> +\tctrlMap[&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(ctrlMap), controls::controls);\n> +}\n> +\n> void IPARkISP1::setControls(unsigned int frame)\n> {\n> \t/*\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 455ee2a0..dae29a2c 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -340,15 +340,19 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> \t\t/*\n> \t\t * If the tuning file isn't found, fall back to the\n> \t\t * 'uncalibrated' configuration file.\n> -\t\t */\n> +\t */\n> \t\tif (ipaTuningFile.empty())\n> \t\t\tipaTuningFile = ipa_->configurationFile(\"uncalibrated.yaml\");\n> \t} else {\n> \t\tipaTuningFile = std::string(configFromEnv);\n> \t}\n>\n> -\tint ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,\n> -\t\t\t &controlInfo_);\n> +\tIPACameraSensorInfo sensorInfo{};\n> +\tint ret = sensor_->sensorInfo(&sensorInfo);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\tret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,\n> +\t\t\t sensorInfo, sensor_->controls(), &controlInfo_);\n> \tif (ret < 0) {\n> \t\tLOG(RkISP1, Error) << \"IPA initialization failure\";\n> \t\treturn ret;\n> @@ -725,7 +729,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \tstd::map entityControls;\n> \tentityControls.emplace(0, data->sensor_->controls());\n>\n> -\tret = data->ipa_->configure(sensorInfo, streamConfig, entityControls);\n> +\tret = data->ipa_->configure(sensorInfo, streamConfig, entityControls, &data->controlInfo_);\n> \tif (ret) {\n> \t\tLOG(RkISP1, Error) << \"failed configuring IPA (\" << ret << \")\";\n> \t\treturn ret;\n> --\n> 2.34.1\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 938C7BDB16\n\tfor ;\n\tMon, 31 Oct 2022 11:42:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C1EDE63032;\n\tMon, 31 Oct 2022 12:42:37 +0100 (CET)","from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7ACAB61F46\n\tfor ;\n\tMon, 31 Oct 2022 12:42:36 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id BED9E1BF211;\n\tMon, 31 Oct 2022 11:42:35 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1667216557;\n\tbh=QQQ3CV6D9jq3x8dQgPs2wbfnw752yX8Te855EADCjb0=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=RLYE47adrG5exnhRLnTGpZhBn1R6c1IJn+WJTqqQ0JOpptiFK1csCP3ZnvtWSpwA/\n\tdfixR2Bazd5IOijjygyEqpfnOidveeTVD8K0rVI07DqVMXCLId8kImhWYuURE/+dLW\n\t4YHtaz6ukJNQJmiD7IYzDiYLVPnD9PkJiVZ0tIcpVf6XGNB5b8OocjCABEDpzdTkCI\n\t0qMdd+muotv+b7bZ6t3mZVBhpJqpUPmJHhQvr51MehzXbsNW8rLo0gbMjc+m+af7nb\n\tZ6PrroeRNACFkVwMZcKUQHiNhYRdKYH06EHpQyrEbZIGB+STOpbuVw4v1mJDERU4um\n\tlEuYytgjBbaGA==","Date":"Mon, 31 Oct 2022 12:42:33 +0100","To":"Nicholas Roth ","Message-ID":"<20221031114233.qe74tdcdqauqilnu@uno.localdomain>","References":"<20221030230500.74842-1-nicholas@rothemail.net>\n\t<20221030230500.74842-3-nicholas@rothemail.net>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221030230500.74842-3-nicholas@rothemail.net>","Subject":"Re: [libcamera-devel] [PATCH v6 2/5] ipa: rkisp1: add\n\tFrameDurationLimits control","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25695,"web_url":"https://patchwork.libcamera.org/comment/25695/","msgid":"<20221031114344.5wsbaxdfhtfrsl3p@uno.localdomain>","date":"2022-10-31T11:43:44","subject":"Re: [libcamera-devel] [PATCH v6 2/5] ipa: rkisp1: add\n\tFrameDurationLimits control","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Ah sorry one more thing\n\nOn Sun, Oct 30, 2022 at 06:04:57PM -0500, Nicholas Roth via libcamera-devel wrote:\n> Currently, the Android HAL does not work on rkisp1-based devices because\n> required FrameDurationLimits metadata is missing from the IPA\n> implementation.\n>\n> This change sets FrameDurationLimits for rkisp1 based on the existing\n> ipu3 implementation, using the sensor's reported range of vertical\n> blanking intervals with the minimum reported horizontal blanking\n> interval.\n>\n> Signed-off-by: Nicholas Roth \n> ---\n> include/libcamera/ipa/rkisp1.mojom | 6 ++-\n> src/ipa/rkisp1/rkisp1.cpp | 57 +++++++++++++++++++++---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 +++--\n> 3 files changed, 64 insertions(+), 11 deletions(-)\n>\n> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\n> index eaf3955e..86ff6d0e 100644\n> --- a/include/libcamera/ipa/rkisp1.mojom\n> +++ b/include/libcamera/ipa/rkisp1.mojom\n> @@ -10,7 +10,9 @@ import \"include/libcamera/ipa/core.mojom\";\n>\n> interface IPARkISP1Interface {\n> \tinit(libcamera.IPASettings settings,\n> -\t uint32 hwRevision)\n> +\t uint32 hwRevision,\n> +\t libcamera.IPACameraSensorInfo sensorInfo,\n> +\t libcamera.ControlInfoMap sensorControls)\n> \t\t=> (int32 ret, libcamera.ControlInfoMap ipaControls);\n> \tstart() => (int32 ret);\n> \tstop();\n> @@ -18,7 +20,7 @@ interface IPARkISP1Interface {\n> \tconfigure(libcamera.IPACameraSensorInfo sensorInfo,\n> \t\t map streamConfig,\n> \t\t map entityControls)\n> -\t\t=> (int32 ret);\n> +\t\t=> (int32 ret, libcamera.ControlInfoMap ipaControls);\n>\n> \tmapBuffers(array buffers);\n> \tunmapBuffers(array ids);\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 069c901b..49239a87 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -49,13 +49,16 @@ public:\n> \tIPARkISP1();\n>\n> \tint init(const IPASettings &settings, unsigned int hwRevision,\n> +\t\t const IPACameraSensorInfo &sensorInfo,\n> +\t\t const ControlInfoMap &sensorControls,\n> \t\t ControlInfoMap *ipaControls) override;\n> \tint start() override;\n> \tvoid stop() override;\n>\n> \tint configure(const IPACameraSensorInfo &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 ControlInfoMap *ipaControls) override;\n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n>\n> @@ -68,6 +71,9 @@ protected:\n> \tstd::string logPrefix() const override;\n>\n> private:\n> +\tvoid updateControls(const IPACameraSensorInfo &sensorInfo,\n> +\t\t\t const ControlInfoMap &sensorControls,\n> +\t\t\t ControlInfoMap *ipaControls);\n> \tvoid setControls(unsigned int frame);\n>\n> \tstd::map buffers_;\n> @@ -115,6 +121,8 @@ std::string IPARkISP1::logPrefix() const\n> }\n>\n> int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> +\t\t const IPACameraSensorInfo &sensorInfo,\n> +\t\t const ControlInfoMap &sensorControls,\n> \t\t ControlInfoMap *ipaControls)\n> {\n> \t/* \\todo Add support for other revisions */\n> @@ -180,9 +188,8 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> \tif (ret)\n> \t\treturn ret;\n>\n> -\t/* Return the controls handled by the IPA. */\n> -\tControlInfoMap::Map ctrlMap = rkisp1Controls;\n> -\t*ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);\n> +\t/* Initialize controls. */\n> +\tupdateControls(sensorInfo, sensorControls, ipaControls);\n>\n> \treturn 0;\n> }\n> @@ -207,7 +214,8 @@ void IPARkISP1::stop()\n> */\n> int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,\n> \t\t\t [[maybe_unused]] const std::map &streamConfig,\n> -\t\t\t const std::map &entityControls)\n> +\t\t\t const std::map &entityControls,\n> +\t\t\t ControlInfoMap *ipaControls)\n> {\n> \tif (entityControls.empty())\n> \t\treturn -EINVAL;\n> @@ -249,6 +257,9 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,\n> \tcontext_.configuration.sensor.size = info.outputSize;\n> \tcontext_.configuration.sensor.lineDuration = info.minLineLength * 1.0s / info.pixelRate;\n>\n> +\t/* Update the camera controls using the new sensor settings. */\n> +\tupdateControls(info, ctrls_, ipaControls);\n> +\n> \t/*\n> \t * When the AGC computes the new exposure values for a frame, it needs\n> \t * to know the limits for shutter speed and analogue gain.\n> @@ -349,6 +360,42 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId\n> \tmetadataReady.emit(frame, metadata);\n> }\n>\n> +void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo,\n> +\t\t\t const ControlInfoMap &sensorControls,\n> +\t\t\t ControlInfoMap *ipaControls)\n> +{\n> +\tControlInfoMap::Map ctrlMap = rkisp1Controls;\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();\n> +\tuint32_t lineLength = sensorInfo.outputSize.width + hblank;\n> +\n> +\tconst ControlInfo &v4l2VBlank = sensorControls.find(V4L2_CID_VBLANK)->second;\n> +\tstd::array frameHeights{\n> +\t\tv4l2VBlank.min().get() + sensorInfo.outputSize.height,\n> +\t\tv4l2VBlank.max().get() + sensorInfo.outputSize.height,\n> +\t\tv4l2VBlank.def().get() + sensorInfo.outputSize.height,\n> +\t};\n> +\n> +\tstd::array 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> +\tctrlMap[&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(ctrlMap), controls::controls);\n> +}\n> +\n> void IPARkISP1::setControls(unsigned int frame)\n> {\n> \t/*\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 455ee2a0..dae29a2c 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -340,15 +340,19 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> \t\t/*\n> \t\t * If the tuning file isn't found, fall back to the\n> \t\t * 'uncalibrated' configuration file.\n> -\t\t */\n> +\t */\n\n ^ Unrelated change\n\n> \t\tif (ipaTuningFile.empty())\n> \t\t\tipaTuningFile = ipa_->configurationFile(\"uncalibrated.yaml\");\n> \t} else {\n> \t\tipaTuningFile = std::string(configFromEnv);\n> \t}\n>\n> -\tint ret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,\n> -\t\t\t &controlInfo_);\n> +\tIPACameraSensorInfo sensorInfo{};\n> +\tint ret = sensor_->sensorInfo(&sensorInfo);\n> +\tif (ret)\n> +\t\treturn ret;\n\nEmpty line here maybe\n\nThanks\n j\n\n> +\tret = ipa_->init({ ipaTuningFile, sensor_->model() }, hwRevision,\n> +\t\t\t sensorInfo, sensor_->controls(), &controlInfo_);\n> \tif (ret < 0) {\n> \t\tLOG(RkISP1, Error) << \"IPA initialization failure\";\n> \t\treturn ret;\n> @@ -725,7 +729,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \tstd::map entityControls;\n> \tentityControls.emplace(0, data->sensor_->controls());\n>\n> -\tret = data->ipa_->configure(sensorInfo, streamConfig, entityControls);\n> +\tret = data->ipa_->configure(sensorInfo, streamConfig, entityControls, &data->controlInfo_);\n> \tif (ret) {\n> \t\tLOG(RkISP1, Error) << \"failed configuring IPA (\" << ret << \")\";\n> \t\treturn ret;\n> --\n> 2.34.1\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 1448FBDB16\n\tfor ;\n\tMon, 31 Oct 2022 11:43:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B05F663035;\n\tMon, 31 Oct 2022 12:43:48 +0100 (CET)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::226])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0EDCC61F46\n\tfor ;\n\tMon, 31 Oct 2022 12:43:47 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 5E1A4C0006;\n\tMon, 31 Oct 2022 11:43:46 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1667216628;\n\tbh=v3URxzZL7QQFMCDPwXZCsU0fNRzUzjkLhHrxPmDQxEE=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=cEYJvVYAo2l4BpNw4pAgo3BI+Hxrvs1sOX32ywYWoQh54nW2y8mv43vitq9Gck7hS\n\tM/yCYjntRoDnI2PX1SvlKVlAUWxm1H2BM2eCPswelSOodQHSBpX7Ix2fVFWdTvU19+\n\tIfkglMBjCP5oGZLjXhdVd9yitctMYGnffhcH5j1sxcymTO5eRcWg8U/wXtQuav4MTd\n\tB01TAv/MIGyl5tW+OaCtfqtO7oPIo5RAxatOCHlynaBdeFSU5KuO4+TTrMUZ6gfhGQ\n\tfZt3Ut5g9r1QGYhW6hWItxvi9pe8UsALF+OQzZCmPL4dJceI8B+69wTeqVS7MrZrJ+\n\tGUCYDUVOVX/tA==","Date":"Mon, 31 Oct 2022 12:43:44 +0100","To":"Nicholas Roth ","Message-ID":"<20221031114344.5wsbaxdfhtfrsl3p@uno.localdomain>","References":"<20221030230500.74842-1-nicholas@rothemail.net>\n\t<20221030230500.74842-3-nicholas@rothemail.net>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221030230500.74842-3-nicholas@rothemail.net>","Subject":"Re: [libcamera-devel] [PATCH v6 2/5] ipa: rkisp1: add\n\tFrameDurationLimits control","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]