[{"id":16079,"web_url":"https://patchwork.libcamera.org/comment/16079/","msgid":"<20210401050014.w2n33jka5e45aui6@basti-TUXEDO-Book-XA1510>","date":"2021-04-01T05:00:14","subject":"Re: [libcamera-devel] [RFC PATCH] libcamera: Request: Add\n\tsetHalfLife","submitter":{"id":78,"url":"https://patchwork.libcamera.org/api/people/78/","name":"Sebastian Fricke","email":"sebastian.fricke@posteo.net"},"content":"Hey Paul,\n\nThank you for the patch.\n\nI have a few questions, which are mostly related to helping understand\nthe issue a bit better.\n\nOn 01.04.2021 12:46, Paul Elder wrote:\n>From the perspective of a Buffer, a Request can suddenly become nullptr\n>in the middle of a stream. As Requests are supposed to be atomic, and\n>they are the nucleus of the libcamera camera streaming API, it would be\n>convenient if we could set the rate of decay of Requests.\n\nSo, what exactly affects the decay of a request? And how can I determine\nwhen it will be unusable in advance, as you introduce a method called\n`setHalfLife`?\n\n>\n>Add a half life member variable to Request, and a setHalfLife() member\n>function to set this. This will affect whether the Request is reusable\n>or not, so Request::reuse() can now fail.\n\nIf I look at the `cam` application for example, we usually take 4\nbuffers, which means that we create 4 requests and then we simply reuse\nthose for any amount of frames. If I make it possible for reuse to fail,\nthen I would have to create a new request, but why do I have to do\nthat?\nI currently don't understand which problems can arise out of re using a\nrequest for too long. I did some tests with the cam application, where I\nhad 4 buffers -> 4 requests and capture 1000 frames at 7,5 fps. The\ncapture took 133 seconds, and I was not able to spot any problems with the\nrequests.\n\n>\n>Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n>\n>---\n>TODO: change the applications to reuse the request if reuse() fails\n>---\n> include/libcamera/request.h |  8 +++++++-\n> src/libcamera/request.cpp   | 36 ++++++++++++++++++++++++++++++++++--\n> 2 files changed, 41 insertions(+), 3 deletions(-)\n>\n>diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n>index 4cf5ff3f..45c707f3 100644\n>--- a/include/libcamera/request.h\n>+++ b/include/libcamera/request.h\n>@@ -7,6 +7,7 @@\n> #ifndef __LIBCAMERA_REQUEST_H__\n> #define __LIBCAMERA_REQUEST_H__\n>\n>+#include <chrono>\n> #include <map>\n> #include <memory>\n> #include <stdint.h>\n>@@ -43,7 +44,7 @@ public:\n> \tRequest(Camera *camera, uint64_t cookie = 0);\n> \t~Request();\n>\n>-\tvoid reuse(ReuseFlag flags = Default);\n>+\tbool reuse(ReuseFlag flags = Default);\n>\n> \tControlList &controls() { return *controls_; }\n> \tControlList &metadata() { return *metadata_; }\n>@@ -57,6 +58,8 @@ public:\n>\n> \tbool hasPendingBuffers() const { return !pending_.empty(); }\n>\n>+\tvoid setHalfLife(std::chrono::duration<double> hl);\n>+\n> \tstd::string toString() const;\n>\n> private:\n>@@ -79,6 +82,9 @@ private:\n> \tconst uint64_t cookie_;\n> \tStatus status_;\n> \tbool cancelled_;\n>+\n>+\tstd::chrono::steady_clock::time_point birthTime_;\n>+\tstd::chrono::duration<double> halfLife_;\n> };\n>\n> } /* namespace libcamera */\n>diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n>index ce2dd7b1..0543abaf 100644\n>--- a/src/libcamera/request.cpp\n>+++ b/src/libcamera/request.cpp\n>@@ -7,7 +7,9 @@\n>\n> #include <libcamera/request.h>\n>\n>+#include <chrono>\n> #include <map>\n>+#include <random>\n> #include <sstream>\n>\n> #include <libcamera/buffer.h>\n>@@ -74,7 +76,9 @@ LOG_DEFINE_CATEGORY(Request)\n>  */\n> Request::Request(Camera *camera, uint64_t cookie)\n> \t: camera_(camera), sequence_(0), cookie_(cookie),\n>-\t  status_(RequestPending), cancelled_(false)\n>+\t  status_(RequestPending), cancelled_(false),\n>+\t  birthTime_(std::chrono::steady_clock::now()),\n>+\t  halfLife_(std::chrono::seconds(10))\n> {\n> \t/**\n> \t * \\todo Should the Camera expose a validator instance, to avoid\n>@@ -111,11 +115,28 @@ Request::~Request()\n>  * prior to queueing the request to the camera, in lieu of constructing a new\n>  * request. The application can reuse the buffers that were previously added\n>  * to the request via addBuffer() by setting \\a flags to ReuseBuffers.\n>+ *\n>+ * The request for reuse may fail, as Requests can decay based on the half\n>+ * life.\n>+ *\n>+ * \\return True on success, false otherwise\n>  */\n>-void Request::reuse(ReuseFlag flags)\n>+bool Request::reuse(ReuseFlag flags)\n> {\n> \tLIBCAMERA_TRACEPOINT(request_reuse, this);\n>\n>+\tstd::chrono::steady_clock::time_point end =\n>+\t\tstd::chrono::steady_clock::now();\n>+\tstd::chrono::duration<double> diff = end - birthTime_;\n\nWhat do we do with this variable `diff`, I don't see it being used\nanywhere?\n\n>+\n>+\tstd::random_device rd{};\n>+\tstd::mt19937 gen{ rd() };\n>+\tdouble hlns =\n>+\t\tstd::chrono::duration_cast<std::chrono::nanoseconds>(halfLife_).count();\n>+\tstd::normal_distribution<> dist{ hlns, hlns / 2 };\n>+\tif (dist(gen) > hlns)\n>+\t\treturn false;\n>+\n\nSo, this code generates a random 32-bit number, so the maximum is\n2^32 = 4294967296, if I am not mistaken.\nThen we get the halflife time in nano seconds so by default that would\nbe 10s = 10000000000ns, and then we get a normal distribution of the\nrandom number with a mean of the halflife time and a standard deviation\nof halfLife time / 2. And when that result is greater than the halfLife\ntime in nano seconds, we declare that request to be not reusable.\nFor me this currently looks like we randomly declare at some point that\na request is not reusable, did I miss something?\n\nGreetings,\nSebastian\n\n> \tpending_.clear();\n> \tif (flags & ReuseBuffers) {\n> \t\tfor (auto pair : bufferMap_) {\n>@@ -133,6 +154,8 @@ void Request::reuse(ReuseFlag flags)\n>\n> \tcontrols_->clear();\n> \tmetadata_->clear();\n>+\n>+\treturn true;\n> }\n>\n> /**\n>@@ -320,6 +343,15 @@ bool Request::completeBuffer(FrameBuffer *buffer)\n> \treturn !hasPendingBuffers();\n> }\n>\n>+/**\n>+ * \\brief Set the half life of the request\n>+ * \\param[in] hl Half-life of the request\n>+ */\n>+void Request::setHalfLife(std::chrono::duration<double> hl)\n>+{\n>+\thalfLife_ = hl;\n>+}\n>+\n> /**\n>  * \\brief Generate a string representation of the Request internals\n>  *\n>-- \n>2.27.0\n>\n>_______________________________________________\n>libcamera-devel mailing list\n>libcamera-devel@lists.libcamera.org\n>https://lists.libcamera.org/listinfo/libcamera-devel","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 573AFC0DA3\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  1 Apr 2021 05:00:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C6AA260517;\n\tThu,  1 Apr 2021 07:00:19 +0200 (CEST)","from mout02.posteo.de (mout02.posteo.de [185.67.36.66])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 60747602D2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  1 Apr 2021 07:00:18 +0200 (CEST)","from submission (posteo.de [89.146.220.130]) \n\tby mout02.posteo.de (Postfix) with ESMTPS id 4449D2400FD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  1 Apr 2021 07:00:17 +0200 (CEST)","from customer (localhost [127.0.0.1])\n\tby submission (posteo.de) with ESMTPSA id 4F9rbN3sM8z6tm6;\n\tThu,  1 Apr 2021 07:00:16 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=posteo.net header.i=@posteo.net\n\theader.b=\"G+OFhrSe\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017;\n\tt=1617253217; bh=gOZLnxR/bKddVrR5ZYsGf6eR+zF0zTC+2EdPpEuxwSU=;\n\th=Date:From:To:Cc:Subject:From;\n\tb=G+OFhrSeuidlZMnT/klqegj+QiOoS3Jrq24FXWWfgt9HFhcGtm7J9oeFZjcayiFUN\n\tAOM9i8two5VqyV2g1HVlEdLyfG3oXbitsBLsdQpkh2YYsirk4c0QarVS7RFhOzTlSY\n\t7vnP3m0fvJ/PiktzVj9NszQtSc2XF9Il0Cvl9IyRdZCvL1hQ0mAaXTlFsOVy0ry/L8\n\t2Mol5Ho/BzZhdGyDTmgNVhWXdZqudd5KloPICmxdDsevuJ7gXyyPOhDKvSYEG7z2Zi\n\to+h/eEnYVjyODHDFKye/K8Y+YjLqRXwiMZRZDJ3Lvsl3lPYUZPiIqTjs3ywon3Y2n5\n\tr3B+kZwA/L06w==","Date":"Thu, 1 Apr 2021 07:00:14 +0200","From":"Sebastian Fricke <sebastian.fricke@posteo.net>","To":"Paul Elder <paul.elder@ideasonboard.com>","Message-ID":"<20210401050014.w2n33jka5e45aui6@basti-TUXEDO-Book-XA1510>","References":"<20210401034648.136223-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210401034648.136223-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH] libcamera: Request: Add\n\tsetHalfLife","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@lists.libcamera.org","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":16081,"web_url":"https://patchwork.libcamera.org/comment/16081/","msgid":"<20210401102032.GE54560@pyrite.rasen.tech>","date":"2021-04-01T10:20:32","subject":"Re: [libcamera-devel] [RFC PATCH] libcamera: Request: Add\n\tsetHalfLife","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Sebastian,\n\nOn Thu, Apr 01, 2021 at 07:00:14AM +0200, Sebastian Fricke wrote:\n> Hey Paul,\n> \n> Thank you for the patch.\n\nThank you for the review.\n\n> \n> I have a few questions, which are mostly related to helping understand\n> the issue a bit better.\n> \n> On 01.04.2021 12:46, Paul Elder wrote:\n> > From the perspective of a Buffer, a Request can suddenly become nullptr\n> > in the middle of a stream. As Requests are supposed to be atomic, and\n> > they are the nucleus of the libcamera camera streaming API, it would be\n> > convenient if we could set the rate of decay of Requests.\n> \n> So, what exactly affects the decay of a request? And how can I determine\n> when it will be unusable in advance, as you introduce a method called\n> `setHalfLife`?\n> \n> > \n> > Add a half life member variable to Request, and a setHalfLife() member\n> > function to set this. This will affect whether the Request is reusable\n> > or not, so Request::reuse() can now fail.\n> \n> If I look at the `cam` application for example, we usually take 4\n> buffers, which means that we create 4 requests and then we simply reuse\n> those for any amount of frames. If I make it possible for reuse to fail,\n> then I would have to create a new request, but why do I have to do\n> that?\n> I currently don't understand which problems can arise out of re using a\n> request for too long. I did some tests with the cam application, where I\n> had 4 buffers -> 4 requests and capture 1000 frames at 7,5 fps. The\n> capture took 133 seconds, and I was not able to spot any problems with the\n> requests.\n> \n> > \n> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> > \n> > ---\n> > TODO: change the applications to reuse the request if reuse() fails\n> > ---\n> > include/libcamera/request.h |  8 +++++++-\n> > src/libcamera/request.cpp   | 36 ++++++++++++++++++++++++++++++++++--\n> > 2 files changed, 41 insertions(+), 3 deletions(-)\n> > \n> > diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> > index 4cf5ff3f..45c707f3 100644\n> > --- a/include/libcamera/request.h\n> > +++ b/include/libcamera/request.h\n> > @@ -7,6 +7,7 @@\n> > #ifndef __LIBCAMERA_REQUEST_H__\n> > #define __LIBCAMERA_REQUEST_H__\n> > \n> > +#include <chrono>\n> > #include <map>\n> > #include <memory>\n> > #include <stdint.h>\n> > @@ -43,7 +44,7 @@ public:\n> > \tRequest(Camera *camera, uint64_t cookie = 0);\n> > \t~Request();\n> > \n> > -\tvoid reuse(ReuseFlag flags = Default);\n> > +\tbool reuse(ReuseFlag flags = Default);\n> > \n> > \tControlList &controls() { return *controls_; }\n> > \tControlList &metadata() { return *metadata_; }\n> > @@ -57,6 +58,8 @@ public:\n> > \n> > \tbool hasPendingBuffers() const { return !pending_.empty(); }\n> > \n> > +\tvoid setHalfLife(std::chrono::duration<double> hl);\n> > +\n> > \tstd::string toString() const;\n> > \n> > private:\n> > @@ -79,6 +82,9 @@ private:\n> > \tconst uint64_t cookie_;\n> > \tStatus status_;\n> > \tbool cancelled_;\n> > +\n> > +\tstd::chrono::steady_clock::time_point birthTime_;\n> > +\tstd::chrono::duration<double> halfLife_;\n> > };\n> > \n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> > index ce2dd7b1..0543abaf 100644\n> > --- a/src/libcamera/request.cpp\n> > +++ b/src/libcamera/request.cpp\n> > @@ -7,7 +7,9 @@\n> > \n> > #include <libcamera/request.h>\n> > \n> > +#include <chrono>\n> > #include <map>\n> > +#include <random>\n> > #include <sstream>\n> > \n> > #include <libcamera/buffer.h>\n> > @@ -74,7 +76,9 @@ LOG_DEFINE_CATEGORY(Request)\n> >  */\n> > Request::Request(Camera *camera, uint64_t cookie)\n> > \t: camera_(camera), sequence_(0), cookie_(cookie),\n> > -\t  status_(RequestPending), cancelled_(false)\n> > +\t  status_(RequestPending), cancelled_(false),\n> > +\t  birthTime_(std::chrono::steady_clock::now()),\n> > +\t  halfLife_(std::chrono::seconds(10))\n> > {\n> > \t/**\n> > \t * \\todo Should the Camera expose a validator instance, to avoid\n> > @@ -111,11 +115,28 @@ Request::~Request()\n> >  * prior to queueing the request to the camera, in lieu of constructing a new\n> >  * request. The application can reuse the buffers that were previously added\n> >  * to the request via addBuffer() by setting \\a flags to ReuseBuffers.\n> > + *\n> > + * The request for reuse may fail, as Requests can decay based on the half\n> > + * life.\n> > + *\n> > + * \\return True on success, false otherwise\n> >  */\n> > -void Request::reuse(ReuseFlag flags)\n> > +bool Request::reuse(ReuseFlag flags)\n> > {\n> > \tLIBCAMERA_TRACEPOINT(request_reuse, this);\n> > \n> > +\tstd::chrono::steady_clock::time_point end =\n> > +\t\tstd::chrono::steady_clock::now();\n> > +\tstd::chrono::duration<double> diff = end - birthTime_;\n> \n> What do we do with this variable `diff`, I don't see it being used\n> anywhere?\n> \n\nOh oops, I meant to use diff in the comparison with the random number\nbelow. I wonder why I didn't get a compiler error for unused variable :/\n\n> > +\n> > +\tstd::random_device rd{};\n> > +\tstd::mt19937 gen{ rd() };\n> > +\tdouble hlns =\n> > +\t\tstd::chrono::duration_cast<std::chrono::nanoseconds>(halfLife_).count();\n> > +\tstd::normal_distribution<> dist{ hlns, hlns / 2 };\n> > +\tif (dist(gen) > hlns)\n> > +\t\treturn false;\n> > +\n> \n> So, this code generates a random 32-bit number, so the maximum is\n> 2^32 = 4294967296, if I am not mistaken.\n> Then we get the halflife time in nano seconds so by default that would\n> be 10s = 10000000000ns, and then we get a normal distribution of the\n> random number with a mean of the halflife time and a standard deviation\n> of halfLife time / 2. And when that result is greater than the halfLife\n> time in nano seconds, we declare that request to be not reusable.\n> For me this currently looks like we randomly declare at some point that\n> a request is not reusable, did I miss something?\n\nI think you're missing... the date :)\n\n\nThanks,\n\nPaul\n\n> \n> > \tpending_.clear();\n> > \tif (flags & ReuseBuffers) {\n> > \t\tfor (auto pair : bufferMap_) {\n> > @@ -133,6 +154,8 @@ void Request::reuse(ReuseFlag flags)\n> > \n> > \tcontrols_->clear();\n> > \tmetadata_->clear();\n> > +\n> > +\treturn true;\n> > }\n> > \n> > /**\n> > @@ -320,6 +343,15 @@ bool Request::completeBuffer(FrameBuffer *buffer)\n> > \treturn !hasPendingBuffers();\n> > }\n> > \n> > +/**\n> > + * \\brief Set the half life of the request\n> > + * \\param[in] hl Half-life of the request\n> > + */\n> > +void Request::setHalfLife(std::chrono::duration<double> hl)\n> > +{\n> > +\thalfLife_ = hl;\n> > +}\n> > +\n> > /**\n> >  * \\brief Generate a string representation of the Request internals\n> >  *\n> > -- \n> > 2.27.0\n> > \n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel","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 D9ACDC0DA3\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  1 Apr 2021 10:20:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5288668784;\n\tThu,  1 Apr 2021 12:20:41 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 34A066877D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  1 Apr 2021 12:20:40 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A61EFF7;\n\tThu,  1 Apr 2021 12:20:38 +0200 (CEST)"],"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=\"vTm84lgO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1617272439;\n\tbh=cPRuXGfpjX1fetAKY26n6MwazIVdv+Km+hO5lYQUNZo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=vTm84lgOKgvSwkWsDEe5mII0u5FUf7ilgW6VHzUdOMb2h/5BNI37eMi/t98+QKIRD\n\t+LNK4u+/14C98GSPDPy1coq2XnpucLvWvyZsHlrD9FHV2/Y8D2owKSnfSxA+qZLcnM\n\tErB3iI3OTHI0tpgJp9cwUUBrUEY2X0buNregt+no=","Date":"Thu, 1 Apr 2021 19:20:32 +0900","From":"paul.elder@ideasonboard.com","To":"Sebastian Fricke <sebastian.fricke@posteo.net>","Message-ID":"<20210401102032.GE54560@pyrite.rasen.tech>","References":"<20210401034648.136223-1-paul.elder@ideasonboard.com>\n\t<20210401050014.w2n33jka5e45aui6@basti-TUXEDO-Book-XA1510>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210401050014.w2n33jka5e45aui6@basti-TUXEDO-Book-XA1510>","Subject":"Re: [libcamera-devel] [RFC PATCH] libcamera: Request: Add\n\tsetHalfLife","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@lists.libcamera.org","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":16082,"web_url":"https://patchwork.libcamera.org/comment/16082/","msgid":"<c9edd5e0-42b5-e642-d79d-15b58135b8fb@ideasonboard.com>","date":"2021-04-01T10:43:36","subject":"Re: [libcamera-devel] [RFC PATCH] libcamera: Request: Add\n\tsetHalfLife","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Paul,\n\nOn 01/04/2021 04:46, Paul Elder wrote:\n> From the perspective of a Buffer, a Request can suddenly become nullptr\n> in the middle of a stream. As Requests are supposed to be atomic, and\n> they are the nucleus of the libcamera camera streaming API, it would be\n> convenient if we could set the rate of decay of Requests.\n> \n> Add a half life member variable to Request, and a setHalfLife() member\n> function to set this. This will affect whether the Request is reusable\n> or not, so Request::reuse() can now fail.\n> \n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> \n> ---\n> TODO: change the applications to reuse the request if reuse() fails\n> ---\n>  include/libcamera/request.h |  8 +++++++-\n>  src/libcamera/request.cpp   | 36 ++++++++++++++++++++++++++++++++++--\n>  2 files changed, 41 insertions(+), 3 deletions(-)\n> \n> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> index 4cf5ff3f..45c707f3 100644\n> --- a/include/libcamera/request.h\n> +++ b/include/libcamera/request.h\n> @@ -7,6 +7,7 @@\n>  #ifndef __LIBCAMERA_REQUEST_H__\n>  #define __LIBCAMERA_REQUEST_H__\n>  \n> +#include <chrono>\n>  #include <map>\n>  #include <memory>\n>  #include <stdint.h>\n> @@ -43,7 +44,7 @@ public:\n>  \tRequest(Camera *camera, uint64_t cookie = 0);\n>  \t~Request();\n>  \n> -\tvoid reuse(ReuseFlag flags = Default);\n> +\tbool reuse(ReuseFlag flags = Default);\n>  \n>  \tControlList &controls() { return *controls_; }\n>  \tControlList &metadata() { return *metadata_; }\n> @@ -57,6 +58,8 @@ public:\n>  \n>  \tbool hasPendingBuffers() const { return !pending_.empty(); }\n>  \n> +\tvoid setHalfLife(std::chrono::duration<double> hl);\n> +\n>  \tstd::string toString() const;\n>  \n>  private:\n> @@ -79,6 +82,9 @@ private:\n>  \tconst uint64_t cookie_;\n>  \tStatus status_;\n>  \tbool cancelled_;\n> +\n> +\tstd::chrono::steady_clock::time_point birthTime_;\n> +\tstd::chrono::duration<double> halfLife_;\n>  };\n>  \n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> index ce2dd7b1..0543abaf 100644\n> --- a/src/libcamera/request.cpp\n> +++ b/src/libcamera/request.cpp\n> @@ -7,7 +7,9 @@\n>  \n>  #include <libcamera/request.h>\n>  \n> +#include <chrono>\n>  #include <map>\n> +#include <random>\n>  #include <sstream>\n>  \n>  #include <libcamera/buffer.h>\n> @@ -74,7 +76,9 @@ LOG_DEFINE_CATEGORY(Request)\n>   */\n>  Request::Request(Camera *camera, uint64_t cookie)\n>  \t: camera_(camera), sequence_(0), cookie_(cookie),\n> -\t  status_(RequestPending), cancelled_(false)\n> +\t  status_(RequestPending), cancelled_(false),\n> +\t  birthTime_(std::chrono::steady_clock::now()),\n> +\t  halfLife_(std::chrono::seconds(10))\n\nI think we should reduce this to just one second in four. or four in\none, it depends on your location I think.\n\nOther than that, looks like a great debug addition, lets do it.\n\nCompletely-Entirely-Reviewed-by: Kieran Bingham\n<kieran.bingham@ideasonboard.com>\n\n\n>  {\n>  \t/**\n>  \t * \\todo Should the Camera expose a validator instance, to avoid\n> @@ -111,11 +115,28 @@ Request::~Request()\n>   * prior to queueing the request to the camera, in lieu of constructing a new\n>   * request. The application can reuse the buffers that were previously added\n>   * to the request via addBuffer() by setting \\a flags to ReuseBuffers.\n> + *\n> + * The request for reuse may fail, as Requests can decay based on the half\n> + * life.\n> + *\n> + * \\return True on success, false otherwise\n>   */\n> -void Request::reuse(ReuseFlag flags)\n> +bool Request::reuse(ReuseFlag flags)\n>  {\n>  \tLIBCAMERA_TRACEPOINT(request_reuse, this);\n>  \n> +\tstd::chrono::steady_clock::time_point end =\n> +\t\tstd::chrono::steady_clock::now();\n> +\tstd::chrono::duration<double> diff = end - birthTime_;\n> +\n> +\tstd::random_device rd{};\n> +\tstd::mt19937 gen{ rd() };\n> +\tdouble hlns =\n> +\t\tstd::chrono::duration_cast<std::chrono::nanoseconds>(halfLife_).count();\n> +\tstd::normal_distribution<> dist{ hlns, hlns / 2 };\n> +\tif (dist(gen) > hlns)\n> +\t\treturn false;\n> +\n>  \tpending_.clear();\n>  \tif (flags & ReuseBuffers) {\n>  \t\tfor (auto pair : bufferMap_) {\n> @@ -133,6 +154,8 @@ void Request::reuse(ReuseFlag flags)\n>  \n>  \tcontrols_->clear();\n>  \tmetadata_->clear();\n> +\n> +\treturn true;\n>  }\n>  \n>  /**\n> @@ -320,6 +343,15 @@ bool Request::completeBuffer(FrameBuffer *buffer)\n>  \treturn !hasPendingBuffers();\n>  }\n>  \n> +/**\n> + * \\brief Set the half life of the request\n> + * \\param[in] hl Half-life of the request\n> + */\n> +void Request::setHalfLife(std::chrono::duration<double> hl)\n> +{\n> +\thalfLife_ = hl;\n> +}\n> +\n>  /**\n>   * \\brief Generate a string representation of the Request internals\n>   *\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 01E68C0DA4\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  1 Apr 2021 10:43:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7030F68781;\n\tThu,  1 Apr 2021 12:43:40 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A292C68780\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  1 Apr 2021 12:43:38 +0200 (CEST)","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 267C2F7;\n\tThu,  1 Apr 2021 12:43:38 +0200 (CEST)"],"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=\"wdTRHBmz\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1617273818;\n\tbh=sBRQAQsvcwRQdol6GUnVBSWMSwDxi1T52T8Np7yMRWo=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=wdTRHBmzH0/Jyu9HEOIXux8KV1yipt4RQ1bAKrOBRbwxevNn7Cv7DW3diZX5YmOpP\n\tiG7VXqKzt0ZQs+gnct83gtaAzKjVy3VmLTXskxwGA+0TAOy2UbdHPfRG0UZw+DoWzd\n\ttyzTWAUjw/WGhFoQiLnzUbulLU4A2yUoiVFvflSk=","To":"Paul Elder <paul.elder@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210401034648.136223-1-paul.elder@ideasonboard.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":"<c9edd5e0-42b5-e642-d79d-15b58135b8fb@ideasonboard.com>","Date":"Thu, 1 Apr 2021 11:43:36 +0100","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":"<20210401034648.136223-1-paul.elder@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [RFC PATCH] libcamera: Request: Add\n\tsetHalfLife","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>"}}]