[libcamera-devel] [PATCH v2 1/2] pipeline: raspberrypi: Use MappedFrameBuffer for embedded data buffers
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Nov 16 13:28:08 CET 2020
Hi Naush,
Thank you for the patch.
On Mon, Nov 16, 2020 at 11:24:57AM +0000, Naushir Patuck wrote:
> Use a MappedFrameBuffer to mmap embedded data buffers for the pipeline
> handler to use in the cases where the sensor does not fill it in. This
> avoids the need to mmap and unmap on every frame.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> .../pipeline/raspberrypi/raspberrypi.cpp | 33 ++++++++++++++-----
> 1 file changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index dd62dfc7..faa06c00 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -24,6 +24,7 @@
> #include <linux/videodev2.h>
>
> #include "libcamera/internal/bayer_format.h"
> +#include "libcamera/internal/buffer.h"
> #include "libcamera/internal/camera_sensor.h"
> #include "libcamera/internal/device_enumerator.h"
> #include "libcamera/internal/ipa_manager.h"
> @@ -165,6 +166,12 @@ public:
> /* Stores the ids of the buffers mapped in the IPA. */
> std::unordered_set<unsigned int> ipaBuffers_;
>
> + /*
> + * Map of (internal) mmaped embedded data buffers, to avoid having to
> + * map/unmap on every frame.
> + */
> + std::map<unsigned int, MappedFrameBuffer> mappedEmbeddedBuffers_;
> +
> /* DMAHEAP allocation helper. */
> RPi::DmaHeap dmaHeap_;
> FileDescriptor lsTable_;
> @@ -1040,6 +1047,13 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)
> return ret;
> }
>
> + if (!data->sensorMetadata_) {
> + for (auto const &it : data->unicam_[Unicam::Embedded].getBuffers()) {
> + data->mappedEmbeddedBuffers_.emplace(it.first,
> + MappedFrameBuffer(it.second, PROT_READ | PROT_WRITE));
That's a fairly long line, could we write
MappedFrameBuffer fb(it.second, PROT_READ | PROT_WRITE);
data->mappedEmbeddedBuffers_.emplace(it.first, std::move(fb));
Alternatively, if you want to avoid the move construction of the
element, you could write
data->mappedEmbeddedBuffers_.emplace(std::piecewise_construct,
std::forward_as_tuple(it.first),
std::forward_as_tuple(it.second,
PROT_READ | PROT_WRITE));
> + }
> + }
> +
> /*
> * Pass the stats and embedded data buffers to the IPA. No other
> * buffers need to be passed.
> @@ -1078,6 +1092,7 @@ void PipelineHandlerRPi::freeBuffers(Camera *camera)
> std::vector<unsigned int> ipaBuffers(data->ipaBuffers_.begin(), data->ipaBuffers_.end());
> data->ipa_->unmapBuffers(ipaBuffers);
> data->ipaBuffers_.clear();
> + data->mappedEmbeddedBuffers_.clear();
>
> for (auto const stream : data->streams_)
> stream->releaseBuffers();
> @@ -1310,14 +1325,16 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)
> * metadata buffer.
> */
> if (!sensorMetadata_) {
> - const FrameBuffer &fb = buffer->planes();
> - uint32_t *mem = static_cast<uint32_t *>(::mmap(nullptr, fb.planes()[0].length,
> - PROT_READ | PROT_WRITE,
> - MAP_SHARED,
> - fb.planes()[0].fd.fd(), 0));
> - mem[0] = ctrl[V4L2_CID_EXPOSURE];
> - mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN];
> - munmap(mem, fb.planes()[0].length);
> + unsigned int bufferId = unicam_[Unicam::Embedded].getBufferId(buffer);
> + auto it = mappedEmbeddedBuffers_.find(bufferId);
> + if (it != mappedEmbeddedBuffers_.end()) {
> + uint32_t *mem = reinterpret_cast<uint32_t *>(it->second.maps()[0].data());
> + mem[0] = ctrl[V4L2_CID_EXPOSURE];
> + mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN];
> + } else {
> + LOG(RPI, Warning) << "Failed to find embedded buffer "
> + << bufferId;
> + }
Out of scope for this series, but I wonder if we shouldn't improve this
with a cleaner API to the IPA instead of faking an embedded data buffer.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> }
> }
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list