[{"id":3413,"web_url":"https://patchwork.libcamera.org/comment/3413/","msgid":"<20200111013525.GP4859@pendragon.ideasonboard.com>","date":"2020-01-11T01:35:25","subject":"Re: [libcamera-devel] [PATCH v3 30/33] cam: Cache buffer memory\n\tmapping","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Fri, Jan 10, 2020 at 08:38:05PM +0100, Niklas Söderlund wrote:\n> With the buffer allocator in use it's possible to cache the dmabuf\n> memory mappings when starting the camera instead of mapping and\n> unmapping them each time.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> src/cam/buffer_writer.cpp | 27 ++++++++++++++++++++++-----\n> src/cam/buffer_writer.h | 5 +++++\n> src/cam/capture.cpp | 3 +++\n> 3 files changed, 30 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp\n> index 1d7366c87714cd91..c5a5eb46224ac3eb 100644\n> --- a/src/cam/buffer_writer.cpp\n> +++ b/src/cam/buffer_writer.cpp\n> @@ -22,6 +22,27 @@ BufferWriter::BufferWriter(const std::string &pattern)\n> {\n> }\n> \n> +BufferWriter::~BufferWriter()\n> +{\n> +\tfor (auto &iter : mappedBuffers_) {\n> +\t\tvoid *memory = iter.second.first;\n> +\t\tunsigned int length = iter.second.second;\n\nUsage of std::pair<> makes the code a bit cryptic :-S I'm tempted to\npropose a refactoring, but we already know we need to implement a buffer\nmmap helper, so this code will go away.\n\nReviewed-by: Laurent Pinchart \n\n> +\t\tmunmap(memory, length);\n> +\t}\n> +\tmappedBuffers_.clear();\n> +}\n> +\n> +void BufferWriter::mapBuffer(FrameBuffer *buffer)\n> +{\n> +\tfor (const FrameBuffer::Plane &plane : buffer->planes()) {\n> +\t\tvoid *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,\n> +\t\t\t\t plane.fd.fd(), 0);\n> +\n> +\t\tmappedBuffers_[plane.fd.fd()] =\n> +\t\t\tstd::make_pair(memory, plane.length);\n> +\t}\n> +}\n> +\n> int BufferWriter::write(FrameBuffer *buffer, const std::string &streamName)\n> {\n> \tstd::string filename;\n> @@ -44,9 +65,7 @@ int BufferWriter::write(FrameBuffer *buffer, const std::string &streamName)\n> \t\treturn -errno;\n> \n> \tfor (const FrameBuffer::Plane &plane : buffer->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\tvoid *data = mappedBuffers_[plane.fd.fd()].first;\n> \t\tunsigned int length = plane.length;\n> \n> \t\tret = ::write(fd, data, length);\n> @@ -61,8 +80,6 @@ int BufferWriter::write(FrameBuffer *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);\n> diff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h\n> index 5917a7dfb5e28106..8c9b2436fdae4fc3 100644\n> --- a/src/cam/buffer_writer.h\n> +++ b/src/cam/buffer_writer.h\n> @@ -7,6 +7,7 @@\n> #ifndef __LIBCAMERA_BUFFER_WRITER_H__\n> #define __LIBCAMERA_BUFFER_WRITER_H__\n> \n> +#include \n> #include \n> \n> #include \n> @@ -15,12 +16,16 @@ class BufferWriter\n> {\n> public:\n> \tBufferWriter(const std::string &pattern = \"frame-#.bin\");\n> +\t~BufferWriter();\n> +\n> +\tvoid mapBuffer(libcamera::FrameBuffer *buffer);\n> \n> \tint write(libcamera::FrameBuffer *buffer,\n> \t\t const std::string &streamName);\n> \n> private:\n> \tstd::string pattern_;\n> +\tstd::map> mappedBuffers_;\n> };\n> \n> #endif /* __LIBCAMERA_BUFFER_WRITER_H__ */\n> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp\n> index dd078eb0ae4a2c62..738fa1c267eb6e36 100644\n> --- a/src/cam/capture.cpp\n> +++ b/src/cam/capture.cpp\n> @@ -116,6 +116,9 @@ int Capture::capture(EventLoop *loop, FrameBufferAllocator *allocator)\n> \t\t\t\t\t << std::endl;\n> \t\t\t\treturn ret;\n> \t\t\t}\n> +\n> +\t\t\tif (writer_)\n> +\t\t\t\twriter_->mapBuffer(buffer.get());\n> \t\t}\n> \n> \t\trequests.push_back(request);","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 934EB606AC\n\tfor ;\n\tSat, 11 Jan 2020 02:35:39 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0CD6E9B1;\n\tSat, 11 Jan 2020 02:35:38 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1578706539;\n\tbh=uF5DXFMjUfsKAtniF2nGPpK0i5XeiX+2IPLxKBvC4Wo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Nqnl9Ln3MGHjI9aDRJ8+vw9MUGu2Ufbe6w3RVboz05qWmJdY3CiKqCe2+pf2mXwph\n\tYvwPeXoiWtSpiZxZW93lcC7CIfL9ogX8ZMn7ygR0mwgeY9myFqGfOfXaeLZXDrtakl\n\tpyv8wL5KXPpc5IOf81oPz1BnqDNDIUr+Do3newms=","Date":"Sat, 11 Jan 2020 03:35:25 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200111013525.GP4859@pendragon.ideasonboard.com>","References":"<20200110193808.2266294-1-niklas.soderlund@ragnatech.se>\n\t<20200110193808.2266294-31-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200110193808.2266294-31-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 30/33] cam: Cache buffer memory\n\tmapping","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 11 Jan 2020 01:35:39 -0000"}}]