[{"id":22234,"web_url":"https://patchwork.libcamera.org/comment/22234/","msgid":"<164682741135.856468.12893199368715218591@Monstersaurus>","date":"2022-03-09T12:03:31","subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Ensure controls\n\texists in updateSessionConfiguration()","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Quoting Umang Jain via libcamera-devel (2022-03-05 17:28:49)\n> Add a control-not-found error checking for the controls being\n> queried in IPAIPU3::updateSessionConfiguration().\n> \n> Signed-off-by: Umang Jain \n> ---\n> src/ipa/ipu3/ipu3.cpp | 31 +++++++++++++++++++++++--------\n> 1 file changed, 23 insertions(+), 8 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 0df358ba..583d6b9a 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -180,16 +180,31 @@ private:\n> */\n> void IPAIPU3::updateSessionConfiguration(const ControlInfoMap &sensorControls)\n> {\n> - const ControlInfo vBlank = sensorControls.find(V4L2_CID_VBLANK)->second;\n> - context_.configuration.sensor.defVBlank = vBlank.def().get();\n> + const auto itVBlank = sensorControls.find(V4L2_CID_VBLANK);\n\nI'm torn whether we should have a single list of mandatory controls that\nwe check at init()/configure() or this which at least checks and obtains\nthe control in one loop.\n\n\nBut I note that this function is returning void, and so you are not\nreturning an error if the controls are not found - So I think that's\nbad. It needs ot have the error propogated back up - ... so that means I\nthink a single check in configure() (or if we can during init()) would\nbe better.\n\n\n> + if (itVBlank == sensorControls.end()) {\n> + LOG(IPAIPU3, Error) << \"Can't find vblank control\";\n> + return;\n> + }\n> \n> - const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second;\n> - int32_t minExposure = v4l2Exposure.min().get();\n> - int32_t maxExposure = v4l2Exposure.max().get();\n> + context_.configuration.sensor.defVBlank = itVBlank->second.def().get();\n> +\n> + const auto itExp = sensorControls.find(V4L2_CID_EXPOSURE);\n> + if (itExp == sensorControls.end()) {\n> + LOG(IPAIPU3, Error) << \"Can't find exposure control\";\n> + return;\n> + }\n> +\n> + int32_t minExposure = itExp->second.min().get();\n> + int32_t maxExposure = itExp->second.max().get();\n> +\n> + const auto itGain = sensorControls.find(V4L2_CID_ANALOGUE_GAIN);\n> + if (itGain == sensorControls.end()) {\n> + LOG(IPAIPU3, Error) << \"Can't find analogue gain control\";\n> + return;\n> + }\n> \n> - const ControlInfo &v4l2Gain = sensorControls.find(V4L2_CID_ANALOGUE_GAIN)->second;\n> - int32_t minGain = v4l2Gain.min().get();\n> - int32_t maxGain = v4l2Gain.max().get();\n> + int32_t minGain = itGain->second.min().get();\n> + int32_t maxGain = itGain->second.max().get();\n> \n> /*\n> * When the AGC computes the new exposure values for a frame, it needs\n> -- \n> 2.31.0\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 F2CBFBE08A\n\tfor ;\n\tWed, 9 Mar 2022 12:03:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6CCFF61189;\n\tWed, 9 Mar 2022 13:03:35 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 257E360475\n\tfor ;\n\tWed, 9 Mar 2022 13:03:34 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B5C748C4;\n\tWed, 9 Mar 2022 13:03:33 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1646827415;\n\tbh=T6L0X3NG8NCz2gKyuUdRzuQunKW8yZZsN/IK4oeggVQ=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=I9k8jYNYnZJf81b7twNRVGG4CYacikZpRvS2Wo7G+BY3swCX8i329nMjthdUdiEa7\n\t+AcFNFrfr583DCqA25PJt6DIjep76SWlnJhkHcNh+Tw+GarmfwTpXI4P1rp/27MQAV\n\tpZ+/Fa2+fsvlpLBR8SGTKBHEvti8un3Dp2AREUvUbvxFEe+yQjOcT/XCpFuDb/wiF5\n\tPqJaZW7Z/YbG3KvFr1KngVyVVTGFkm15MXJEMTIqmS+xGzCQZAUSq2XFl5VIhp3TeQ\n\tTpBMDJYKhEn3VxGjweWtEfhiERhxb5q1m4chGx0jnTONxQKKXk8yJNGdi7SZOQsmtq\n\tskbDyZVkhgpwQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1646827413;\n\tbh=T6L0X3NG8NCz2gKyuUdRzuQunKW8yZZsN/IK4oeggVQ=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=JZUoaSTE2F/k7+g/rvOq2TpMhLWJ5QheyJlqI95JGAcFSUhuG66yAWNLkRJE3U//r\n\tvfVPmqpFDgU31zF0WAm7V8ROdevUtiKc63tdxGSS5/ksolNsPebSGXpPFNH2bw54aB\n\tix8DUfTehPFrERDPW+Yqa0Lt7QFmdmS4kJFFhFXw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"JZUoaSTE\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220305172849.1022923-4-umang.jain@ideasonboard.com>","References":"<20220305172849.1022923-1-umang.jain@ideasonboard.com>\n\t<20220305172849.1022923-4-umang.jain@ideasonboard.com>","To":"Umang Jain ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 09 Mar 2022 12:03:31 +0000","Message-ID":"<164682741135.856468.12893199368715218591@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 3/3] ipa: ipu3: Ensure controls\n\texists in updateSessionConfiguration()","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":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]