Patch Detail
Show a patch.
GET /api/patches/2592/?format=api
{ "id": 2592, "url": "https://patchwork.libcamera.org/api/patches/2592/?format=api", "web_url": "https://patchwork.libcamera.org/patch/2592/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20200112010212.2609025-11-niklas.soderlund@ragnatech.se>", "date": "2020-01-12T01:01:50", "name": "[libcamera-devel,v4,10/32] libcamera: buffer: Switch from Plane to FrameBuffer::Plane", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "d025d7193c02f2e6c71b71a061499d6603a6f4fe", "submitter": { "id": 5, "url": "https://patchwork.libcamera.org/api/people/5/?format=api", "name": "Niklas Söderlund", "email": "niklas.soderlund@ragnatech.se" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/2592/mbox/", "series": [ { "id": 617, "url": "https://patchwork.libcamera.org/api/series/617/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=617", "date": "2020-01-12T01:01:40", "name": "libcamera: Rework buffer API", "version": 4, "mbox": "https://patchwork.libcamera.org/series/617/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/2592/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/2592/checks/", "tags": {}, "headers": { "Return-Path": "<niklas.soderlund@ragnatech.se>", "Received": [ "from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net\n\t[195.74.38.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E8ACE606CE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 12 Jan 2020 02:03:06 +0100 (CET)", "from bismarck.berto.se (p54ac5d7b.dip0.t-ipconnect.de\n\t[84.172.93.123]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA\n\tid 4864035b-34d7-11ea-b6d8-005056917f90;\n\tSun, 12 Jan 2020 02:03:02 +0100 (CET)" ], "X-Halon-ID": "4864035b-34d7-11ea-b6d8-005056917f90", "Authorized-sender": "niklas@soderlund.pp.se", "From": "=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Sun, 12 Jan 2020 02:01:50 +0100", "Message-Id": "<20200112010212.2609025-11-niklas.soderlund@ragnatech.se>", "X-Mailer": "git-send-email 2.24.1", "In-Reply-To": "<20200112010212.2609025-1-niklas.soderlund@ragnatech.se>", "References": "<20200112010212.2609025-1-niklas.soderlund@ragnatech.se>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=UTF-8", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v4 10/32] libcamera: buffer: Switch from\n\tPlane to FrameBuffer::Plane", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "X-List-Received-Date": "Sun, 12 Jan 2020 01:03:07 -0000" }, "content": "It is not libcamera's responsibility to handle memory mappings. Switch\nfrom the soon to be removed Plane class which deals with memory\nmappings to FrameBuffer::Plane which just describes it. This makes the\ntransition to the full FrameBuffer easier.\n\nAs the full FrameBuffer interface has not yet spread to all parts of\nlibcamera core it is hard to create efficient caching of memory mappings\nin the qcam application. This will be fixed in a later patch, for now\nthe dmabuf is mapped and unmapped each time it is seen by the\napplication.\n\nSigned-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n* Changes since v2\n- Add todo about adding caching for cam mmap() call\n- Use {param,stat}Pool_.buffers()[i].planes() directly instead of\n creating a new plane.\n- Use push_back() instead of emplace_back() for already constructed\n objects.\n- s/todo:/todo/\n---\n include/libcamera/buffer.h | 6 +++---\n src/cam/buffer_writer.cpp | 11 ++++++++---\n src/libcamera/buffer.cpp | 4 ++--\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 ++----------\n src/libcamera/stream.cpp | 6 ++++--\n src/libcamera/v4l2_videodevice.cpp | 13 +++++++------\n src/qcam/main_window.cpp | 14 ++++++++++----\n src/v4l2/v4l2_camera.cpp | 2 +-\n test/camera/buffer_import.cpp | 2 +-\n 9 files changed, 38 insertions(+), 32 deletions(-)", "diff": "diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h\nindex e66e9c9cf828160a..d61efad10f7d364d 100644\n--- a/include/libcamera/buffer.h\n+++ b/include/libcamera/buffer.h\n@@ -95,11 +95,11 @@ private:\n class BufferMemory final\n {\n public:\n-\tconst std::vector<Plane> &planes() const { return planes_; }\n-\tstd::vector<Plane> &planes() { return planes_; }\n+\tconst std::vector<FrameBuffer::Plane> &planes() const { return planes_; }\n+\tstd::vector<FrameBuffer::Plane> &planes() { return planes_; }\n \n private:\n-\tstd::vector<Plane> planes_;\n+\tstd::vector<FrameBuffer::Plane> planes_;\n };\n \n class BufferPool final\ndiff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp\nindex c33e99c5f8173db8..765a176238e58dff 100644\n--- a/src/cam/buffer_writer.cpp\n+++ b/src/cam/buffer_writer.cpp\n@@ -10,6 +10,7 @@\n #include <iostream>\n #include <sstream>\n #include <string.h>\n+#include <sys/mman.h>\n #include <unistd.h>\n \n #include \"buffer_writer.h\"\n@@ -43,9 +44,11 @@ int BufferWriter::write(Buffer *buffer, const std::string &streamName)\n \t\treturn -errno;\n \n \tBufferMemory *mem = buffer->mem();\n-\tfor (Plane &plane : mem->planes()) {\n-\t\tvoid *data = plane.mem();\n-\t\tunsigned int length = plane.length();\n+\tfor (const FrameBuffer::Plane &plane : mem->planes()) {\n+\t\t/* \\todo Once the FrameBuffer is done cache mapped memory. */\n+\t\tvoid *data = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,\n+\t\t\t\t plane.fd.fd(), 0);\n+\t\tunsigned int length = plane.length;\n \n \t\tret = ::write(fd, data, length);\n \t\tif (ret < 0) {\n@@ -59,6 +62,8 @@ int BufferWriter::write(Buffer *buffer, const std::string &streamName)\n \t\t\t\t << length << std::endl;\n \t\t\tbreak;\n \t\t}\n+\n+\t\tmunmap(data, length);\n \t}\n \n \tclose(fd);\ndiff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp\nindex 2178bd2fe17111dc..ec7c614dd654e4eb 100644\n--- a/src/libcamera/buffer.cpp\n+++ b/src/libcamera/buffer.cpp\n@@ -257,13 +257,13 @@ void *Plane::mem()\n /**\n * \\fn BufferMemory::planes() const\n * \\brief Retrieve the planes within the buffer\n- * \\return A const reference to a vector holding all Planes within the buffer\n+ * \\return A const reference to a vector holding all planes within the buffer\n */\n \n /**\n * \\fn BufferMemory::planes()\n * \\brief Retrieve the planes within the buffer\n- * \\return A reference to a vector holding all Planes within the buffer\n+ * \\return A reference to a vector holding all planes within the buffer\n */\n \n /**\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex d7ee95ded0f76027..1e44571589a6003d 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -687,22 +687,14 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,\n \t}\n \n \tfor (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) {\n-\t\tFrameBuffer::Plane plane;\n-\t\tplane.fd = FileDescriptor(paramPool_.buffers()[i].planes()[0].dmabuf());\n-\t\tplane.length = paramPool_.buffers()[i].planes()[0].length();\n-\n \t\tdata->ipaBuffers_.push_back({ .id = RKISP1_PARAM_BASE | i,\n-\t\t\t\t\t .planes = { plane } });\n+\t\t\t\t\t .planes = paramPool_.buffers()[i].planes() });\n \t\tparamBuffers_.push(new Buffer(i));\n \t}\n \n \tfor (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) {\n-\t\tFrameBuffer::Plane plane;\n-\t\tplane.fd = FileDescriptor(statPool_.buffers()[i].planes()[0].dmabuf());\n-\t\tplane.length = statPool_.buffers()[i].planes()[0].length();\n-\n \t\tdata->ipaBuffers_.push_back({ .id = RKISP1_STAT_BASE | i,\n-\t\t\t\t\t .planes = { plane } });\n+\t\t\t\t\t .planes = statPool_.buffers()[i].planes() });\n \t\tstatBuffers_.push(new Buffer(i));\n \t}\n \ndiff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\nindex 45f31ae1e2daeb53..16a323f135b2022a 100644\n--- a/src/libcamera/stream.cpp\n+++ b/src/libcamera/stream.cpp\n@@ -577,8 +577,10 @@ int Stream::mapBuffer(const Buffer *buffer)\n \t\tif (dmabufs[i] == -1)\n \t\t\tbreak;\n \n-\t\tmem->planes().emplace_back();\n-\t\tmem->planes().back().setDmabuf(dmabufs[i], 0);\n+\t\tFrameBuffer::Plane plane;\n+\t\tplane.fd = FileDescriptor(dmabufs[i]);\n+\t\tplane.length = 0;\n+\t\tmem->planes().push_back(plane);\n \t}\n \n \t/* Remove the buffer from the cache and return its index. */\ndiff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\nindex 033f7cc0d809ea8a..1dc9e19350f825a9 100644\n--- a/src/libcamera/v4l2_videodevice.cpp\n+++ b/src/libcamera/v4l2_videodevice.cpp\n@@ -922,9 +922,10 @@ int V4L2VideoDevice::createPlane(BufferMemory *buffer, unsigned int index,\n \t\treturn ret;\n \t}\n \n-\tbuffer->planes().emplace_back();\n-\tPlane &plane = buffer->planes().back();\n-\tplane.setDmabuf(expbuf.fd, length);\n+\tFrameBuffer::Plane plane;\n+\tplane.fd = FileDescriptor(expbuf.fd);\n+\tplane.length = length;\n+\tbuffer->planes().push_back(plane);\n \t::close(expbuf.fd);\n \n \treturn 0;\n@@ -987,14 +988,14 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer)\n \n \tbool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type);\n \tBufferMemory *mem = &bufferPool_->buffers()[buf.index];\n-\tconst std::vector<Plane> &planes = mem->planes();\n+\tconst std::vector<FrameBuffer::Plane> &planes = mem->planes();\n \n \tif (buf.memory == V4L2_MEMORY_DMABUF) {\n \t\tif (multiPlanar) {\n \t\t\tfor (unsigned int p = 0; p < planes.size(); ++p)\n-\t\t\t\tv4l2Planes[p].m.fd = planes[p].dmabuf();\n+\t\t\t\tv4l2Planes[p].m.fd = planes[p].fd.fd();\n \t\t} else {\n-\t\t\tbuf.m.fd = planes[0].dmabuf();\n+\t\t\tbuf.m.fd = planes[0].fd.fd();\n \t\t}\n \t}\n \ndiff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\nindex 0c7ca61ac12ec41c..0a353e8ba247caf9 100644\n--- a/src/qcam/main_window.cpp\n+++ b/src/qcam/main_window.cpp\n@@ -8,6 +8,7 @@\n #include <iomanip>\n #include <iostream>\n #include <string>\n+#include <sys/mman.h>\n \n #include <QCoreApplication>\n #include <QInputDialog>\n@@ -296,13 +297,18 @@ void MainWindow::requestComplete(Request *request)\n \n int MainWindow::display(Buffer *buffer)\n {\n-\tBufferMemory *mem = buffer->mem();\n-\tif (mem->planes().size() != 1)\n+\tif (buffer->mem()->planes().size() != 1)\n \t\treturn -EINVAL;\n \n-\tPlane &plane = mem->planes().front();\n-\tunsigned char *raw = static_cast<unsigned char *>(plane.mem());\n+\t/* \\todo Once the FrameBuffer is done cache mapped memory. */\n+\tconst FrameBuffer::Plane &plane = buffer->mem()->planes().front();\n+\tvoid *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,\n+\t\t\t plane.fd.fd(), 0);\n+\n+\tunsigned char *raw = static_cast<unsigned char *>(memory);\n \tviewfinder_->display(raw, buffer->bytesused());\n \n+\tmunmap(memory, plane.length);\n+\n \treturn 0;\n }\ndiff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp\nindex 07f05a31261c1b25..7221755ddb5db4bf 100644\n--- a/src/v4l2/v4l2_camera.cpp\n+++ b/src/v4l2/v4l2_camera.cpp\n@@ -135,7 +135,7 @@ void V4L2Camera::freeBuffers()\n FileDescriptor V4L2Camera::getBufferFd(unsigned int index)\n {\n \tStream *stream = *camera_->streams().begin();\n-\treturn FileDescriptor(stream->buffers()[index].planes()[0].dmabuf());\n+\treturn stream->buffers()[index].planes()[0].fd;\n }\n \n int V4L2Camera::streamOn()\ndiff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp\nindex 3efe02704c02f691..171540edd96f9fca 100644\n--- a/test/camera/buffer_import.cpp\n+++ b/test/camera/buffer_import.cpp\n@@ -178,7 +178,7 @@ private:\n \n \t\tuint64_t cookie = index;\n \t\tBufferMemory &mem = pool_.buffers()[index];\n-\t\tint dmabuf = mem.planes()[0].dmabuf();\n+\t\tint dmabuf = mem.planes()[0].fd.fd();\n \n \t\trequestReady.emit(cookie, dmabuf);\n \n", "prefixes": [ "libcamera-devel", "v4", "10/32" ] }