[{"id":15915,"web_url":"https://patchwork.libcamera.org/comment/15915/","msgid":"","date":"2021-03-26T02:05:35","subject":"Re: [libcamera-devel] [PATCH v3 03/11] libcamera: request: Provide\n\ta sequence number","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 Thu, Mar 25, 2021 at 01:42:23PM +0000, Kieran Bingham wrote:\n> Provide a sequence number on Requests which are added by the pipeline\n> handler.\n> \n> Each pipeline handler keeps a requestSequence per CameraData and\n> increments everytime a request is queued on that camera.\n> \n> The sequence number is associated with the Request and can be utilised\n> for assisting with debugging, and printing the queueing sequence of in\n> flight requests.\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> include/libcamera/internal/pipeline_handler.h | 4 +++-\n> include/libcamera/request.h | 2 ++\n> src/libcamera/pipeline_handler.cpp | 2 ++\n> src/libcamera/request.cpp | 11 +++++++++--\n> 4 files changed, 16 insertions(+), 3 deletions(-)\n> \n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index 6aca0b46f43d..9bdda8f3b799 100644\n> --- a/include/libcamera/internal/pipeline_handler.h\n> +++ b/include/libcamera/internal/pipeline_handler.h\n> @@ -38,7 +38,7 @@ class CameraData\n> {\n> public:\n> \texplicit CameraData(PipelineHandler *pipe)\n> -\t\t: pipe_(pipe)\n> +\t\t: pipe_(pipe), requestSequence_(0)\n> \t{\n> \t}\n> \tvirtual ~CameraData() = default;\n> @@ -48,6 +48,8 @@ public:\n> \tControlInfoMap controlInfo_;\n> \tControlList properties_;\n> \n> +\tuint32_t requestSequence_;\n> +\n> private:\n> \tLIBCAMERA_DISABLE_COPY(CameraData)\n> };\n> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> index 6e5aad5f6b75..cd5a24741f8a 100644\n> --- a/include/libcamera/request.h\n> +++ b/include/libcamera/request.h\n> @@ -50,6 +50,7 @@ public:\n> \tint addBuffer(const Stream *stream, FrameBuffer *buffer);\n> \tFrameBuffer *findBuffer(const Stream *stream) const;\n> \n> +\tuint32_t sequence() const { return sequence_; }\n> \tuint64_t cookie() const { return cookie_; }\n> \tStatus status() const { return status_; }\n> \n> @@ -71,6 +72,7 @@ private:\n> \tBufferMap bufferMap_;\n> \tstd::unordered_set pending_;\n> \n> +\tuint32_t sequence_;\n> \tconst uint64_t cookie_;\n> \tStatus status_;\n> \tbool cancelled_;\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index d22991d318c9..e3d4975d9ffd 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -382,6 +382,8 @@ int PipelineHandler::queueRequest(Request *request)\n> \tCameraData *data = cameraData(camera);\n> \tdata->queuedRequests_.push_back(request);\n> \n> +\trequest->sequence_ = data->requestSequence_++;\n> +\n> \tint ret = queueRequestDevice(camera, request);\n> \tif (ret)\n> \t\tdata->queuedRequests_.remove(request);\n> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> index 3ad83f3b8418..fc16b148a599 100644\n> --- a/src/libcamera/request.cpp\n> +++ b/src/libcamera/request.cpp\n> @@ -72,8 +72,8 @@ LOG_DEFINE_CATEGORY(Request)\n> *\n> */\n> Request::Request(Camera *camera, uint64_t cookie)\n> -\t: camera_(camera), cookie_(cookie), status_(RequestPending),\n> -\t cancelled_(false)\n> +\t: camera_(camera), sequence_(0), cookie_(cookie),\n> +\t status_(RequestPending), cancelled_(false)\n> {\n> \t/**\n> \t * \\todo Should the Camera expose a validator instance, to avoid\n> @@ -126,6 +126,7 @@ void Request::reuse(ReuseFlag flags)\n> \t\tbufferMap_.clear();\n> \t}\n> \n> +\tsequence_ = 0;\n> \tstatus_ = RequestPending;\n> \tcancelled_ = false;\n> \n> @@ -227,6 +228,12 @@ FrameBuffer *Request::findBuffer(const Stream *stream) const\n> * \\return The metadata associated with the request\n> */\n> \n> +/**\n> + * \\fn Request::sequence()\n> + * \\brief Retrieve the sequence number for the request\n> + * \\return The request sequence number\n\nAs the sequence number is exposed in the public API, could you elaborate\na bit more, to explain what it is and what it's used for ? Otherwise,\nthe patch looks good to me.\n\n> + */\n> +\n> /**\n> * \\fn Request::cookie()\n> * \\brief Retrieve the cookie set when the request was created","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 6FBF1C32EA\n\tfor ;\n\tFri, 26 Mar 2021 02:06:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DA2D76084F;\n\tFri, 26 Mar 2021 03:06:20 +0100 (CET)","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 7A9DC602E3\n\tfor ;\n\tFri, 26 Mar 2021 03:06:19 +0100 (CET)","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 E5D4BB6F;\n\tFri, 26 Mar 2021 03:06:18 +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=\"CjvidkhD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616724379;\n\tbh=aDiticmtGdV4VQsCVK509J90CcExyeAc4HSxPEOnmVE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=CjvidkhDRWNLh9hr8UjE9Xfq4t1yQQwsrt05I5pzsFLDpANbVI7WPtt0Gy2UTlxPe\n\t4JTTRTeeXr7FnqOb+UDrcyVrIQhvstDEVdZAB9CxY14Pr+Z3mMsNEiQSuftGMc9TOA\n\tIAQqTS8IXy3L0U32Fzzg0dGekcFETLTT0/gewj+0=","Date":"Fri, 26 Mar 2021 04:05:35 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210325134231.1400051-1-kieran.bingham@ideasonboard.com>\n\t<20210325134231.1400051-6-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210325134231.1400051-6-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 03/11] libcamera: request: Provide\n\ta sequence number","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","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15947,"web_url":"https://patchwork.libcamera.org/comment/15947/","msgid":"<02b6a1a2-b97c-4103-0ff8-634a35c76392@ideasonboard.com>","date":"2021-03-26T14:54:41","subject":"Re: [libcamera-devel] [PATCH v3 03/11] libcamera: request: Provide\n\ta sequence number","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 26/03/2021 02:05, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Thu, Mar 25, 2021 at 01:42:23PM +0000, Kieran Bingham wrote:\n>> Provide a sequence number on Requests which are added by the pipeline\n>> handler.\n>>\n>> Each pipeline handler keeps a requestSequence per CameraData and\n>> increments everytime a request is queued on that camera.\n>>\n>> The sequence number is associated with the Request and can be utilised\n>> for assisting with debugging, and printing the queueing sequence of in\n>> flight requests.\n>>\n>> Signed-off-by: Kieran Bingham \n>> ---\n>> include/libcamera/internal/pipeline_handler.h | 4 +++-\n>> include/libcamera/request.h | 2 ++\n>> src/libcamera/pipeline_handler.cpp | 2 ++\n>> src/libcamera/request.cpp | 11 +++++++++--\n>> 4 files changed, 16 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n>> index 6aca0b46f43d..9bdda8f3b799 100644\n>> --- a/include/libcamera/internal/pipeline_handler.h\n>> +++ b/include/libcamera/internal/pipeline_handler.h\n>> @@ -38,7 +38,7 @@ class CameraData\n>> {\n>> public:\n>> \texplicit CameraData(PipelineHandler *pipe)\n>> -\t\t: pipe_(pipe)\n>> +\t\t: pipe_(pipe), requestSequence_(0)\n>> \t{\n>> \t}\n>> \tvirtual ~CameraData() = default;\n>> @@ -48,6 +48,8 @@ public:\n>> \tControlInfoMap controlInfo_;\n>> \tControlList properties_;\n>> \n>> +\tuint32_t requestSequence_;\n>> +\n>> private:\n>> \tLIBCAMERA_DISABLE_COPY(CameraData)\n>> };\n>> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n>> index 6e5aad5f6b75..cd5a24741f8a 100644\n>> --- a/include/libcamera/request.h\n>> +++ b/include/libcamera/request.h\n>> @@ -50,6 +50,7 @@ public:\n>> \tint addBuffer(const Stream *stream, FrameBuffer *buffer);\n>> \tFrameBuffer *findBuffer(const Stream *stream) const;\n>> \n>> +\tuint32_t sequence() const { return sequence_; }\n>> \tuint64_t cookie() const { return cookie_; }\n>> \tStatus status() const { return status_; }\n>> \n>> @@ -71,6 +72,7 @@ private:\n>> \tBufferMap bufferMap_;\n>> \tstd::unordered_set pending_;\n>> \n>> +\tuint32_t sequence_;\n>> \tconst uint64_t cookie_;\n>> \tStatus status_;\n>> \tbool cancelled_;\n>> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n>> index d22991d318c9..e3d4975d9ffd 100644\n>> --- a/src/libcamera/pipeline_handler.cpp\n>> +++ b/src/libcamera/pipeline_handler.cpp\n>> @@ -382,6 +382,8 @@ int PipelineHandler::queueRequest(Request *request)\n>> \tCameraData *data = cameraData(camera);\n>> \tdata->queuedRequests_.push_back(request);\n>> \n>> +\trequest->sequence_ = data->requestSequence_++;\n>> +\n>> \tint ret = queueRequestDevice(camera, request);\n>> \tif (ret)\n>> \t\tdata->queuedRequests_.remove(request);\n>> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n>> index 3ad83f3b8418..fc16b148a599 100644\n>> --- a/src/libcamera/request.cpp\n>> +++ b/src/libcamera/request.cpp\n>> @@ -72,8 +72,8 @@ LOG_DEFINE_CATEGORY(Request)\n>> *\n>> */\n>> Request::Request(Camera *camera, uint64_t cookie)\n>> -\t: camera_(camera), cookie_(cookie), status_(RequestPending),\n>> -\t cancelled_(false)\n>> +\t: camera_(camera), sequence_(0), cookie_(cookie),\n>> +\t status_(RequestPending), cancelled_(false)\n>> {\n>> \t/**\n>> \t * \\todo Should the Camera expose a validator instance, to avoid\n>> @@ -126,6 +126,7 @@ void Request::reuse(ReuseFlag flags)\n>> \t\tbufferMap_.clear();\n>> \t}\n>> \n>> +\tsequence_ = 0;\n>> \tstatus_ = RequestPending;\n>> \tcancelled_ = false;\n>> \n>> @@ -227,6 +228,12 @@ FrameBuffer *Request::findBuffer(const Stream *stream) const\n>> * \\return The metadata associated with the request\n>> */\n>> \n>> +/**\n>> + * \\fn Request::sequence()\n>> + * \\brief Retrieve the sequence number for the request\n>> + * \\return The request sequence number\n> \n> As the sequence number is exposed in the public API, could you elaborate\n> a bit more, to explain what it is and what it's used for ? Otherwise,\n> the patch looks good to me.\n\nI can - but I also expect this is likely to move to the private\nstructures when Request becomes Extensible. Unless you'd prefer it to be\npart of the public API?\n\nMaybe it has some value to applications?\n\n\\brief Retrieve the sequence number for the request\n\nWhen requests are queued, they are given a sequential number to track\nthe order in which requests are queued to a camera. This number counts\nall requests given to a camera through its lifetime, and is not reset to\nzero between camera stop/start sequences.\n\nIt can be used to support debugging and identifying the flow of requests\nthrough a pipeline, but does not guarantee to represent the sequence\nnumber of any images in the stream. The sequence number is stored as an\nunsigned integer and will wrap when overflowed.\n\n\\return The request sequence number\n\n\n> \n>> + */\n>> +\n>> /**\n>> * \\fn Request::cookie()\n>> * \\brief Retrieve the cookie set when the request was created\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 9E7FDC32EB\n\tfor ;\n\tFri, 26 Mar 2021 14:54:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D91A668D6A;\n\tFri, 26 Mar 2021 15:54:48 +0100 (CET)","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 573F6602D7\n\tfor ;\n\tFri, 26 Mar 2021 15:54:47 +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 99502885;\n\tFri, 26 Mar 2021 15:54:46 +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=\"GAriQGW6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616770486;\n\tbh=9rSm0Ik0mxB4MSkd6jhVgmP3k9M+zpVqKdkv8Y6KRsY=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=GAriQGW6PogpLGjOysqF+Vp5BZ5jr+CY0LHr60pJXWBQWlRO3sXT3Yph+1kDWq2s/\n\tK6tslGE2fCxAzXVVZsmfW85rFhXxG0/F39+8V/pbAS6M15cRzYAPbp/Vp4HX8+soj1\n\tOH/QuYUNaZcshYav2yTQCNB/W2YYzj4/B3o1qs6Y=","To":"Laurent Pinchart ","References":"<20210325134231.1400051-1-kieran.bingham@ideasonboard.com>\n\t<20210325134231.1400051-6-kieran.bingham@ideasonboard.com>\n\t","From":"Kieran Bingham ","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":"<02b6a1a2-b97c-4103-0ff8-634a35c76392@ideasonboard.com>","Date":"Fri, 26 Mar 2021 14:54:41 +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":"","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v3 03/11] libcamera: request: Provide\n\ta sequence number","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15953,"web_url":"https://patchwork.libcamera.org/comment/15953/","msgid":"","date":"2021-03-26T15:50:16","subject":"Re: [libcamera-devel] [PATCH v3 03/11] libcamera: request: Provide\n\ta sequence number","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Fri, Mar 26, 2021 at 02:54:41PM +0000, Kieran Bingham wrote:\n> On 26/03/2021 02:05, Laurent Pinchart wrote:\n> > On Thu, Mar 25, 2021 at 01:42:23PM +0000, Kieran Bingham wrote:\n> >> Provide a sequence number on Requests which are added by the pipeline\n> >> handler.\n> >>\n> >> Each pipeline handler keeps a requestSequence per CameraData and\n> >> increments everytime a request is queued on that camera.\n> >>\n> >> The sequence number is associated with the Request and can be utilised\n> >> for assisting with debugging, and printing the queueing sequence of in\n> >> flight requests.\n> >>\n> >> Signed-off-by: Kieran Bingham \n> >> ---\n> >> include/libcamera/internal/pipeline_handler.h | 4 +++-\n> >> include/libcamera/request.h | 2 ++\n> >> src/libcamera/pipeline_handler.cpp | 2 ++\n> >> src/libcamera/request.cpp | 11 +++++++++--\n> >> 4 files changed, 16 insertions(+), 3 deletions(-)\n> >>\n> >> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> >> index 6aca0b46f43d..9bdda8f3b799 100644\n> >> --- a/include/libcamera/internal/pipeline_handler.h\n> >> +++ b/include/libcamera/internal/pipeline_handler.h\n> >> @@ -38,7 +38,7 @@ class CameraData\n> >> {\n> >> public:\n> >> \texplicit CameraData(PipelineHandler *pipe)\n> >> -\t\t: pipe_(pipe)\n> >> +\t\t: pipe_(pipe), requestSequence_(0)\n> >> \t{\n> >> \t}\n> >> \tvirtual ~CameraData() = default;\n> >> @@ -48,6 +48,8 @@ public:\n> >> \tControlInfoMap controlInfo_;\n> >> \tControlList properties_;\n> >> \n> >> +\tuint32_t requestSequence_;\n> >> +\n> >> private:\n> >> \tLIBCAMERA_DISABLE_COPY(CameraData)\n> >> };\n> >> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> >> index 6e5aad5f6b75..cd5a24741f8a 100644\n> >> --- a/include/libcamera/request.h\n> >> +++ b/include/libcamera/request.h\n> >> @@ -50,6 +50,7 @@ public:\n> >> \tint addBuffer(const Stream *stream, FrameBuffer *buffer);\n> >> \tFrameBuffer *findBuffer(const Stream *stream) const;\n> >> \n> >> +\tuint32_t sequence() const { return sequence_; }\n> >> \tuint64_t cookie() const { return cookie_; }\n> >> \tStatus status() const { return status_; }\n> >> \n> >> @@ -71,6 +72,7 @@ private:\n> >> \tBufferMap bufferMap_;\n> >> \tstd::unordered_set pending_;\n> >> \n> >> +\tuint32_t sequence_;\n> >> \tconst uint64_t cookie_;\n> >> \tStatus status_;\n> >> \tbool cancelled_;\n> >> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> >> index d22991d318c9..e3d4975d9ffd 100644\n> >> --- a/src/libcamera/pipeline_handler.cpp\n> >> +++ b/src/libcamera/pipeline_handler.cpp\n> >> @@ -382,6 +382,8 @@ int PipelineHandler::queueRequest(Request *request)\n> >> \tCameraData *data = cameraData(camera);\n> >> \tdata->queuedRequests_.push_back(request);\n> >> \n> >> +\trequest->sequence_ = data->requestSequence_++;\n> >> +\n> >> \tint ret = queueRequestDevice(camera, request);\n> >> \tif (ret)\n> >> \t\tdata->queuedRequests_.remove(request);\n> >> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> >> index 3ad83f3b8418..fc16b148a599 100644\n> >> --- a/src/libcamera/request.cpp\n> >> +++ b/src/libcamera/request.cpp\n> >> @@ -72,8 +72,8 @@ LOG_DEFINE_CATEGORY(Request)\n> >> *\n> >> */\n> >> Request::Request(Camera *camera, uint64_t cookie)\n> >> -\t: camera_(camera), cookie_(cookie), status_(RequestPending),\n> >> -\t cancelled_(false)\n> >> +\t: camera_(camera), sequence_(0), cookie_(cookie),\n> >> +\t status_(RequestPending), cancelled_(false)\n> >> {\n> >> \t/**\n> >> \t * \\todo Should the Camera expose a validator instance, to avoid\n> >> @@ -126,6 +126,7 @@ void Request::reuse(ReuseFlag flags)\n> >> \t\tbufferMap_.clear();\n> >> \t}\n> >> \n> >> +\tsequence_ = 0;\n> >> \tstatus_ = RequestPending;\n> >> \tcancelled_ = false;\n> >> \n> >> @@ -227,6 +228,12 @@ FrameBuffer *Request::findBuffer(const Stream *stream) const\n> >> * \\return The metadata associated with the request\n> >> */\n> >> \n> >> +/**\n> >> + * \\fn Request::sequence()\n> >> + * \\brief Retrieve the sequence number for the request\n> >> + * \\return The request sequence number\n> > \n> > As the sequence number is exposed in the public API, could you elaborate\n> > a bit more, to explain what it is and what it's used for ? Otherwise,\n> > the patch looks good to me.\n> \n> I can - but I also expect this is likely to move to the private\n> structures when Request becomes Extensible. Unless you'd prefer it to be\n> part of the public API?\n> \n> Maybe it has some value to applications?\n\nGood question. I think it may, but I'm not sure.\n\n> \\brief Retrieve the sequence number for the request\n> \n> When requests are queued, they are given a sequential number to track\n> the order in which requests are queued to a camera. This number counts\n> all requests given to a camera through its lifetime, and is not reset to\n> zero between camera stop/start sequences.\n> \n> It can be used to support debugging and identifying the flow of requests\n> through a pipeline, but does not guarantee to represent the sequence\n> number of any images in the stream. The sequence number is stored as an\n> unsigned integer and will wrap when overflowed.\n> \n> \\return The request sequence number\n\nReviewed-by: Laurent Pinchart \n\n> >> + */\n> >> +\n> >> /**\n> >> * \\fn Request::cookie()\n> >> * \\brief Retrieve the cookie set when the request was created","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 D06E1C32EB\n\tfor ;\n\tFri, 26 Mar 2021 15:51:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 35CBD68D6B;\n\tFri, 26 Mar 2021 16:51:02 +0100 (CET)","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 7BA966051A\n\tfor ;\n\tFri, 26 Mar 2021 16:51:00 +0100 (CET)","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 E9405443;\n\tFri, 26 Mar 2021 16:50:59 +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=\"kRLDUaKe\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616773860;\n\tbh=P5TCQjnvm+jgELDHNq2907h3lUEEqzC4liC3fsflWPc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kRLDUaKej4wpD7+HIj8JRBMbiACKLQEraM1paTyAQ6RccuijfyssbZDg7cNgchK8/\n\tBRQLtvXLAkCoYMc2UK4dvbghvX+iHmEOQFgHH7IEZz9vIwcSXt11S3ttPORLRUMZva\n\tKcofnkS8Bttt4/jOH1l0/8nKqRe+qb3C/j92/SRo=","Date":"Fri, 26 Mar 2021 17:50:16 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210325134231.1400051-1-kieran.bingham@ideasonboard.com>\n\t<20210325134231.1400051-6-kieran.bingham@ideasonboard.com>\n\t\n\t<02b6a1a2-b97c-4103-0ff8-634a35c76392@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<02b6a1a2-b97c-4103-0ff8-634a35c76392@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 03/11] libcamera: request: Provide\n\ta sequence number","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","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]