[{"id":713,"web_url":"https://patchwork.libcamera.org/comment/713/","msgid":"<20190131091638.GB4197@pendragon.ideasonboard.com>","date":"2019-01-31T09:16:38","subject":"Re: [libcamera-devel] [PATCH 4/4] libcamera: buffer: Provide Buffer\n\tPlanes","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Tue, Jan 29, 2019 at 01:53:57PM +0000, Kieran Bingham wrote:\n> Extend the Buffer management to support multi-planar formats.\n> \n> An image within the system may use one or more Plane objects to track each\n> plane in the case of multi-planar image formats. The Buffer class manages all\n> of the data required to render or interpret the raw image data.\n> \n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>  include/libcamera/buffer.h |  38 +++++++-\n>  src/libcamera/buffer.cpp   | 189 +++++++++++++++++++++++++++++++++++--\n>  2 files changed, 215 insertions(+), 12 deletions(-)\n> \n> diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h\n> index dda5075f2879..08a74fb70b88 100644\n> --- a/include/libcamera/buffer.h\n> +++ b/include/libcamera/buffer.h\n> @@ -11,20 +11,50 @@\n>  \n>  namespace libcamera {\n>  \n> -class Buffer\n> +class Plane\n>  {\n>  public:\n> -\tBuffer();\n> -\t~Buffer();\n> +\tPlane();\n> +\tPlane(unsigned int length, unsigned int offset);\n> +\t~Plane();\n>  \n>  \tint dmabuf() const { return fd_; };\n>  \tint setDmabuf(int fd);\n>  \n> +\tint mmap(int fd);\n>  \tint mmap();\n> -\tvoid munmap();\n> +\tint munmap();\n> +\n> +\tunsigned int length() const { return length_; };\n> +\tvoid *mem() const { return mem_; };\n>  \n>  private:\n>  \tint fd_;\n> +\tunsigned int length_;\n> +\tunsigned int offset_;\n> +\tvoid *mem_;\n> +};\n> +\n> +class Buffer\n> +{\n> +public:\n> +\tBuffer();\n> +\tvirtual ~Buffer();\n> +\n> +\tint mmap(int fd);\n> +\tint munmap();\n> +\n> +\tunsigned int index() const { return index_; };\n> +\tconst std::vector<Plane *> &planes() { return planes_; };\n> +\n> +private:\n> +\tunsigned int index_;\n> +\n> +\tunsigned int format_;\n> +\tunsigned int width_;\n> +\tunsigned int height_;\n> +\n> +\tstd::vector<Plane *> planes_;\n>  };\n>  \n>  class BufferPool\n> diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp\n> index 4a870df77e92..7b84a97dcd2b 100644\n> --- a/src/libcamera/buffer.cpp\n> +++ b/src/libcamera/buffer.cpp\n> @@ -6,10 +6,14 @@\n>   */\n>  \n>  #include <errno.h>\n> +#include <string.h>\n> +#include <sys/mman.h>\n>  #include <unistd.h>\n>  \n>  #include <libcamera/buffer.h>\n>  \n> +#include \"log.h\"\n> +\n>  /**\n>   * \\file buffer.h\n>   * \\brief Buffer handling\n> @@ -17,25 +21,46 @@\n>  \n>  namespace libcamera {\n>  \n> +LOG_DEFINE_CATEGORY(Buffer)\n> +\n>  /**\n> - * \\class Buffer\n> - * \\brief A memory buffer to store a frame\n> + * \\class Plane\n> + * \\brief A memory buffer to store a single plane of a frame\n\nDefining Plane as a memory buffer, and Buffer as a set of Plane, is too\nconfusing. We need better than that.\n\n>   *\n> - * The Buffer class represents a memory buffer used to store a frame.\n> + * The Plane class represents a memory region used to store a single plane. It\n\nAnd here defining Plane as a plane won't really help understanding the\nAPI. We need an overview of how the Buffer and Plane objects interact.\n\n> + * may be backed by it's own dmaBuf handle, and represents a single plane from a\n\ns/it's/its/\n\n> + * Buffer. In the case of multi-planar image formats, multiple Plane objects are\n> + * attached to a Buffer to manage the whole picture.\n\nTry reading this paragraph with a newcommer's point of view and I think\nyou'll realize we need better :-)\n\n>   */\n> -Buffer::Buffer()\n> -\t: fd_(-1)\n> +\n> +Plane::Plane()\n> +\t: fd_(-1), length_(0), offset_(0), mem_(0)\n>  {\n>  }\n>  \n> -Buffer::~Buffer()\n> +/**\n> + * \\brief Construct a Plane memory region for CPU mappings\n> + * \\param[in] length The size of the memory region which should be mapped\n> + * \\param[in] offset The offset into the file descriptor base at which the\n> + * buffer commences\n> + *\n> + * \\sa mmap(int fd)\n> + */\n> +Plane::Plane(unsigned int length, unsigned int offset)\n> +\t: fd_(-1), length_(length), offset_(offset), mem_(0)\n> +{\n> +}\n> +\n> +Plane::~Plane()\n>  {\n> +\tmunmap();\n> +\n>  \tif (fd_ != -1)\n>  \t\tclose(fd_);\n>  }\n>  \n>  /**\n> - * \\fn Buffer::dmabuf()\n> + * \\fn Plane::dmabuf()\n>   * \\brief Get the dmabuf file handle backing the buffer\n>   */\n>  \n> @@ -44,7 +69,7 @@ Buffer::~Buffer()\n>   *\n>   * The \\a fd dmabuf file handle is duplicated and stored.\n>   */\n> -int Buffer::setDmabuf(int fd)\n> +int Plane::setDmabuf(int fd)\n>  {\n>  \tif (fd_ != -1) {\n>  \t\tclose(fd_);\n> @@ -61,6 +86,154 @@ int Buffer::setDmabuf(int fd)\n>  \treturn 0;\n>  }\n>  \n> +/**\n> + * \\brief Map the plane memory data to a CPU accessible address\n> + *\n> + * The \\a fd specifies the file descriptor to map the memory from.\n> + *\n> + * \\return 0 on success or a negative error value otherwise.\n> + */\n> +int Plane::mmap(int fd)\n> +{\n> +\tvoid *map;\n> +\n> +\tmap = ::mmap(NULL, length_, PROT_READ | PROT_WRITE, MAP_SHARED, fd,\n> +\t\t     offset_);\n> +\tif (map == reinterpret_cast<void *>(-1)) {\n> +\t\tint ret = -errno;\n> +\t\tLOG(Buffer, Error)\n> +\t\t\t<< \"Failed to mmap buffer: \" << strerror(ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tmem_ = map;\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\brief Map the plane memory data to a CPU accessible address\n> + *\n> + * The file descriptor to map the memory from must be set by a call to\n> + * setDmaBuf before calling this function.\n> + *\n> + * \\sa setDmaBuf\n> + *\n> + * \\return 0 on success or a negative error value otherwise.\n\nI don't understand why you need two version of mmap().\n\n> + */\n> +int Plane::mmap()\n> +{\n> +\treturn mmap(fd_);\n> +}\n> +\n> +/**\n> + * \\brief Unmap any existing CPU accessible mapping\n> + *\n> + * Unmap the memory mapped by an earlier call to mmap.\n> + *\n> + * \\return 0 on success or a negative error value otherwise.\n> + */\n> +int Plane::munmap()\n> +{\n> +\tint ret = 0;\n> +\n> +\tif (mem_)\n> +\t\tret = ::munmap(mem_, length_);\n> +\n> +\tif (ret) {\n> +\t\tret = -errno;\n> +\t\tLOG(Buffer, Warning)\n> +\t\t\t<< \"Failed to unmap buffer: \" << strerror(ret);\n> +\t} else {\n> +\t\tmem_ = 0;\n> +\t}\n> +\n> +\treturn ret;\n> +}\n> +\n> +/**\n> + * \\fn Plane::length()\n> + * \\brief Get the length of the memory buffer\n> + */\n> +\n> +/**\n> + * \\fn Plane::mem()\n> + * \\brief Get the CPU accessible memory address if mapped\n> + */\n> +\n> +/**\n> + * \\class Buffer\n> + * \\brief A memory buffer to store an image\n> + *\n> + * The Buffer class represents the memory buffers used to store a\n> + * full frame image, which may contain multiple separate memory Plane\n> + * objects if the image format is multi-planar.\n> + */\n> +\n> +Buffer::Buffer()\n> +\t: index_(-1), format_(0), width_(0), height_(0)\n> +{\n> +}\n> +\n> +Buffer::~Buffer()\n> +{\n> +\tfor (Plane *plane : planes_)\n> +\t\tdelete plane;\n> +\n> +\tplanes_.clear();\n> +}\n> +\n> +/**\n> + * \\brief Map each buffer plane to a CPU accessible address\n> + *\n> + * Utilise the \\a fd file descriptor to map each of the Buffer's planes to a CPU\n> + * accessible address.\n> + *\n> + * \\return 0 on success or a negative error value otherwise.\n> + */\n> +int Buffer::mmap(int fd)\n> +{\n> +\tint ret;\n> +\n> +\tfor (Plane *plane : planes_) {\n> +\t\tret = plane->mmap(fd);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\brief Unmap any existing CPU accessible mapping for each plane\n> + *\n> + * Unmap the memory mapped by an earlier call to mmap.\n> + *\n> + * \\return 0 on success or a negative error value otherwise.\n> + */\n> +int Buffer::munmap()\n> +{\n> +\tint ret;\n> +\n> +\tfor (Plane *plane : planes_) {\n> +\t\tret = plane->munmap();\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\fn Buffer::index()\n> + * \\brief Get the Buffer index value\n> + */\n> +\n> +/**\n> + * \\fn Buffer::planes()\n> + * \\brief Return a reference to the vector holding all Planes within the buffer\n> + */\n> +\n>  /**\n>   * \\class BufferPool\n>   * \\brief A pool of buffers","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 631F160B2D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 31 Jan 2019 10:16:42 +0100 (CET)","from pendragon.ideasonboard.com (85-76-34-136-nat.elisa-mobile.fi\n\t[85.76.34.136])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0E2D541;\n\tThu, 31 Jan 2019 10:16:40 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548926201;\n\tbh=zXgsOFuIwj/UJGPS4mPJejDAdijc7C+4mZDjqnq52Ns=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qdkFUW1vaC0aNJRlVKPlipi+bkGje512i75i9aJofue5RPSymrXLS7tiLe5bOleFD\n\t0r0Kxycuj5XTt0g/PYdjuJFB/LqW7t3jttuMiXk0PoiCDQ3ce5nrjfymW4waPp4Pfd\n\tHTMKE1L4aKnpQ/MTv7RdctAg/1R/j2USXd0BadK0=","Date":"Thu, 31 Jan 2019 11:16:38 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"LibCamera Devel <libcamera-devel@lists.libcamera.org>","Message-ID":"<20190131091638.GB4197@pendragon.ideasonboard.com>","References":"<20190129135357.32339-1-kieran.bingham@ideasonboard.com>\n\t<20190129135357.32339-5-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190129135357.32339-5-kieran.bingham@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 4/4] libcamera: buffer: Provide Buffer\n\tPlanes","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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":"Thu, 31 Jan 2019 09:16:42 -0000"}},{"id":725,"web_url":"https://patchwork.libcamera.org/comment/725/","msgid":"<1f6c7120-45f8-d996-a284-cb8b75502166@ideasonboard.com>","date":"2019-01-31T19:31:43","subject":"Re: [libcamera-devel] [PATCH 4/4] libcamera: buffer: Provide Buffer\n\tPlanes","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 31/01/2019 09:16, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Tue, Jan 29, 2019 at 01:53:57PM +0000, Kieran Bingham wrote:\n>> Extend the Buffer management to support multi-planar formats.\n>>\n>> An image within the system may use one or more Plane objects to track each\n>> plane in the case of multi-planar image formats. The Buffer class manages all\n>> of the data required to render or interpret the raw image data.\n>>\n>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>> ---\n>>  include/libcamera/buffer.h |  38 +++++++-\n>>  src/libcamera/buffer.cpp   | 189 +++++++++++++++++++++++++++++++++++--\n>>  2 files changed, 215 insertions(+), 12 deletions(-)\n>>\n>> diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h\n>> index dda5075f2879..08a74fb70b88 100644\n>> --- a/include/libcamera/buffer.h\n>> +++ b/include/libcamera/buffer.h\n>> @@ -11,20 +11,50 @@\n>>  \n>>  namespace libcamera {\n>>  \n>> -class Buffer\n>> +class Plane\n>>  {\n>>  public:\n>> -\tBuffer();\n>> -\t~Buffer();\n>> +\tPlane();\n>> +\tPlane(unsigned int length, unsigned int offset);\n>> +\t~Plane();\n>>  \n>>  \tint dmabuf() const { return fd_; };\n>>  \tint setDmabuf(int fd);\n>>  \n>> +\tint mmap(int fd);\n>>  \tint mmap();\n>> -\tvoid munmap();\n>> +\tint munmap();\n>> +\n>> +\tunsigned int length() const { return length_; };\n>> +\tvoid *mem() const { return mem_; };\n>>  \n>>  private:\n>>  \tint fd_;\n>> +\tunsigned int length_;\n>> +\tunsigned int offset_;\n>> +\tvoid *mem_;\n>> +};\n>> +\n>> +class Buffer\n>> +{\n>> +public:\n>> +\tBuffer();\n>> +\tvirtual ~Buffer();\n>> +\n>> +\tint mmap(int fd);\n>> +\tint munmap();\n>> +\n>> +\tunsigned int index() const { return index_; };\n>> +\tconst std::vector<Plane *> &planes() { return planes_; };\n>> +\n>> +private:\n>> +\tunsigned int index_;\n>> +\n>> +\tunsigned int format_;\n>> +\tunsigned int width_;\n>> +\tunsigned int height_;\n>> +\n>> +\tstd::vector<Plane *> planes_;\n>>  };\n>>  \n>>  class BufferPool\n>> diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp\n>> index 4a870df77e92..7b84a97dcd2b 100644\n>> --- a/src/libcamera/buffer.cpp\n>> +++ b/src/libcamera/buffer.cpp\n>> @@ -6,10 +6,14 @@\n>>   */\n>>  \n>>  #include <errno.h>\n>> +#include <string.h>\n>> +#include <sys/mman.h>\n>>  #include <unistd.h>\n>>  \n>>  #include <libcamera/buffer.h>\n>>  \n>> +#include \"log.h\"\n>> +\n>>  /**\n>>   * \\file buffer.h\n>>   * \\brief Buffer handling\n>> @@ -17,25 +21,46 @@\n>>  \n>>  namespace libcamera {\n>>  \n>> +LOG_DEFINE_CATEGORY(Buffer)\n>> +\n>>  /**\n>> - * \\class Buffer\n>> - * \\brief A memory buffer to store a frame\n>> + * \\class Plane\n>> + * \\brief A memory buffer to store a single plane of a frame\n> \n> Defining Plane as a memory buffer, and Buffer as a set of Plane, is too\n> confusing. We need better than that.\n\nMemory Region as below?\n\n> \n>>   *\n>> - * The Buffer class represents a memory buffer used to store a frame.\n>> + * The Plane class represents a memory region used to store a single plane. It\n> \n> And here defining Plane as a plane won't really help understanding the\n> API. We need an overview of how the Buffer and Plane objects interact.\n> \n>> + * may be backed by it's own dmaBuf handle, and represents a single plane from a\n> \n> s/it's/its/\n> \n>> + * Buffer. In the case of multi-planar image formats, multiple Plane objects are\n>> + * attached to a Buffer to manage the whole picture.\n> \n> Try reading this paragraph with a newcommer's point of view and I think\n> you'll realize we need better :-)\n\n> \n>>   */\n>> -Buffer::Buffer()\n>> -\t: fd_(-1)\n>> +\n>> +Plane::Plane()\n>> +\t: fd_(-1), length_(0), offset_(0), mem_(0)\n>>  {\n>>  }\n>>  \n>> -Buffer::~Buffer()\n>> +/**\n>> + * \\brief Construct a Plane memory region for CPU mappings\n>> + * \\param[in] length The size of the memory region which should be mapped\n>> + * \\param[in] offset The offset into the file descriptor base at which the\n>> + * buffer commences\n>> + *\n>> + * \\sa mmap(int fd)\n>> + */\n>> +Plane::Plane(unsigned int length, unsigned int offset)\n>> +\t: fd_(-1), length_(length), offset_(offset), mem_(0)\n>> +{\n>> +}\n>> +\n>> +Plane::~Plane()\n>>  {\n>> +\tmunmap();\n>> +\n>>  \tif (fd_ != -1)\n>>  \t\tclose(fd_);\n>>  }\n>>  \n>>  /**\n>> - * \\fn Buffer::dmabuf()\n>> + * \\fn Plane::dmabuf()\n>>   * \\brief Get the dmabuf file handle backing the buffer\n>>   */\n>>  \n>> @@ -44,7 +69,7 @@ Buffer::~Buffer()\n>>   *\n>>   * The \\a fd dmabuf file handle is duplicated and stored.\n>>   */\n>> -int Buffer::setDmabuf(int fd)\n>> +int Plane::setDmabuf(int fd)\n>>  {\n>>  \tif (fd_ != -1) {\n>>  \t\tclose(fd_);\n>> @@ -61,6 +86,154 @@ int Buffer::setDmabuf(int fd)\n>>  \treturn 0;\n>>  }\n>>  \n>> +/**\n>> + * \\brief Map the plane memory data to a CPU accessible address\n>> + *\n>> + * The \\a fd specifies the file descriptor to map the memory from.\n>> + *\n>> + * \\return 0 on success or a negative error value otherwise.\n>> + */\n>> +int Plane::mmap(int fd)\n>> +{\n>> +\tvoid *map;\n>> +\n>> +\tmap = ::mmap(NULL, length_, PROT_READ | PROT_WRITE, MAP_SHARED, fd,\n>> +\t\t     offset_);\n>> +\tif (map == reinterpret_cast<void *>(-1)) {\n>> +\t\tint ret = -errno;\n>> +\t\tLOG(Buffer, Error)\n>> +\t\t\t<< \"Failed to mmap buffer: \" << strerror(ret);\n>> +\t\treturn ret;\n>> +\t}\n>> +\n>> +\tmem_ = map;\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +/**\n>> + * \\brief Map the plane memory data to a CPU accessible address\n>> + *\n>> + * The file descriptor to map the memory from must be set by a call to\n>> + * setDmaBuf before calling this function.\n>> + *\n>> + * \\sa setDmaBuf\n>> + *\n>> + * \\return 0 on success or a negative error value otherwise.\n> \n> I don't understand why you need two version of mmap().\n\nOne allows me to map the internal buffers with the FD from the\nv4l2device open call directly without necessitating a dup() for every\nbuffer.\n\nWould you prefer that every operation goes through the dmaBuf ?\n\nWon't that then enforce a lot of file description duplication where it\nmight not always be necessary?\n\n\n\n\n>> + */\n>> +int Plane::mmap()\n>> +{\n>> +\treturn mmap(fd_);\n>> +}\n>> +\n>> +/**\n>> + * \\brief Unmap any existing CPU accessible mapping\n>> + *\n>> + * Unmap the memory mapped by an earlier call to mmap.\n>> + *\n>> + * \\return 0 on success or a negative error value otherwise.\n>> + */\n>> +int Plane::munmap()\n>> +{\n>> +\tint ret = 0;\n>> +\n>> +\tif (mem_)\n>> +\t\tret = ::munmap(mem_, length_);\n>> +\n>> +\tif (ret) {\n>> +\t\tret = -errno;\n>> +\t\tLOG(Buffer, Warning)\n>> +\t\t\t<< \"Failed to unmap buffer: \" << strerror(ret);\n>> +\t} else {\n>> +\t\tmem_ = 0;\n>> +\t}\n>> +\n>> +\treturn ret;\n>> +}\n>> +\n>> +/**\n>> + * \\fn Plane::length()\n>> + * \\brief Get the length of the memory buffer\n>> + */\n>> +\n>> +/**\n>> + * \\fn Plane::mem()\n>> + * \\brief Get the CPU accessible memory address if mapped\n>> + */\n>> +\n>> +/**\n>> + * \\class Buffer\n>> + * \\brief A memory buffer to store an image\n>> + *\n>> + * The Buffer class represents the memory buffers used to store a\n>> + * full frame image, which may contain multiple separate memory Plane\n>> + * objects if the image format is multi-planar.\n>> + */\n>> +\n>> +Buffer::Buffer()\n>> +\t: index_(-1), format_(0), width_(0), height_(0)\n>> +{\n>> +}\n>> +\n>> +Buffer::~Buffer()\n>> +{\n>> +\tfor (Plane *plane : planes_)\n>> +\t\tdelete plane;\n>> +\n>> +\tplanes_.clear();\n>> +}\n>> +\n>> +/**\n>> + * \\brief Map each buffer plane to a CPU accessible address\n>> + *\n>> + * Utilise the \\a fd file descriptor to map each of the Buffer's planes to a CPU\n>> + * accessible address.\n>> + *\n>> + * \\return 0 on success or a negative error value otherwise.\n>> + */\n>> +int Buffer::mmap(int fd)\n>> +{\n>> +\tint ret;\n>> +\n>> +\tfor (Plane *plane : planes_) {\n>> +\t\tret = plane->mmap(fd);\n>> +\t\tif (ret)\n>> +\t\t\treturn ret;\n>> +\t}\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +/**\n>> + * \\brief Unmap any existing CPU accessible mapping for each plane\n>> + *\n>> + * Unmap the memory mapped by an earlier call to mmap.\n>> + *\n>> + * \\return 0 on success or a negative error value otherwise.\n>> + */\n>> +int Buffer::munmap()\n>> +{\n>> +\tint ret;\n>> +\n>> +\tfor (Plane *plane : planes_) {\n>> +\t\tret = plane->munmap();\n>> +\t\tif (ret)\n>> +\t\t\treturn ret;\n>> +\t}\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +/**\n>> + * \\fn Buffer::index()\n>> + * \\brief Get the Buffer index value\n>> + */\n>> +\n>> +/**\n>> + * \\fn Buffer::planes()\n>> + * \\brief Return a reference to the vector holding all Planes within the buffer\n>> + */\n>> +\n>>  /**\n>>   * \\class BufferPool\n>>   * \\brief A pool of buffers\n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 948FC60C6A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 31 Jan 2019 20:31:47 +0100 (CET)","from [10.10.152.147] (218.182-78-194.adsl-static.isp.belgacom.be\n\t[194.78.182.218])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F28F141;\n\tThu, 31 Jan 2019 20:31:46 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548963107;\n\tbh=f626/vgD2KrQPttoAHokbr8vXDevmtsBSM2cyHXdsNw=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=P1OOSEdZl9W9b8mlX+sbt3OhAbPZA4EobVEjo2IwbtQp0i8QCHJ9zB+Du/0dc29QP\n\t/glmd0Kieemy7DfBFUwBnb9skRyfPyLgtsI5QdeWJnYazUOVhobwRpi3hLwRnE2+IE\n\tWKn2JzEItnfohFMH/v4iHonr/9kPri0IlireM5AM=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"LibCamera Devel <libcamera-devel@lists.libcamera.org>","References":"<20190129135357.32339-1-kieran.bingham@ideasonboard.com>\n\t<20190129135357.32339-5-kieran.bingham@ideasonboard.com>\n\t<20190131091638.GB4197@pendragon.ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<1f6c7120-45f8-d996-a284-cb8b75502166@ideasonboard.com>","Date":"Thu, 31 Jan 2019 20:31:43 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.4.0","MIME-Version":"1.0","In-Reply-To":"<20190131091638.GB4197@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 4/4] libcamera: buffer: Provide Buffer\n\tPlanes","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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":"Thu, 31 Jan 2019 19:31:47 -0000"}},{"id":729,"web_url":"https://patchwork.libcamera.org/comment/729/","msgid":"<20190201004010.GB23060@pendragon.ideasonboard.com>","date":"2019-02-01T00:40:10","subject":"Re: [libcamera-devel] [PATCH 4/4] libcamera: buffer: Provide Buffer\n\tPlanes","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Thu, Jan 31, 2019 at 08:31:43PM +0100, Kieran Bingham wrote:\n> On 31/01/2019 09:16, Laurent Pinchart wrote:\n> > On Tue, Jan 29, 2019 at 01:53:57PM +0000, Kieran Bingham wrote:\n> >> Extend the Buffer management to support multi-planar formats.\n> >>\n> >> An image within the system may use one or more Plane objects to track each\n> >> plane in the case of multi-planar image formats. The Buffer class manages all\n> >> of the data required to render or interpret the raw image data.\n> >>\n> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> >> ---\n> >>  include/libcamera/buffer.h |  38 +++++++-\n> >>  src/libcamera/buffer.cpp   | 189 +++++++++++++++++++++++++++++++++++--\n> >>  2 files changed, 215 insertions(+), 12 deletions(-)\n> >>\n> >> diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h\n> >> index dda5075f2879..08a74fb70b88 100644\n> >> --- a/include/libcamera/buffer.h\n> >> +++ b/include/libcamera/buffer.h\n> >> @@ -11,20 +11,50 @@\n> >>  \n> >>  namespace libcamera {\n> >>  \n> >> -class Buffer\n> >> +class Plane\n> >>  {\n> >>  public:\n> >> -\tBuffer();\n> >> -\t~Buffer();\n> >> +\tPlane();\n> >> +\tPlane(unsigned int length, unsigned int offset);\n> >> +\t~Plane();\n> >>  \n> >>  \tint dmabuf() const { return fd_; };\n> >>  \tint setDmabuf(int fd);\n> >>  \n> >> +\tint mmap(int fd);\n> >>  \tint mmap();\n> >> -\tvoid munmap();\n> >> +\tint munmap();\n> >> +\n> >> +\tunsigned int length() const { return length_; };\n> >> +\tvoid *mem() const { return mem_; };\n> >>  \n> >>  private:\n> >>  \tint fd_;\n> >> +\tunsigned int length_;\n> >> +\tunsigned int offset_;\n> >> +\tvoid *mem_;\n> >> +};\n> >> +\n> >> +class Buffer\n> >> +{\n> >> +public:\n> >> +\tBuffer();\n> >> +\tvirtual ~Buffer();\n> >> +\n> >> +\tint mmap(int fd);\n> >> +\tint munmap();\n> >> +\n> >> +\tunsigned int index() const { return index_; };\n> >> +\tconst std::vector<Plane *> &planes() { return planes_; };\n> >> +\n> >> +private:\n> >> +\tunsigned int index_;\n> >> +\n> >> +\tunsigned int format_;\n> >> +\tunsigned int width_;\n> >> +\tunsigned int height_;\n> >> +\n> >> +\tstd::vector<Plane *> planes_;\n> >>  };\n> >>  \n> >>  class BufferPool\n> >> diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp\n> >> index 4a870df77e92..7b84a97dcd2b 100644\n> >> --- a/src/libcamera/buffer.cpp\n> >> +++ b/src/libcamera/buffer.cpp\n> >> @@ -6,10 +6,14 @@\n> >>   */\n> >>  \n> >>  #include <errno.h>\n> >> +#include <string.h>\n> >> +#include <sys/mman.h>\n> >>  #include <unistd.h>\n> >>  \n> >>  #include <libcamera/buffer.h>\n> >>  \n> >> +#include \"log.h\"\n> >> +\n> >>  /**\n> >>   * \\file buffer.h\n> >>   * \\brief Buffer handling\n> >> @@ -17,25 +21,46 @@\n> >>  \n> >>  namespace libcamera {\n> >>  \n> >> +LOG_DEFINE_CATEGORY(Buffer)\n> >> +\n> >>  /**\n> >> - * \\class Buffer\n> >> - * \\brief A memory buffer to store a frame\n> >> + * \\class Plane\n> >> + * \\brief A memory buffer to store a single plane of a frame\n> > \n> > Defining Plane as a memory buffer, and Buffer as a set of Plane, is too\n> > confusing. We need better than that.\n> \n> Memory Region as below?\n\nThat's better, but I still think we need to improve the documentation.\n\n> >>   *\n> >> - * The Buffer class represents a memory buffer used to store a frame.\n> >> + * The Plane class represents a memory region used to store a single plane. It\n> > \n> > And here defining Plane as a plane won't really help understanding the\n> > API. We need an overview of how the Buffer and Plane objects interact.\n> > \n> >> + * may be backed by it's own dmaBuf handle, and represents a single plane from a\n> > \n> > s/it's/its/\n> > \n> >> + * Buffer. In the case of multi-planar image formats, multiple Plane objects are\n> >> + * attached to a Buffer to manage the whole picture.\n> > \n> > Try reading this paragraph with a newcommer's point of view and I think\n> > you'll realize we need better :-)\n> > \n> >>   */\n> >> -Buffer::Buffer()\n> >> -\t: fd_(-1)\n> >> +\n> >> +Plane::Plane()\n> >> +\t: fd_(-1), length_(0), offset_(0), mem_(0)\n> >>  {\n> >>  }\n> >>  \n> >> -Buffer::~Buffer()\n> >> +/**\n> >> + * \\brief Construct a Plane memory region for CPU mappings\n> >> + * \\param[in] length The size of the memory region which should be mapped\n> >> + * \\param[in] offset The offset into the file descriptor base at which the\n> >> + * buffer commences\n> >> + *\n> >> + * \\sa mmap(int fd)\n> >> + */\n> >> +Plane::Plane(unsigned int length, unsigned int offset)\n> >> +\t: fd_(-1), length_(length), offset_(offset), mem_(0)\n> >> +{\n> >> +}\n> >> +\n> >> +Plane::~Plane()\n> >>  {\n> >> +\tmunmap();\n> >> +\n> >>  \tif (fd_ != -1)\n> >>  \t\tclose(fd_);\n> >>  }\n> >>  \n> >>  /**\n> >> - * \\fn Buffer::dmabuf()\n> >> + * \\fn Plane::dmabuf()\n> >>   * \\brief Get the dmabuf file handle backing the buffer\n> >>   */\n> >>  \n> >> @@ -44,7 +69,7 @@ Buffer::~Buffer()\n> >>   *\n> >>   * The \\a fd dmabuf file handle is duplicated and stored.\n> >>   */\n> >> -int Buffer::setDmabuf(int fd)\n> >> +int Plane::setDmabuf(int fd)\n> >>  {\n> >>  \tif (fd_ != -1) {\n> >>  \t\tclose(fd_);\n> >> @@ -61,6 +86,154 @@ int Buffer::setDmabuf(int fd)\n> >>  \treturn 0;\n> >>  }\n> >>  \n> >> +/**\n> >> + * \\brief Map the plane memory data to a CPU accessible address\n> >> + *\n> >> + * The \\a fd specifies the file descriptor to map the memory from.\n> >> + *\n> >> + * \\return 0 on success or a negative error value otherwise.\n> >> + */\n> >> +int Plane::mmap(int fd)\n> >> +{\n> >> +\tvoid *map;\n> >> +\n> >> +\tmap = ::mmap(NULL, length_, PROT_READ | PROT_WRITE, MAP_SHARED, fd,\n> >> +\t\t     offset_);\n> >> +\tif (map == reinterpret_cast<void *>(-1)) {\n> >> +\t\tint ret = -errno;\n> >> +\t\tLOG(Buffer, Error)\n> >> +\t\t\t<< \"Failed to mmap buffer: \" << strerror(ret);\n> >> +\t\treturn ret;\n> >> +\t}\n> >> +\n> >> +\tmem_ = map;\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief Map the plane memory data to a CPU accessible address\n> >> + *\n> >> + * The file descriptor to map the memory from must be set by a call to\n> >> + * setDmaBuf before calling this function.\n> >> + *\n> >> + * \\sa setDmaBuf\n> >> + *\n> >> + * \\return 0 on success or a negative error value otherwise.\n> > \n> > I don't understand why you need two version of mmap().\n> \n> One allows me to map the internal buffers with the FD from the\n> v4l2device open call directly without necessitating a dup() for every\n> buffer.\n> \n> Would you prefer that every operation goes through the dmaBuf ?\n> \n> Won't that then enforce a lot of file description duplication where it\n> might not always be necessary?\n\nThe dup() is necessary for external allocation (dmabuf import) to ensure\nthat the file descriptor won't be closed with a buffer still referencing\nit. For internal allocation, there's no requirement to duplicate the\nV4L2 device fd, but it should not be passed to the mmap() method. The\nmmap() method is meant to be called by applications to mmap the buffer,\nand applications are not aware of the underlying device. You should thus\npass the V4L2Device when creating the buffers and use its fd for mapping\nin the Plane::mmap() call, transparently for applications. It's\nimportant that the V4L2Device doesn't appear in the public API.\n\n> >> + */\n> >> +int Plane::mmap()\n> >> +{\n> >> +\treturn mmap(fd_);\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief Unmap any existing CPU accessible mapping\n> >> + *\n> >> + * Unmap the memory mapped by an earlier call to mmap.\n> >> + *\n> >> + * \\return 0 on success or a negative error value otherwise.\n> >> + */\n> >> +int Plane::munmap()\n> >> +{\n> >> +\tint ret = 0;\n> >> +\n> >> +\tif (mem_)\n> >> +\t\tret = ::munmap(mem_, length_);\n> >> +\n> >> +\tif (ret) {\n> >> +\t\tret = -errno;\n> >> +\t\tLOG(Buffer, Warning)\n> >> +\t\t\t<< \"Failed to unmap buffer: \" << strerror(ret);\n> >> +\t} else {\n> >> +\t\tmem_ = 0;\n> >> +\t}\n> >> +\n> >> +\treturn ret;\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\fn Plane::length()\n> >> + * \\brief Get the length of the memory buffer\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\fn Plane::mem()\n> >> + * \\brief Get the CPU accessible memory address if mapped\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\class Buffer\n> >> + * \\brief A memory buffer to store an image\n> >> + *\n> >> + * The Buffer class represents the memory buffers used to store a\n> >> + * full frame image, which may contain multiple separate memory Plane\n> >> + * objects if the image format is multi-planar.\n> >> + */\n> >> +\n> >> +Buffer::Buffer()\n> >> +\t: index_(-1), format_(0), width_(0), height_(0)\n> >> +{\n> >> +}\n> >> +\n> >> +Buffer::~Buffer()\n> >> +{\n> >> +\tfor (Plane *plane : planes_)\n> >> +\t\tdelete plane;\n> >> +\n> >> +\tplanes_.clear();\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief Map each buffer plane to a CPU accessible address\n> >> + *\n> >> + * Utilise the \\a fd file descriptor to map each of the Buffer's planes to a CPU\n> >> + * accessible address.\n> >> + *\n> >> + * \\return 0 on success or a negative error value otherwise.\n> >> + */\n> >> +int Buffer::mmap(int fd)\n> >> +{\n> >> +\tint ret;\n> >> +\n> >> +\tfor (Plane *plane : planes_) {\n> >> +\t\tret = plane->mmap(fd);\n> >> +\t\tif (ret)\n> >> +\t\t\treturn ret;\n> >> +\t}\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief Unmap any existing CPU accessible mapping for each plane\n> >> + *\n> >> + * Unmap the memory mapped by an earlier call to mmap.\n> >> + *\n> >> + * \\return 0 on success or a negative error value otherwise.\n> >> + */\n> >> +int Buffer::munmap()\n> >> +{\n> >> +\tint ret;\n> >> +\n> >> +\tfor (Plane *plane : planes_) {\n> >> +\t\tret = plane->munmap();\n> >> +\t\tif (ret)\n> >> +\t\t\treturn ret;\n> >> +\t}\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\fn Buffer::index()\n> >> + * \\brief Get the Buffer index value\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\fn Buffer::planes()\n> >> + * \\brief Return a reference to the vector holding all Planes within the buffer\n> >> + */\n> >> +\n> >>  /**\n> >>   * \\class BufferPool\n> >>   * \\brief A pool of buffers","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4264260C78\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  1 Feb 2019 01:40:16 +0100 (CET)","from pendragon.ideasonboard.com (85-76-34-136-nat.elisa-mobile.fi\n\t[85.76.34.136])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E8F1641;\n\tFri,  1 Feb 2019 01:40:13 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548981615;\n\tbh=ymWk/LYgMFunjnx8y7W7evQVEZrrSxQIfowMlFN4v+Q=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=b5g35mKAuin7dziMGKwwWd0obMIIbFZW2gt0XI+O5hgs/oyMc5hPiySzi+OkOBlM6\n\tSpnz8HK9cuhqcvVGXlLo32GBIWDryZ03vMxQO1EsNKFudjFcqCkSYfV274U0Q+Y1x2\n\tM1/iNz91Lqo9zNovMbXq7JanHlCiOIJelpGQRt7U=","Date":"Fri, 1 Feb 2019 02:40:10 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"LibCamera Devel <libcamera-devel@lists.libcamera.org>","Message-ID":"<20190201004010.GB23060@pendragon.ideasonboard.com>","References":"<20190129135357.32339-1-kieran.bingham@ideasonboard.com>\n\t<20190129135357.32339-5-kieran.bingham@ideasonboard.com>\n\t<20190131091638.GB4197@pendragon.ideasonboard.com>\n\t<1f6c7120-45f8-d996-a284-cb8b75502166@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<1f6c7120-45f8-d996-a284-cb8b75502166@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 4/4] libcamera: buffer: Provide Buffer\n\tPlanes","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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":"Fri, 01 Feb 2019 00:40:16 -0000"}}]