[{"id":38497,"web_url":"https://patchwork.libcamera.org/comment/38497/","msgid":"","date":"2026-04-07T09:54:47","subject":"Re: [PATCH v1] libcamera: pipeline: rkisp1: Fix buffer queue\n\tmemleaks","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Fri, Apr 03, 2026 at 07:31:06PM +0200, Barnabás Pőcze wrote:\n> `RkISP1Frames::{clear,destroy}()` always push all buffers from the\n> `RkISP1FrameInfo` object to the corresponding `available*Buffers_`\n> queues. This is not entirely correct.\n>\n> If no dewarper is used, then `availableMainPathBuffers_` is never\n> consumed, so it will grow with each request. Fix it by only queueing\n> the buffer if `RkISP1CameraData::usesDewarper_`.\n>\n> Additionally, in raw capture mode, no parameter or statistics buffers are\n> used, so those queues will be filled up with `nullptr`s without limit.\n> Fix that by only queueing them if they are not null.\n>\n> Fixes: c8f63760e55a (\"pipeline: rkisp1: Support raw Bayer capture at runtime\")\n> Fixes: 12b553d691d4 (\"libcamera: rkisp1: Plumb the dw100 dewarper as V4L2M2M converter\")\n> Signed-off-by: Barnabás Pőcze \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 26 +++++++++++++++---------\n> 1 file changed, 16 insertions(+), 10 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index faeeecd96..a8756cefa 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -86,6 +86,8 @@ public:\n> \tRkISP1FrameInfo *find(Request *request);\n>\n> private:\n> +\tvoid recycleBuffers(RkISP1FrameInfo &info);\n> +\n> \tPipelineHandlerRkISP1 *pipe_;\n> \tstd::map frameInfo_;\n> };\n> @@ -316,12 +318,7 @@ int RkISP1Frames::destroy(unsigned int frame)\n> \tif (it == frameInfo_.end())\n> \t\treturn -ENOENT;\n>\n> -\tauto &info = it->second;\n> -\n> -\tpipe_->availableParamBuffers_.push(info.paramBuffer);\n> -\tpipe_->availableStatBuffers_.push(info.statBuffer);\n> -\tpipe_->availableMainPathBuffers_.push(info.mainPathBuffer);\n> -\n> +\trecycleBuffers(it->second);\n> \tframeInfo_.erase(it);\n>\n> \treturn 0;\n> @@ -329,13 +326,22 @@ int RkISP1Frames::destroy(unsigned int frame)\n>\n> void RkISP1Frames::clear()\n> {\n> -\tfor (const auto &[frame, info] : frameInfo_) {\n> +\tfor (auto &[frame, info] : frameInfo_)\n> +\t\trecycleBuffers(info);\n> +\n> +\tframeInfo_.clear();\n> +}\n> +\n> +void RkISP1Frames::recycleBuffers(RkISP1FrameInfo &info)\n> +{\n> +\tif (info.paramBuffer)\n> \t\tpipe_->availableParamBuffers_.push(info.paramBuffer);\n> +\n> +\tif (info.statBuffer)\n> \t\tpipe_->availableStatBuffers_.push(info.statBuffer);\n> -\t\tpipe_->availableMainPathBuffers_.push(info.mainPathBuffer);\n> -\t}\n>\n> -\tframeInfo_.clear();\n> +\tif (pipe_->cameraData(pipe_->activeCamera_)->usesDewarper_)\n\nI'm wondering if this check is enough or we should also check for\ninfo.mainPathBuffer validity.\n\nLooking at RkISP1Frames::create()\n\n FrameBuffer *mainPathBuffer = nullptr;\n\tif (!isRaw) {\n\n ...\n\n\t\tif (data->usesDewarper_) {\n\t\t\tmainPathBuffer = pipe_->availableMainPathBuffers_.front();\n ...\n }\n }\n\n\tif (!mainPathBuffer)\n\t\tmainPathBuffer = request->findBuffer(&data->mainPathStream_);\n\n\tinfo.mainPathBuffer = mainPathBuffer;\n\nIt seems that it will always be populated and the only case where it\nhas to be returned to the pool is actually caught by the above\ncondition.\n\nGood catch, we're certainly wasting memory.\nAlso, one more reason to get rid of the several differernt\nimplementation of *FrameInfo in all our pipelines.\n\nReviewed-by: Jacopo Mondi \n\n\n\n> +\t\tpipe_->availableMainPathBuffers_.push(info.mainPathBuffer);\n> }\n>\n> RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame)\n> --\n> 2.53.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 4BC6DBDCBD\n\tfor ;\n\tTue, 7 Apr 2026 09:54:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AA41062D52;\n\tTue, 7 Apr 2026 11:54:52 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8F02E62010\n\tfor ;\n\tTue, 7 Apr 2026 11:54:51 +0200 (CEST)","from ideasonboard.com (mob-109-113-47-41.net.vodafone.it\n\t[109.113.47.41])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E4C9D6DC;\n\tTue, 7 Apr 2026 11:53:23 +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=\"iFuelGvk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1775555604;\n\tbh=KCSQaiiKFGLFfK38t0mg/DT5yznAnJfMQl6zdEKl5t0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iFuelGvkatOqNgaamH/oHuV2+91hytxEehLdijlmWhPgT4DB/ok9nwopPr5mO146O\n\tPOEvHxPvRwXtxdS28vL56yVKCD3Xyt8Q0XwYARyOhEC/nc19aQyz1/Quv91YZ5lTL5\n\tW35dc4UPOCBNGb1JwF39pONdAELBRRqcZecNkQSI=","Date":"Tue, 7 Apr 2026 11:54:47 +0200","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] libcamera: pipeline: rkisp1: Fix buffer queue\n\tmemleaks","Message-ID":"","References":"<20260403173106.473431-1-barnabas.pocze@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20260403173106.473431-1-barnabas.pocze@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":38577,"web_url":"https://patchwork.libcamera.org/comment/38577/","msgid":"<86825f2d-86a2-46bf-846b-41727a2bdf78@ideasonboard.com>","date":"2026-04-13T07:04:58","subject":"Re: [PATCH v1] libcamera: pipeline: rkisp1: Fix buffer queue\n\tmemleaks","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 04. 07. 11:54 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Fri, Apr 03, 2026 at 07:31:06PM +0200, Barnabás Pőcze wrote:\n>> `RkISP1Frames::{clear,destroy}()` always push all buffers from the\n>> `RkISP1FrameInfo` object to the corresponding `available*Buffers_`\n>> queues. This is not entirely correct.\n>>\n>> If no dewarper is used, then `availableMainPathBuffers_` is never\n>> consumed, so it will grow with each request. Fix it by only queueing\n>> the buffer if `RkISP1CameraData::usesDewarper_`.\n>>\n>> Additionally, in raw capture mode, no parameter or statistics buffers are\n>> used, so those queues will be filled up with `nullptr`s without limit.\n>> Fix that by only queueing them if they are not null.\n>>\n>> Fixes: c8f63760e55a (\"pipeline: rkisp1: Support raw Bayer capture at runtime\")\n>> Fixes: 12b553d691d4 (\"libcamera: rkisp1: Plumb the dw100 dewarper as V4L2M2M converter\")\n>> Signed-off-by: Barnabás Pőcze \n>> ---\n>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 26 +++++++++++++++---------\n>> 1 file changed, 16 insertions(+), 10 deletions(-)\n>>\n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> index faeeecd96..a8756cefa 100644\n>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> @@ -86,6 +86,8 @@ public:\n>> \tRkISP1FrameInfo *find(Request *request);\n>>\n>> private:\n>> +\tvoid recycleBuffers(RkISP1FrameInfo &info);\n>> +\n>> \tPipelineHandlerRkISP1 *pipe_;\n>> \tstd::map frameInfo_;\n>> };\n>> @@ -316,12 +318,7 @@ int RkISP1Frames::destroy(unsigned int frame)\n>> \tif (it == frameInfo_.end())\n>> \t\treturn -ENOENT;\n>>\n>> -\tauto &info = it->second;\n>> -\n>> -\tpipe_->availableParamBuffers_.push(info.paramBuffer);\n>> -\tpipe_->availableStatBuffers_.push(info.statBuffer);\n>> -\tpipe_->availableMainPathBuffers_.push(info.mainPathBuffer);\n>> -\n>> +\trecycleBuffers(it->second);\n>> \tframeInfo_.erase(it);\n>>\n>> \treturn 0;\n>> @@ -329,13 +326,22 @@ int RkISP1Frames::destroy(unsigned int frame)\n>>\n>> void RkISP1Frames::clear()\n>> {\n>> -\tfor (const auto &[frame, info] : frameInfo_) {\n>> +\tfor (auto &[frame, info] : frameInfo_)\n>> +\t\trecycleBuffers(info);\n>> +\n>> +\tframeInfo_.clear();\n>> +}\n>> +\n>> +void RkISP1Frames::recycleBuffers(RkISP1FrameInfo &info)\n>> +{\n>> +\tif (info.paramBuffer)\n>> \t\tpipe_->availableParamBuffers_.push(info.paramBuffer);\n>> +\n>> +\tif (info.statBuffer)\n>> \t\tpipe_->availableStatBuffers_.push(info.statBuffer);\n>> -\t\tpipe_->availableMainPathBuffers_.push(info.mainPathBuffer);\n>> -\t}\n>>\n>> -\tframeInfo_.clear();\n>> +\tif (pipe_->cameraData(pipe_->activeCamera_)->usesDewarper_)\n> \n> I'm wondering if this check is enough or we should also check for\n> info.mainPathBuffer validity.\n\nNot sure, it shouldn't be possible for it to be be nullptr if usesDewarper_,\nbut I suppose it cannot hurt.\n\n\n> \n> Looking at RkISP1Frames::create()\n> \n> FrameBuffer *mainPathBuffer = nullptr;\n> \tif (!isRaw) {\n> \n> ...\n> \n> \t\tif (data->usesDewarper_) {\n> \t\t\tmainPathBuffer = pipe_->availableMainPathBuffers_.front();\n> ...\n> }\n> }\n> \n> \tif (!mainPathBuffer)\n> \t\tmainPathBuffer = request->findBuffer(&data->mainPathStream_);\n> \n> \tinfo.mainPathBuffer = mainPathBuffer;\n> \n> It seems that it will always be populated and the only case where it\n> has to be returned to the pool is actually caught by the above\n> condition.\n> \n> Good catch, we're certainly wasting memory.\n> Also, one more reason to get rid of the several differernt\n> implementation of *FrameInfo in all our pipelines.\n> \n> Reviewed-by: Jacopo Mondi \n> \n> \n> \n>> +\t\tpipe_->availableMainPathBuffers_.push(info.mainPathBuffer);\n>> }\n>>\n>> RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame)\n>> --\n>> 2.53.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 1933CC32BB\n\tfor ;\n\tMon, 13 Apr 2026 07:05:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C49B962E5A;\n\tMon, 13 Apr 2026 09:05:03 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4ACC862846\n\tfor ;\n\tMon, 13 Apr 2026 09:05:02 +0200 (CEST)","from [192.168.33.49] (185.182.214.8.nat.pool.zt.hu [185.182.214.8])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 948887FE;\n\tMon, 13 Apr 2026 09:03:30 +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=\"Q1p1Etsw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1776063810;\n\tbh=kLXzL1zDrPvfuA6cmcEvVpoFUYGW3A6LB0Q0zRw07tU=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=Q1p1EtswijRq/LIHcaxAPGdVifIKiZplym2puFwgpjY0aYESf9/0Ax5X7xaj9J7Ic\n\t9X7w9m5KXdLM3Viv4V0bZ4XblPGgBlwMXplXK3W4wLhefqOJrFT2DYQ73vw4968grd\n\tWd/tcN8zAHksnY9UJy7gTGabt/9/B1vXNejhuAag=","Message-ID":"<86825f2d-86a2-46bf-846b-41727a2bdf78@ideasonboard.com>","Date":"Mon, 13 Apr 2026 09:04:58 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v1] libcamera: pipeline: rkisp1: Fix buffer queue\n\tmemleaks","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20260403173106.473431-1-barnabas.pocze@ideasonboard.com>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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\" "}}]