[libcamera-devel] [PATCH v7 10/12] pipeline: raspberrypi: Use an unordered_map for the stream buffer list
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Sep 15 15:40:31 CEST 2020
Hi Naush,
On 08/09/2020 08:49, 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>
> ---
> .../pipeline/raspberrypi/raspberrypi.cpp | 23 ++++++---------
> .../pipeline/raspberrypi/rpi_stream.cpp | 28 ++++++++++---------
> .../pipeline/raspberrypi/rpi_stream.h | 14 +++++++---
> 3 files changed, 34 insertions(+), 31 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 341ee6ce..6aa13034 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;
> @@ -1432,8 +1427,8 @@ 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 bayerIndex = unicam_[Unicam::Image].getBufferId(bayerBuffer);
> + unsigned int embeddedIndex = unicam_[Unicam::Embedded].getBufferId(embeddedBuffer);
We assign an ID to an Index here, but I don't think it matters too much.
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> LOG(RPI, Debug) << "Signalling RPI_IPA_EVENT_SIGNAL_ISP_PREPARE:"
> << " Bayer buffer id: " << bayerIndex
> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> index 80170d64..879e25ba 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_++, 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);
> @@ -196,12 +198,12 @@ void RPiStream::clearBuffers()
> availableBuffers_ = std::queue<FrameBuffer *>{};
> requestBuffers_ = std::queue<FrameBuffer *>{};
> internalBuffers_.clear();
> - bufferList_.clear();
> + bufferMap_.clear();
> }
>
> 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..ed517c22 100644
> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> @@ -9,6 +9,7 @@
>
> #include <queue>
> #include <string>
> +#include <unordered_map>
> #include <vector>
>
> #include <libcamera/stream.h>
> @@ -19,6 +20,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.
> @@ -32,7 +35,7 @@ public:
>
> 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_(0)
> {
> }
>
> @@ -45,8 +48,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);
> @@ -74,8 +77,11 @@ private:
> /* The actual device stream. */
> std::unique_ptr<V4L2VideoDevice> dev_;
>
> + /* Tracks a unique id key for the bufferMap_ */
> + unsigned int 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
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list