[{"id":15439,"web_url":"https://patchwork.libcamera.org/comment/15439/","msgid":"<07ce8505-b1a0-ba5d-70ee-177cb32a0972@ideasonboard.com>","date":"2021-03-03T11:00:23","subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: delayed_controls:\n\tFix off-by-one error in get()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Naush,\n\nOn 01/03/2021 13:31, Naushir Patuck wrote:\n> There was an off-by-one error in DelayedControls::get() when picking\n> controls from the queue to return back to the pipeline handler.\n> This is only noticeable as small oscillations in brightness when closely\n> viewing frame while AGC is running. The old StaggeredCtrl did not show\n> this error as the startup queuing mechanism has changed in\n> DelayedControls.\n> \n> Fix this by indexing to the correct position in the queue.\n\nIt seems this behaviour is currently expected by the test framework.\n\nRunning the tests (whole suite with `ninja test`) identifies a failure on:\n\n\n> 44/61 libcamera / delayed_contols                                        FAIL            0.12s   (exit status 255 or signal 127 SIGinvalid)\n\n\nRunning this test alone on each patch in the series:\n\n> git rebase -i libcamera.org/master -x \"ninja -C build && ./build/test/delayed_contols\"\n\nstops at this patch.\n\nCould you also investigate the test, and update it accordingly in this\npatch to make sure that it is correct and doesn't fail please?\n\nI anticipate that this is just a case of needing to update the tests to\nmatch, if we assume it is the test that is wrong in this instance, but\nwe should check that too, and make sure we're not breaking some other\nassumptions elsewhere.\n\n--\nKieran\n\n\n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> Reported-by: David Plowman <david.plowman@raspberrypi.com>\n> Fixes: 3d4b7b005911 (\"libcamera: delayed_controls: Add helper for controls that apply with a delay\")\n> Tested-by: David Plowman <david.plowman@raspberrypi.com>\n> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n> ---\n>  src/libcamera/delayed_controls.cpp | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp\n> index 5a05741c0285..138761c9852e 100644\n> --- a/src/libcamera/delayed_controls.cpp\n> +++ b/src/libcamera/delayed_controls.cpp\n> @@ -184,7 +184,7 @@ bool DelayedControls::push(const ControlList &controls)\n>   */\n>  ControlList DelayedControls::get(uint32_t sequence)\n>  {\n> -\tuint32_t adjustedSeq = sequence - firstSequence_ + 1;\n> +\tuint32_t adjustedSeq = sequence - firstSequence_;\n>  \tunsigned int index = std::max<int>(0, adjustedSeq - maxDelay_);\n>  \n>  \tControlList out(device_->controls());\n>","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 B3E45BD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  3 Mar 2021 11:00:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6F87568A98;\n\tWed,  3 Mar 2021 12:00:28 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3B9CC68A91\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  3 Mar 2021 12:00:27 +0100 (CET)","from [192.168.0.20]\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 D331AE9;\n\tWed,  3 Mar 2021 12:00:26 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"lDmstNrv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1614769227;\n\tbh=lPr3Od+VMky7JNWf64EeWxaC1Zm/AEa+NYOpTATZOQw=;\n\th=From:Subject:Reply-To:To:References:Date:In-Reply-To:From;\n\tb=lDmstNrvODtow5owUQqS4qDtF1YPMvb2k9JD/R6159LESY3SCdlk+HKpCFupwR76L\n\tbx0R0cPX6zUwo4pFkpXZEsAnPn6BRMeJfPcDNETw+HOc/6bwwBaWIf4dZ5TEKdigQ1\n\t6T1tOmbe5X0LCRIyyUITXJNKfQh49YNN8cmokg3U=","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Naushir Patuck <naush@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210301133159.4179129-1-naush@raspberrypi.com>\n\t<20210301133159.4179129-6-naush@raspberrypi.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<07ce8505-b1a0-ba5d-70ee-177cb32a0972@ideasonboard.com>","Date":"Wed, 3 Mar 2021 11:00:23 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20210301133159.4179129-6-naush@raspberrypi.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: delayed_controls:\n\tFix off-by-one error in get()","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>","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":15440,"web_url":"https://patchwork.libcamera.org/comment/15440/","msgid":"<CAEmqJPoi+hvDeOe6dGr2dYP39wTG6A8p1eqadgvmue2Ecz=4_g@mail.gmail.com>","date":"2021-03-03T11:24:24","subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: delayed_controls:\n\tFix off-by-one error in get()","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Kieran,\n\nOn Wed, 3 Mar 2021 at 11:00, Kieran Bingham <kieran.bingham@ideasonboard.com>\nwrote:\n\n> Hi Naush,\n>\n> On 01/03/2021 13:31, Naushir Patuck wrote:\n> > There was an off-by-one error in DelayedControls::get() when picking\n> > controls from the queue to return back to the pipeline handler.\n> > This is only noticeable as small oscillations in brightness when closely\n> > viewing frame while AGC is running. The old StaggeredCtrl did not show\n> > this error as the startup queuing mechanism has changed in\n> > DelayedControls.\n> >\n> > Fix this by indexing to the correct position in the queue.\n>\n> It seems this behaviour is currently expected by the test framework.\n>\n> Running the tests (whole suite with `ninja test`) identifies a failure on:\n>\n>\n> > 44/61 libcamera / delayed_contols\n> FAIL            0.12s   (exit status 255 or signal 127 SIGinvalid)\n>\n>\n> Running this test alone on each patch in the series:\n>\n> > git rebase -i libcamera.org/master -x \"ninja -C build &&\n> ./build/test/delayed_contols\"\n>\n> stops at this patch.\n>\n> Could you also investigate the test, and update it accordingly in this\n> patch to make sure that it is correct and doesn't fail please?\n>\n\nSorry, my bad.  I had run the tests, and did not see a failure.  However,\non closer\ninspection, the test was marked as SKIPPED, as I do not have the \"vivid\nvideo\"\ndevice driver available.\n\nLet me see what's involved in getting this device available to use on our\nkernel, and I will update the tests to reflect the new state of the world.\n\nRegards,\nNaush\n\n\n>\n> I anticipate that this is just a case of needing to update the tests to\n> match, if we assume it is the test that is wrong in this instance, but\n> we should check that too, and make sure we're not breaking some other\n> assumptions elsewhere.\n>\n> --\n> Kieran\n>\n>\n> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > Reported-by: David Plowman <david.plowman@raspberrypi.com>\n> > Fixes: 3d4b7b005911 (\"libcamera: delayed_controls: Add helper for\n> controls that apply with a delay\")\n> > Tested-by: David Plowman <david.plowman@raspberrypi.com>\n> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n> > ---\n> >  src/libcamera/delayed_controls.cpp | 2 +-\n> >  1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> > diff --git a/src/libcamera/delayed_controls.cpp\n> b/src/libcamera/delayed_controls.cpp\n> > index 5a05741c0285..138761c9852e 100644\n> > --- a/src/libcamera/delayed_controls.cpp\n> > +++ b/src/libcamera/delayed_controls.cpp\n> > @@ -184,7 +184,7 @@ bool DelayedControls::push(const ControlList\n> &controls)\n> >   */\n> >  ControlList DelayedControls::get(uint32_t sequence)\n> >  {\n> > -     uint32_t adjustedSeq = sequence - firstSequence_ + 1;\n> > +     uint32_t adjustedSeq = sequence - firstSequence_;\n> >       unsigned int index = std::max<int>(0, adjustedSeq - maxDelay_);\n> >\n> >       ControlList out(device_->controls());\n> >\n>\n> --\n> Regards\n> --\n> Kieran\n>","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 13DC6BD80C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  3 Mar 2021 11:24:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 94DA568A91;\n\tWed,  3 Mar 2021 12:24:42 +0100 (CET)","from mail-lj1-x235.google.com (mail-lj1-x235.google.com\n\t[IPv6:2a00:1450:4864:20::235])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9AC4368A7E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  3 Mar 2021 12:24:40 +0100 (CET)","by mail-lj1-x235.google.com with SMTP id y12so15236190ljj.12\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 03 Mar 2021 03:24:40 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"PLfbpPmy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=GoXdnk4NHDNz7JDt1PBh2vPZou15wt6/FlCgeC3gtZw=;\n\tb=PLfbpPmyd3XQQbSdmHqJmkriRVq1gwxBIfrVSy5K/VY6zVohaXeOCzRcTU8tiBGnyj\n\t/O7yvc0dI/aifWZeL4RwRlmbQZKNLv/mRCMAzUX+pZl0XeWPxqRay8vJCVYCLTPzYvT2\n\t/ybynglMNaeiFls9b47pCYXOq/lY+vc1unsGI/JuZlUyVI1LHw2Q8tLL9k6SQred5Nup\n\t2txqqF3SxOMMelglibsIaXyCU8P+BkKHIZ3efj4/4qa2abjZBc9D8/e6BgNeiWZeCZgf\n\tG09xN4tzJcMPNzsE2dJdAQLzKAR3N1Y1t7/l57lZb1CVdJPm+cUcDRgaZf6gEUb2vnww\n\tJzBw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=GoXdnk4NHDNz7JDt1PBh2vPZou15wt6/FlCgeC3gtZw=;\n\tb=eieCGFatLO3yijo8YLf3mCN/YvLSO7lrEc4JJW52dNzDQEtWjdsJ5Uauzsr7GBY9jS\n\tlB7qDkgpS40QdaH7sVSMoK0lWDwHpiBukPe5u6cALw1i4DzIubAdY+jbcGqZBffs36Dw\n\tFY/Bd4Usvld2LttLRLZYhfT1N2O4CMknfgqWBsOhn0fJmF31iufQPHbartJsbhgdkFvt\n\tbDw+dNg/iXWw3qZUOy0ujR4KP9yTV+iN0+X8pg2iDcIXzjF26MKr7nZYhzOFWqXlEwhb\n\tRPo9OM8WaSPz/VoEK0npU1Tq5ARLk0k0FBiZeS4yAs7J969D3OAUjBXrpBq4orzXmgNq\n\tNOFw==","X-Gm-Message-State":"AOAM531zWubcvAtXBk/uBcN1sc6wb+bfPYxZQeZULH1Is/8lUjyFBrjU\n\tBJMdZ/tAZ5b+Q7df4Zj76oeDjEAMjLP8owASF/9U9Q==","X-Google-Smtp-Source":"ABdhPJy5nKJ3LtNUQWMVFchEK068/fVnhjfc/3mNFlJQHYE0FuWWY4+Vy5bKKKnvAVMy4h+DKLC0qpSdaacieOaoe3I=","X-Received":"by 2002:a2e:a58f:: with SMTP id\n\tm15mr14192722ljp.400.1614770679986; \n\tWed, 03 Mar 2021 03:24:39 -0800 (PST)","MIME-Version":"1.0","References":"<20210301133159.4179129-1-naush@raspberrypi.com>\n\t<20210301133159.4179129-6-naush@raspberrypi.com>\n\t<07ce8505-b1a0-ba5d-70ee-177cb32a0972@ideasonboard.com>","In-Reply-To":"<07ce8505-b1a0-ba5d-70ee-177cb32a0972@ideasonboard.com>","From":"Naushir Patuck <naush@raspberrypi.com>","Date":"Wed, 3 Mar 2021 11:24:24 +0000","Message-ID":"<CAEmqJPoi+hvDeOe6dGr2dYP39wTG6A8p1eqadgvmue2Ecz=4_g@mail.gmail.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: delayed_controls:\n\tFix off-by-one error in get()","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>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"multipart/mixed;\n\tboundary=\"===============7562396122972227712==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":15441,"web_url":"https://patchwork.libcamera.org/comment/15441/","msgid":"<4caeb41e-94b4-863f-4403-c4fb48b0a060@ideasonboard.com>","date":"2021-03-03T11:32:01","subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: delayed_controls:\n\tFix off-by-one error in get()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Naush,\n\nOn 03/03/2021 11:24, Naushir Patuck wrote:\n> Hi Kieran,\n> \n> On Wed, 3 Mar 2021 at 11:00, Kieran Bingham\n> <kieran.bingham@ideasonboard.com\n> <mailto:kieran.bingham@ideasonboard.com>> wrote:\n> \n>     Hi Naush,\n> \n>     On 01/03/2021 13:31, Naushir Patuck wrote:\n>     > There was an off-by-one error in DelayedControls::get() when picking\n>     > controls from the queue to return back to the pipeline handler.\n>     > This is only noticeable as small oscillations in brightness when\n>     closely\n>     > viewing frame while AGC is running. The old StaggeredCtrl did not show\n>     > this error as the startup queuing mechanism has changed in\n>     > DelayedControls.\n>     >\n>     > Fix this by indexing to the correct position in the queue.\n> \n>     It seems this behaviour is currently expected by the test framework.\n> \n>     Running the tests (whole suite with `ninja test`) identifies a\n>     failure on:\n> \n> \n>     > 44/61 libcamera / delayed_contols                                 \n>           FAIL            0.12s   (exit status 255 or signal 127 SIGinvalid)\n> \n> \n>     Running this test alone on each patch in the series:\n> \n>     > git rebase -i libcamera.org/master <http://libcamera.org/master>\n>     -x \"ninja -C build && ./build/test/delayed_contols\"\n> \n>     stops at this patch.\n> \n>     Could you also investigate the test, and update it accordingly in this\n>     patch to make sure that it is correct and doesn't fail please?\n> \n> \n> Sorry, my bad.  I had run the tests, and did not see a failure. \n> However, on closer\n> inspection, the test was marked as SKIPPED, as I do not have the \"vivid\n> video\"\n> device driver available.\n> \n> Let me see what's involved in getting this device available to use on our\n> kernel, and I will update the tests to reflect the new state of the world.\n\nAha yes, we need vimc, vivid and vim2m to be able to run the tests\nwithout hardware.\n\nYou can run that on the RPi, or on your host laptop with those modules\nloaded.\n\nNiklas' \"lc-compliance\" tool which I hope we'll see start to integrate\nsoon will do deeper per-pipeline tests on a running system.\n\nFor now the unit-tests are supposed to run 'without' hardware.\n\n--\nKieran\n\n> Regards,\n> Naush\n>  \n> \n> \n>     I anticipate that this is just a case of needing to update the tests to\n>     match, if we assume it is the test that is wrong in this instance, but\n>     we should check that too, and make sure we're not breaking some other\n>     assumptions elsewhere.\n> \n>     --\n>     Kieran\n> \n> \n>     > Signed-off-by: Naushir Patuck <naush@raspberrypi.com\n>     <mailto:naush@raspberrypi.com>>\n>     > Reported-by: David Plowman <david.plowman@raspberrypi.com\n>     <mailto:david.plowman@raspberrypi.com>>\n>     > Fixes: 3d4b7b005911 (\"libcamera: delayed_controls: Add helper for\n>     controls that apply with a delay\")\n>     > Tested-by: David Plowman <david.plowman@raspberrypi.com\n>     <mailto:david.plowman@raspberrypi.com>>\n>     > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com\n>     <mailto:paul.elder@ideasonboard.com>>\n>     > ---\n>     >  src/libcamera/delayed_controls.cpp | 2 +-\n>     >  1 file changed, 1 insertion(+), 1 deletion(-)\n>     >\n>     > diff --git a/src/libcamera/delayed_controls.cpp\n>     b/src/libcamera/delayed_controls.cpp\n>     > index 5a05741c0285..138761c9852e 100644\n>     > --- a/src/libcamera/delayed_controls.cpp\n>     > +++ b/src/libcamera/delayed_controls.cpp\n>     > @@ -184,7 +184,7 @@ bool DelayedControls::push(const ControlList\n>     &controls)\n>     >   */\n>     >  ControlList DelayedControls::get(uint32_t sequence)\n>     >  {\n>     > -     uint32_t adjustedSeq = sequence - firstSequence_ + 1;\n>     > +     uint32_t adjustedSeq = sequence - firstSequence_;\n>     >       unsigned int index = std::max<int>(0, adjustedSeq - maxDelay_);\n>     > \n>     >       ControlList out(device_->controls());","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 EC45CBD1F1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  3 Mar 2021 11:32:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7164168A98;\n\tWed,  3 Mar 2021 12:32:05 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 39DE768A7E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  3 Mar 2021 12:32:04 +0100 (CET)","from [192.168.0.20]\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 A2A428CA;\n\tWed,  3 Mar 2021 12:32:03 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"H/fC+gvq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1614771123;\n\tbh=EUxhaCAPU/J4l6M3QmRXqyEcPMNOWEK3oL4atGgtC2Q=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=H/fC+gvqx1TWoQSZYvfjRh2ceUczLTp6CECWqsrdbFfdmQ9h/TCc7qRu/VHwGs/8l\n\tYQ1FhNVNtmENsovx2mh/amBUuUZBk35k1N2qDx78xUDRcc8yCf1HQjeLTniBXppcPi\n\tTXzBUHKbzbAF8MWzzRmdajm5Eu4HaXGbLye6DaKY=","To":"Naushir Patuck <naush@raspberrypi.com>","References":"<20210301133159.4179129-1-naush@raspberrypi.com>\n\t<20210301133159.4179129-6-naush@raspberrypi.com>\n\t<07ce8505-b1a0-ba5d-70ee-177cb32a0972@ideasonboard.com>\n\t<CAEmqJPoi+hvDeOe6dGr2dYP39wTG6A8p1eqadgvmue2Ecz=4_g@mail.gmail.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<4caeb41e-94b4-863f-4403-c4fb48b0a060@ideasonboard.com>","Date":"Wed, 3 Mar 2021 11:32:01 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<CAEmqJPoi+hvDeOe6dGr2dYP39wTG6A8p1eqadgvmue2Ecz=4_g@mail.gmail.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v3 5/5] libcamera: delayed_controls:\n\tFix off-by-one error in get()","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>","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]