[libcamera-devel] [PATCH v2 10/11] libcamera: ipu3: Add helper for parameter and statistic buffers
Niklas Söderlund
niklas.soderlund at ragnatech.se
Wed Feb 3 11:24:32 CET 2021
Hi Laurent,
Thanks for your comments.
On 2021-01-10 23:01:29 +0200, Laurent Pinchart wrote:
> Hi Niklas,
>
> Thank you for the patch.
>
> On Tue, Dec 29, 2020 at 05:03:17PM +0100, Niklas Söderlund wrote:
> > Add a helper class to aid in associating a parameter and statistic
> > buffer with each request queued to the pipeline. The helper helps with
> > tracking the state of the extra buffers and in completing the request
> > once all extra processing is done.
> >
> > This change only adds the helper more work is needed to integrate it
> > with the pipeline and an IPA.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> > * Changes since v1
> > - Move mapping of buffers to IPA to the pipeline handler and a separate
> > patch.
> > - Move all pipeline interactions out of helper.
> > ---
> > src/libcamera/pipeline/ipu3/frames.cpp | 141 ++++++++++++++++++++++++
> > src/libcamera/pipeline/ipu3/frames.h | 63 +++++++++++
> > src/libcamera/pipeline/ipu3/meson.build | 1 +
> > 3 files changed, 205 insertions(+)
> > create mode 100644 src/libcamera/pipeline/ipu3/frames.cpp
> > create mode 100644 src/libcamera/pipeline/ipu3/frames.h
> >
> > diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp
> > new file mode 100644
> > index 0000000000000000..1ab078db056ce193
> > --- /dev/null
> > +++ b/src/libcamera/pipeline/ipu3/frames.cpp
> > @@ -0,0 +1,141 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * frames.cpp - Intel IPU3 Frames helper
> > + */
> > +
> > +#include "frames.h"
> > +
> > +#include <libcamera/buffer.h>
> > +#include <libcamera/request.h>
> > +
> > +#include "libcamera/internal/pipeline_handler.h"
> > +#include "libcamera/internal/v4l2_videodevice.h"
> > +
> > +namespace libcamera {
> > +
> > +LOG_DECLARE_CATEGORY(IPU3)
> > +
> > +IPU3Frames::IPU3Frames()
> > + : nextId_(0)
> > +{
> > +}
> > +
> > +void IPU3Frames::init(const std::vector<std::unique_ptr<FrameBuffer>> ¶mBuffers,
> > + const std::vector<std::unique_ptr<FrameBuffer>> &statBuffers)
> > +{
> > + for (const std::unique_ptr<FrameBuffer> &buffer : paramBuffers)
> > + availableParamBuffers_.push(buffer.get());
> > +
> > + for (const std::unique_ptr<FrameBuffer> &buffer : statBuffers)
> > + availableStatBuffers_.push(buffer.get());
> > +
> > + nextId_ = 0;
> > + frameInfo_.clear();
> > +}
> > +
> > +void IPU3Frames::clear()
> > +{
> > + availableParamBuffers_ = {};
> > + availableStatBuffers_ = {};
> > +}
> > +
> > +IPU3Frames::Info *IPU3Frames::create(Request *request)
> > +{
> > + unsigned int id = nextId_++;
> > +
> > + if (availableParamBuffers_.empty()) {
> > + LOG(IPU3, Error) << "Parameters buffer underrun";
> > + return nullptr;
> > + }
> > + FrameBuffer *paramBuffer = availableParamBuffers_.front();
> > +
> > + if (availableStatBuffers_.empty()) {
> > + LOG(IPU3, Error) << "Statisitc buffer underrun";
> > + return nullptr;
> > + }
> > + FrameBuffer *statBuffer = availableStatBuffers_.front();
> > +
> > + availableParamBuffers_.pop();
> > + availableStatBuffers_.pop();
> > +
> > + std::unique_ptr<Info> info = std::make_unique<Info>();
>
> It would be nice if we could preallocate the Info instances to lower the
> number of dynamic allocations at runtime.
I agree, I have some crazy ideas for how we can make pipelines life
easier by interacting with the pipeline depth at a core Camera level.
That would allow this allocation unnecessary. I will add a todo here so
we don't forget it.
>
> > +
> > + info->id = id;
> > + info->request = request;
> > + info->rawBuffer = nullptr;
> > + info->paramBuffer = paramBuffer;
> > + info->statBuffer = statBuffer;
> > + info->paramFilled = false;
> > + info->paramDequeued = false;
> > + info->metadataProcessed = false;
> > +
> > + frameInfo_[id] = std::move(info);
> > +
> > + return frameInfo_[id].get();
> > +}
> > +
> > +bool IPU3Frames::tryComplete(IPU3Frames::Info *info)
> > +{
> > + Request *request = info->request;
> > +
> > + if (request->hasPendingBuffers())
> > + return false;
> > +
> > + if (!info->metadataProcessed)
> > + return false;
> > +
> > + if (!info->paramDequeued)
> > + return false;
> > +
> > + /* Return params and stat buffer for reuse. */
> > + availableParamBuffers_.push(info->paramBuffer);
> > + availableStatBuffers_.push(info->statBuffer);
> > +
> > + /* Delete the extended frame information. */
> > + frameInfo_.erase(info->id);
> > +
> > + return true;
> > +}
> > +
> > +IPU3Frames::Info *IPU3Frames::find(unsigned int id)
> > +{
> > + const auto &itInfo = frameInfo_.find(id);
> > +
> > + if (itInfo != frameInfo_.end())
> > + return itInfo->second.get();
> > +
> > + return nullptr;
> > +}
> > +
> > +IPU3Frames::Info *IPU3Frames::find(FrameBuffer *buffer)
> > +{
> > + for (auto const &itInfo : frameInfo_) {
> > + Info *info = itInfo.second.get();
> > +
> > + for (auto const itBuffers : info->request->buffers())
> > + if (itBuffers.second == buffer)
> > + return info;
> > +
> > + if (info->rawBuffer == buffer || info->paramBuffer == buffer ||
> > + info->statBuffer == buffer)
> > + return info;
> > + }
> > +
> > + return nullptr;
> > +}
> > +
> > +IPU3Frames::Info *IPU3Frames::find(Request *request)
> > +{
> > + for (auto const &itInfo : frameInfo_) {
> > + Info *info = itInfo.second.get();
> > +
> > + if (info->request == request)
> > + return info;
> > + }
> > +
> > + return nullptr;
> > +}
>
> I think this function isn't used.
You are correct, it was added for completeness as I had intentions to
profligate this to the RkISP1 pipeline handler. But if that happens it
may be added back, will drop for next version.
>
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h
> > new file mode 100644
> > index 0000000000000000..06b2678be4fb04d7
> > --- /dev/null
> > +++ b/src/libcamera/pipeline/ipu3/frames.h
> > @@ -0,0 +1,63 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2020, Google Inc.
> > + *
> > + * frames.h - Intel IPU3 Frames helper
> > + */
> > +#ifndef __LIBCAMERA_PIPELINE_IPU3_FRAMES_H__
> > +#define __LIBCAMERA_PIPELINE_IPU3_FRAMES_H__
> > +
> > +#include <map>
> > +#include <memory>
> > +#include <queue>
> > +#include <vector>
> > +
> > +namespace libcamera {
> > +
> > +class FrameBuffer;
> > +class IPAProxy;
> > +class PipelineHandler;
> > +class Request;
> > +class V4L2VideoDevice;
> > +struct IPABuffer;
> > +
> > +class IPU3Frames
> > +{
> > +public:
> > + struct Info {
> > + unsigned int id;
> > + Request *request;
> > +
> > + FrameBuffer *rawBuffer;
> > + FrameBuffer *paramBuffer;
> > + FrameBuffer *statBuffer;
> > +
> > + bool paramFilled;
> > + bool paramDequeued;
> > + bool metadataProcessed;
> > + };
> > +
> > + IPU3Frames();
> > +
> > + void init(const std::vector<std::unique_ptr<FrameBuffer>> ¶mBuffers,
> > + const std::vector<std::unique_ptr<FrameBuffer>> &statBuffers);
> > + void clear();
> > +
> > + Info *create(Request *request);
> > + bool tryComplete(IPU3Frames::Info *info);
> > +
> > + Info *find(unsigned int id);
> > + Info *find(FrameBuffer *buffer);
> > + Info *find(Request *request);
> > +
> > +private:
> > + std::queue<FrameBuffer *> availableParamBuffers_;
> > + std::queue<FrameBuffer *> availableStatBuffers_;
> > +
> > + unsigned int nextId_;
> > + std::map<unsigned int, std::unique_ptr<Info>> frameInfo_;
> > +};
> > +
> > +} /* namespace libcamera */
> > +
> > +#endif /* __LIBCAMERA_PIPELINE_IPU3_FRAMES_H__ */
> > diff --git a/src/libcamera/pipeline/ipu3/meson.build b/src/libcamera/pipeline/ipu3/meson.build
> > index d60e07ae6ccac2bc..a1b0b31ac5bcf864 100644
> > --- a/src/libcamera/pipeline/ipu3/meson.build
> > +++ b/src/libcamera/pipeline/ipu3/meson.build
> > @@ -2,6 +2,7 @@
> >
> > libcamera_sources += files([
> > 'cio2.cpp',
> > + 'frames.cpp',
> > 'imgu.cpp',
> > 'ipu3.cpp',
> > ])
>
> --
> Regards,
>
> Laurent Pinchart
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list