[{"id":25198,"web_url":"https://patchwork.libcamera.org/comment/25198/","msgid":"","date":"2022-09-30T20:27:00","subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Correct context during\n\tconfigure()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Fri, Sep 30, 2022 at 08:08:21PM +0100, Kieran Bingham via libcamera-devel wrote:\n> The introduction of the FCQueue in the IPU3 inadvertently introduced a\n> bug which cleared the initialisation of the session configuration\n> immediately after some parameters had been set.\n> \n> Furthermore, it cleared and never re-initialised the sensor line\n> duration property, which was previously only set during the call to\n> init().\n> \n> Move the clearing of the contexts from the updateSessionConfiguration()\n> call to the earliest opportunity in configure(), and immediately\n> re-initialise the sensor parameters.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=151\n> Fixes: 85c5c47325ab (\"ipa: ipu3: Use the FCQueue\")\n> Signed-off-by: Kieran Bingham \n> ---\n> src/ipa/ipu3/ipu3.cpp | 13 ++++++++-----\n> 1 file changed, 8 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index d1ea081d595d..7fb62c749fde 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -217,11 +217,6 @@ void IPAIPU3::updateSessionConfiguration(const ControlInfoMap &sensorControls)\n> \tint32_t minGain = v4l2Gain.min().get();\n> \tint32_t maxGain = v4l2Gain.max().get();\n> \n> -\t/* Clear the IPA context before the streaming session. */\n> -\tcontext_.configuration = {};\n> -\tcontext_.activeState = {};\n> -\tcontext_.frameContexts.clear();\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> @@ -498,6 +493,14 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> \n> \tlensCtrls_ = configInfo.lensControls;\n> \n> +\t/* Clear the IPA context for the new streaming session. */\n> +\tcontext_.activeState = {};\n> +\tcontext_.configuration = {};\n> +\tcontext_.frameContexts.clear();\n> +\n> +\t/* Intialise the sensor configuration */\n> +\tcontext_.configuration.sensor.lineDuration = sensorInfo_.lineLength * 1.0s / sensorInfo_.pixelRate;\n\nThis could be dropped from init(), but that can be done in a different\npatch, we should revisit the init()/configure() split in the IPU3 IPA\nmodule implementation.\n\nReviewed-by: Laurent Pinchart \n\n> +\n> \t/*\n> \t * Compute the sensor V4L2 controls to be used by the algorithms and\n> \t * to be set on the sensor.","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 0166DC0DA4\n\tfor ;\n\tFri, 30 Sep 2022 20:27:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3CB8462841;\n\tFri, 30 Sep 2022 22:27:04 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6F2CC61FB1\n\tfor ;\n\tFri, 30 Sep 2022 22:27:03 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B953147C;\n\tFri, 30 Sep 2022 22:27:02 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1664569624;\n\tbh=k/AUWtuTkIcxfjeKTX0hKqoLYgUUfrgmG6/cAmI+H20=;\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=LgvStVDGmVZ9diMQLuAm41gANHraZdTxFFKPL6vQvVzKnUNXKxRydEtXuyZVA7Nf7\n\tIWQsxPGbnOE4DJNscDZxN/9KZmmiRAjZnaO5ivcGDJY8uMeioXiFs2OjHwAzgMywAE\n\tpexKXbnifb+iwEzEF2o+j84jjol+l/SOf9LwUjKLjMVmIXOKSQnTZyLkdx2eJ6hbeq\n\tX8vZmLCTRjKuy3z8osvlctCi3ylPk7nKkX7JLA+eTEsGHuBiaesdOeayYgTL2WAgp2\n\tSx5EwNGDkqICBxHYdtaCQyTMPEz74tAIchbyiW+wulV3CvaFyhmQPOEjBTdLnrUKZP\n\t+llKoe0I6LTDA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1664569622;\n\tbh=k/AUWtuTkIcxfjeKTX0hKqoLYgUUfrgmG6/cAmI+H20=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=aQVAHMrXXWzWNBxlpNn4aqP0SuTKlb7MG3Jc2+UW5Lj2uo5lp5gCuCmqDVhJlvdxW\n\t3En8Xi18q2RwbJeXcUQQ9O4r+se/CURVijW6i2RbZFu1qYltBDpOaTg4IaGzpoZOVJ\n\tE+Kmecouig6beYMs2LBJCKss1qek+tsqiNngFXXQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"aQVAHMrX\"; dkim-atps=neutral","Date":"Fri, 30 Sep 2022 23:27:00 +0300","To":"Kieran Bingham ","Message-ID":"","References":"<20220930190821.4100474-1-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220930190821.4100474-1-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Correct context during\n\tconfigure()","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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25215,"web_url":"https://patchwork.libcamera.org/comment/25215/","msgid":"<20221001080932.5zp7fygvywbqio6v@uno.localdomain>","date":"2022-10-01T08:09:32","subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Correct context during\n\tconfigure()","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"On Fri, Sep 30, 2022 at 08:08:21PM +0100, Kieran Bingham via libcamera-devel wrote:\n> The introduction of the FCQueue in the IPU3 inadvertently introduced a\n> bug which cleared the initialisation of the session configuration\n> immediately after some parameters had been set.\n>\n> Furthermore, it cleared and never re-initialised the sensor line\n> duration property, which was previously only set during the call to\n> init().\n>\n> Move the clearing of the contexts from the updateSessionConfiguration()\n> call to the earliest opportunity in configure(), and immediately\n> re-initialise the sensor parameters.\n>\n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=151\n> Fixes: 85c5c47325ab (\"ipa: ipu3: Use the FCQueue\")\n> Signed-off-by: Kieran Bingham \n> ---\n> src/ipa/ipu3/ipu3.cpp | 13 ++++++++-----\n> 1 file changed, 8 insertions(+), 5 deletions(-)\n>\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index d1ea081d595d..7fb62c749fde 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -217,11 +217,6 @@ void IPAIPU3::updateSessionConfiguration(const ControlInfoMap &sensorControls)\n> \tint32_t minGain = v4l2Gain.min().get();\n> \tint32_t maxGain = v4l2Gain.max().get();\n>\n> -\t/* Clear the IPA context before the streaming session. */\n> -\tcontext_.configuration = {};\n> -\tcontext_.activeState = {};\n> -\tcontext_.frameContexts.clear();\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> @@ -498,6 +493,14 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n>\n> \tlensCtrls_ = configInfo.lensControls;\n>\n> +\t/* Clear the IPA context for the new streaming session. */\n> +\tcontext_.activeState = {};\n> +\tcontext_.configuration = {};\n> +\tcontext_.frameContexts.clear();\n> +\n> +\t/* Intialise the sensor configuration */\n> +\tcontext_.configuration.sensor.lineDuration = sensorInfo_.lineLength * 1.0s / sensorInfo_.pixelRate;\n\nIt is currently calculated at init time one, so it's correct to move\nit here (possibily removing it from init(), it doesn't seem to be used\nin any function called before configure().)\n\nIt might make sense to intiialize lineDuration in updateControls(),\nbut that would make it less clear that updateControls() shall be\ncalled before updateSessionConfiguration(), so I guess it's fine to\nhave it here.\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> +\n> \t/*\n> \t * Compute the sensor V4L2 controls to be used by the algorithms and\n> \t * to be set on the sensor.\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 A58A7BD16B\n\tfor ;\n\tSat, 1 Oct 2022 08:09:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EBB2B6285B;\n\tSat, 1 Oct 2022 10:09:36 +0200 (CEST)","from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::225])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D663B622B4\n\tfor ;\n\tSat, 1 Oct 2022 10:09:34 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 5B58B1C0002;\n\tSat, 1 Oct 2022 08:09:33 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1664611777;\n\tbh=QunGMNSvVBLQfLPIknXKuFND1P18DvQIjiInFfAZnRE=;\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=ENSLPwUTZN3oWSWWgkC8wOXRs4Udl6vewA9QF5k7AP+dDW419bA+r27ue7XAHhncv\n\tAZ0XSjLerFGpnsoY+gCznDYsYtGZqsNhQ7v5ASkERazcxzDdGf9qc2PX36ipQJsXob\n\tUwbzlDeJ5Q51nHizOFl4ON/cXsKB6oGmdWHsNtaL1YhA2QAM4G7c1kJeJv620SSHRb\n\te09olVe23pYpQYhscTBGy0QhCRAkPcTarchVx8/zZSbSijEc3nqvawGKFHrmqCoDVf\n\tHOFg0SVjMqYHQh8ogKA2WYUPLJ3CSBSI80OTMhCAegdOAPrFrnWj3YjCJSsxAbISha\n\tW2RUKhWd5sMhQ==","Date":"Sat, 1 Oct 2022 10:09:32 +0200","To":"Kieran Bingham ","Message-ID":"<20221001080932.5zp7fygvywbqio6v@uno.localdomain>","References":"<20220930190821.4100474-1-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220930190821.4100474-1-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Correct context during\n\tconfigure()","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]