| Message ID | 20250815113400.20623-6-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series |
|
| Related | show |
Hi 2025. 08. 15. 13:33 keltezéssel, Laurent Pinchart írta: > 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> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 16 +++++++------ > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 24 ++++++++++---------- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 22 +++++++++--------- > 3 files changed, 32 insertions(+), 30 deletions(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index ad20810e6a26..7ae85e5063db 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -678,15 +678,17 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera) > /* Map buffers to the IPA. */ > unsigned int ipaBufferId = 1; > > - for (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) { > + auto pushBuffer = [&](std::vector<IPABuffer> &buffers, > + const std::unique_ptr<FrameBuffer> &buffer) { I feel like the `buffers` argument is a bit unnecessary since it's only ever `ipaBuffers_`. > buffer->setCookie(ipaBufferId++); > - ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes()); > - } > + buffers.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()); > - } > + for (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) > + pushBuffer(ipaBuffers_, buffer); > + > + for (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) > + pushBuffer(ipaBuffers_, buffer); > > 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..f03a03fef35c 100644 > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > @@ -1133,27 +1133,27 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera) > data->dsStream_.configuration().bufferCount, > }); > > + auto pushBuffer = [&](std::queue<FrameBuffer *> &queue, > + std::vector<IPABuffer> &buffers, > + const std::unique_ptr<FrameBuffer> &buffer) { > + buffer->setCookie(ipaBufferId++); > + buffers.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()); > - } > + for (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_) > + pushBuffer(availableStatsBuffers_, data->ipaStatBuffers_, buffer); > > ret = params_->allocateBuffers(bufferCount, ¶msBuffers_); > 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()); > - } > + for (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_) > + pushBuffer(availableParamsBuffers_, data->ipaParamBuffers_, buffer); > > 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..99347c9f6258 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -1028,19 +1028,19 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) > availableMainPathBuffers_.push(buffer.get()); > } > > - for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) { > + auto pushBuffer = [&](std::vector<IPABuffer> &buffers, > + std::queue<FrameBuffer *> &queue, > + const std::unique_ptr<FrameBuffer> &buffer) { Same here. Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > buffer->setCookie(ipaBufferId++); > - data->ipaBuffers_.emplace_back(buffer->cookie(), > - buffer->planes()); > - availableParamBuffers_.push(buffer.get()); > - } > + buffers.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()); > - } > + for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) > + pushBuffer(data->ipaBuffers_, availableParamBuffers_, buffer); > + > + for (std::unique_ptr<FrameBuffer> &buffer : statBuffers_) > + pushBuffer(data->ipaBuffers_, availableStatBuffers_, buffer); > > data->ipa_->mapBuffers(data->ipaBuffers_); > > -- > Regards, > > Laurent Pinchart >
On Tue, Aug 19, 2025 at 02:15:44PM +0200, Barnabás Pőcze wrote: > 2025. 08. 15. 13:33 keltezéssel, Laurent Pinchart írta: > > 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> > > --- > > src/libcamera/pipeline/ipu3/ipu3.cpp | 16 +++++++------ > > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 24 ++++++++++---------- > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 22 +++++++++--------- > > 3 files changed, 32 insertions(+), 30 deletions(-) > > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > index ad20810e6a26..7ae85e5063db 100644 > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > @@ -678,15 +678,17 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera) > > /* Map buffers to the IPA. */ > > unsigned int ipaBufferId = 1; > > > > - for (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) { > > + auto pushBuffer = [&](std::vector<IPABuffer> &buffers, > > + const std::unique_ptr<FrameBuffer> &buffer) { > > I feel like the `buffers` argument is a bit unnecessary since it's > only ever `ipaBuffers_`. You know, one of the reasons I like your reviews is because you point out things that I wasn't sure about myself when sending patches :-) I've gone back and forth on this, and decided to add the buffers argument for consistency between pipeline handlers. If you think it's best to drop it, I'll do that. What's your preference ? > > buffer->setCookie(ipaBufferId++); > > - ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes()); > > - } > > + buffers.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()); > > - } > > + for (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) > > + pushBuffer(ipaBuffers_, buffer); > > + > > + for (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) > > + pushBuffer(ipaBuffers_, buffer); > > > > 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..f03a03fef35c 100644 > > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > @@ -1133,27 +1133,27 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera) > > data->dsStream_.configuration().bufferCount, > > }); > > > > + auto pushBuffer = [&](std::queue<FrameBuffer *> &queue, > > + std::vector<IPABuffer> &buffers, > > + const std::unique_ptr<FrameBuffer> &buffer) { > > + buffer->setCookie(ipaBufferId++); > > + buffers.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()); > > - } > > + for (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_) > > + pushBuffer(availableStatsBuffers_, data->ipaStatBuffers_, buffer); > > > > ret = params_->allocateBuffers(bufferCount, ¶msBuffers_); > > 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()); > > - } > > + for (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_) > > + pushBuffer(availableParamsBuffers_, data->ipaParamBuffers_, buffer); > > > > 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..99347c9f6258 100644 > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > @@ -1028,19 +1028,19 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) > > availableMainPathBuffers_.push(buffer.get()); > > } > > > > - for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) { > > + auto pushBuffer = [&](std::vector<IPABuffer> &buffers, > > + std::queue<FrameBuffer *> &queue, > > + const std::unique_ptr<FrameBuffer> &buffer) { > > Same here. > > Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > > buffer->setCookie(ipaBufferId++); > > - data->ipaBuffers_.emplace_back(buffer->cookie(), > > - buffer->planes()); > > - availableParamBuffers_.push(buffer.get()); > > - } > > + buffers.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()); > > - } > > + for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) > > + pushBuffer(data->ipaBuffers_, availableParamBuffers_, buffer); > > + > > + for (std::unique_ptr<FrameBuffer> &buffer : statBuffers_) > > + pushBuffer(data->ipaBuffers_, availableStatBuffers_, buffer); > > > > data->ipa_->mapBuffers(data->ipaBuffers_); > >
2025. 08. 19. 14:37 keltezéssel, Laurent Pinchart írta: > On Tue, Aug 19, 2025 at 02:15:44PM +0200, Barnabás Pőcze wrote: >> 2025. 08. 15. 13:33 keltezéssel, Laurent Pinchart írta: >>> 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> >>> --- >>> src/libcamera/pipeline/ipu3/ipu3.cpp | 16 +++++++------ >>> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 24 ++++++++++---------- >>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 22 +++++++++--------- >>> 3 files changed, 32 insertions(+), 30 deletions(-) >>> >>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp >>> index ad20810e6a26..7ae85e5063db 100644 >>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp >>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp >>> @@ -678,15 +678,17 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera) >>> /* Map buffers to the IPA. */ >>> unsigned int ipaBufferId = 1; >>> >>> - for (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) { >>> + auto pushBuffer = [&](std::vector<IPABuffer> &buffers, >>> + const std::unique_ptr<FrameBuffer> &buffer) { >> >> I feel like the `buffers` argument is a bit unnecessary since it's >> only ever `ipaBuffers_`. > > You know, one of the reasons I like your reviews is because you point > out things that I wasn't sure about myself when sending patches :-) I've > gone back and forth on this, and decided to add the buffers argument for > consistency between pipeline handlers. If you think it's best to drop > it, I'll do that. What's your preference ? My thinking is that it's highly specific, so might as well go for the simplest option. But I am not sure, I don't think it matters much in the end. > >>> buffer->setCookie(ipaBufferId++); >>> - ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes()); >>> - } >>> + buffers.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()); >>> - } >>> + for (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) >>> + pushBuffer(ipaBuffers_, buffer); >>> + >>> + for (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) >>> + pushBuffer(ipaBuffers_, buffer); >>> >>> 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..f03a03fef35c 100644 >>> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp >>> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp >>> @@ -1133,27 +1133,27 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera) >>> data->dsStream_.configuration().bufferCount, >>> }); >>> >>> + auto pushBuffer = [&](std::queue<FrameBuffer *> &queue, >>> + std::vector<IPABuffer> &buffers, >>> + const std::unique_ptr<FrameBuffer> &buffer) { >>> + buffer->setCookie(ipaBufferId++); >>> + buffers.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()); >>> - } >>> + for (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_) >>> + pushBuffer(availableStatsBuffers_, data->ipaStatBuffers_, buffer); >>> >>> ret = params_->allocateBuffers(bufferCount, ¶msBuffers_); >>> 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()); >>> - } >>> + for (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_) >>> + pushBuffer(availableParamsBuffers_, data->ipaParamBuffers_, buffer); >>> >>> 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..99347c9f6258 100644 >>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp >>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp >>> @@ -1028,19 +1028,19 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) >>> availableMainPathBuffers_.push(buffer.get()); >>> } >>> >>> - for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) { >>> + auto pushBuffer = [&](std::vector<IPABuffer> &buffers, >>> + std::queue<FrameBuffer *> &queue, >>> + const std::unique_ptr<FrameBuffer> &buffer) { >> >> Same here. >> >> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> >>> buffer->setCookie(ipaBufferId++); >>> - data->ipaBuffers_.emplace_back(buffer->cookie(), >>> - buffer->planes()); >>> - availableParamBuffers_.push(buffer.get()); >>> - } >>> + buffers.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()); >>> - } >>> + for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) >>> + pushBuffer(data->ipaBuffers_, availableParamBuffers_, buffer); >>> + >>> + for (std::unique_ptr<FrameBuffer> &buffer : statBuffers_) >>> + pushBuffer(data->ipaBuffers_, availableStatBuffers_, buffer); >>> >>> data->ipa_->mapBuffers(data->ipaBuffers_); >>> >
Hi On Tue, Aug 19, 2025 at 03:01:50PM +0200, Barnabás Pőcze wrote: > 2025. 08. 19. 14:37 keltezéssel, Laurent Pinchart írta: > > On Tue, Aug 19, 2025 at 02:15:44PM +0200, Barnabás Pőcze wrote: > > > 2025. 08. 15. 13:33 keltezéssel, Laurent Pinchart írta: > > > > 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> > > > > --- > > > > src/libcamera/pipeline/ipu3/ipu3.cpp | 16 +++++++------ > > > > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 24 ++++++++++---------- > > > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 22 +++++++++--------- > > > > 3 files changed, 32 insertions(+), 30 deletions(-) > > > > > > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > > > index ad20810e6a26..7ae85e5063db 100644 > > > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > > > @@ -678,15 +678,17 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera) > > > > /* Map buffers to the IPA. */ > > > > unsigned int ipaBufferId = 1; > > > > > > > > - for (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) { > > > > + auto pushBuffer = [&](std::vector<IPABuffer> &buffers, > > > > + const std::unique_ptr<FrameBuffer> &buffer) { > > > > > > I feel like the `buffers` argument is a bit unnecessary since it's > > > only ever `ipaBuffers_`. > > Or you can pass the argument but avoid the default capture ? But then you won't be able to access ipaBufferId.. (yes, you could capture that one only, but I'm not sure all of this is worth it ?) > > You know, one of the reasons I like your reviews is because you point > > out things that I wasn't sure about myself when sending patches :-) I've > > gone back and forth on this, and decided to add the buffers argument for > > consistency between pipeline handlers. If you think it's best to drop > > it, I'll do that. What's your preference ? > > My thinking is that it's highly specific, so might as well go for the simplest > option. But I am not sure, I don't think it matters much in the end. > > > > > > > > buffer->setCookie(ipaBufferId++); > > > > - ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes()); > > > > - } > > > > + buffers.emplace_back(buffer->cookie(), buffer->planes()); > > > > + }; > > > > > > > > - for (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) { I wonder if you could pass 'imgu->statsBuffers_' instead and for-loop in the lambda.. Anyways Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > - buffer->setCookie(ipaBufferId++); > > > > - ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes()); > > > > - } > > > > + for (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) > > > > + pushBuffer(ipaBuffers_, buffer); > > > > + > > > > + for (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) > > > > + pushBuffer(ipaBuffers_, buffer); > > > > > > > > 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..f03a03fef35c 100644 > > > > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > > > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > > > @@ -1133,27 +1133,27 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera) > > > > data->dsStream_.configuration().bufferCount, > > > > }); > > > > > > > > + auto pushBuffer = [&](std::queue<FrameBuffer *> &queue, > > > > + std::vector<IPABuffer> &buffers, > > > > + const std::unique_ptr<FrameBuffer> &buffer) { > > > > + buffer->setCookie(ipaBufferId++); > > > > + buffers.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()); > > > > - } > > > > + for (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_) > > > > + pushBuffer(availableStatsBuffers_, data->ipaStatBuffers_, buffer); > > > > > > > > ret = params_->allocateBuffers(bufferCount, ¶msBuffers_); > > > > 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()); > > > > - } > > > > + for (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_) > > > > + pushBuffer(availableParamsBuffers_, data->ipaParamBuffers_, buffer); > > > > > > > > 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..99347c9f6258 100644 > > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > > > @@ -1028,19 +1028,19 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) > > > > availableMainPathBuffers_.push(buffer.get()); > > > > } > > > > > > > > - for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) { > > > > + auto pushBuffer = [&](std::vector<IPABuffer> &buffers, > > > > + std::queue<FrameBuffer *> &queue, > > > > + const std::unique_ptr<FrameBuffer> &buffer) { > > > > > > Same here. > > > > > > Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > > > > > > buffer->setCookie(ipaBufferId++); > > > > - data->ipaBuffers_.emplace_back(buffer->cookie(), > > > > - buffer->planes()); > > > > - availableParamBuffers_.push(buffer.get()); > > > > - } > > > > + buffers.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()); > > > > - } > > > > + for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) > > > > + pushBuffer(data->ipaBuffers_, availableParamBuffers_, buffer); > > > > + > > > > + for (std::unique_ptr<FrameBuffer> &buffer : statBuffers_) > > > > + pushBuffer(data->ipaBuffers_, availableStatBuffers_, buffer); > > > > > > > > data->ipa_->mapBuffers(data->ipaBuffers_); > > > > > > >
On Tue, Aug 19, 2025 at 05:31:59PM +0200, Jacopo Mondi wrote: > On Tue, Aug 19, 2025 at 03:01:50PM +0200, Barnabás Pőcze wrote: > > 2025. 08. 19. 14:37 keltezéssel, Laurent Pinchart írta: > > > On Tue, Aug 19, 2025 at 02:15:44PM +0200, Barnabás Pőcze wrote: > > > > 2025. 08. 15. 13:33 keltezéssel, Laurent Pinchart írta: > > > > > 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> > > > > > --- > > > > > src/libcamera/pipeline/ipu3/ipu3.cpp | 16 +++++++------ > > > > > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 24 ++++++++++---------- > > > > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 22 +++++++++--------- > > > > > 3 files changed, 32 insertions(+), 30 deletions(-) > > > > > > > > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > > > > index ad20810e6a26..7ae85e5063db 100644 > > > > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > > > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > > > > @@ -678,15 +678,17 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera) > > > > > /* Map buffers to the IPA. */ > > > > > unsigned int ipaBufferId = 1; > > > > > > > > > > - for (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) { > > > > > + auto pushBuffer = [&](std::vector<IPABuffer> &buffers, > > > > > + const std::unique_ptr<FrameBuffer> &buffer) { > > > > > > > > I feel like the `buffers` argument is a bit unnecessary since it's > > > > only ever `ipaBuffers_`. > > Or you can pass the argument but avoid the default capture ? But then > you won't be able to access ipaBufferId.. (yes, you could capture that > one only, but I'm not sure all of this is worth it ?) > > > > You know, one of the reasons I like your reviews is because you point > > > out things that I wasn't sure about myself when sending patches :-) I've > > > gone back and forth on this, and decided to add the buffers argument for > > > consistency between pipeline handlers. If you think it's best to drop > > > it, I'll do that. What's your preference ? > > > > My thinking is that it's highly specific, so might as well go for the simplest > > option. But I am not sure, I don't think it matters much in the end. > > > > > > > buffer->setCookie(ipaBufferId++); > > > > > - ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes()); > > > > > - } > > > > > + buffers.emplace_back(buffer->cookie(), buffer->planes()); > > > > > + }; > > > > > > > > > > - for (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) { > > I wonder if you could pass 'imgu->statsBuffers_' instead and for-loop > in the lambda.. Interesting idea. I'll post of a v2 of this patch, you and Barnabás can decide what you like best. > Anyways > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > > > - buffer->setCookie(ipaBufferId++); > > > > > - ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes()); > > > > > - } > > > > > + for (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) > > > > > + pushBuffer(ipaBuffers_, buffer); > > > > > + > > > > > + for (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) > > > > > + pushBuffer(ipaBuffers_, buffer); > > > > > > > > > > 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..f03a03fef35c 100644 > > > > > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > > > > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > > > > @@ -1133,27 +1133,27 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera) > > > > > data->dsStream_.configuration().bufferCount, > > > > > }); > > > > > > > > > > + auto pushBuffer = [&](std::queue<FrameBuffer *> &queue, > > > > > + std::vector<IPABuffer> &buffers, > > > > > + const std::unique_ptr<FrameBuffer> &buffer) { > > > > > + buffer->setCookie(ipaBufferId++); > > > > > + buffers.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()); > > > > > - } > > > > > + for (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_) > > > > > + pushBuffer(availableStatsBuffers_, data->ipaStatBuffers_, buffer); > > > > > > > > > > ret = params_->allocateBuffers(bufferCount, ¶msBuffers_); > > > > > 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()); > > > > > - } > > > > > + for (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_) > > > > > + pushBuffer(availableParamsBuffers_, data->ipaParamBuffers_, buffer); > > > > > > > > > > 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..99347c9f6258 100644 > > > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > > > > @@ -1028,19 +1028,19 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) > > > > > availableMainPathBuffers_.push(buffer.get()); > > > > > } > > > > > > > > > > - for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) { > > > > > + auto pushBuffer = [&](std::vector<IPABuffer> &buffers, > > > > > + std::queue<FrameBuffer *> &queue, > > > > > + const std::unique_ptr<FrameBuffer> &buffer) { > > > > > > > > Same here. > > > > > > > > Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > > > > > > > > buffer->setCookie(ipaBufferId++); > > > > > - data->ipaBuffers_.emplace_back(buffer->cookie(), > > > > > - buffer->planes()); > > > > > - availableParamBuffers_.push(buffer.get()); > > > > > - } > > > > > + buffers.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()); > > > > > - } > > > > > + for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) > > > > > + pushBuffer(data->ipaBuffers_, availableParamBuffers_, buffer); > > > > > + > > > > > + for (std::unique_ptr<FrameBuffer> &buffer : statBuffers_) > > > > > + pushBuffer(data->ipaBuffers_, availableStatBuffers_, buffer); > > > > > > > > > > data->ipa_->mapBuffers(data->ipaBuffers_); > > > > >
On Tue, Aug 19, 2025 at 07:09:03PM +0300, Laurent Pinchart wrote: > On Tue, Aug 19, 2025 at 05:31:59PM +0200, Jacopo Mondi wrote: > > On Tue, Aug 19, 2025 at 03:01:50PM +0200, Barnabás Pőcze wrote: > > > 2025. 08. 19. 14:37 keltezéssel, Laurent Pinchart írta: > > > > On Tue, Aug 19, 2025 at 02:15:44PM +0200, Barnabás Pőcze wrote: > > > > > 2025. 08. 15. 13:33 keltezéssel, Laurent Pinchart írta: > > > > > > 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> > > > > > > --- > > > > > > src/libcamera/pipeline/ipu3/ipu3.cpp | 16 +++++++------ > > > > > > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 24 ++++++++++---------- > > > > > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 22 +++++++++--------- > > > > > > 3 files changed, 32 insertions(+), 30 deletions(-) > > > > > > > > > > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > > > > > index ad20810e6a26..7ae85e5063db 100644 > > > > > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > > > > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > > > > > @@ -678,15 +678,17 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera) > > > > > > /* Map buffers to the IPA. */ > > > > > > unsigned int ipaBufferId = 1; > > > > > > > > > > > > - for (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) { > > > > > > + auto pushBuffer = [&](std::vector<IPABuffer> &buffers, > > > > > > + const std::unique_ptr<FrameBuffer> &buffer) { > > > > > > > > > > I feel like the `buffers` argument is a bit unnecessary since it's > > > > > only ever `ipaBuffers_`. > > > > Or you can pass the argument but avoid the default capture ? But then > > you won't be able to access ipaBufferId.. (yes, you could capture that > > one only, but I'm not sure all of this is worth it ?) > > > > > > You know, one of the reasons I like your reviews is because you point > > > > out things that I wasn't sure about myself when sending patches :-) I've > > > > gone back and forth on this, and decided to add the buffers argument for > > > > consistency between pipeline handlers. If you think it's best to drop > > > > it, I'll do that. What's your preference ? > > > > > > My thinking is that it's highly specific, so might as well go for the simplest > > > option. But I am not sure, I don't think it matters much in the end. > > > > > > > > > buffer->setCookie(ipaBufferId++); > > > > > > - ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes()); > > > > > > - } > > > > > > + buffers.emplace_back(buffer->cookie(), buffer->planes()); > > > > > > + }; > > > > > > > > > > > > - for (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) { > > > > I wonder if you could pass 'imgu->statsBuffers_' instead and for-loop > > in the lambda.. > > Interesting idea. I'll post of a v2 of this patch, you and Barnabás can > decide what you like best. Only if you really prefer, I think it doesn't make much difference :) > > > Anyways > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > > > > > - buffer->setCookie(ipaBufferId++); > > > > > > - ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes()); > > > > > > - } > > > > > > + for (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) > > > > > > + pushBuffer(ipaBuffers_, buffer); > > > > > > + > > > > > > + for (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) > > > > > > + pushBuffer(ipaBuffers_, buffer); > > > > > > > > > > > > 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..f03a03fef35c 100644 > > > > > > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > > > > > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp > > > > > > @@ -1133,27 +1133,27 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera) > > > > > > data->dsStream_.configuration().bufferCount, > > > > > > }); > > > > > > > > > > > > + auto pushBuffer = [&](std::queue<FrameBuffer *> &queue, > > > > > > + std::vector<IPABuffer> &buffers, > > > > > > + const std::unique_ptr<FrameBuffer> &buffer) { > > > > > > + buffer->setCookie(ipaBufferId++); > > > > > > + buffers.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()); > > > > > > - } > > > > > > + for (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_) > > > > > > + pushBuffer(availableStatsBuffers_, data->ipaStatBuffers_, buffer); > > > > > > > > > > > > ret = params_->allocateBuffers(bufferCount, ¶msBuffers_); > > > > > > 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()); > > > > > > - } > > > > > > + for (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_) > > > > > > + pushBuffer(availableParamsBuffers_, data->ipaParamBuffers_, buffer); > > > > > > > > > > > > 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..99347c9f6258 100644 > > > > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > > > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > > > > > @@ -1028,19 +1028,19 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) > > > > > > availableMainPathBuffers_.push(buffer.get()); > > > > > > } > > > > > > > > > > > > - for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) { > > > > > > + auto pushBuffer = [&](std::vector<IPABuffer> &buffers, > > > > > > + std::queue<FrameBuffer *> &queue, > > > > > > + const std::unique_ptr<FrameBuffer> &buffer) { > > > > > > > > > > Same here. > > > > > > > > > > Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > > > > > > > > > > buffer->setCookie(ipaBufferId++); > > > > > > - data->ipaBuffers_.emplace_back(buffer->cookie(), > > > > > > - buffer->planes()); > > > > > > - availableParamBuffers_.push(buffer.get()); > > > > > > - } > > > > > > + buffers.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()); > > > > > > - } > > > > > > + for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) > > > > > > + pushBuffer(data->ipaBuffers_, availableParamBuffers_, buffer); > > > > > > + > > > > > > + for (std::unique_ptr<FrameBuffer> &buffer : statBuffers_) > > > > > > + pushBuffer(data->ipaBuffers_, availableStatBuffers_, buffer); > > > > > > > > > > > > data->ipa_->mapBuffers(data->ipaBuffers_); > > > > > > > > -- > Regards, > > Laurent Pinchart
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index ad20810e6a26..7ae85e5063db 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -678,15 +678,17 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera) /* Map buffers to the IPA. */ unsigned int ipaBufferId = 1; - for (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) { + auto pushBuffer = [&](std::vector<IPABuffer> &buffers, + const std::unique_ptr<FrameBuffer> &buffer) { buffer->setCookie(ipaBufferId++); - ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes()); - } + buffers.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()); - } + for (const std::unique_ptr<FrameBuffer> &buffer : imgu->paramBuffers_) + pushBuffer(ipaBuffers_, buffer); + + for (const std::unique_ptr<FrameBuffer> &buffer : imgu->statBuffers_) + pushBuffer(ipaBuffers_, buffer); 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..f03a03fef35c 100644 --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp @@ -1133,27 +1133,27 @@ int PipelineHandlerMaliC55::allocateBuffers(Camera *camera) data->dsStream_.configuration().bufferCount, }); + auto pushBuffer = [&](std::queue<FrameBuffer *> &queue, + std::vector<IPABuffer> &buffers, + const std::unique_ptr<FrameBuffer> &buffer) { + buffer->setCookie(ipaBufferId++); + buffers.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()); - } + for (std::unique_ptr<FrameBuffer> &buffer : statsBuffers_) + pushBuffer(availableStatsBuffers_, data->ipaStatBuffers_, buffer); ret = params_->allocateBuffers(bufferCount, ¶msBuffers_); 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()); - } + for (std::unique_ptr<FrameBuffer> &buffer : paramsBuffers_) + pushBuffer(availableParamsBuffers_, data->ipaParamBuffers_, buffer); 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..99347c9f6258 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -1028,19 +1028,19 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) availableMainPathBuffers_.push(buffer.get()); } - for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) { + auto pushBuffer = [&](std::vector<IPABuffer> &buffers, + std::queue<FrameBuffer *> &queue, + const std::unique_ptr<FrameBuffer> &buffer) { buffer->setCookie(ipaBufferId++); - data->ipaBuffers_.emplace_back(buffer->cookie(), - buffer->planes()); - availableParamBuffers_.push(buffer.get()); - } + buffers.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()); - } + for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) + pushBuffer(data->ipaBuffers_, availableParamBuffers_, buffer); + + for (std::unique_ptr<FrameBuffer> &buffer : statBuffers_) + pushBuffer(data->ipaBuffers_, availableStatBuffers_, buffer); data->ipa_->mapBuffers(data->ipaBuffers_);
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> --- src/libcamera/pipeline/ipu3/ipu3.cpp | 16 +++++++------ src/libcamera/pipeline/mali-c55/mali-c55.cpp | 24 ++++++++++---------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 22 +++++++++--------- 3 files changed, 32 insertions(+), 30 deletions(-)