[{"id":23718,"web_url":"https://patchwork.libcamera.org/comment/23718/","msgid":"<20220704142122.oqofebcyb4bcisrw@uno.localdomain>","date":"2022-07-04T14:21:22","subject":"Re: [libcamera-devel] [PATCH] delayed_controls: Remove reduandant\n\tfirstSequence_","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran\n\nOn Mon, Jul 04, 2022 at 02:37:51PM +0100, Kieran Bingham via libcamera-devel wrote:\n> The DelayedControls implementation tracked the sequence numbers to\n> determine the offset if a device did not commence with a sequence number\n> of 0.\n>\n> This guarantee is now handled by the V4L2VideoDevice.\n>\n> Remove the firstSequence_ offset and the corresponding running_ flag\n> which was used to track setting firstSequence_ from the DelayedControls.\n>\n> Signed-off-by: Kieran Bingham \n\nIt seems correct to me!\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n>\n> This will conflict with Jacopo's series, but is still applicable\n> conceptually, whichever gets to go first.\n>\n> include/libcamera/internal/delayed_controls.h | 3 ---\n> src/libcamera/delayed_controls.cpp | 12 ++----------\n> 2 files changed, 2 insertions(+), 13 deletions(-)\n>\n> diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h\n> index 703fdb66bc27..f560c823d4e9 100644\n> --- a/include/libcamera/internal/delayed_controls.h\n> +++ b/include/libcamera/internal/delayed_controls.h\n> @@ -72,9 +72,6 @@ private:\n> \tstd::unordered_map controlParams_;\n> \tunsigned int maxDelay_;\n>\n> -\tbool running_;\n> -\tuint32_t firstSequence_;\n> -\n> \tuint32_t queueCount_;\n> \tuint32_t writeCount_;\n> \t/* \\todo Evaluate if we should index on ControlId * or unsigned int */\n> diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp\n> index 9667187e9a4a..527c319113ac 100644\n> --- a/src/libcamera/delayed_controls.cpp\n> +++ b/src/libcamera/delayed_controls.cpp\n> @@ -115,8 +115,6 @@ DelayedControls::DelayedControls(V4L2Device *device,\n> */\n> void DelayedControls::reset()\n> {\n> -\trunning_ = false;\n> -\tfirstSequence_ = 0;\n> \tqueueCount_ = 1;\n> \twriteCount_ = 0;\n>\n> @@ -204,8 +202,7 @@ bool DelayedControls::push(const ControlList &controls)\n> */\n> ControlList DelayedControls::get(uint32_t sequence)\n> {\n> -\tuint32_t adjustedSeq = sequence - firstSequence_;\n> -\tunsigned int index = std::max(0, adjustedSeq - maxDelay_);\n> +\tunsigned int index = std::max(0, sequence - maxDelay_);\n>\n> \tControlList out(device_->controls());\n> \tfor (const auto &ctrl : values_) {\n> @@ -236,11 +233,6 @@ void DelayedControls::applyControls(uint32_t sequence)\n> {\n> \tLOG(DelayedControls, Debug) << \"frame \" << sequence << \" started\";\n>\n> -\tif (!running_) {\n> -\t\tfirstSequence_ = sequence;\n> -\t\trunning_ = true;\n> -\t}\n> -\n> \t/*\n> \t * Create control list peeking ahead in the value queue to ensure\n> \t * values are set in time to satisfy the sensor delay.\n> @@ -279,7 +271,7 @@ void DelayedControls::applyControls(uint32_t sequence)\n> \t\t}\n> \t}\n>\n> -\twriteCount_ = sequence - firstSequence_ + 1;\n> +\twriteCount_ = sequence + 1;\n>\n> \twhile (writeCount_ > queueCount_) {\n> \t\tLOG(DelayedControls, Debug)\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 4AD3CBE173\n\tfor ;\n\tMon, 4 Jul 2022 14:21:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ABA556564A;\n\tMon, 4 Jul 2022 16:21:25 +0200 (CEST)","from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4C80261FB1\n\tfor ;\n\tMon, 4 Jul 2022 16:21:24 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id C0038E0007;\n\tMon, 4 Jul 2022 14:21:23 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656944485;\n\tbh=MMEG6erxaTCRk8UEXhH18c20nEnpD4v4Y9I2VRXVo48=;\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=E/fYmHGphD9Hq+rqhXekPZFIGqf9YK2lJ9CvenSJos9w5YGI60qvHq+8+iS48sr7c\n\tq2FaHeaL4vM6D77cXt3DwWivhPIpSRsW17+qX+9Q6XUC5X/XoKonMmr99kHEl2P0N6\n\tlGs6jo7x4UiGkvT5sAWWhEAilx3tUALoLxHYIK6FaBbXr0OkQQ1uUxsEo/Lq/XWq7W\n\tHPgvLSxZZiTBLTtP4+GjOJZawglcuHxloqVRgBYedYqPWzpFluZbhI0+l3zubufU/W\n\tZvvJoqlm5b/zPt5x2goWOGLbvTi5I2EDmBbCtOWux7NKCIPmOOX4/1q8cxwIr//l6Q\n\tdJZk1CjMiORgw==","Date":"Mon, 4 Jul 2022 16:21:22 +0200","To":"Kieran Bingham ","Message-ID":"<20220704142122.oqofebcyb4bcisrw@uno.localdomain>","References":"<20220704133751.1582959-1-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220704133751.1582959-1-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] delayed_controls: Remove reduandant\n\tfirstSequence_","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\" "}},{"id":23726,"web_url":"https://patchwork.libcamera.org/comment/23726/","msgid":"","date":"2022-07-04T18:23:10","subject":"Re: [libcamera-devel] [PATCH] delayed_controls: Remove reduandant\n\tfirstSequence_","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 Mon, Jul 04, 2022 at 02:37:51PM +0100, Kieran Bingham via libcamera-devel wrote:\n> The DelayedControls implementation tracked the sequence numbers to\n> determine the offset if a device did not commence with a sequence number\n> of 0.\n> \n> This guarantee is now handled by the V4L2VideoDevice.\n> \n> Remove the firstSequence_ offset and the corresponding running_ flag\n> which was used to track setting firstSequence_ from the DelayedControls.\n> \n> Signed-off-by: Kieran Bingham \n\nWith the typo in the commit message fixed,\n\nReviewed-by: Laurent Pinchart \n\n> ---\n> \n> This will conflict with Jacopo's series, but is still applicable\n> conceptually, whichever gets to go first.\n> \n> include/libcamera/internal/delayed_controls.h | 3 ---\n> src/libcamera/delayed_controls.cpp | 12 ++----------\n> 2 files changed, 2 insertions(+), 13 deletions(-)\n> \n> diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h\n> index 703fdb66bc27..f560c823d4e9 100644\n> --- a/include/libcamera/internal/delayed_controls.h\n> +++ b/include/libcamera/internal/delayed_controls.h\n> @@ -72,9 +72,6 @@ private:\n> \tstd::unordered_map controlParams_;\n> \tunsigned int maxDelay_;\n> \n> -\tbool running_;\n> -\tuint32_t firstSequence_;\n> -\n> \tuint32_t queueCount_;\n> \tuint32_t writeCount_;\n> \t/* \\todo Evaluate if we should index on ControlId * or unsigned int */\n> diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp\n> index 9667187e9a4a..527c319113ac 100644\n> --- a/src/libcamera/delayed_controls.cpp\n> +++ b/src/libcamera/delayed_controls.cpp\n> @@ -115,8 +115,6 @@ DelayedControls::DelayedControls(V4L2Device *device,\n> */\n> void DelayedControls::reset()\n> {\n> -\trunning_ = false;\n> -\tfirstSequence_ = 0;\n> \tqueueCount_ = 1;\n> \twriteCount_ = 0;\n> \n> @@ -204,8 +202,7 @@ bool DelayedControls::push(const ControlList &controls)\n> */\n> ControlList DelayedControls::get(uint32_t sequence)\n> {\n> -\tuint32_t adjustedSeq = sequence - firstSequence_;\n> -\tunsigned int index = std::max(0, adjustedSeq - maxDelay_);\n> +\tunsigned int index = std::max(0, sequence - maxDelay_);\n> \n> \tControlList out(device_->controls());\n> \tfor (const auto &ctrl : values_) {\n> @@ -236,11 +233,6 @@ void DelayedControls::applyControls(uint32_t sequence)\n> {\n> \tLOG(DelayedControls, Debug) << \"frame \" << sequence << \" started\";\n> \n> -\tif (!running_) {\n> -\t\tfirstSequence_ = sequence;\n> -\t\trunning_ = true;\n> -\t}\n> -\n> \t/*\n> \t * Create control list peeking ahead in the value queue to ensure\n> \t * values are set in time to satisfy the sensor delay.\n> @@ -279,7 +271,7 @@ void DelayedControls::applyControls(uint32_t sequence)\n> \t\t}\n> \t}\n> \n> -\twriteCount_ = sequence - firstSequence_ + 1;\n> +\twriteCount_ = sequence + 1;\n> \n> \twhile (writeCount_ > queueCount_) {\n> \t\tLOG(DelayedControls, Debug)","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 43DCBBE173\n\tfor ;\n\tMon, 4 Jul 2022 18:23:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AAB336564A;\n\tMon, 4 Jul 2022 20:23:35 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4C62B61FB1\n\tfor ;\n\tMon, 4 Jul 2022 20:23:33 +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 CA74D8D0;\n\tMon, 4 Jul 2022 20:23:32 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656959015;\n\tbh=2R5aCHFMnnnxzVhMs6F/Ku8Xvadr6wfdsHtBWu4j8Ac=;\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=nO4U3/fS7AePwxRLyMWyj4j6fKmax8KVdCBLFgw//Zi57noVXWk0TVnuWwb9R6i50\n\tr75u6bwoQiCL6Cj9BuJMsAx/XOwfM4YXfIX4IzHyxoMLncMQGZxe/7Flb7coQNDFG9\n\tvxRFbaEgLeW7HWMKSmJJSq94Ynq53jUJ7bnvrDdaFFeF8H0cpkYjHQuhCfRJ3MJHvt\n\tyaDd2e1VH4YitFuHh75yR+gIclXgO4esaN++ZZi7/053naCLQSvxer3rTqkiRXPyGQ\n\tPKGGDhA+pfzaGltPgjGLq7Ykdbv+099jftz0cX9hrjPyWibM9E6eMqyj2U9GPSqwtg\n\twgCOmXVa6PCSQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1656959013;\n\tbh=2R5aCHFMnnnxzVhMs6F/Ku8Xvadr6wfdsHtBWu4j8Ac=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YfakPeP0yBxt1DS+HpbycSfcjOFiCcwqvrw4ESEbIl9Y1NTzVF2tz4WQ1PNCwwkBs\n\tyUwMLj84wUzo1PHiffOkxUq8FIxWtp8pg2BAf9VKtMcRfDKBbINkc9TJavE3Dj/QCi\n\tZ2OIf2akz0pJttw/9NuSQf1Xn6AQwG1EtjX7Pzmg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"YfakPeP0\"; dkim-atps=neutral","Date":"Mon, 4 Jul 2022 21:23:10 +0300","To":"Kieran Bingham ","Message-ID":"","References":"<20220704133751.1582959-1-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220704133751.1582959-1-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] delayed_controls: Remove reduandant\n\tfirstSequence_","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\" "}}]