[v3,1/2] pipelines: Use lambda functions to factor out buffer mapping code

24283 diff mbox series

Multiple pipeline handlers duplicate code related to mapping params and
stats buffers to IPA modules. Factor out the code to lambda functions to
share it between the two buffer types.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
Changes since v1:

- Move for loop to pushBuffers() function
- Drop unneeded lambda function arguments
---
 src/libcamera/pipeline/ipu3/ipu3.cpp         | 16 ++++++------
 src/libcamera/pipeline/mali-c55/mali-c55.cpp | 26 +++++++++++---------
 src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 23 +++++++++--------
 3 files changed, 33 insertions(+), 32 deletions(-)

Quoting Laurent Pinchart (2025-09-04 14:46:40)
> Multiple pipeline handlers duplicate code related to mapping params and
> stats buffers to IPA modules. Factor out the code to lambda functions to
> share it between the two buffer types.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
> Changes since v1:
> 
> - Move for loop to pushBuffers() function
> - Drop unneeded lambda function arguments
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp         | 16 ++++++------
>  src/libcamera/pipeline/mali-c55/mali-c55.cpp | 26 +++++++++++---------
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp     | 23 +++++++++--------
>  3 files changed, 33 insertions(+), 32 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index ad20810e6a26..bfbc80af6a3c 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -678,15 +678,15 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera)
>         /* Map buffers to the IPA. */
>         unsigned int ipaBufferId = 1;
>  
> -       for (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) {
> -               buffer->setCookie(ipaBufferId++);
> -               ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());
> -       }
> +       auto pushBuffers = [&](const std::vector<std::unique_ptr<FrameBuffer>> &buffers) {
> +               for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
> +                       buffer->setCookie(ipaBufferId++);
> +                       ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());
> +               }
> +       };
>  
> -       for (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) {
> -               buffer->setCookie(ipaBufferId++);
> -               ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());
> -       }
> +       pushBuffers(imgu->paramBuffers_);
> +       pushBuffers(imgu->statBuffers_);

Seems ok to me. I'm not sure I would have named this pushBuffers but I
haven't got a better name so :


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

>  
>         data->ipa_->mapBuffers(ipaBuffers_);
>  
> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> index 76341ed3f363..97996399fc51 100644
> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> @@ -1133,27 +1133,29 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera)
>                 data->dsStream_.configuration().bufferCount,
>         });
>  
> +       auto pushBuffers = [&](const std::vector<std::unique_ptr<FrameBuffer>> &buffers,
> +                              std::queue<FrameBuffer *> &queue,
> +                              std::vector<IPABuffer> &ipaBuffers) {
> +               for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
> +                       buffer->setCookie(ipaBufferId++);
> +                       ipaBuffers.emplace_back(buffer->cookie(), buffer->planes());
> +                       queue.push(buffer.get());
> +               }
> +       };
> +
>         ret = stats_->allocateBuffers(bufferCount, &statsBuffers_);
>         if (ret < 0)
>                 return ret;
>  
> -       for (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_) {
> -               buffer->setCookie(ipaBufferId++);
> -               data->ipaStatBuffers_.emplace_back(buffer->cookie(),
> -                                                  buffer->planes());
> -               availableStatsBuffers_.push(buffer.get());
> -       }
> +       pushBuffers(statsBuffers_, availableStatsBuffers_,
> +                   data->ipaStatBuffers_);
>  
>         ret = params_->allocateBuffers(bufferCount, &paramsBuffers_);
>         if (ret < 0)
>                 return ret;
>  
> -       for (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_) {
> -               buffer->setCookie(ipaBufferId++);
> -               data->ipaParamBuffers_.emplace_back(buffer->cookie(),
> -                                                   buffer->planes());
> -               availableParamsBuffers_.push(buffer.get());
> -       }
> +       pushBuffers(paramsBuffers_, availableParamsBuffers_,
> +                   data->ipaParamBuffers_);
>  
>         if (data->ipa_) {
>                 data->ipa_->mapBuffers(data->ipaStatBuffers_, true);
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 55d7d4442caf..291f96836c5e 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -1028,19 +1028,18 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
>                         availableMainPathBuffers_.push(buffer.get());
>         }
>  
> -       for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) {
> -               buffer->setCookie(ipaBufferId++);
> -               data->ipaBuffers_.emplace_back(buffer->cookie(),
> -                                              buffer->planes());
> -               availableParamBuffers_.push(buffer.get());
> -       }
> +       auto pushBuffers = [&](const std::vector<std::unique_ptr<FrameBuffer>> &buffers,
> +                              std::queue<FrameBuffer *> &queue) {
> +               for (const std::unique_ptr<FrameBuffer> &buffer : buffers) {
> +                       buffer->setCookie(ipaBufferId++);
> +                       data->ipaBuffers_.emplace_back(buffer->cookie(),
> +                                                      buffer->planes());
> +                       queue.push(buffer.get());
> +               }
> +       };
>  
> -       for (std::unique_ptr<FrameBuffer> &buffer : statBuffers_) {
> -               buffer->setCookie(ipaBufferId++);
> -               data->ipaBuffers_.emplace_back(buffer->cookie(),
> -                                              buffer->planes());
> -               availableStatBuffers_.push(buffer.get());
> -       }
> +       pushBuffers(paramBuffers_, availableParamBuffers_);
> +       pushBuffers(statBuffers_, availableStatBuffers_);
>  
>         data->ipa_->mapBuffers(data->ipaBuffers_);
>  
> -- 
> Regards,
> 
> Laurent Pinchart
>