[libcamera-devel] [PATCH 01/20] pipeline: rpi: Add RequiresMmap flag to RPi::Stream
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Thu Oct 12 09:21:06 CEST 2023
Hi Naush
On Fri, Oct 06, 2023 at 02:19:41PM +0100, Naushir Patuck via libcamera-devel wrote:
> Add a new RequiresMmap flag to the RPi::Stream class indicating that
> buffers handled by the stream must be mmapped after allocation and
> cached internally.
>
> Add a new member function getBuffer(id) which can be used to obtain the
> mapped buffers for a given buffer id.
>
> Add a new member function acquireBuffer() which can be used to obtain
> any mapped buffer that has not already been acquired by the caller.
>
> As a drive-by, add the <algorithm> header to rpi_stream.cpp as it is
> needed for the std::find_if() function.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> ---
> .../pipeline/rpi/common/pipeline_base.cpp | 2 +-
> .../pipeline/rpi/common/rpi_stream.cpp | 50 +++++++++++++++++--
> .../pipeline/rpi/common/rpi_stream.h | 32 +++++++++++-
> src/libcamera/pipeline/rpi/vc4/vc4.cpp | 6 +--
> 4 files changed, 81 insertions(+), 9 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index fad710a63c46..825eae80d014 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -888,7 +888,7 @@ void PipelineHandlerBase::mapBuffers(Camera *camera, const BufferMap &buffers, u
> */
> for (auto const &it : buffers) {
> bufferIds.push_back(IPABuffer(mask | it.first,
> - it.second->planes()));
> + it.second.buffer->planes()));
> data->bufferIds_.insert(mask | it.first);
> }
>
> diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> index 7319f510a371..83c2205f7ca0 100644
> --- a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> @@ -6,6 +6,10 @@
> */
> #include "rpi_stream.h"
>
> +#include <algorithm>
> +#include <tuple>
> +#include <utility>
> +
> #include <libcamera/base/log.h>
>
> /* Maximum number of buffer slots to allocate in the V4L2 device driver. */
> @@ -17,8 +21,13 @@ LOG_DEFINE_CATEGORY(RPISTREAM)
>
> namespace RPi {
>
> +const BufferObject Stream::errorBufferObject{ nullptr, false };
> +
> void Stream::setFlags(StreamFlags flags)
> {
> + /* We don't want dynamic mmapping. */
> + ASSERT(!(flags & StreamFlag::RequiresMmap));
> +
> flags_ |= flags;
>
> /* Import streams cannot be external. */
> @@ -27,6 +36,9 @@ void Stream::setFlags(StreamFlags flags)
>
> void Stream::clearFlags(StreamFlags flags)
> {
> + /* We don't want dynamic mmapping. */
> + ASSERT(!(flags & StreamFlag::RequiresMmap));
> +
> flags_ &= ~flags;
> }
>
> @@ -56,7 +68,7 @@ void Stream::resetBuffers()
> void Stream::setExportedBuffers(std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> {
> for (auto const &buffer : *buffers)
> - bufferMap_.emplace(++id_, buffer.get());
> + bufferEmplace(++id_, buffer.get());
> }
>
> const BufferMap &Stream::getBuffers() const
> @@ -71,7 +83,7 @@ unsigned int Stream::getBufferId(FrameBuffer *buffer) const
>
> /* 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; });
> + [&buffer](auto const &p) { return p.second.buffer == buffer; });
>
> if (it == bufferMap_.end())
> return 0;
> @@ -81,7 +93,7 @@ unsigned int Stream::getBufferId(FrameBuffer *buffer) const
>
> void Stream::setExportedBuffer(FrameBuffer *buffer)
> {
> - bufferMap_.emplace(++id_, buffer);
> + bufferEmplace(++id_, buffer);
> }
>
> int Stream::prepareBuffers(unsigned int count)
> @@ -180,6 +192,27 @@ void Stream::returnBuffer(FrameBuffer *buffer)
> }
> }
>
> +const BufferObject &Stream::getBuffer(unsigned int id)
> +{
> + auto const &it = bufferMap_.find(id);
> + if (it == bufferMap_.end())
> + return errorBufferObject;
> +
> + return it->second;
> +}
> +
> +const BufferObject &Stream::acquireBuffer()
> +{
> + /* No id provided, so pick up the next available buffer if possible. */
> + if (availableBuffers_.empty())
> + return errorBufferObject;
> +
> + unsigned int id = getBufferId(availableBuffers_.front());
> + availableBuffers_.pop();
> +
> + return getBuffer(id);
> +}
> +
> int Stream::queueAllBuffers()
> {
> int ret;
> @@ -204,6 +237,17 @@ void Stream::releaseBuffers()
> clearBuffers();
> }
>
> +void Stream::bufferEmplace(unsigned int id, FrameBuffer *buffer)
> +{
> + if (flags_ & StreamFlag::RequiresMmap) {
> + bufferMap_.emplace(std::piecewise_construct, std::forward_as_tuple(id),
> + std::forward_as_tuple(buffer, true));
> + } else {
> + bufferMap_.emplace(std::piecewise_construct, std::forward_as_tuple(id),
> + std::forward_as_tuple(buffer, false));
> + }
nit: drop {}
can be done when applying
Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
Thanks
j
> +}
> +
> void Stream::clearBuffers()
> {
> availableBuffers_ = std::queue<FrameBuffer *>{};
> diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.h b/src/libcamera/pipeline/rpi/common/rpi_stream.h
> index 889b499782a4..861e9c8e7dab 100644
> --- a/src/libcamera/pipeline/rpi/common/rpi_stream.h
> +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.h
> @@ -7,22 +7,23 @@
>
> #pragma once
>
> +#include <optional>
> #include <queue>
> #include <string>
> #include <unordered_map>
> #include <vector>
>
> #include <libcamera/base/flags.h>
> +
> #include <libcamera/stream.h>
>
> +#include "libcamera/internal/mapped_framebuffer.h"
> #include "libcamera/internal/v4l2_videodevice.h"
>
> namespace libcamera {
>
> namespace RPi {
>
> -using BufferMap = std::unordered_map<unsigned int, FrameBuffer *>;
> -
> enum BufferMask {
> MaskID = 0x00ffff,
> MaskStats = 0x010000,
> @@ -30,6 +31,21 @@ enum BufferMask {
> MaskBayerData = 0x040000,
> };
>
> +struct BufferObject {
> + BufferObject(FrameBuffer *b, bool requiresMmap)
> + : buffer(b), mapped(std::nullopt)
> + {
> + if (requiresMmap)
> + mapped = std::make_optional<MappedFrameBuffer>
> + (b, MappedFrameBuffer::MapFlag::ReadWrite);
> + }
> +
> + FrameBuffer *buffer;
> + std::optional<MappedFrameBuffer> mapped;
> +};
> +
> +using BufferMap = std::unordered_map<unsigned int, BufferObject>;
> +
> /*
> * Device stream abstraction for either an internal or external stream.
> * Used for both Unicam and the ISP.
> @@ -49,6 +65,11 @@ public:
> * buffers might be provided by (and returned to) the application.
> */
> External = (1 << 1),
> + /*
> + * Indicates that the stream buffers need to be mmaped and returned
> + * to the pipeline handler when requested.
> + */
> + RequiresMmap = (1 << 2),
> };
>
> using StreamFlags = Flags<StreamFlag>;
> @@ -82,10 +103,17 @@ public:
> int queueBuffer(FrameBuffer *buffer);
> void returnBuffer(FrameBuffer *buffer);
>
> + const BufferObject &getBuffer(unsigned int id);
> + const BufferObject &acquireBuffer();
> +
> int queueAllBuffers();
> void releaseBuffers();
>
> + /* For error handling. */
> + static const BufferObject errorBufferObject;
> +
> private:
> + void bufferEmplace(unsigned int id, FrameBuffer *buffer);
> void clearBuffers();
> int queueToDevice(FrameBuffer *buffer);
>
> diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> index c189abaabc87..bc90d6324777 100644
> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp
> @@ -825,7 +825,7 @@ void Vc4CameraData::processStatsComplete(const ipa::RPi::BufferIds &buffers)
> if (!isRunning())
> return;
>
> - FrameBuffer *buffer = isp_[Isp::Stats].getBuffers().at(buffers.stats & RPi::MaskID);
> + FrameBuffer *buffer = isp_[Isp::Stats].getBuffers().at(buffers.stats & RPi::MaskID).buffer;
>
> handleStreamBuffer(buffer, &isp_[Isp::Stats]);
>
> @@ -842,7 +842,7 @@ void Vc4CameraData::prepareIspComplete(const ipa::RPi::BufferIds &buffers)
> if (!isRunning())
> return;
>
> - buffer = unicam_[Unicam::Image].getBuffers().at(bayer & RPi::MaskID);
> + buffer = unicam_[Unicam::Image].getBuffers().at(bayer & RPi::MaskID).buffer;
> LOG(RPI, Debug) << "Input re-queue to ISP, buffer id " << (bayer & RPi::MaskID)
> << ", timestamp: " << buffer->metadata().timestamp;
>
> @@ -850,7 +850,7 @@ void Vc4CameraData::prepareIspComplete(const ipa::RPi::BufferIds &buffers)
> ispOutputCount_ = 0;
>
> if (sensorMetadata_ && embeddedId) {
> - buffer = unicam_[Unicam::Embedded].getBuffers().at(embeddedId & RPi::MaskID);
> + buffer = unicam_[Unicam::Embedded].getBuffers().at(embeddedId & RPi::MaskID).buffer;
> handleStreamBuffer(buffer, &unicam_[Unicam::Embedded]);
> }
>
> --
> 2.34.1
>
More information about the libcamera-devel
mailing list