[libcamera-devel] [PATCH v8 10/12] pipeline: raspberrypi: Use an unordered_map for the stream buffer list
Niklas Söderlund
niklas.soderlund at ragnatech.se
Sat Sep 19 14:58:08 CEST 2020
Hi Naushir,
Thanks for your work.
On 2020-09-18 10:42:31 +0100, Naushir Patuck wrote:
> By using a map container, we can easily insert/remove buffers from the
> buffer list. This will be required to track buffers allocated externally
> and passed to the pipeline handler through a Request.
>
> Replace the buffer index tracking with an id generated internally by the
> stream object.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> .../pipeline/raspberrypi/raspberrypi.cpp | 31 +++++------
> .../pipeline/raspberrypi/rpi_stream.cpp | 32 ++++++-----
> .../pipeline/raspberrypi/rpi_stream.h | 54 +++++++++++++++++--
> 3 files changed, 82 insertions(+), 35 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 7d91188c..51544233 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -890,7 +890,6 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera)
> int PipelineHandlerRPi::prepareBuffers(Camera *camera)
> {
> RPiCameraData *data = cameraData(camera);
> - unsigned int index;
> int ret;
>
> /*
> @@ -917,18 +916,14 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)
> * This will allow us to identify buffers passed between the pipeline
> * handler and the IPA.
> */
> - index = 0;
> for (auto const &b : data->isp_[Isp::Stats].getBuffers()) {
> - data->ipaBuffers_.push_back({ .id = RPiIpaMask::STATS | index,
> - .planes = b->planes() });
> - index++;
> + data->ipaBuffers_.push_back({ .id = RPiIpaMask::STATS | b.first,
> + .planes = b.second->planes() });
> }
>
> - index = 0;
> for (auto const &b : data->unicam_[Unicam::Embedded].getBuffers()) {
> - data->ipaBuffers_.push_back({ .id = RPiIpaMask::EMBEDDED_DATA | index,
> - .planes = b->planes() });
> - index++;
> + data->ipaBuffers_.push_back({ .id = RPiIpaMask::EMBEDDED_DATA | b.first,
> + .planes = b.second->planes() });
> }
>
> data->ipa_->mapBuffers(data->ipaBuffers_);
> @@ -1127,7 +1122,7 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)
> return;
>
> for (RPi::RPiStream &s : unicam_) {
> - index = s.getBufferIndex(buffer);
> + index = s.getBufferId(buffer);
> if (index != -1) {
> stream = &s;
> break;
> @@ -1178,7 +1173,7 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer)
> return;
>
> LOG(RPI, Debug) << "Stream ISP Input buffer complete"
> - << ", buffer id " << unicam_[Unicam::Image].getBufferIndex(buffer)
> + << ", buffer id " << unicam_[Unicam::Image].getBufferId(buffer)
> << ", timestamp: " << buffer->metadata().timestamp;
>
> /* The ISP input buffer gets re-queued into Unicam. */
> @@ -1195,7 +1190,7 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)
> return;
>
> for (RPi::RPiStream &s : isp_) {
> - index = s.getBufferIndex(buffer);
> + index = s.getBufferId(buffer);
> if (index != -1) {
> stream = &s;
> break;
> @@ -1436,16 +1431,16 @@ void RPiCameraData::tryRunPipeline()
> /* Set our state to say the pipeline is active. */
> state_ = State::Busy;
>
> - unsigned int bayerIndex = unicam_[Unicam::Image].getBufferIndex(bayerBuffer);
> - unsigned int embeddedIndex = unicam_[Unicam::Embedded].getBufferIndex(embeddedBuffer);
> + unsigned int bayerId = unicam_[Unicam::Image].getBufferId(bayerBuffer);
> + unsigned int embeddedId = unicam_[Unicam::Embedded].getBufferId(embeddedBuffer);
>
> LOG(RPI, Debug) << "Signalling RPI_IPA_EVENT_SIGNAL_ISP_PREPARE:"
> - << " Bayer buffer id: " << bayerIndex
> - << " Embedded buffer id: " << embeddedIndex;
> + << " Bayer buffer id: " << bayerId
> + << " Embedded buffer id: " << embeddedId;
>
> op.operation = RPI_IPA_EVENT_SIGNAL_ISP_PREPARE;
> - op.data = { RPiIpaMask::EMBEDDED_DATA | embeddedIndex,
> - RPiIpaMask::BAYER_DATA | bayerIndex };
> + op.data = { RPiIpaMask::EMBEDDED_DATA | embeddedId,
> + RPiIpaMask::BAYER_DATA | bayerId };
> ipa_->processEvent(op);
> }
>
> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> index 80170d64..aee0aa2d 100644
> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> @@ -44,26 +44,28 @@ bool RPiStream::isExternal() const
>
> void RPiStream::setExportedBuffers(std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> {
> - std::transform(buffers->begin(), buffers->end(), std::back_inserter(bufferList_),
> - [](std::unique_ptr<FrameBuffer> &b) { return b.get(); });
> + for (auto const &buffer : *buffers)
> + bufferMap_.emplace(id_.get(), buffer.get());
> }
>
> -const std::vector<FrameBuffer *> &RPiStream::getBuffers() const
> +const BufferMap &RPiStream::getBuffers() const
> {
> - return bufferList_;
> + return bufferMap_;
> }
>
> -int RPiStream::getBufferIndex(FrameBuffer *buffer) const
> +int RPiStream::getBufferId(FrameBuffer *buffer) const
> {
> if (importOnly_)
> return -1;
>
> - /* Find the buffer in the list, and return the index position. */
> - auto it = std::find(bufferList_.begin(), bufferList_.end(), buffer);
> - if (it != bufferList_.end())
> - return std::distance(bufferList_.begin(), it);
> + /* Find the buffer in the map, and return the buffer id. */
> + auto it = std::find_if(bufferMap_.begin(), bufferMap_.end(),
> + [&buffer](auto const &p) { return p.second == buffer; });
>
> - return -1;
> + if (it == bufferMap_.end())
> + return -1;
> +
> + return it->first;
> }
>
> int RPiStream::prepareBuffers(unsigned int count)
> @@ -86,7 +88,7 @@ int RPiStream::prepareBuffers(unsigned int count)
> }
>
> /* We must import all internal/external exported buffers. */
> - count = bufferList_.size();
> + count = bufferMap_.size();
> }
>
> return dev_->importBuffers(count);
> @@ -139,6 +141,9 @@ void RPiStream::returnBuffer(FrameBuffer *buffer)
> /* Push this buffer back into the queue to be used again. */
> availableBuffers_.push(buffer);
>
> + /* Allow the buffer id to be reused. */
> + id_.release(getBufferId(buffer));
> +
> /*
> * Do we have any Request buffers that are waiting to be queued?
> * If so, do it now as availableBuffers_ will not be empty.
> @@ -196,12 +201,13 @@ void RPiStream::clearBuffers()
> availableBuffers_ = std::queue<FrameBuffer *>{};
> requestBuffers_ = std::queue<FrameBuffer *>{};
> internalBuffers_.clear();
> - bufferList_.clear();
> + bufferMap_.clear();
> + id_.reset();
> }
>
> int RPiStream::queueToDevice(FrameBuffer *buffer)
> {
> - LOG(RPISTREAM, Debug) << "Queuing buffer " << getBufferIndex(buffer)
> + LOG(RPISTREAM, Debug) << "Queuing buffer " << getBufferId(buffer)
> << " for " << name_;
>
> int ret = dev_->queueBuffer(buffer);
> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> index 959e9147..df986367 100644
> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> @@ -9,8 +9,10 @@
>
> #include <queue>
> #include <string>
> +#include <unordered_map>
> #include <vector>
>
> +#include <libcamera/ipa/raspberrypi.h>
> #include <libcamera/stream.h>
>
> #include "libcamera/internal/v4l2_videodevice.h"
> @@ -19,6 +21,8 @@ namespace libcamera {
>
> namespace RPi {
>
> +using BufferMap = std::unordered_map<unsigned int, FrameBuffer *>;
> +
> /*
> * Device stream abstraction for either an internal or external stream.
> * Used for both Unicam and the ISP.
> @@ -27,12 +31,13 @@ class RPiStream : public Stream
> {
> public:
> RPiStream()
> + : id_(RPiIpaMask::ID)
> {
> }
>
> RPiStream(const char *name, MediaEntity *dev, bool importOnly = false)
> : external_(false), importOnly_(importOnly), name_(name),
> - dev_(std::make_unique<V4L2VideoDevice>(dev))
> + dev_(std::make_unique<V4L2VideoDevice>(dev)), id_(RPiIpaMask::ID)
> {
> }
>
> @@ -45,8 +50,8 @@ public:
> bool isExternal() const;
>
> void setExportedBuffers(std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> - const std::vector<FrameBuffer *> &getBuffers() const;
> - int getBufferIndex(FrameBuffer *buffer) const;
> + const BufferMap &getBuffers() const;
> + int getBufferId(FrameBuffer *buffer) const;
>
> int prepareBuffers(unsigned int count);
> int queueBuffer(FrameBuffer *buffer);
> @@ -56,6 +61,44 @@ public:
> void releaseBuffers();
>
> private:
> + class IdGenerator
> + {
> + public:
> + IdGenerator(int max)
> + : max_(max), id_(0)
> + {
> + }
> +
> + int get()
> + {
> + int id;
> + if (!recycle_.empty()) {
> + id = recycle_.front();
> + recycle_.pop();
> + } else {
> + id = id_++;
> + ASSERT(id_ <= max_);
> + }
> + return id;
> + }
> +
> + void release(int id)
> + {
> + recycle_.push(id);
> + }
> +
> + void reset()
> + {
> + id_ = 0;
> + recycle_ = {};
> + }
> +
> + private:
> + int max_;
> + int id_;
> + std::queue<int> recycle_;
> + };
> +
> void clearBuffers();
> int queueToDevice(FrameBuffer *buffer);
>
> @@ -74,8 +117,11 @@ private:
> /* The actual device stream. */
> std::unique_ptr<V4L2VideoDevice> dev_;
>
> + /* Tracks a unique id key for the bufferMap_ */
> + IdGenerator id_;
> +
> /* All frame buffers associated with this device stream. */
> - std::vector<FrameBuffer *> bufferList_;
> + BufferMap bufferMap_;
>
> /*
> * List of frame buffers that we can use if none have been provided by
> --
> 2.25.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list