[{"id":3381,"web_url":"https://patchwork.libcamera.org/comment/3381/","msgid":"<20200108013416.GW4871@pendragon.ideasonboard.com>","date":"2020-01-08T01:34:16","subject":"Re: [libcamera-devel] [PATCH v2 15/25] libcamera: pipeline: rkisp1:\n\tDestroy frame information before completing request","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Mon, Dec 30, 2019 at 01:05:00PM +0100, Niklas Söderlund wrote:\n> The FrameBuffer interface will allow reuse of FrameBuffers form the\n\ns/FrameBuffers/FrameBuffer instances/\ns/form/from/\n\n> request completion handler. For this reason the pipeline must destroy\n> its cached information freeing the statistics and parameters buffer used\n> to allow them to be reused directly.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 46df871a51105ee4..9cd0ab3ad88b35cc 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -989,9 +989,9 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)\n> \tif (!info->paramDequeued)\n> \t\treturn;\n> \n> -\tcompleteRequest(activeCamera_, request);\n> -\n> \tdata->frameInfo_.destroy(info->frame);\n> +\n> +\tcompleteRequest(activeCamera_, request);\n\nThe change itself seems fine, but I don't see how it relates the commit\nmessage. As far as I can tell, we can already queue a Buffer (albeit a\nnew one) from the request completion handler, and the only thing that\nthe pipeline handler does with the buffer is to store it in a newly\nallocated RkISP1FrameInfo that is stored in the frameInfo_ map in a new\nentry. I don't see what resource is now being reused that wasn't being\nreused before and that requires the RkISP1FrameInfo to be destroyed\nfirst.\n\nFeel free to keep the change, but please update the commit message\n(possibly to explain why I'm wrong :-)).\n\n> }\n> \n> void PipelineHandlerRkISP1::bufferReady(Buffer *buffer)","headers":{"Return-Path":"","Received":["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 4AC2460612\n\tfor ;\n\tWed, 8 Jan 2020 02:34:28 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C5E0B52F;\n\tWed, 8 Jan 2020 02:34:27 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1578447268;\n\tbh=BRH6jVPeT75NQDhrSZsRCT2I534+y8stzx/78YgY/zA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QI4DijrwId1KBzQ9CqlQd91HVMupez4Nl+Jmows+CEOe5oEN2X2GYnKpJmNcH0p57\n\tSLZa264ZO/GRSu52fDWJiYC4Ug/9FS2zm75Xc8KoXKe/OmShu1p61+PNPa6PKc25io\n\tpQoivbfsj8uSlvbkyvWgBl114P167rk1k6xsIBnE=","Date":"Wed, 8 Jan 2020 03:34:16 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200108013416.GW4871@pendragon.ideasonboard.com>","References":"<20191230120510.938333-1-niklas.soderlund@ragnatech.se>\n\t<20191230120510.938333-16-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191230120510.938333-16-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 15/25] libcamera: pipeline: rkisp1:\n\tDestroy frame information before completing request","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","X-List-Received-Date":"Wed, 08 Jan 2020 01:34:28 -0000"}}]