| Message ID | 20231006132000.23504-2-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@raspberrypi.com> > Reviewed-by: David Plowman <david.plowman@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@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 >
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)); + } +} + 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]); }