[{"id":29254,"web_url":"https://patchwork.libcamera.org/comment/29254/","msgid":"<171343493022.342316.4906885895509411521@ping.linuxembedded.co.uk>","date":"2024-04-18T10:08:50","subject":"Re: [PATCH 1/2] pipeline: rkisp1: Use cached Request pointer","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Umang Jain (2024-04-18 08:49:44)\n> The Request pointer is cached in RkISP1FrameInfo. Use it directly\n> instead of retrieving it via buffer->request().\n\nI don't think there's any specific speed up here. I think the compiler\nwill map this into a single data read in both cases ?\n\nAs this is in the bufferReady() context, I think useing the request\nassociat3ed with the buffer is 'currently' correct.\n\nThat said, I think this is an area that's likely about to get reworked\nin two fronts. Dan has looked at how to clean up the FrameInfo handling,\nand Stefan has been looking at how to be able to split/spearate buffers\nand requests.\n\nSo I suspect that we should already merge 2/2 from this series but\nprobably leave this one out for now...\n\n\n\n\n> \n> Signed-off-by: Umang Jain \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index abb21968..5a6c9e1e 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -1261,7 +1261,7 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n> return;\n> \n> const FrameMetadata &metadata = buffer->metadata();\n> - Request *request = buffer->request();\n> + Request *request = info->request;\n> \n> if (metadata.status != FrameMetadata::FrameCancelled) {\n> /*\n> -- \n> 2.44.0\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 AC6E4C3213\n\tfor ;\n\tThu, 18 Apr 2024 10:08:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6AD14633F4;\n\tThu, 18 Apr 2024 12:08:54 +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 560D461B4F\n\tfor ;\n\tThu, 18 Apr 2024 12:08:53 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B1087827;\n\tThu, 18 Apr 2024 12:08:05 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"mrIXbTes\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713434885;\n\tbh=OYim2VH5bgZTiYfz18thlDTzMwmWHn+aTUYDQAiAMDE=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=mrIXbTesGqX3PaJUbLjZSa9KTsgssvi0ssh6Np4OSYKASMBsAAeow+AmkaxRNIo+E\n\tyflzB0G1YRgkfkOmnVnZR8q7eiu07TbhsjFAjArEFp8GfOz4alH7vXPktzV/DFd0rv\n\tK9kKLbOV+4u6xbZ8qSqFUP3Z8eLRVIrVsxpEIFhk=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240418074945.47349-2-umang.jain@ideasonboard.com>","References":"<20240418074945.47349-1-umang.jain@ideasonboard.com>\n\t<20240418074945.47349-2-umang.jain@ideasonboard.com>","Subject":"Re: [PATCH 1/2] pipeline: rkisp1: Use cached Request pointer","From":"Kieran Bingham ","Cc":"Umang Jain ","To":"Umang Jain ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 18 Apr 2024 11:08:50 +0100","Message-ID":"<171343493022.342316.4906885895509411521@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29256,"web_url":"https://patchwork.libcamera.org/comment/29256/","msgid":"<20240418101152.GT12561@pendragon.ideasonboard.com>","date":"2024-04-18T10:11:52","subject":"Re: [PATCH 1/2] pipeline: rkisp1: Use cached Request pointer","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Thu, Apr 18, 2024 at 01:19:44PM +0530, Umang Jain wrote:\n> The Request pointer is cached in RkISP1FrameInfo. Use it directly\n> instead of retrieving it via buffer->request().\n\nhttps://cbea.ms/git-commit/#why-not-how\n\n> Signed-off-by: Umang Jain \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index abb21968..5a6c9e1e 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -1261,7 +1261,7 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n> \t\treturn;\n> \n> \tconst FrameMetadata &metadata = buffer->metadata();\n> -\tRequest *request = buffer->request();\n> +\tRequest *request = info->request;\n> \n> \tif (metadata.status != FrameMetadata::FrameCancelled) {\n> \t\t/*","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 7C4F6C3213\n\tfor ;\n\tThu, 18 Apr 2024 10:12:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8853F633F8;\n\tThu, 18 Apr 2024 12:12:00 +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 2558F633EE\n\tfor ;\n\tThu, 18 Apr 2024 12:11:59 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(117.145-247-81.adsl-dyn.isp.belgacom.be [81.247.145.117])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6133E827;\n\tThu, 18 Apr 2024 12:11:11 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"fdkp6iFu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713435071;\n\tbh=w9AvNFtG5+50edsu9sc+v518cPsplv/LkTaliY6NBBU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=fdkp6iFusgFsDHWze9bP9e6gNDKiqzG9/4OQtBP0fDqUdbxouu3tMh1gAx09cx3Sh\n\tb79RxpcdChVw2IX+TxdsMke0NHkkqMcljtjvfMODNtqdswtQnP7g2bDILcZQDoylWc\n\tIzVP1vnX+l8uzfaKXf+U8HdGIe6aaNbnrLK4aBzo=","Date":"Thu, 18 Apr 2024 13:11:52 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/2] pipeline: rkisp1: Use cached Request pointer","Message-ID":"<20240418101152.GT12561@pendragon.ideasonboard.com>","References":"<20240418074945.47349-1-umang.jain@ideasonboard.com>\n\t<20240418074945.47349-2-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240418074945.47349-2-umang.jain@ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29257,"web_url":"https://patchwork.libcamera.org/comment/29257/","msgid":"<0a900e06-b2dc-4b4a-a30a-80aca8d55706@ideasonboard.com>","date":"2024-04-18T10:19:31","subject":"Re: [PATCH 1/2] pipeline: rkisp1: Use cached Request pointer","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Kieran\n\nOn 18/04/24 3:38 pm, Kieran Bingham wrote:\n> Quoting Umang Jain (2024-04-18 08:49:44)\n>> The Request pointer is cached in RkISP1FrameInfo. Use it directly\n>> instead of retrieving it via buffer->request().\n> I don't think there's any specific speed up here. I think the compiler\n> will map this into a single data read in both cases ?\n>\n> As this is in the bufferReady() context, I think useing the request\n> associat3ed with the buffer is 'currently' correct.\n>\n> That said, I think this is an area that's likely about to get reworked\n> in two fronts. Dan has looked at how to clean up the FrameInfo handling,\n> and Stefan has been looking at how to be able to split/spearate buffers\n> and requests.\n\nThere is a third front in this case, where the buffer is the internal \nbuffer which pipes into another component. In that case, \nbuffer->request() will fail (since buffer is an internal buffer in this \ncase).\n\nRegarding the latter point, I thought it already is in line with \nseparation of request/buffers ... since we are using cached value of \nrequest, instead of getting it from buffer\n\n\n>\n> So I suspect that we should already merge 2/2 from this series but\n> probably leave this one out for now...\n>\n\nOK\n>\n>\n>> Signed-off-by: Umang Jain \n>> ---\n>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> index abb21968..5a6c9e1e 100644\n>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> @@ -1261,7 +1261,7 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n>> return;\n>> \n>> const FrameMetadata &metadata = buffer->metadata();\n>> - Request *request = buffer->request();\n>> + Request *request = info->request;\n>> \n>> if (metadata.status != FrameMetadata::FrameCancelled) {\n>> /*\n>> -- \n>> 2.44.0\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 0192EBE08B\n\tfor ;\n\tThu, 18 Apr 2024 10:19:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 19A5161B4F;\n\tThu, 18 Apr 2024 12:19:39 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 76C7E61B4F\n\tfor ;\n\tThu, 18 Apr 2024 12:19:37 +0200 (CEST)","from [192.168.1.105] (unknown [103.251.226.7])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0AEFB827;\n\tThu, 18 Apr 2024 12:18:48 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"N7IRxYtX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713435529;\n\tbh=dZR66upuAOVbp4ZCLYLTAHZ4VcutB5V1PUoFM6j6ir4=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=N7IRxYtXCfYu37tCjsEl6BnvbWzU1ElfLFMZkByr/chdbqwT7pIagu57zWCDL7UY6\n\tz5dEKlRdpZMRoSYruuYG1Uf95Qyykr+yAnmn/fu7V1sXwX2Ng92HeKpr1IbhR+otiU\n\tPFD3xFUetNvrVnbzvA+9vcyLWYhXjzD50+lkMybA=","Message-ID":"<0a900e06-b2dc-4b4a-a30a-80aca8d55706@ideasonboard.com>","Date":"Thu, 18 Apr 2024 15:49:31 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 1/2] pipeline: rkisp1: Use cached Request pointer","Content-Language":"en-US","To":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20240418074945.47349-1-umang.jain@ideasonboard.com>\n\t<20240418074945.47349-2-umang.jain@ideasonboard.com>\n\t<171343493022.342316.4906885895509411521@ping.linuxembedded.co.uk>","From":"Umang Jain ","In-Reply-To":"<171343493022.342316.4906885895509411521@ping.linuxembedded.co.uk>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]