From ae9a05847cead7ddedba0ae150e538562afbb13f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20S=C3=B6derlund?= Date: Tue, 26 Nov 2019 00:04:53 +0100 Subject: [PATCH] libcamera: buffer: Switch from Plane to FrameBuffer::Plane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is not libcamera's responsibility to handle memory mappings. Switch from the soon to be removed Plane class which deals with memory mappings to FrameBuffer::Plane which just describes it. This makes the transition to the full FrameBuffer easier. As the full FrameBuffer interface has not yet spread to all parts of libcamera core it is hard to create efficient caching of memory mappings in the qcam application. This will be fixed in a later patch, for now the dmabuf is mapped and unmapped each time it is seen by the application. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- include/libcamera/buffer.h | 6 +++--- src/cam/buffer_writer.cpp | 11 ++++++++--- src/libcamera/buffer.cpp | 4 ++-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 ++---------- src/libcamera/stream.cpp | 6 ++++-- src/libcamera/v4l2_videodevice.cpp | 13 +++++++------ src/qcam/main_window.cpp | 14 ++++++++++---- src/v4l2/v4l2_camera.cpp | 2 +- test/camera/buffer_import.cpp | 2 +- 9 files changed, 38 insertions(+), 32 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index e66e9c9cf..d61efad10 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -95,11 +95,11 @@ private: class BufferMemory final { public: - const std::vector &planes() const { return planes_; } - std::vector &planes() { return planes_; } + const std::vector &planes() const { return planes_; } + std::vector &planes() { return planes_; } private: - std::vector planes_; + std::vector planes_; }; class BufferPool final diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp index c33e99c5f..765a17623 100644 --- a/src/cam/buffer_writer.cpp +++ b/src/cam/buffer_writer.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include "buffer_writer.h" @@ -43,9 +44,11 @@ int BufferWriter::write(Buffer *buffer, const std::string &streamName) return -errno; BufferMemory *mem = buffer->mem(); - for (Plane &plane : mem->planes()) { - void *data = plane.mem(); - unsigned int length = plane.length(); + for (const FrameBuffer::Plane &plane : mem->planes()) { + /* \todo Once the FrameBuffer is done cache mapped memory. */ + void *data = mmap(NULL, plane.length, PROT_READ, MAP_SHARED, + plane.fd.fd(), 0); + unsigned int length = plane.length; ret = ::write(fd, data, length); if (ret < 0) { @@ -59,6 +62,8 @@ int BufferWriter::write(Buffer *buffer, const std::string &streamName) << length << std::endl; break; } + + munmap(data, length); } close(fd); diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 2178bd2fe..ec7c614dd 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -257,13 +257,13 @@ void *Plane::mem() /** * \fn BufferMemory::planes() const * \brief Retrieve the planes within the buffer - * \return A const reference to a vector holding all Planes within the buffer + * \return A const reference to a vector holding all planes within the buffer */ /** * \fn BufferMemory::planes() * \brief Retrieve the planes within the buffer - * \return A reference to a vector holding all Planes within the buffer + * \return A reference to a vector holding all planes within the buffer */ /** diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index d7ee95ded..1e4457158 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -687,22 +687,14 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera, } for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) { - FrameBuffer::Plane plane; - plane.fd = FileDescriptor(paramPool_.buffers()[i].planes()[0].dmabuf()); - plane.length = paramPool_.buffers()[i].planes()[0].length(); - data->ipaBuffers_.push_back({ .id = RKISP1_PARAM_BASE | i, - .planes = { plane } }); + .planes = paramPool_.buffers()[i].planes() }); paramBuffers_.push(new Buffer(i)); } for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) { - FrameBuffer::Plane plane; - plane.fd = FileDescriptor(statPool_.buffers()[i].planes()[0].dmabuf()); - plane.length = statPool_.buffers()[i].planes()[0].length(); - data->ipaBuffers_.push_back({ .id = RKISP1_STAT_BASE | i, - .planes = { plane } }); + .planes = statPool_.buffers()[i].planes() }); statBuffers_.push(new Buffer(i)); } diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index 45f31ae1e..16a323f13 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -577,8 +577,10 @@ int Stream::mapBuffer(const Buffer *buffer) if (dmabufs[i] == -1) break; - mem->planes().emplace_back(); - mem->planes().back().setDmabuf(dmabufs[i], 0); + FrameBuffer::Plane plane; + plane.fd = FileDescriptor(dmabufs[i]); + plane.length = 0; + mem->planes().push_back(plane); } /* Remove the buffer from the cache and return its index. */ diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 033f7cc0d..1dc9e1935 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -922,9 +922,10 @@ int V4L2VideoDevice::createPlane(BufferMemory *buffer, unsigned int index, return ret; } - buffer->planes().emplace_back(); - Plane &plane = buffer->planes().back(); - plane.setDmabuf(expbuf.fd, length); + FrameBuffer::Plane plane; + plane.fd = FileDescriptor(expbuf.fd); + plane.length = length; + buffer->planes().push_back(plane); ::close(expbuf.fd); return 0; @@ -987,14 +988,14 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer) bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); BufferMemory *mem = &bufferPool_->buffers()[buf.index]; - const std::vector &planes = mem->planes(); + const std::vector &planes = mem->planes(); if (buf.memory == V4L2_MEMORY_DMABUF) { if (multiPlanar) { for (unsigned int p = 0; p < planes.size(); ++p) - v4l2Planes[p].m.fd = planes[p].dmabuf(); + v4l2Planes[p].m.fd = planes[p].fd.fd(); } else { - buf.m.fd = planes[0].dmabuf(); + buf.m.fd = planes[0].fd.fd(); } } diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 0c7ca61ac..0a353e8ba 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -296,13 +297,18 @@ void MainWindow::requestComplete(Request *request) int MainWindow::display(Buffer *buffer) { - BufferMemory *mem = buffer->mem(); - if (mem->planes().size() != 1) + if (buffer->mem()->planes().size() != 1) return -EINVAL; - Plane &plane = mem->planes().front(); - unsigned char *raw = static_cast(plane.mem()); + /* \todo Once the FrameBuffer is done cache mapped memory. */ + const FrameBuffer::Plane &plane = buffer->mem()->planes().front(); + void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED, + plane.fd.fd(), 0); + + unsigned char *raw = static_cast(memory); viewfinder_->display(raw, buffer->bytesused()); + munmap(memory, plane.length); + return 0; } diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index 07f05a312..7221755dd 100644 --- a/src/v4l2/v4l2_camera.cpp +++ b/src/v4l2/v4l2_camera.cpp @@ -135,7 +135,7 @@ void V4L2Camera::freeBuffers() FileDescriptor V4L2Camera::getBufferFd(unsigned int index) { Stream *stream = *camera_->streams().begin(); - return FileDescriptor(stream->buffers()[index].planes()[0].dmabuf()); + return stream->buffers()[index].planes()[0].fd; } int V4L2Camera::streamOn() diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index 3efe02704..171540edd 100644 --- a/test/camera/buffer_import.cpp +++ b/test/camera/buffer_import.cpp @@ -178,7 +178,7 @@ private: uint64_t cookie = index; BufferMemory &mem = pool_.buffers()[index]; - int dmabuf = mem.planes()[0].dmabuf(); + int dmabuf = mem.planes()[0].fd.fd(); requestReady.emit(cookie, dmabuf);