[{"id":3408,"web_url":"https://patchwork.libcamera.org/comment/3408/","msgid":"<20200111004842.GK4859@pendragon.ideasonboard.com>","date":"2020-01-11T00:48:42","subject":"Re: [libcamera-devel] [PATCH v3 21/33] 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 Fri, Jan 10, 2020 at 08:37:56PM +0100, Niklas Söderlund wrote:\n> It's common for applications to create and queue a new request in a\n> previous request completion handler. When the new request gets queued to\n> the RkISP1 pipeline handler it tries to find a parameters and statistic\n> buffer to be used with the request. The problem is if the pipeline depth\n> is already filled there are no internal buffers free to be used by the\n> new request.\n> \n> This was solved by allocation one more parameters and statistic buffer\n> then the pipeline depth, this is waste full. Instead free the request\n> that is about to be completed resources before it is signaled to the\n\n\"free the resources of the request that has completed\"\n\n> application, this way if the pipeline depth is full it can reuse the\n> internal resources and the waste full allocation can be removed.\n\ns/waste full/wasteful/\n\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> * Changes since v2\n> - Rewrote commit message.\n> - Actually remove the wasteful allocation of extra buffers in this\n> commit instead of depending on the switch to FrameBuffer to do it for\n> us.\n\nAh this makes much more sense to me now :-)\n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 ++++++------\n> 1 file changed, 6 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 979b670e4cb75512..607ff85539a22324 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -671,14 +671,14 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,\n> \tif (ret)\n> \t\treturn ret;\n> \n> -\tparamPool_.createBuffers(stream->configuration().bufferCount + 1);\n> +\tparamPool_.createBuffers(stream->configuration().bufferCount);\n> \tret = param_->exportBuffers(¶mPool_);\n> \tif (ret) {\n> \t\tvideo_->releaseBuffers();\n> \t\treturn ret;\n> \t}\n> \n> -\tstatPool_.createBuffers(stream->configuration().bufferCount + 1);\n> +\tstatPool_.createBuffers(stream->configuration().bufferCount);\n> \tret = stat_->exportBuffers(&statPool_);\n> \tif (ret) {\n> \t\tparam_->releaseBuffers();\n> @@ -686,13 +686,13 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,\n> \t\treturn ret;\n> \t}\n> \n> -\tfor (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) {\n> +\tfor (unsigned int i = 0; i < stream->configuration().bufferCount; i++) {\n> \t\tdata->ipaBuffers_.push_back({ .id = RKISP1_PARAM_BASE | i,\n> \t\t\t\t\t .planes = paramPool_.buffers()[i].planes() });\n> \t\tparamBuffers_.push(new Buffer(i));\n> \t}\n> \n> -\tfor (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) {\n> +\tfor (unsigned int i = 0; i < stream->configuration().bufferCount; i++) {\n> \t\tdata->ipaBuffers_.push_back({ .id = RKISP1_STAT_BASE | i,\n> \t\t\t\t\t .planes = statPool_.buffers()[i].planes() });\n> \t\tstatBuffers_.push(new Buffer(i));\n> @@ -981,9 +981,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> }\n> \n> void PipelineHandlerRkISP1::bufferReady(Buffer *buffer)","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E4EF8606AC\n\tfor ;\n\tSat, 11 Jan 2020 01:48:55 +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 3BD9352F;\n\tSat, 11 Jan 2020 01:48:55 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1578703735;\n\tbh=FFORomF5EaObhPGlyBWb5WxRpUUtjUNyu/ceOEm0DMY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kndGtgd+B12IUt2lTDegQUazGA1yKlK5QTdslB30hS4L5oDcjkPEa5OjW0ng9MfmZ\n\tXwq1IB8BVGcU6hsPN6U6ZfhtZdloeHpxYHuQ09rU1HBHHMLj4oUsTZmxcxQh21/Vsh\n\tG3zLhyp9C9/XtTQeMYHBk5yV7rq9JpiuViFNDP6E=","Date":"Sat, 11 Jan 2020 02:48:42 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200111004842.GK4859@pendragon.ideasonboard.com>","References":"<20200110193808.2266294-1-niklas.soderlund@ragnatech.se>\n\t<20200110193808.2266294-22-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":"<20200110193808.2266294-22-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 21/33] 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":"Sat, 11 Jan 2020 00:48:56 -0000"}}]