[{"id":15670,"web_url":"https://patchwork.libcamera.org/comment/15670/","msgid":"<YE1wdmwM/B8f5wqC@pendragon.ideasonboard.com>","date":"2021-03-14T02:09:58","subject":"Re: [libcamera-devel] [PATCH 7/8] libcamera: pipeline: ipu3:\n\tframes: Associate buffers with the reqeust","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\ns/reqeust/request/ in the subject line\n\nOn Fri, Mar 12, 2021 at 06:11:30AM +0000, Kieran Bingham wrote:\n> Ensure that the buffers are associated with the request even if they are\n\ns/the request/a request/ (or their request) ?\n\n> used internally to be able to correctly map back to the resources they\n> are being used to fulfil.\n\nSounds a bit weird, but I get what you mean :-)\n\n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\nI was a bit worried we could expand the usage of the request_ pointer in\nthe FrameBuffer class in the future to handle more operations\nautomatically, leading to issues if we associate internal buffers with\nrequests, but thinking about it, your proposal makes the most sense. We\neven state that this is the expected use case in the\nFrameBuffer::setRequest() documentation :-)\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nWhile at it, should we also apply the following ?\n\ndiff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\nindex 0ef3bc04659b..3cd777d1b742 100644\n--- a/src/libcamera/pipeline/ipu3/cio2.cpp\n+++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n@@ -278,10 +278,9 @@ FrameBuffer *CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer)\n\n \t\tbuffer = availableBuffers_.front();\n \t\tavailableBuffers_.pop();\n+\t\tbuffer->setRequest(request);\n \t}\n\n-\tbuffer->setRequest(request);\n-\n \tint ret = output_->queueBuffer(buffer);\n \tif (ret)\n \t\treturn nullptr;\n\n> ---\n>  src/libcamera/pipeline/ipu3/frames.cpp | 3 +++\n>  1 file changed, 3 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp\n> index 7a7c5643df43..2a0590258d03 100644\n> --- a/src/libcamera/pipeline/ipu3/frames.cpp\n> +++ b/src/libcamera/pipeline/ipu3/frames.cpp\n> @@ -56,6 +56,9 @@ IPU3Frames::Info *IPU3Frames::create(Request *request)\n>  \tFrameBuffer *paramBuffer = availableParamBuffers_.front();\n>  \tFrameBuffer *statBuffer = availableStatBuffers_.front();\n>  \n> +\tparamBuffer->setRequest(request);\n> +\tstatBuffer->setRequest(request);\n> +\n>  \tavailableParamBuffers_.pop();\n>  \tavailableStatBuffers_.pop();\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 0C8A8BD80C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 14 Mar 2021 02:10:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7FE386084E;\n\tSun, 14 Mar 2021 03:10:35 +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 5BB686084D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 14 Mar 2021 03:10:34 +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 C236155C;\n\tSun, 14 Mar 2021 03:10:33 +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=\"HSNoezUB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615687834;\n\tbh=XUrjTe67d2QAt1gb+Ipi5X1qZxL61Wzyv2rbRPb30S8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HSNoezUBGeiCvxRl/1abI9N1goSgWUEQa08GR9uC8Mmjgmhe+EkN4vdhkoJhsPQRZ\n\tz54cfIVa/eRvtcvxJsd4L6wIEPDBZtq7I0LVrkhR6h+rW5J0v7kYm2Qc+sQ/YkSiwg\n\tqOh7C3ZjkESEz6ZURhaNasZejQCwnZn7oNLr9e+U=","Date":"Sun, 14 Mar 2021 04:09:58 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<YE1wdmwM/B8f5wqC@pendragon.ideasonboard.com>","References":"<20210312061131.854849-1-kieran.bingham@ideasonboard.com>\n\t<20210312061131.854849-8-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210312061131.854849-8-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 7/8] libcamera: pipeline: ipu3:\n\tframes: Associate buffers with the reqeust","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":"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":15696,"web_url":"https://patchwork.libcamera.org/comment/15696/","msgid":"<abd9c782-3286-1ffa-b82b-c74ffc26c755@ideasonboard.com>","date":"2021-03-15T11:38:24","subject":"Re: [libcamera-devel] [PATCH 7/8] libcamera: pipeline: ipu3:\n\tframes: Associate buffers with the reqeust","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 14/03/2021 02:09, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> s/reqeust/request/ in the subject line\n> \n> On Fri, Mar 12, 2021 at 06:11:30AM +0000, Kieran Bingham wrote:\n>> Ensure that the buffers are associated with the request even if they are\n> \n> s/the request/a request/ (or their request) ?\n> \n>> used internally to be able to correctly map back to the resources they\n>> are being used to fulfil.\n> \n> Sounds a bit weird, but I get what you mean :-)\n> \n>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> I was a bit worried we could expand the usage of the request_ pointer in\n> the FrameBuffer class in the future to handle more operations\n> automatically, leading to issues if we associate internal buffers with\n> requests, but thinking about it, your proposal makes the most sense. We\n> even state that this is the expected use case in the\n> FrameBuffer::setRequest() documentation :-)\n> \n\nYes, this patch is a bit of a wild card.\n\nBut I believe (at least while debugging) we should be able to identify\nall resources, internal or not, against the request that has caused that\nresource to be handled.\n\nNiklas has suggested that sometimes there won't be a request, so that\nmight be a corner case to consider, but I think I've seen that you\nreplied that would be an invalid use case currently.\n\nThe FrameInfos class currently looks up which FrameInfo instance is\nassociated with a buffer by doing a lookup. I'd be tempted to have an\ninternal cookie for requests to allow a pipeline handler to obtain any\ninternal state associated with a request without performing a lookup.\nAnd then a buffer could map to it's associated context without needing\nto do a search in a vector or list.\n\n\n\n\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> While at it, should we also apply the following ?\n> \n> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> index 0ef3bc04659b..3cd777d1b742 100644\n> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> @@ -278,10 +278,9 @@ FrameBuffer *CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer)\n> \n>  \t\tbuffer = availableBuffers_.front();\n>  \t\tavailableBuffers_.pop();\n> +\t\tbuffer->setRequest(request);\n>  \t}\n> \n> -\tbuffer->setRequest(request);\n> -\n\nErr ... Doesn't this stop setting the Request when a raw buffer is\nprovided by the application?\n\n\n>  \tint ret = output_->queueBuffer(buffer);\n>  \tif (ret)\n>  \t\treturn nullptr;\n> \n>> ---\n>>  src/libcamera/pipeline/ipu3/frames.cpp | 3 +++\n>>  1 file changed, 3 insertions(+)\n>>\n>> diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp\n>> index 7a7c5643df43..2a0590258d03 100644\n>> --- a/src/libcamera/pipeline/ipu3/frames.cpp\n>> +++ b/src/libcamera/pipeline/ipu3/frames.cpp\n>> @@ -56,6 +56,9 @@ IPU3Frames::Info *IPU3Frames::create(Request *request)\n>>  \tFrameBuffer *paramBuffer = availableParamBuffers_.front();\n>>  \tFrameBuffer *statBuffer = availableStatBuffers_.front();\n>>  \n>> +\tparamBuffer->setRequest(request);\n>> +\tstatBuffer->setRequest(request);\n>> +\n>>  \tavailableParamBuffers_.pop();\n>>  \tavailableStatBuffers_.pop();\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 39FF9BD80C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 15 Mar 2021 11:38:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9C97A68D40;\n\tMon, 15 Mar 2021 12:38:27 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 04DE760106\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 15 Mar 2021 12:38: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 6F476556;\n\tMon, 15 Mar 2021 12:38: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=\"I87W1dSI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615808306;\n\tbh=B2v3+wCgSZ+N9Ewls2lyrLhlbMQxvuzhajH3+mc9BOU=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=I87W1dSI/TJ/GyXaDKOxciXw+wQsOknwR3SQuSqWHS7ZNm3o+f3mJVmI1FN1qidC/\n\t0Z1oEcYL5mugjd5/YOHeWyrG6Zj4R2iLQR/hyz0l8bIoN/gBIgc8mmj7KMcq2evP1w\n\tRx/RvVBIng4FjOapVUsHGuZtMXXM4q0L2ziFTWOE=","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","References":"<20210312061131.854849-1-kieran.bingham@ideasonboard.com>\n\t<20210312061131.854849-8-kieran.bingham@ideasonboard.com>\n\t<YE1wdmwM/B8f5wqC@pendragon.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":"<abd9c782-3286-1ffa-b82b-c74ffc26c755@ideasonboard.com>","Date":"Mon, 15 Mar 2021 11:38:24 +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":"<YE1wdmwM/B8f5wqC@pendragon.ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 7/8] libcamera: pipeline: ipu3:\n\tframes: Associate buffers with the reqeust","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=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":15708,"web_url":"https://patchwork.libcamera.org/comment/15708/","msgid":"<YE+O12+cphwaVTtU@pendragon.ideasonboard.com>","date":"2021-03-15T16:44:07","subject":"Re: [libcamera-devel] [PATCH 7/8] libcamera: pipeline: ipu3:\n\tframes: Associate buffers with the reqeust","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Mon, Mar 15, 2021 at 11:38:24AM +0000, Kieran Bingham wrote:\n> On 14/03/2021 02:09, Laurent Pinchart wrote:\n> > Hi Kieran,\n> > \n> > Thank you for the patch.\n> > \n> > s/reqeust/request/ in the subject line\n> > \n> > On Fri, Mar 12, 2021 at 06:11:30AM +0000, Kieran Bingham wrote:\n> >> Ensure that the buffers are associated with the request even if they are\n> > \n> > s/the request/a request/ (or their request) ?\n> > \n> >> used internally to be able to correctly map back to the resources they\n> >> are being used to fulfil.\n> > \n> > Sounds a bit weird, but I get what you mean :-)\n> > \n> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > \n> > I was a bit worried we could expand the usage of the request_ pointer in\n> > the FrameBuffer class in the future to handle more operations\n> > automatically, leading to issues if we associate internal buffers with\n> > requests, but thinking about it, your proposal makes the most sense. We\n> > even state that this is the expected use case in the\n> > FrameBuffer::setRequest() documentation :-)\n> > \n> \n> Yes, this patch is a bit of a wild card.\n> \n> But I believe (at least while debugging) we should be able to identify\n> all resources, internal or not, against the request that has caused that\n> resource to be handled.\n> \n> Niklas has suggested that sometimes there won't be a request, so that\n> might be a corner case to consider, but I think I've seen that you\n> replied that would be an invalid use case currently.\n> \n> The FrameInfos class currently looks up which FrameInfo instance is\n> associated with a buffer by doing a lookup. I'd be tempted to have an\n> internal cookie for requests to allow a pipeline handler to obtain any\n> internal state associated with a request without performing a lookup.\n> And then a buffer could map to it's associated context without needing\n> to do a search in a vector or list.\n\nIt's a good idea. If you implement this, can you make Request Extensible\nfirst, and store this in the Private class ?\n\n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > \n> > While at it, should we also apply the following ?\n> > \n> > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > index 0ef3bc04659b..3cd777d1b742 100644\n> > --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> > @@ -278,10 +278,9 @@ FrameBuffer *CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer)\n> > \n> >  \t\tbuffer = availableBuffers_.front();\n> >  \t\tavailableBuffers_.pop();\n> > +\t\tbuffer->setRequest(request);\n> >  \t}\n> > \n> > -\tbuffer->setRequest(request);\n> > -\n> \n> Err ... Doesn't this stop setting the Request when a raw buffer is\n> provided by the application?\n\nIsn't it set by Request::addBuffer() ?\n\n> >  \tint ret = output_->queueBuffer(buffer);\n> >  \tif (ret)\n> >  \t\treturn nullptr;\n> > \n> >> ---\n> >>  src/libcamera/pipeline/ipu3/frames.cpp | 3 +++\n> >>  1 file changed, 3 insertions(+)\n> >>\n> >> diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp\n> >> index 7a7c5643df43..2a0590258d03 100644\n> >> --- a/src/libcamera/pipeline/ipu3/frames.cpp\n> >> +++ b/src/libcamera/pipeline/ipu3/frames.cpp\n> >> @@ -56,6 +56,9 @@ IPU3Frames::Info *IPU3Frames::create(Request *request)\n> >>  \tFrameBuffer *paramBuffer = availableParamBuffers_.front();\n> >>  \tFrameBuffer *statBuffer = availableStatBuffers_.front();\n> >>  \n> >> +\tparamBuffer->setRequest(request);\n> >> +\tstatBuffer->setRequest(request);\n> >> +\n> >>  \tavailableParamBuffers_.pop();\n> >>  \tavailableStatBuffers_.pop();\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 DAC4EBD80C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 15 Mar 2021 16:44:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 99B8168D46;\n\tMon, 15 Mar 2021 17:44:44 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2F1B768D3E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 15 Mar 2021 17:44:43 +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 A4F55316;\n\tMon, 15 Mar 2021 17:44:42 +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=\"cFHLcciL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615826682;\n\tbh=PcHSiMlHaq+NPK38Pvi/ULTJPOvnfjPv4EGMlikybfk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cFHLcciL5MRHe3R8lXqslwlFk9Q+VSGW5/2rvQXn+e/ysQFZ07uUFi2doGU2WUcab\n\tDSMzxxF49mp6zLCrzm4wYCG6s6OoOSGsKlW2HWueo1kOpee8S59TDcNo6Q/zS/xyui\n\tZOjqTLE8jfgRxQ8ihDvDTsVyH1LCAj+yKcwixKRI=","Date":"Mon, 15 Mar 2021 18:44:07 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<YE+O12+cphwaVTtU@pendragon.ideasonboard.com>","References":"<20210312061131.854849-1-kieran.bingham@ideasonboard.com>\n\t<20210312061131.854849-8-kieran.bingham@ideasonboard.com>\n\t<YE1wdmwM/B8f5wqC@pendragon.ideasonboard.com>\n\t<abd9c782-3286-1ffa-b82b-c74ffc26c755@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<abd9c782-3286-1ffa-b82b-c74ffc26c755@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 7/8] libcamera: pipeline: ipu3:\n\tframes: Associate buffers with the reqeust","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":"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>"}}]