[{"id":35509,"web_url":"https://patchwork.libcamera.org/comment/35509/","msgid":"<803158a1-4fd4-46c4-9370-9745fde7f463@ideasonboard.com>","date":"2025-08-19T12:15:44","subject":"Re: [PATCH v2 5/8] pipelines: Use lambda functions to factor out\n\tbuffer mapping code","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 08. 15. 13:33 keltezéssel, Laurent Pinchart írta:\n> Multiple pipeline handlers duplicate code related to mapping params and\n> stats buffers to IPA modules. Factor out the code to lambda functions to\n> share it between the two buffer types.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>   src/libcamera/pipeline/ipu3/ipu3.cpp         | 16 +++++++------\n>   src/libcamera/pipeline/mali-c55/mali-c55.cpp | 24 ++++++++++----------\n>   src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 22 +++++++++---------\n>   3 files changed, 32 insertions(+), 30 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index ad20810e6a26..7ae85e5063db 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -678,15 +678,17 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n>   \t/* Map buffers to the IPA. */\n>   \tunsigned int ipaBufferId = 1;\n> \n> -\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) {\n> +\tauto pushBuffer = [&](std::vector<IPABuffer> &buffers,\n> +\t\t\t      const std::unique_ptr<FrameBuffer> &buffer) {\n\nI feel like the `buffers` argument is a bit unnecessary since it's only ever `ipaBuffers_`.\n\n\n>   \t\tbuffer->setCookie(ipaBufferId++);\n> -\t\tipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n> -\t}\n> +\t\tbuffers.emplace_back(buffer->cookie(), buffer->planes());\n> +\t};\n> \n> -\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) {\n> -\t\tbuffer->setCookie(ipaBufferId++);\n> -\t\tipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n> -\t}\n> +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_)\n> +\t\tpushBuffer(ipaBuffers_, buffer);\n> +\n> +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_)\n> +\t\tpushBuffer(ipaBuffers_, buffer);\n> \n>   \tdata->ipa_->mapBuffers(ipaBuffers_);\n> \n> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> index 76341ed3f363..f03a03fef35c 100644\n> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> @@ -1133,27 +1133,27 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera)\n>   \t\tdata->dsStream_.configuration().bufferCount,\n>   \t});\n> \n> +\tauto pushBuffer = [&](std::queue<FrameBuffer *> &queue,\n> +\t\t\t      std::vector<IPABuffer> &buffers,\n> +\t\t\t      const std::unique_ptr<FrameBuffer> &buffer) {\n> +\t\tbuffer->setCookie(ipaBufferId++);\n> +\t\tbuffers.emplace_back(buffer->cookie(), buffer->planes());\n> +\t\tqueue.push(buffer.get());\n> +\t};\n> +\n>   \tret = stats_->allocateBuffers(bufferCount, &statsBuffers_);\n>   \tif (ret < 0)\n>   \t\treturn ret;\n> \n> -\tfor (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_) {\n> -\t\tbuffer->setCookie(ipaBufferId++);\n> -\t\tdata->ipaStatBuffers_.emplace_back(buffer->cookie(),\n> -\t\t\t\t\t\t   buffer->planes());\n> -\t\tavailableStatsBuffers_.push(buffer.get());\n> -\t}\n> +\tfor (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_)\n> +\t\tpushBuffer(availableStatsBuffers_, data->ipaStatBuffers_, buffer);\n> \n>   \tret = params_->allocateBuffers(bufferCount, &paramsBuffers_);\n>   \tif (ret < 0)\n>   \t\treturn ret;\n> \n> -\tfor (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_) {\n> -\t\tbuffer->setCookie(ipaBufferId++);\n> -\t\tdata->ipaParamBuffers_.emplace_back(buffer->cookie(),\n> -\t\t\t\t\t\t    buffer->planes());\n> -\t\tavailableParamsBuffers_.push(buffer.get());\n> -\t}\n> +\tfor (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_)\n> +\t\tpushBuffer(availableParamsBuffers_, data->ipaParamBuffers_, buffer);\n> \n>   \tif (data->ipa_) {\n>   \t\tdata->ipa_->mapBuffers(data->ipaStatBuffers_, true);\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 55d7d4442caf..99347c9f6258 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -1028,19 +1028,19 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n>   \t\t\tavailableMainPathBuffers_.push(buffer.get());\n>   \t}\n> \n> -\tfor (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) {\n> +\tauto pushBuffer = [&](std::vector<IPABuffer> &buffers,\n> +\t\t\t      std::queue<FrameBuffer *> &queue,\n> +\t\t\t      const std::unique_ptr<FrameBuffer> &buffer) {\n\nSame here.\n\nReviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n\n\n>   \t\tbuffer->setCookie(ipaBufferId++);\n> -\t\tdata->ipaBuffers_.emplace_back(buffer->cookie(),\n> -\t\t\t\t\t       buffer->planes());\n> -\t\tavailableParamBuffers_.push(buffer.get());\n> -\t}\n> +\t\tbuffers.emplace_back(buffer->cookie(), buffer->planes());\n> +\t\tqueue.push(buffer.get());\n> +\t};\n> \n> -\tfor (std::unique_ptr<FrameBuffer> &buffer : statBuffers_) {\n> -\t\tbuffer->setCookie(ipaBufferId++);\n> -\t\tdata->ipaBuffers_.emplace_back(buffer->cookie(),\n> -\t\t\t\t\t       buffer->planes());\n> -\t\tavailableStatBuffers_.push(buffer.get());\n> -\t}\n> +\tfor (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_)\n> +\t\tpushBuffer(data->ipaBuffers_, availableParamBuffers_, buffer);\n> +\n> +\tfor (std::unique_ptr<FrameBuffer> &buffer : statBuffers_)\n> +\t\tpushBuffer(data->ipaBuffers_, availableStatBuffers_, buffer);\n> \n>   \tdata->ipa_->mapBuffers(data->ipaBuffers_);\n> \n> --\n> Regards,\n> \n> Laurent Pinchart\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 304F2BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 19 Aug 2025 12:15:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 80402692CF;\n\tTue, 19 Aug 2025 14:15:53 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 885D7692CE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Aug 2025 14:15:51 +0200 (CEST)","from [192.168.33.20] (185.221.141.188.nat.pool.zt.hu\n\t[185.221.141.188])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 889BA2391;\n\tTue, 19 Aug 2025 14:14:52 +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=\"a7kQn4Yr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1755605693;\n\tbh=mRp5wmhaP2ncI/12qkuHnJ2uABuoqixvMvlFXGZ0XQg=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=a7kQn4Yrj03Ul/QwY0ZfyfrxFr57SQm24OODZQ51LyU0vrzXraHluaNXHJAiWlaDp\n\trLrZgbg3f8YYAALbiJRaUIVSJUtLPp1zg8u/BDCBfjzb+k0Equ3NmamEJShIV6acS2\n\tFwHWQAmT0srIgWt/6nUHh44AUY1Jflkb/py2jTUU=","Message-ID":"<803158a1-4fd4-46c4-9370-9745fde7f463@ideasonboard.com>","Date":"Tue, 19 Aug 2025 14:15:44 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 5/8] pipelines: Use lambda functions to factor out\n\tbuffer mapping code","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Cc":"=?utf-8?q?Daniel_R=C3=A1kos?= <daniel.rakos@rastergrid.com>","References":"<20250815113400.20623-1-laurent.pinchart@ideasonboard.com>\n\t<HfCaQHEgoEqjy73NffYZVwR8gEgDItxLBhkRxvbizMvW-m_kBGD71KEvPLEMhHZDtdTfkYc69RxsHzF_oxs_Rg==@protonmail.internalid>\n\t<20250815113400.20623-6-laurent.pinchart@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250815113400.20623-6-laurent.pinchart@ideasonboard.com>","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":"<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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35511,"web_url":"https://patchwork.libcamera.org/comment/35511/","msgid":"<20250819123724.GT5862@pendragon.ideasonboard.com>","date":"2025-08-19T12:37:24","subject":"Re: [PATCH v2 5/8] pipelines: Use lambda functions to factor out\n\tbuffer mapping code","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Aug 19, 2025 at 02:15:44PM +0200, Barnabás Pőcze wrote:\n> 2025. 08. 15. 13:33 keltezéssel, Laurent Pinchart írta:\n> > Multiple pipeline handlers duplicate code related to mapping params and\n> > stats buffers to IPA modules. Factor out the code to lambda functions to\n> > share it between the two buffer types.\n> > \n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >   src/libcamera/pipeline/ipu3/ipu3.cpp         | 16 +++++++------\n> >   src/libcamera/pipeline/mali-c55/mali-c55.cpp | 24 ++++++++++----------\n> >   src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 22 +++++++++---------\n> >   3 files changed, 32 insertions(+), 30 deletions(-)\n> > \n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index ad20810e6a26..7ae85e5063db 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -678,15 +678,17 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n> >   \t/* Map buffers to the IPA. */\n> >   \tunsigned int ipaBufferId = 1;\n> > \n> > -\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) {\n> > +\tauto pushBuffer = [&](std::vector<IPABuffer> &buffers,\n> > +\t\t\t      const std::unique_ptr<FrameBuffer> &buffer) {\n> \n> I feel like the `buffers` argument is a bit unnecessary since it's\n> only ever `ipaBuffers_`.\n\nYou know, one of the reasons I like your reviews is because you point\nout things that I wasn't sure about myself when sending patches :-) I've\ngone back and forth on this, and decided to add the buffers argument for\nconsistency between pipeline handlers. If you think it's best to drop\nit, I'll do that. What's your preference ?\n\n> >   \t\tbuffer->setCookie(ipaBufferId++);\n> > -\t\tipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n> > -\t}\n> > +\t\tbuffers.emplace_back(buffer->cookie(), buffer->planes());\n> > +\t};\n> > \n> > -\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) {\n> > -\t\tbuffer->setCookie(ipaBufferId++);\n> > -\t\tipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n> > -\t}\n> > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_)\n> > +\t\tpushBuffer(ipaBuffers_, buffer);\n> > +\n> > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_)\n> > +\t\tpushBuffer(ipaBuffers_, buffer);\n> > \n> >   \tdata->ipa_->mapBuffers(ipaBuffers_);\n> > \n> > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > index 76341ed3f363..f03a03fef35c 100644\n> > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > @@ -1133,27 +1133,27 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera)\n> >   \t\tdata->dsStream_.configuration().bufferCount,\n> >   \t});\n> > \n> > +\tauto pushBuffer = [&](std::queue<FrameBuffer *> &queue,\n> > +\t\t\t      std::vector<IPABuffer> &buffers,\n> > +\t\t\t      const std::unique_ptr<FrameBuffer> &buffer) {\n> > +\t\tbuffer->setCookie(ipaBufferId++);\n> > +\t\tbuffers.emplace_back(buffer->cookie(), buffer->planes());\n> > +\t\tqueue.push(buffer.get());\n> > +\t};\n> > +\n> >   \tret = stats_->allocateBuffers(bufferCount, &statsBuffers_);\n> >   \tif (ret < 0)\n> >   \t\treturn ret;\n> > \n> > -\tfor (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_) {\n> > -\t\tbuffer->setCookie(ipaBufferId++);\n> > -\t\tdata->ipaStatBuffers_.emplace_back(buffer->cookie(),\n> > -\t\t\t\t\t\t   buffer->planes());\n> > -\t\tavailableStatsBuffers_.push(buffer.get());\n> > -\t}\n> > +\tfor (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_)\n> > +\t\tpushBuffer(availableStatsBuffers_, data->ipaStatBuffers_, buffer);\n> > \n> >   \tret = params_->allocateBuffers(bufferCount, &paramsBuffers_);\n> >   \tif (ret < 0)\n> >   \t\treturn ret;\n> > \n> > -\tfor (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_) {\n> > -\t\tbuffer->setCookie(ipaBufferId++);\n> > -\t\tdata->ipaParamBuffers_.emplace_back(buffer->cookie(),\n> > -\t\t\t\t\t\t    buffer->planes());\n> > -\t\tavailableParamsBuffers_.push(buffer.get());\n> > -\t}\n> > +\tfor (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_)\n> > +\t\tpushBuffer(availableParamsBuffers_, data->ipaParamBuffers_, buffer);\n> > \n> >   \tif (data->ipa_) {\n> >   \t\tdata->ipa_->mapBuffers(data->ipaStatBuffers_, true);\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 55d7d4442caf..99347c9f6258 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -1028,19 +1028,19 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n> >   \t\t\tavailableMainPathBuffers_.push(buffer.get());\n> >   \t}\n> > \n> > -\tfor (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) {\n> > +\tauto pushBuffer = [&](std::vector<IPABuffer> &buffers,\n> > +\t\t\t      std::queue<FrameBuffer *> &queue,\n> > +\t\t\t      const std::unique_ptr<FrameBuffer> &buffer) {\n> \n> Same here.\n> \n> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> \n> >   \t\tbuffer->setCookie(ipaBufferId++);\n> > -\t\tdata->ipaBuffers_.emplace_back(buffer->cookie(),\n> > -\t\t\t\t\t       buffer->planes());\n> > -\t\tavailableParamBuffers_.push(buffer.get());\n> > -\t}\n> > +\t\tbuffers.emplace_back(buffer->cookie(), buffer->planes());\n> > +\t\tqueue.push(buffer.get());\n> > +\t};\n> > \n> > -\tfor (std::unique_ptr<FrameBuffer> &buffer : statBuffers_) {\n> > -\t\tbuffer->setCookie(ipaBufferId++);\n> > -\t\tdata->ipaBuffers_.emplace_back(buffer->cookie(),\n> > -\t\t\t\t\t       buffer->planes());\n> > -\t\tavailableStatBuffers_.push(buffer.get());\n> > -\t}\n> > +\tfor (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_)\n> > +\t\tpushBuffer(data->ipaBuffers_, availableParamBuffers_, buffer);\n> > +\n> > +\tfor (std::unique_ptr<FrameBuffer> &buffer : statBuffers_)\n> > +\t\tpushBuffer(data->ipaBuffers_, availableStatBuffers_, buffer);\n> > \n> >   \tdata->ipa_->mapBuffers(data->ipaBuffers_);\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 49F99BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 19 Aug 2025 12:37:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 49798692CF;\n\tTue, 19 Aug 2025 14:37:48 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C1CA269252\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Aug 2025 14:37:46 +0200 (CEST)","from pendragon.ideasonboard.com (85-76-43-215-nat.elisa-mobile.fi\n\t[85.76.43.215])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id AC96A2F81; \n\tTue, 19 Aug 2025 14:36: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=\"B7BTr3KF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1755607008;\n\tbh=Zat6B80D7iZ7y096ls6gojco55wKIqOAnGs+I5r1DFE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=B7BTr3KF5mHmI33xX/NjcCq0KqUHxBwhQSQwIq1efVDzPcUQbKPcKyvVGPOGXaBBi\n\tHUnpPlKsij0r6/NufmJRfwUXO8bZRvHqtPi7393Te1Fcwbk0KGcH7tizedfqhI/10f\n\tUTWYoNNG2P6y7LAzMiDrHmVVDNZD9y/7NQ/eDXgM=","Date":"Tue, 19 Aug 2025 15:37:24 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, Daniel =?utf-8?b?UsOha29z?=\n\t<daniel.rakos@rastergrid.com>","Subject":"Re: [PATCH v2 5/8] pipelines: Use lambda functions to factor out\n\tbuffer mapping code","Message-ID":"<20250819123724.GT5862@pendragon.ideasonboard.com>","References":"<20250815113400.20623-1-laurent.pinchart@ideasonboard.com>\n\t<HfCaQHEgoEqjy73NffYZVwR8gEgDItxLBhkRxvbizMvW-m_kBGD71KEvPLEMhHZDtdTfkYc69RxsHzF_oxs_Rg==@protonmail.internalid>\n\t<20250815113400.20623-6-laurent.pinchart@ideasonboard.com>\n\t<803158a1-4fd4-46c4-9370-9745fde7f463@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<803158a1-4fd4-46c4-9370-9745fde7f463@ideasonboard.com>","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35513,"web_url":"https://patchwork.libcamera.org/comment/35513/","msgid":"<800f2856-4467-472c-8066-3090657ed0d6@ideasonboard.com>","date":"2025-08-19T13:01:50","subject":"Re: [PATCH v2 5/8] pipelines: Use lambda functions to factor out\n\tbuffer mapping code","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 08. 19. 14:37 keltezéssel, Laurent Pinchart írta:\n> On Tue, Aug 19, 2025 at 02:15:44PM +0200, Barnabás Pőcze wrote:\n>> 2025. 08. 15. 13:33 keltezéssel, Laurent Pinchart írta:\n>>> Multiple pipeline handlers duplicate code related to mapping params and\n>>> stats buffers to IPA modules. Factor out the code to lambda functions to\n>>> share it between the two buffer types.\n>>>\n>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>>> ---\n>>>    src/libcamera/pipeline/ipu3/ipu3.cpp         | 16 +++++++------\n>>>    src/libcamera/pipeline/mali-c55/mali-c55.cpp | 24 ++++++++++----------\n>>>    src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 22 +++++++++---------\n>>>    3 files changed, 32 insertions(+), 30 deletions(-)\n>>>\n>>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> index ad20810e6a26..7ae85e5063db 100644\n>>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> @@ -678,15 +678,17 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n>>>    \t/* Map buffers to the IPA. */\n>>>    \tunsigned int ipaBufferId = 1;\n>>>\n>>> -\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) {\n>>> +\tauto pushBuffer = [&](std::vector<IPABuffer> &buffers,\n>>> +\t\t\t      const std::unique_ptr<FrameBuffer> &buffer) {\n>>\n>> I feel like the `buffers` argument is a bit unnecessary since it's\n>> only ever `ipaBuffers_`.\n> \n> You know, one of the reasons I like your reviews is because you point\n> out things that I wasn't sure about myself when sending patches :-) I've\n> gone back and forth on this, and decided to add the buffers argument for\n> consistency between pipeline handlers. If you think it's best to drop\n> it, I'll do that. What's your preference ?\n\nMy thinking is that it's highly specific, so might as well go for the simplest\noption. But I am not sure, I don't think it matters much in the end.\n\n\n> \n>>>    \t\tbuffer->setCookie(ipaBufferId++);\n>>> -\t\tipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n>>> -\t}\n>>> +\t\tbuffers.emplace_back(buffer->cookie(), buffer->planes());\n>>> +\t};\n>>>\n>>> -\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) {\n>>> -\t\tbuffer->setCookie(ipaBufferId++);\n>>> -\t\tipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n>>> -\t}\n>>> +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_)\n>>> +\t\tpushBuffer(ipaBuffers_, buffer);\n>>> +\n>>> +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_)\n>>> +\t\tpushBuffer(ipaBuffers_, buffer);\n>>>\n>>>    \tdata->ipa_->mapBuffers(ipaBuffers_);\n>>>\n>>> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>>> index 76341ed3f363..f03a03fef35c 100644\n>>> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>>> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n>>> @@ -1133,27 +1133,27 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera)\n>>>    \t\tdata->dsStream_.configuration().bufferCount,\n>>>    \t});\n>>>\n>>> +\tauto pushBuffer = [&](std::queue<FrameBuffer *> &queue,\n>>> +\t\t\t      std::vector<IPABuffer> &buffers,\n>>> +\t\t\t      const std::unique_ptr<FrameBuffer> &buffer) {\n>>> +\t\tbuffer->setCookie(ipaBufferId++);\n>>> +\t\tbuffers.emplace_back(buffer->cookie(), buffer->planes());\n>>> +\t\tqueue.push(buffer.get());\n>>> +\t};\n>>> +\n>>>    \tret = stats_->allocateBuffers(bufferCount, &statsBuffers_);\n>>>    \tif (ret < 0)\n>>>    \t\treturn ret;\n>>>\n>>> -\tfor (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_) {\n>>> -\t\tbuffer->setCookie(ipaBufferId++);\n>>> -\t\tdata->ipaStatBuffers_.emplace_back(buffer->cookie(),\n>>> -\t\t\t\t\t\t   buffer->planes());\n>>> -\t\tavailableStatsBuffers_.push(buffer.get());\n>>> -\t}\n>>> +\tfor (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_)\n>>> +\t\tpushBuffer(availableStatsBuffers_, data->ipaStatBuffers_, buffer);\n>>>\n>>>    \tret = params_->allocateBuffers(bufferCount, &paramsBuffers_);\n>>>    \tif (ret < 0)\n>>>    \t\treturn ret;\n>>>\n>>> -\tfor (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_) {\n>>> -\t\tbuffer->setCookie(ipaBufferId++);\n>>> -\t\tdata->ipaParamBuffers_.emplace_back(buffer->cookie(),\n>>> -\t\t\t\t\t\t    buffer->planes());\n>>> -\t\tavailableParamsBuffers_.push(buffer.get());\n>>> -\t}\n>>> +\tfor (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_)\n>>> +\t\tpushBuffer(availableParamsBuffers_, data->ipaParamBuffers_, buffer);\n>>>\n>>>    \tif (data->ipa_) {\n>>>    \t\tdata->ipa_->mapBuffers(data->ipaStatBuffers_, true);\n>>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>> index 55d7d4442caf..99347c9f6258 100644\n>>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>> @@ -1028,19 +1028,19 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n>>>    \t\t\tavailableMainPathBuffers_.push(buffer.get());\n>>>    \t}\n>>>\n>>> -\tfor (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) {\n>>> +\tauto pushBuffer = [&](std::vector<IPABuffer> &buffers,\n>>> +\t\t\t      std::queue<FrameBuffer *> &queue,\n>>> +\t\t\t      const std::unique_ptr<FrameBuffer> &buffer) {\n>>\n>> Same here.\n>>\n>> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n>>\n>>>    \t\tbuffer->setCookie(ipaBufferId++);\n>>> -\t\tdata->ipaBuffers_.emplace_back(buffer->cookie(),\n>>> -\t\t\t\t\t       buffer->planes());\n>>> -\t\tavailableParamBuffers_.push(buffer.get());\n>>> -\t}\n>>> +\t\tbuffers.emplace_back(buffer->cookie(), buffer->planes());\n>>> +\t\tqueue.push(buffer.get());\n>>> +\t};\n>>>\n>>> -\tfor (std::unique_ptr<FrameBuffer> &buffer : statBuffers_) {\n>>> -\t\tbuffer->setCookie(ipaBufferId++);\n>>> -\t\tdata->ipaBuffers_.emplace_back(buffer->cookie(),\n>>> -\t\t\t\t\t       buffer->planes());\n>>> -\t\tavailableStatBuffers_.push(buffer.get());\n>>> -\t}\n>>> +\tfor (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_)\n>>> +\t\tpushBuffer(data->ipaBuffers_, availableParamBuffers_, buffer);\n>>> +\n>>> +\tfor (std::unique_ptr<FrameBuffer> &buffer : statBuffers_)\n>>> +\t\tpushBuffer(data->ipaBuffers_, availableStatBuffers_, buffer);\n>>>\n>>>    \tdata->ipa_->mapBuffers(data->ipaBuffers_);\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 086ACBEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 19 Aug 2025 13:01:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0FEE7692D4;\n\tTue, 19 Aug 2025 15:01:56 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8709569252\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Aug 2025 15:01:54 +0200 (CEST)","from [192.168.33.20] (185.221.141.188.nat.pool.zt.hu\n\t[185.221.141.188])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4DCCF2391;\n\tTue, 19 Aug 2025 15:00:56 +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=\"TetvkGha\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1755608456;\n\tbh=r2UDZbyHMoD3FVOItVOzTEyybwwWbgWGGQGc05UI6Wo=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=TetvkGhaL/VdZQWRnuXv94AwP1EUVz0vcspDeHYmZ83GK1XENVaLEIdbclbifoV6K\n\tYOFM0zPPcbnqDZhoPFNo77J/BeSLRBT5gUH8kIXRnsWT56BJ6+wCDvnWZHNhdBxHCn\n\tfj0G/qNFVbgXdmsjakF4c+1dXe/pXH/xXCziDGQU=","Message-ID":"<800f2856-4467-472c-8066-3090657ed0d6@ideasonboard.com>","Date":"Tue, 19 Aug 2025 15:01:50 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 5/8] pipelines: Use lambda functions to factor out\n\tbuffer mapping code","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org, =?utf-8?q?Daniel_R=C3=A1kos?=\n\t<daniel.rakos@rastergrid.com>","References":"<20250815113400.20623-1-laurent.pinchart@ideasonboard.com>\n\t<HfCaQHEgoEqjy73NffYZVwR8gEgDItxLBhkRxvbizMvW-m_kBGD71KEvPLEMhHZDtdTfkYc69RxsHzF_oxs_Rg==@protonmail.internalid>\n\t<20250815113400.20623-6-laurent.pinchart@ideasonboard.com>\n\t<803158a1-4fd4-46c4-9370-9745fde7f463@ideasonboard.com>\n\t<20250819123724.GT5862@pendragon.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250819123724.GT5862@pendragon.ideasonboard.com>","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":"<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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35520,"web_url":"https://patchwork.libcamera.org/comment/35520/","msgid":"<kmwcelzrhlupzbyw65ezj52iqb3efpylyel6x7qsabyjdbuaub@grtop5tt7pib>","date":"2025-08-19T15:31:59","subject":"Re: [PATCH v2 5/8] pipelines: Use lambda functions to factor out\n\tbuffer mapping code","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi\n\nOn Tue, Aug 19, 2025 at 03:01:50PM +0200, Barnabás Pőcze wrote:\n> 2025. 08. 19. 14:37 keltezéssel, Laurent Pinchart írta:\n> > On Tue, Aug 19, 2025 at 02:15:44PM +0200, Barnabás Pőcze wrote:\n> > > 2025. 08. 15. 13:33 keltezéssel, Laurent Pinchart írta:\n> > > > Multiple pipeline handlers duplicate code related to mapping params and\n> > > > stats buffers to IPA modules. Factor out the code to lambda functions to\n> > > > share it between the two buffer types.\n> > > >\n> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > > ---\n> > > >    src/libcamera/pipeline/ipu3/ipu3.cpp         | 16 +++++++------\n> > > >    src/libcamera/pipeline/mali-c55/mali-c55.cpp | 24 ++++++++++----------\n> > > >    src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 22 +++++++++---------\n> > > >    3 files changed, 32 insertions(+), 30 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > index ad20810e6a26..7ae85e5063db 100644\n> > > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > @@ -678,15 +678,17 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n> > > >    \t/* Map buffers to the IPA. */\n> > > >    \tunsigned int ipaBufferId = 1;\n> > > >\n> > > > -\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) {\n> > > > +\tauto pushBuffer = [&](std::vector<IPABuffer> &buffers,\n> > > > +\t\t\t      const std::unique_ptr<FrameBuffer> &buffer) {\n> > >\n> > > I feel like the `buffers` argument is a bit unnecessary since it's\n> > > only ever `ipaBuffers_`.\n> >\n\nOr you can pass the argument but avoid the default capture ? But then\nyou won't be able to access ipaBufferId.. (yes, you could capture that\none only, but I'm not sure all of this is worth it ?)\n\n\n> > You know, one of the reasons I like your reviews is because you point\n> > out things that I wasn't sure about myself when sending patches :-) I've\n> > gone back and forth on this, and decided to add the buffers argument for\n> > consistency between pipeline handlers. If you think it's best to drop\n> > it, I'll do that. What's your preference ?\n>\n> My thinking is that it's highly specific, so might as well go for the simplest\n> option. But I am not sure, I don't think it matters much in the end.\n>\n>\n> >\n> > > >    \t\tbuffer->setCookie(ipaBufferId++);\n> > > > -\t\tipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n> > > > -\t}\n> > > > +\t\tbuffers.emplace_back(buffer->cookie(), buffer->planes());\n> > > > +\t};\n> > > >\n> > > > -\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) {\n\nI wonder if you could pass 'imgu->statsBuffers_' instead and for-loop\nin the lambda..\n\nAnyways\nReviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n\n> > > > -\t\tbuffer->setCookie(ipaBufferId++);\n> > > > -\t\tipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n> > > > -\t}\n> > > > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_)\n> > > > +\t\tpushBuffer(ipaBuffers_, buffer);\n> > > > +\n> > > > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_)\n> > > > +\t\tpushBuffer(ipaBuffers_, buffer);\n> > > >\n> > > >    \tdata->ipa_->mapBuffers(ipaBuffers_);\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > > index 76341ed3f363..f03a03fef35c 100644\n> > > > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > > @@ -1133,27 +1133,27 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera)\n> > > >    \t\tdata->dsStream_.configuration().bufferCount,\n> > > >    \t});\n> > > >\n> > > > +\tauto pushBuffer = [&](std::queue<FrameBuffer *> &queue,\n> > > > +\t\t\t      std::vector<IPABuffer> &buffers,\n> > > > +\t\t\t      const std::unique_ptr<FrameBuffer> &buffer) {\n> > > > +\t\tbuffer->setCookie(ipaBufferId++);\n> > > > +\t\tbuffers.emplace_back(buffer->cookie(), buffer->planes());\n> > > > +\t\tqueue.push(buffer.get());\n> > > > +\t};\n> > > > +\n> > > >    \tret = stats_->allocateBuffers(bufferCount, &statsBuffers_);\n> > > >    \tif (ret < 0)\n> > > >    \t\treturn ret;\n> > > >\n> > > > -\tfor (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_) {\n> > > > -\t\tbuffer->setCookie(ipaBufferId++);\n> > > > -\t\tdata->ipaStatBuffers_.emplace_back(buffer->cookie(),\n> > > > -\t\t\t\t\t\t   buffer->planes());\n> > > > -\t\tavailableStatsBuffers_.push(buffer.get());\n> > > > -\t}\n> > > > +\tfor (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_)\n> > > > +\t\tpushBuffer(availableStatsBuffers_, data->ipaStatBuffers_, buffer);\n> > > >\n> > > >    \tret = params_->allocateBuffers(bufferCount, &paramsBuffers_);\n> > > >    \tif (ret < 0)\n> > > >    \t\treturn ret;\n> > > >\n> > > > -\tfor (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_) {\n> > > > -\t\tbuffer->setCookie(ipaBufferId++);\n> > > > -\t\tdata->ipaParamBuffers_.emplace_back(buffer->cookie(),\n> > > > -\t\t\t\t\t\t    buffer->planes());\n> > > > -\t\tavailableParamsBuffers_.push(buffer.get());\n> > > > -\t}\n> > > > +\tfor (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_)\n> > > > +\t\tpushBuffer(availableParamsBuffers_, data->ipaParamBuffers_, buffer);\n> > > >\n> > > >    \tif (data->ipa_) {\n> > > >    \t\tdata->ipa_->mapBuffers(data->ipaStatBuffers_, true);\n> > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > index 55d7d4442caf..99347c9f6258 100644\n> > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > @@ -1028,19 +1028,19 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n> > > >    \t\t\tavailableMainPathBuffers_.push(buffer.get());\n> > > >    \t}\n> > > >\n> > > > -\tfor (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) {\n> > > > +\tauto pushBuffer = [&](std::vector<IPABuffer> &buffers,\n> > > > +\t\t\t      std::queue<FrameBuffer *> &queue,\n> > > > +\t\t\t      const std::unique_ptr<FrameBuffer> &buffer) {\n> > >\n> > > Same here.\n> > >\n> > > Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> > >\n> > > >    \t\tbuffer->setCookie(ipaBufferId++);\n> > > > -\t\tdata->ipaBuffers_.emplace_back(buffer->cookie(),\n> > > > -\t\t\t\t\t       buffer->planes());\n> > > > -\t\tavailableParamBuffers_.push(buffer.get());\n> > > > -\t}\n> > > > +\t\tbuffers.emplace_back(buffer->cookie(), buffer->planes());\n> > > > +\t\tqueue.push(buffer.get());\n> > > > +\t};\n> > > >\n> > > > -\tfor (std::unique_ptr<FrameBuffer> &buffer : statBuffers_) {\n> > > > -\t\tbuffer->setCookie(ipaBufferId++);\n> > > > -\t\tdata->ipaBuffers_.emplace_back(buffer->cookie(),\n> > > > -\t\t\t\t\t       buffer->planes());\n> > > > -\t\tavailableStatBuffers_.push(buffer.get());\n> > > > -\t}\n> > > > +\tfor (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_)\n> > > > +\t\tpushBuffer(data->ipaBuffers_, availableParamBuffers_, buffer);\n> > > > +\n> > > > +\tfor (std::unique_ptr<FrameBuffer> &buffer : statBuffers_)\n> > > > +\t\tpushBuffer(data->ipaBuffers_, availableStatBuffers_, buffer);\n> > > >\n> > > >    \tdata->ipa_->mapBuffers(data->ipaBuffers_);\n> > > >\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 3057EBEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 19 Aug 2025 15:32:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 101BF692E1;\n\tTue, 19 Aug 2025 17:32:06 +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 242DF692DA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Aug 2025 17:32:04 +0200 (CEST)","from ideasonboard.com (mob-5-90-52-92.net.vodafone.it [5.90.52.92])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 892792F81;\n\tTue, 19 Aug 2025 17:31: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=\"GW1f35sF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1755617465;\n\tbh=OwpAOUPiyf03p8XxtcJqAute72URKX+XbDv5SiJD5Fg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GW1f35sFpghFX3if6mtQNrpoEoUf9Bq0H4uRneWTKIwhqKyILIHFJkscPOZWiBUak\n\two3c7M+r5SPUJrVir/HRUFmHh8hXcwwPMOSI/W8DzmLztHQ6GbxV/DkQLx9a8MTXsw\n\tzWRJiiPclVrXPYt7OdzADk0lSrGY9zk52byKR2uE=","Date":"Tue, 19 Aug 2025 17:31:59 +0200","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>","Cc":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>, \n\tlibcamera-devel@lists.libcamera.org, Daniel =?utf-8?b?UsOha29z?=\n\t<daniel.rakos@rastergrid.com>","Subject":"Re: [PATCH v2 5/8] pipelines: Use lambda functions to factor out\n\tbuffer mapping code","Message-ID":"<kmwcelzrhlupzbyw65ezj52iqb3efpylyel6x7qsabyjdbuaub@grtop5tt7pib>","References":"<20250815113400.20623-1-laurent.pinchart@ideasonboard.com>\n\t<HfCaQHEgoEqjy73NffYZVwR8gEgDItxLBhkRxvbizMvW-m_kBGD71KEvPLEMhHZDtdTfkYc69RxsHzF_oxs_Rg==@protonmail.internalid>\n\t<20250815113400.20623-6-laurent.pinchart@ideasonboard.com>\n\t<803158a1-4fd4-46c4-9370-9745fde7f463@ideasonboard.com>\n\t<20250819123724.GT5862@pendragon.ideasonboard.com>\n\t<800f2856-4467-472c-8066-3090657ed0d6@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<800f2856-4467-472c-8066-3090657ed0d6@ideasonboard.com>","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35522,"web_url":"https://patchwork.libcamera.org/comment/35522/","msgid":"<20250819160903.GA25064@pendragon.ideasonboard.com>","date":"2025-08-19T16:09:03","subject":"Re: [PATCH v2 5/8] pipelines: Use lambda functions to factor out\n\tbuffer mapping code","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Aug 19, 2025 at 05:31:59PM +0200, Jacopo Mondi wrote:\n> On Tue, Aug 19, 2025 at 03:01:50PM +0200, Barnabás Pőcze wrote:\n> > 2025. 08. 19. 14:37 keltezéssel, Laurent Pinchart írta:\n> > > On Tue, Aug 19, 2025 at 02:15:44PM +0200, Barnabás Pőcze wrote:\n> > > > 2025. 08. 15. 13:33 keltezéssel, Laurent Pinchart írta:\n> > > > > Multiple pipeline handlers duplicate code related to mapping params and\n> > > > > stats buffers to IPA modules. Factor out the code to lambda functions to\n> > > > > share it between the two buffer types.\n> > > > >\n> > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > > > ---\n> > > > >    src/libcamera/pipeline/ipu3/ipu3.cpp         | 16 +++++++------\n> > > > >    src/libcamera/pipeline/mali-c55/mali-c55.cpp | 24 ++++++++++----------\n> > > > >    src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 22 +++++++++---------\n> > > > >    3 files changed, 32 insertions(+), 30 deletions(-)\n> > > > >\n> > > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > > index ad20810e6a26..7ae85e5063db 100644\n> > > > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > > @@ -678,15 +678,17 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n> > > > >    \t/* Map buffers to the IPA. */\n> > > > >    \tunsigned int ipaBufferId = 1;\n> > > > >\n> > > > > -\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) {\n> > > > > +\tauto pushBuffer = [&](std::vector<IPABuffer> &buffers,\n> > > > > +\t\t\t      const std::unique_ptr<FrameBuffer> &buffer) {\n> > > >\n> > > > I feel like the `buffers` argument is a bit unnecessary since it's\n> > > > only ever `ipaBuffers_`.\n> \n> Or you can pass the argument but avoid the default capture ? But then\n> you won't be able to access ipaBufferId.. (yes, you could capture that\n> one only, but I'm not sure all of this is worth it ?)\n> \n> > > You know, one of the reasons I like your reviews is because you point\n> > > out things that I wasn't sure about myself when sending patches :-) I've\n> > > gone back and forth on this, and decided to add the buffers argument for\n> > > consistency between pipeline handlers. If you think it's best to drop\n> > > it, I'll do that. What's your preference ?\n> >\n> > My thinking is that it's highly specific, so might as well go for the simplest\n> > option. But I am not sure, I don't think it matters much in the end.\n> >\n> > > > >    \t\tbuffer->setCookie(ipaBufferId++);\n> > > > > -\t\tipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n> > > > > -\t}\n> > > > > +\t\tbuffers.emplace_back(buffer->cookie(), buffer->planes());\n> > > > > +\t};\n> > > > >\n> > > > > -\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) {\n> \n> I wonder if you could pass 'imgu->statsBuffers_' instead and for-loop\n> in the lambda..\n\nInteresting idea. I'll post of a v2 of this patch, you and Barnabás can\ndecide what you like best.\n\n> Anyways\n> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> \n> > > > > -\t\tbuffer->setCookie(ipaBufferId++);\n> > > > > -\t\tipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n> > > > > -\t}\n> > > > > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_)\n> > > > > +\t\tpushBuffer(ipaBuffers_, buffer);\n> > > > > +\n> > > > > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_)\n> > > > > +\t\tpushBuffer(ipaBuffers_, buffer);\n> > > > >\n> > > > >    \tdata->ipa_->mapBuffers(ipaBuffers_);\n> > > > >\n> > > > > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > > > index 76341ed3f363..f03a03fef35c 100644\n> > > > > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > > > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > > > @@ -1133,27 +1133,27 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera)\n> > > > >    \t\tdata->dsStream_.configuration().bufferCount,\n> > > > >    \t});\n> > > > >\n> > > > > +\tauto pushBuffer = [&](std::queue<FrameBuffer *> &queue,\n> > > > > +\t\t\t      std::vector<IPABuffer> &buffers,\n> > > > > +\t\t\t      const std::unique_ptr<FrameBuffer> &buffer) {\n> > > > > +\t\tbuffer->setCookie(ipaBufferId++);\n> > > > > +\t\tbuffers.emplace_back(buffer->cookie(), buffer->planes());\n> > > > > +\t\tqueue.push(buffer.get());\n> > > > > +\t};\n> > > > > +\n> > > > >    \tret = stats_->allocateBuffers(bufferCount, &statsBuffers_);\n> > > > >    \tif (ret < 0)\n> > > > >    \t\treturn ret;\n> > > > >\n> > > > > -\tfor (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_) {\n> > > > > -\t\tbuffer->setCookie(ipaBufferId++);\n> > > > > -\t\tdata->ipaStatBuffers_.emplace_back(buffer->cookie(),\n> > > > > -\t\t\t\t\t\t   buffer->planes());\n> > > > > -\t\tavailableStatsBuffers_.push(buffer.get());\n> > > > > -\t}\n> > > > > +\tfor (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_)\n> > > > > +\t\tpushBuffer(availableStatsBuffers_, data->ipaStatBuffers_, buffer);\n> > > > >\n> > > > >    \tret = params_->allocateBuffers(bufferCount, &paramsBuffers_);\n> > > > >    \tif (ret < 0)\n> > > > >    \t\treturn ret;\n> > > > >\n> > > > > -\tfor (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_) {\n> > > > > -\t\tbuffer->setCookie(ipaBufferId++);\n> > > > > -\t\tdata->ipaParamBuffers_.emplace_back(buffer->cookie(),\n> > > > > -\t\t\t\t\t\t    buffer->planes());\n> > > > > -\t\tavailableParamsBuffers_.push(buffer.get());\n> > > > > -\t}\n> > > > > +\tfor (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_)\n> > > > > +\t\tpushBuffer(availableParamsBuffers_, data->ipaParamBuffers_, buffer);\n> > > > >\n> > > > >    \tif (data->ipa_) {\n> > > > >    \t\tdata->ipa_->mapBuffers(data->ipaStatBuffers_, true);\n> > > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > > index 55d7d4442caf..99347c9f6258 100644\n> > > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > > @@ -1028,19 +1028,19 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n> > > > >    \t\t\tavailableMainPathBuffers_.push(buffer.get());\n> > > > >    \t}\n> > > > >\n> > > > > -\tfor (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) {\n> > > > > +\tauto pushBuffer = [&](std::vector<IPABuffer> &buffers,\n> > > > > +\t\t\t      std::queue<FrameBuffer *> &queue,\n> > > > > +\t\t\t      const std::unique_ptr<FrameBuffer> &buffer) {\n> > > >\n> > > > Same here.\n> > > >\n> > > > Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> > > >\n> > > > >    \t\tbuffer->setCookie(ipaBufferId++);\n> > > > > -\t\tdata->ipaBuffers_.emplace_back(buffer->cookie(),\n> > > > > -\t\t\t\t\t       buffer->planes());\n> > > > > -\t\tavailableParamBuffers_.push(buffer.get());\n> > > > > -\t}\n> > > > > +\t\tbuffers.emplace_back(buffer->cookie(), buffer->planes());\n> > > > > +\t\tqueue.push(buffer.get());\n> > > > > +\t};\n> > > > >\n> > > > > -\tfor (std::unique_ptr<FrameBuffer> &buffer : statBuffers_) {\n> > > > > -\t\tbuffer->setCookie(ipaBufferId++);\n> > > > > -\t\tdata->ipaBuffers_.emplace_back(buffer->cookie(),\n> > > > > -\t\t\t\t\t       buffer->planes());\n> > > > > -\t\tavailableStatBuffers_.push(buffer.get());\n> > > > > -\t}\n> > > > > +\tfor (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_)\n> > > > > +\t\tpushBuffer(data->ipaBuffers_, availableParamBuffers_, buffer);\n> > > > > +\n> > > > > +\tfor (std::unique_ptr<FrameBuffer> &buffer : statBuffers_)\n> > > > > +\t\tpushBuffer(data->ipaBuffers_, availableStatBuffers_, buffer);\n> > > > >\n> > > > >    \tdata->ipa_->mapBuffers(data->ipaBuffers_);\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 1834DBD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 19 Aug 2025 16:09:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 75ABE692D5;\n\tTue, 19 Aug 2025 18:09:27 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C3600692CA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Aug 2025 18:09:25 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 77F212391; \n\tTue, 19 Aug 2025 18:08:27 +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=\"aERLuAID\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1755619707;\n\tbh=msw5MF5HJJrjNPvokwnlPhefnwroNnXJqznwj3zWnyA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=aERLuAIDvll0JMsnI1TwRUbveJvdBaaoI1h5RTG5bRoa54u+M0Je+T705F/1buJ00\n\tjupjXHPcuNwvV4w/lj6GZkDhltPpU7QBp8BMs+UKz/Az4KP7FByJSs8cMDe+5sUTjn\n\tBOG3N3SB6BvjcIt+6s9EGHznINkf3NetNgnAQbyA=","Date":"Tue, 19 Aug 2025 19:09:03 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org, Daniel =?utf-8?b?UsOha29z?=\n\t<daniel.rakos@rastergrid.com>","Subject":"Re: [PATCH v2 5/8] pipelines: Use lambda functions to factor out\n\tbuffer mapping code","Message-ID":"<20250819160903.GA25064@pendragon.ideasonboard.com>","References":"<20250815113400.20623-1-laurent.pinchart@ideasonboard.com>\n\t<HfCaQHEgoEqjy73NffYZVwR8gEgDItxLBhkRxvbizMvW-m_kBGD71KEvPLEMhHZDtdTfkYc69RxsHzF_oxs_Rg==@protonmail.internalid>\n\t<20250815113400.20623-6-laurent.pinchart@ideasonboard.com>\n\t<803158a1-4fd4-46c4-9370-9745fde7f463@ideasonboard.com>\n\t<20250819123724.GT5862@pendragon.ideasonboard.com>\n\t<800f2856-4467-472c-8066-3090657ed0d6@ideasonboard.com>\n\t<kmwcelzrhlupzbyw65ezj52iqb3efpylyel6x7qsabyjdbuaub@grtop5tt7pib>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<kmwcelzrhlupzbyw65ezj52iqb3efpylyel6x7qsabyjdbuaub@grtop5tt7pib>","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":35523,"web_url":"https://patchwork.libcamera.org/comment/35523/","msgid":"<xy7qceyahvkttgtxszsi7fqnrnbf5zj2mwvc6gerq4wwr6jp35@7nponj4i2aqg>","date":"2025-08-19T18:00:12","subject":"Re: [PATCH v2 5/8] pipelines: Use lambda functions to factor out\n\tbuffer mapping code","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"On Tue, Aug 19, 2025 at 07:09:03PM +0300, Laurent Pinchart wrote:\n> On Tue, Aug 19, 2025 at 05:31:59PM +0200, Jacopo Mondi wrote:\n> > On Tue, Aug 19, 2025 at 03:01:50PM +0200, Barnabás Pőcze wrote:\n> > > 2025. 08. 19. 14:37 keltezéssel, Laurent Pinchart írta:\n> > > > On Tue, Aug 19, 2025 at 02:15:44PM +0200, Barnabás Pőcze wrote:\n> > > > > 2025. 08. 15. 13:33 keltezéssel, Laurent Pinchart írta:\n> > > > > > Multiple pipeline handlers duplicate code related to mapping params and\n> > > > > > stats buffers to IPA modules. Factor out the code to lambda functions to\n> > > > > > share it between the two buffer types.\n> > > > > >\n> > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > > > > ---\n> > > > > >    src/libcamera/pipeline/ipu3/ipu3.cpp         | 16 +++++++------\n> > > > > >    src/libcamera/pipeline/mali-c55/mali-c55.cpp | 24 ++++++++++----------\n> > > > > >    src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 22 +++++++++---------\n> > > > > >    3 files changed, 32 insertions(+), 30 deletions(-)\n> > > > > >\n> > > > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > > > index ad20810e6a26..7ae85e5063db 100644\n> > > > > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > > > @@ -678,15 +678,17 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n> > > > > >    \t/* Map buffers to the IPA. */\n> > > > > >    \tunsigned int ipaBufferId = 1;\n> > > > > >\n> > > > > > -\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) {\n> > > > > > +\tauto pushBuffer = [&](std::vector<IPABuffer> &buffers,\n> > > > > > +\t\t\t      const std::unique_ptr<FrameBuffer> &buffer) {\n> > > > >\n> > > > > I feel like the `buffers` argument is a bit unnecessary since it's\n> > > > > only ever `ipaBuffers_`.\n> >\n> > Or you can pass the argument but avoid the default capture ? But then\n> > you won't be able to access ipaBufferId.. (yes, you could capture that\n> > one only, but I'm not sure all of this is worth it ?)\n> >\n> > > > You know, one of the reasons I like your reviews is because you point\n> > > > out things that I wasn't sure about myself when sending patches :-) I've\n> > > > gone back and forth on this, and decided to add the buffers argument for\n> > > > consistency between pipeline handlers. If you think it's best to drop\n> > > > it, I'll do that. What's your preference ?\n> > >\n> > > My thinking is that it's highly specific, so might as well go for the simplest\n> > > option. But I am not sure, I don't think it matters much in the end.\n> > >\n> > > > > >    \t\tbuffer->setCookie(ipaBufferId++);\n> > > > > > -\t\tipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n> > > > > > -\t}\n> > > > > > +\t\tbuffers.emplace_back(buffer->cookie(), buffer->planes());\n> > > > > > +\t};\n> > > > > >\n> > > > > > -\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) {\n> >\n> > I wonder if you could pass 'imgu->statsBuffers_' instead and for-loop\n> > in the lambda..\n>\n> Interesting idea. I'll post of a v2 of this patch, you and Barnabás can\n> decide what you like best.\n\nOnly if you really prefer, I think it doesn't make much difference :)\n\n>\n> > Anyways\n> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n> >\n> > > > > > -\t\tbuffer->setCookie(ipaBufferId++);\n> > > > > > -\t\tipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());\n> > > > > > -\t}\n> > > > > > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_)\n> > > > > > +\t\tpushBuffer(ipaBuffers_, buffer);\n> > > > > > +\n> > > > > > +\tfor (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_)\n> > > > > > +\t\tpushBuffer(ipaBuffers_, buffer);\n> > > > > >\n> > > > > >    \tdata->ipa_->mapBuffers(ipaBuffers_);\n> > > > > >\n> > > > > > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > > > > index 76341ed3f363..f03a03fef35c 100644\n> > > > > > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > > > > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp\n> > > > > > @@ -1133,27 +1133,27 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera)\n> > > > > >    \t\tdata->dsStream_.configuration().bufferCount,\n> > > > > >    \t});\n> > > > > >\n> > > > > > +\tauto pushBuffer = [&](std::queue<FrameBuffer *> &queue,\n> > > > > > +\t\t\t      std::vector<IPABuffer> &buffers,\n> > > > > > +\t\t\t      const std::unique_ptr<FrameBuffer> &buffer) {\n> > > > > > +\t\tbuffer->setCookie(ipaBufferId++);\n> > > > > > +\t\tbuffers.emplace_back(buffer->cookie(), buffer->planes());\n> > > > > > +\t\tqueue.push(buffer.get());\n> > > > > > +\t};\n> > > > > > +\n> > > > > >    \tret = stats_->allocateBuffers(bufferCount, &statsBuffers_);\n> > > > > >    \tif (ret < 0)\n> > > > > >    \t\treturn ret;\n> > > > > >\n> > > > > > -\tfor (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_) {\n> > > > > > -\t\tbuffer->setCookie(ipaBufferId++);\n> > > > > > -\t\tdata->ipaStatBuffers_.emplace_back(buffer->cookie(),\n> > > > > > -\t\t\t\t\t\t   buffer->planes());\n> > > > > > -\t\tavailableStatsBuffers_.push(buffer.get());\n> > > > > > -\t}\n> > > > > > +\tfor (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_)\n> > > > > > +\t\tpushBuffer(availableStatsBuffers_, data->ipaStatBuffers_, buffer);\n> > > > > >\n> > > > > >    \tret = params_->allocateBuffers(bufferCount, &paramsBuffers_);\n> > > > > >    \tif (ret < 0)\n> > > > > >    \t\treturn ret;\n> > > > > >\n> > > > > > -\tfor (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_) {\n> > > > > > -\t\tbuffer->setCookie(ipaBufferId++);\n> > > > > > -\t\tdata->ipaParamBuffers_.emplace_back(buffer->cookie(),\n> > > > > > -\t\t\t\t\t\t    buffer->planes());\n> > > > > > -\t\tavailableParamsBuffers_.push(buffer.get());\n> > > > > > -\t}\n> > > > > > +\tfor (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_)\n> > > > > > +\t\tpushBuffer(availableParamsBuffers_, data->ipaParamBuffers_, buffer);\n> > > > > >\n> > > > > >    \tif (data->ipa_) {\n> > > > > >    \t\tdata->ipa_->mapBuffers(data->ipaStatBuffers_, true);\n> > > > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > > > index 55d7d4442caf..99347c9f6258 100644\n> > > > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > > > @@ -1028,19 +1028,19 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n> > > > > >    \t\t\tavailableMainPathBuffers_.push(buffer.get());\n> > > > > >    \t}\n> > > > > >\n> > > > > > -\tfor (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) {\n> > > > > > +\tauto pushBuffer = [&](std::vector<IPABuffer> &buffers,\n> > > > > > +\t\t\t      std::queue<FrameBuffer *> &queue,\n> > > > > > +\t\t\t      const std::unique_ptr<FrameBuffer> &buffer) {\n> > > > >\n> > > > > Same here.\n> > > > >\n> > > > > Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>\n> > > > >\n> > > > > >    \t\tbuffer->setCookie(ipaBufferId++);\n> > > > > > -\t\tdata->ipaBuffers_.emplace_back(buffer->cookie(),\n> > > > > > -\t\t\t\t\t       buffer->planes());\n> > > > > > -\t\tavailableParamBuffers_.push(buffer.get());\n> > > > > > -\t}\n> > > > > > +\t\tbuffers.emplace_back(buffer->cookie(), buffer->planes());\n> > > > > > +\t\tqueue.push(buffer.get());\n> > > > > > +\t};\n> > > > > >\n> > > > > > -\tfor (std::unique_ptr<FrameBuffer> &buffer : statBuffers_) {\n> > > > > > -\t\tbuffer->setCookie(ipaBufferId++);\n> > > > > > -\t\tdata->ipaBuffers_.emplace_back(buffer->cookie(),\n> > > > > > -\t\t\t\t\t       buffer->planes());\n> > > > > > -\t\tavailableStatBuffers_.push(buffer.get());\n> > > > > > -\t}\n> > > > > > +\tfor (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_)\n> > > > > > +\t\tpushBuffer(data->ipaBuffers_, availableParamBuffers_, buffer);\n> > > > > > +\n> > > > > > +\tfor (std::unique_ptr<FrameBuffer> &buffer : statBuffers_)\n> > > > > > +\t\tpushBuffer(data->ipaBuffers_, availableStatBuffers_, buffer);\n> > > > > >\n> > > > > >    \tdata->ipa_->mapBuffers(data->ipaBuffers_);\n> > > > > >\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 A7CFFBD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 19 Aug 2025 18:00:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 33FB9692CA;\n\tTue, 19 Aug 2025 20:00:19 +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 BC9DC692CA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Aug 2025 20:00:16 +0200 (CEST)","from ideasonboard.com (mob-5-90-52-92.net.vodafone.it [5.90.52.92])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8448C107;\n\tTue, 19 Aug 2025 19:59:18 +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=\"q60huwmX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1755626358;\n\tbh=S0iUz1eNdsFrpYtA9gr0/WBHfEvdyWsDS8nWBn695PU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=q60huwmXTqzWaa5Uuatt/XYFmhjcb/W2FytJ5XCPHT4AfaRDx7vzLNLw6VaTl0Uv4\n\t+NS3JJh2xMoxbg7uP0/FOGvnMGR30QVa6UOGIfdVbl5pcZW2bi1Yoo8umc+7o9qgQ8\n\twxFKH5fSmyae5bf4MUFAIh1jJxnFipqMw+DwiPQ4=","Date":"Tue, 19 Aug 2025 20:00:12 +0200","From":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>, =?utf-8?b?QmFybmFiw6Fz?=\n\t=?utf-8?q?_P=C5=91cze?= <barnabas.pocze@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org, Daniel =?utf-8?b?UsOha29z?=\n\t<daniel.rakos@rastergrid.com>","Subject":"Re: [PATCH v2 5/8] pipelines: Use lambda functions to factor out\n\tbuffer mapping code","Message-ID":"<xy7qceyahvkttgtxszsi7fqnrnbf5zj2mwvc6gerq4wwr6jp35@7nponj4i2aqg>","References":"<20250815113400.20623-1-laurent.pinchart@ideasonboard.com>\n\t<HfCaQHEgoEqjy73NffYZVwR8gEgDItxLBhkRxvbizMvW-m_kBGD71KEvPLEMhHZDtdTfkYc69RxsHzF_oxs_Rg==@protonmail.internalid>\n\t<20250815113400.20623-6-laurent.pinchart@ideasonboard.com>\n\t<803158a1-4fd4-46c4-9370-9745fde7f463@ideasonboard.com>\n\t<20250819123724.GT5862@pendragon.ideasonboard.com>\n\t<800f2856-4467-472c-8066-3090657ed0d6@ideasonboard.com>\n\t<kmwcelzrhlupzbyw65ezj52iqb3efpylyel6x7qsabyjdbuaub@grtop5tt7pib>\n\t<20250819160903.GA25064@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250819160903.GA25064@pendragon.ideasonboard.com>","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]