[{"id":2098,"web_url":"https://patchwork.libcamera.org/comment/2098/","msgid":"<20190701230730.GE11004@bigcity.dyn.berto.se>","date":"2019-07-01T23:07:30","subject":"Re: [libcamera-devel] [RFC 6/8] libcamera: stream: Add operation to\n\tmap buffers","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your work.\n\nOn 2019-06-30 20:10:47 +0200, Jacopo Mondi wrote:\n> Add and operation to map external buffers provided by applications in\n> a Request to the Stream's internal buffers used by the pipeline handlers\n> to interact with the video device.\n> \n> For streams using internal memory allocation, the two buffers are the\n> same as applications effectively use Buffers from the Stream's pool\n> where the video device memory has been exported to.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  include/libcamera/buffer.h |  1 +\n>  include/libcamera/stream.h |  5 +++\n>  src/libcamera/stream.cpp   | 66 ++++++++++++++++++++++++++++++++++++++\n>  3 files changed, 72 insertions(+)\n> \n> diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h\n> index 260a62e9e77e..d5d3dc90a096 100644\n> --- a/include/libcamera/buffer.h\n> +++ b/include/libcamera/buffer.h\n> @@ -59,6 +59,7 @@ private:\n>  \tfriend class BufferPool;\n>  \tfriend class PipelineHandler;\n>  \tfriend class Request;\n> +\tfriend class Stream;\n>  \tfriend class V4L2VideoDevice;\n>  \n>  \tvoid cancel();\n> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> index 796f1aff2602..4c034c113ddb 100644\n> --- a/include/libcamera/stream.h\n> +++ b/include/libcamera/stream.h\n> @@ -85,6 +85,8 @@ public:\n>  \tconst StreamConfiguration &configuration() const { return configuration_; }\n>  \tMemoryType memoryType() const { return memoryType_; }\n>  \n> +\tBuffer *mapBuffer(Buffer *requestBuffer);\n> +\n>  protected:\n>  \tfriend class Camera;\n>  \n> @@ -94,6 +96,9 @@ protected:\n>  \tBufferPool bufferPool_;\n>  \tStreamConfiguration configuration_;\n>  \tMemoryType memoryType_;\n> +\n> +\tstd::vector<Buffer *> mappableBuffers_;\n> +\tstd::map<unsigned int, Buffer *> bufferMaps_;\n>  };\n>  \n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> index b6292427d3a2..f36336857ad6 100644\n> --- a/src/libcamera/stream.cpp\n> +++ b/src/libcamera/stream.cpp\n> @@ -13,6 +13,8 @@\n>  #include <iomanip>\n>  #include <sstream>\n>  \n> +#include <libcamera/request.h>\n> +\n>  #include \"log.h\"\n>  \n>  /**\n> @@ -445,6 +447,26 @@ void Stream::createBuffers(unsigned int count)\n>  \t\treturn;\n>  \n>  \tbufferPool_.createBuffers(count);\n> +\n> +\t/* Streams with internal memory usage do not need buffer mapping. */\n> +\tif (memoryType_ == InternalMemory)\n> +\t\treturn;\n> +\n> +\t/*\n> +\t * Prepare for buffer mapping by queuing all buffers from the internal\n> +\t * pool. Each external buffer presented by application will be mapped\n> +\t * on an internal one.\n> +\t */\n> +\tmappableBuffers_.clear();\n> +\tfor (Buffer &buffer : bufferPool_.buffers()) {\n> +\t\t/* Reserve all planes to support mapping multiplanar buffers. */\n> +\t\tbuffer.planes().clear();\n> +\t\t/* \\todo: I would use VIDEO_MAX_PLANES but that's V4L2 stuff.. */\n> +\t\tfor (unsigned int i = 0; i < 3; ++i)\n> +\t\t\tbuffer.planes().emplace_back();\n> +\n> +\t\tmappableBuffers_.push_back(&buffer);\n> +\t}\n>  }\n>  \n>  /**\n> @@ -457,6 +479,50 @@ void Stream::destroyBuffers()\n>  \tcreateBuffers(0);\n>  }\n>  \n> +/**\n> + * \\brief Map the buffer an application has associated with a Request to an\n> + * internl one\n> + *\n> + * \\todo Rewrite documentation\n> + * If the Stream uses external memory, we need to map the externally\n> + * provided buffer to an internal one, trying to keep a best effort\n> + * association based on the Buffer's last usage time.\n\nI would not use the word last here as it (at lest to me) implies that \nthe buffer selected is the most recently used buffer and not the oldest \nbuffer.\n\n> + * External and internal buffers are associated by using the dmabuf\n> + * fds as key.\n> + */\n> +Buffer *Stream::mapBuffer(Buffer *requestBuffer)\n> +{\n> +\t/*\n> +\t * \\todo Multiplane APIs have one fd per plane, the key should be\n> +\t * hashed using all the planes fds.\n> +\t */\n> +\tunsigned int key = requestBuffer->planes()[0].dmabuf();\n> +\n> +\t/* If the buffer has already been mapped, just return it. */\n> +\tauto mapped = bufferMaps_.find(key);\n> +\tif (mapped != bufferMaps_.end())\n> +\t\treturn mapped->second;\n\nOn a hit should you not update the buffers position in mappableBuffers_ \nto increase the hit rate and try to keep the cache as hot as possible?  \n\nOne idea is to make mappableBuffers_ a std::set and index it on a \ntimestmap property, that way you could just update the timestamp when a \nbuffer is resued and when you need to evict a buffer from the list the \noldes buffer will always be at the front (or end) of the set.\n\n> +\n> +\t/*\n> +\t * Remove the last recently used buffer from the circular list and\n> +\t * use it for mapping.\n> +\t */\n> +\tauto mappable = mappableBuffers_.begin();\n> +\tBuffer *buffer = *mappable;\n> +\tmappableBuffers_.erase(mappable);\n> +\tmappableBuffers_.push_back(buffer);\n\nIt's simpler to rotate the vector,\n\n  std::rotate(mappableBuffers_.begin(), mappableBuffers_.begin() + 1, mappableBuffers_.end());\n\n> +\n> +\t/* \\todo: Support multiplanar external buffers. */\n> +\tbuffer->planes()[0].setDmabuf(key, 0);\n> +\n> +\t/* Pipeline handlers use request_ at buffer completion time. */\n> +\tbuffer->request_ = requestBuffer->request();\n> +\n> +\tbufferMaps_[key] = buffer;\n\nI'm no expert on our buffer handling, but when you evict a buffer from \nmappableBuffers_ should it not also be removed from bufferMaps_? Else if \nevery buffer imported is a new dmabuf we will consume a lot of memory.\n\n> +\n> +\treturn buffer;\n> +}\n> +\n>  /**\n>   * \\var Stream::bufferPool_\n>   * \\brief The pool of buffers associated with the stream\n> -- \n> 2.21.0\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lf1-x129.google.com (mail-lf1-x129.google.com\n\t[IPv6:2a00:1450:4864:20::129])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9122A60BF8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  2 Jul 2019 01:07:32 +0200 (CEST)","by mail-lf1-x129.google.com with SMTP id j29so9891179lfk.10\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 01 Jul 2019 16:07:32 -0700 (PDT)","from localhost (customer-145-14-112-32.stosn.net. [145.14.112.32])\n\tby smtp.gmail.com with ESMTPSA id\n\tw15sm3887203ljh.0.2019.07.01.16.07.30\n\t(version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256);\n\tMon, 01 Jul 2019 16:07:31 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=5RgXT6pigcFL28Nh5y5qwiVc8QlseJ2SMBVJLlwMZvk=;\n\tb=rjCj7mLsPbcWPfFSIQ+vkaemWOt1z8qKbWqoLL1MUyWqhor6YC0OwKtYISe8OgkT5h\n\tND+qzGfulMq0UsyYW0b67PGvrEGY1QL56HR+m8npk8XGZQthFEyzTz2c8pHzBFwaszEe\n\t/OUZloXzROCa0B8p7Tt08ndFBDQeKOg7u4vG48FOJKKSXffphPD6B4M03q074r1EDoBT\n\tLxyNUWIUcK64TkgZbA5lSIAp4h6riPQ1cLj4Pf/rcD0fGioriaO1PRc97S6Mvll6VDRP\n\tpwmWhUeqPwL8EqMEVv7RJj3PN90GXDizNbIpMzw6uYuU01Dcc/8VghKkCxsLsEU+fugp\n\tUvnA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=5RgXT6pigcFL28Nh5y5qwiVc8QlseJ2SMBVJLlwMZvk=;\n\tb=ohbWob4kGf+BSj8iHHXnJnM/l6nTTw+Q8599CEGl3CHWWinIoY1M7RyWB9T8rXaLwz\n\teYdPyDe1cdPSkojJwrpLRUBZ09LER/GmRGVUDpdB/OtUvHs7zx+R26esMl39vdr2Vxac\n\ttXZbnkK5lQhxm+UmeoPSCgyvqBFjC72Uj4yrB6b/vJ7gawn25Pt3ax2ku3ishpOBly7E\n\tisg1Az80KsZF2+WI2ojRnOu6XjiZWJRkE5Gc6NTl9P6kdSE5fhf6/bGQeqQlve6XiVEG\n\tgTzA8H709HCbvRLNv851GHO/Al4pp79iuhpNHuP8G04K7lwzsXC0VywrgpGr7UEU3yeh\n\tr6hg==","X-Gm-Message-State":"APjAAAWtxNfYrgBf2RJNWCh476nQExpFDcMvLUOL+/gSQpYpxIMRMYTh\n\tkfjl2LsVat0AmM+T8g8M6liXcGFdHQE=","X-Google-Smtp-Source":"APXvYqzMYU+LLREpOnOBKozwbRyKoNE1Q+vOqTL17p/bzE6UZFWH0B7J+40PkXbgPDs6W5fuBqG0uQ==","X-Received":"by 2002:a05:6512:51c:: with SMTP id\n\to28mr13431027lfb.67.1562022452010; \n\tMon, 01 Jul 2019 16:07:32 -0700 (PDT)","Date":"Tue, 2 Jul 2019 01:07:30 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190701230730.GE11004@bigcity.dyn.berto.se>","References":"<20190630181049.9548-1-jacopo@jmondi.org>\n\t<20190630181049.9548-7-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190630181049.9548-7-jacopo@jmondi.org>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [RFC 6/8] libcamera: stream: Add operation to\n\tmap buffers","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":"Mon, 01 Jul 2019 23:07:32 -0000"}},{"id":2117,"web_url":"https://patchwork.libcamera.org/comment/2117/","msgid":"<20190702073958.taz2g3svd2zkagvg@uno.localdomain>","date":"2019-07-02T07:39:58","subject":"Re: [libcamera-devel] [RFC 6/8] libcamera: stream: Add operation to\n\tmap buffers","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Tue, Jul 02, 2019 at 01:07:30AM +0200, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> Thanks for your work.\n>\n> On 2019-06-30 20:10:47 +0200, Jacopo Mondi wrote:\n> > Add and operation to map external buffers provided by applications in\n> > a Request to the Stream's internal buffers used by the pipeline handlers\n> > to interact with the video device.\n> >\n> > For streams using internal memory allocation, the two buffers are the\n> > same as applications effectively use Buffers from the Stream's pool\n> > where the video device memory has been exported to.\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > ---\n> >  include/libcamera/buffer.h |  1 +\n> >  include/libcamera/stream.h |  5 +++\n> >  src/libcamera/stream.cpp   | 66 ++++++++++++++++++++++++++++++++++++++\n> >  3 files changed, 72 insertions(+)\n> >\n> > diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h\n> > index 260a62e9e77e..d5d3dc90a096 100644\n> > --- a/include/libcamera/buffer.h\n> > +++ b/include/libcamera/buffer.h\n> > @@ -59,6 +59,7 @@ private:\n> >  \tfriend class BufferPool;\n> >  \tfriend class PipelineHandler;\n> >  \tfriend class Request;\n> > +\tfriend class Stream;\n> >  \tfriend class V4L2VideoDevice;\n> >\n> >  \tvoid cancel();\n> > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> > index 796f1aff2602..4c034c113ddb 100644\n> > --- a/include/libcamera/stream.h\n> > +++ b/include/libcamera/stream.h\n> > @@ -85,6 +85,8 @@ public:\n> >  \tconst StreamConfiguration &configuration() const { return configuration_; }\n> >  \tMemoryType memoryType() const { return memoryType_; }\n> >\n> > +\tBuffer *mapBuffer(Buffer *requestBuffer);\n> > +\n> >  protected:\n> >  \tfriend class Camera;\n> >\n> > @@ -94,6 +96,9 @@ protected:\n> >  \tBufferPool bufferPool_;\n> >  \tStreamConfiguration configuration_;\n> >  \tMemoryType memoryType_;\n> > +\n> > +\tstd::vector<Buffer *> mappableBuffers_;\n> > +\tstd::map<unsigned int, Buffer *> bufferMaps_;\n> >  };\n> >\n> >  } /* namespace libcamera */\n> > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> > index b6292427d3a2..f36336857ad6 100644\n> > --- a/src/libcamera/stream.cpp\n> > +++ b/src/libcamera/stream.cpp\n> > @@ -13,6 +13,8 @@\n> >  #include <iomanip>\n> >  #include <sstream>\n> >\n> > +#include <libcamera/request.h>\n> > +\n> >  #include \"log.h\"\n> >\n> >  /**\n> > @@ -445,6 +447,26 @@ void Stream::createBuffers(unsigned int count)\n> >  \t\treturn;\n> >\n> >  \tbufferPool_.createBuffers(count);\n> > +\n> > +\t/* Streams with internal memory usage do not need buffer mapping. */\n> > +\tif (memoryType_ == InternalMemory)\n> > +\t\treturn;\n> > +\n> > +\t/*\n> > +\t * Prepare for buffer mapping by queuing all buffers from the internal\n> > +\t * pool. Each external buffer presented by application will be mapped\n> > +\t * on an internal one.\n> > +\t */\n> > +\tmappableBuffers_.clear();\n> > +\tfor (Buffer &buffer : bufferPool_.buffers()) {\n> > +\t\t/* Reserve all planes to support mapping multiplanar buffers. */\n> > +\t\tbuffer.planes().clear();\n> > +\t\t/* \\todo: I would use VIDEO_MAX_PLANES but that's V4L2 stuff.. */\n> > +\t\tfor (unsigned int i = 0; i < 3; ++i)\n> > +\t\t\tbuffer.planes().emplace_back();\n> > +\n> > +\t\tmappableBuffers_.push_back(&buffer);\n> > +\t}\n> >  }\n> >\n> >  /**\n> > @@ -457,6 +479,50 @@ void Stream::destroyBuffers()\n> >  \tcreateBuffers(0);\n> >  }\n> >\n> > +/**\n> > + * \\brief Map the buffer an application has associated with a Request to an\n> > + * internl one\n> > + *\n> > + * \\todo Rewrite documentation\n> > + * If the Stream uses external memory, we need to map the externally\n> > + * provided buffer to an internal one, trying to keep a best effort\n> > + * association based on the Buffer's last usage time.\n>\n> I would not use the word last here as it (at lest to me) implies that\n> the buffer selected is the most recently used buffer and not the oldest\n> buffer.\n>\n> > + * External and internal buffers are associated by using the dmabuf\n> > + * fds as key.\n> > + */\n> > +Buffer *Stream::mapBuffer(Buffer *requestBuffer)\n> > +{\n> > +\t/*\n> > +\t * \\todo Multiplane APIs have one fd per plane, the key should be\n> > +\t * hashed using all the planes fds.\n> > +\t */\n> > +\tunsigned int key = requestBuffer->planes()[0].dmabuf();\n> > +\n> > +\t/* If the buffer has already been mapped, just return it. */\n> > +\tauto mapped = bufferMaps_.find(key);\n> > +\tif (mapped != bufferMaps_.end())\n> > +\t\treturn mapped->second;\n>\n> On a hit should you not update the buffers position in mappableBuffers_\n> to increase the hit rate and try to keep the cache as hot as possible?\n\nYou are right!\n\n>\n> One idea is to make mappableBuffers_ a std::set and index it on a\n> timestmap property, that way you could just update the timestamp when a\n> buffer is resued and when you need to evict a buffer from the list the\n> oldes buffer will always be at the front (or end) of the set.\n\nOr I could remove the buffer from the vector of used buffers and push it back\nagain. Your one is probably more elegant, but I do expect a very\nlimited number of buffers to cycle.\n>\n> > +\n> > +\t/*\n> > +\t * Remove the last recently used buffer from the circular list and\n> > +\t * use it for mapping.\n> > +\t */\n> > +\tauto mappable = mappableBuffers_.begin();\n> > +\tBuffer *buffer = *mappable;\n> > +\tmappableBuffers_.erase(mappable);\n> > +\tmappableBuffers_.push_back(buffer);\n>\n> It's simpler to rotate the vector,\n>\n>   std::rotate(mappableBuffers_.begin(), mappableBuffers_.begin() + 1, mappableBuffers_.end());\n>\n\nThanks! Also, if you have better names to suggest in place of\nmappableBuffers_ I would be happy to hear that.\n\n> > +\n> > +\t/* \\todo: Support multiplanar external buffers. */\n> > +\tbuffer->planes()[0].setDmabuf(key, 0);\n> > +\n> > +\t/* Pipeline handlers use request_ at buffer completion time. */\n> > +\tbuffer->request_ = requestBuffer->request();\n> > +\n> > +\tbufferMaps_[key] = buffer;\n>\n> I'm no expert on our buffer handling, but when you evict a buffer from\n> mappableBuffers_ should it not also be removed from bufferMaps_? Else if\n> every buffer imported is a new dmabuf we will consume a lot of memory.\n>\n\nTrue, the map could grow if we keep receiving new buffers. Again, I\nexpect application to cycle on a limited number of buffers so I was\nnot too concerned, but yes, when a a buffer mapping is removed, the\nentry on the map corresponding to that buffer key should be erased.\n\nThanks for spotting these!\n\n> > +\n> > +\treturn buffer;\n> > +}\n> > +\n> >  /**\n> >   * \\var Stream::bufferPool_\n> >   * \\brief The pool of buffers associated with the stream\n> > --\n> > 2.21.0\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n>\n> --\n> Regards,\n> Niklas Söderlund","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ED27660C01\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  2 Jul 2019 09:38:43 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 772EB1BF206;\n\tTue,  2 Jul 2019 07:38:41 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Tue, 2 Jul 2019 09:39:58 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190702073958.taz2g3svd2zkagvg@uno.localdomain>","References":"<20190630181049.9548-1-jacopo@jmondi.org>\n\t<20190630181049.9548-7-jacopo@jmondi.org>\n\t<20190701230730.GE11004@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"6zdi5l55sndh2m5z\"","Content-Disposition":"inline","In-Reply-To":"<20190701230730.GE11004@bigcity.dyn.berto.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [RFC 6/8] libcamera: stream: Add operation to\n\tmap buffers","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":"Tue, 02 Jul 2019 07:38:44 -0000"}},{"id":2129,"web_url":"https://patchwork.libcamera.org/comment/2129/","msgid":"<fc4ce14c-cc0f-12bf-c4a4-260fa96a4e1b@ideasonboard.com>","date":"2019-07-03T10:47:05","subject":"Re: [libcamera-devel] [RFC 6/8] libcamera: stream: Add operation to\n\tmap buffers","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 30/06/2019 19:10, Jacopo Mondi wrote:\n> Add and operation to map external buffers provided by applications in\n> a Request to the Stream's internal buffers used by the pipeline handlers\n> to interact with the video device.\n> \n> For streams using internal memory allocation, the two buffers are the\n> same as applications effectively use Buffers from the Stream's pool\n> where the video device memory has been exported to.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  include/libcamera/buffer.h |  1 +\n>  include/libcamera/stream.h |  5 +++\n>  src/libcamera/stream.cpp   | 66 ++++++++++++++++++++++++++++++++++++++\n>  3 files changed, 72 insertions(+)\n> \n> diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h\n> index 260a62e9e77e..d5d3dc90a096 100644\n> --- a/include/libcamera/buffer.h\n> +++ b/include/libcamera/buffer.h\n> @@ -59,6 +59,7 @@ private:\n>  \tfriend class BufferPool;\n>  \tfriend class PipelineHandler;\n>  \tfriend class Request;\n> +\tfriend class Stream;\n>  \tfriend class V4L2VideoDevice;\n>  \n>  \tvoid cancel();\n> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> index 796f1aff2602..4c034c113ddb 100644\n> --- a/include/libcamera/stream.h\n> +++ b/include/libcamera/stream.h\n> @@ -85,6 +85,8 @@ public:\n>  \tconst StreamConfiguration &configuration() const { return configuration_; }\n>  \tMemoryType memoryType() const { return memoryType_; }\n>  \n> +\tBuffer *mapBuffer(Buffer *requestBuffer);\n> +\n>  protected:\n>  \tfriend class Camera;\n>  \n> @@ -94,6 +96,9 @@ protected:\n>  \tBufferPool bufferPool_;\n>  \tStreamConfiguration configuration_;\n>  \tMemoryType memoryType_;\n> +\n> +\tstd::vector<Buffer *> mappableBuffers_;\n> +\tstd::map<unsigned int, Buffer *> bufferMaps_;\n>  };\n>  \n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> index b6292427d3a2..f36336857ad6 100644\n> --- a/src/libcamera/stream.cpp\n> +++ b/src/libcamera/stream.cpp\n> @@ -13,6 +13,8 @@\n>  #include <iomanip>\n>  #include <sstream>\n>  \n> +#include <libcamera/request.h>\n> +\n>  #include \"log.h\"\n>  \n>  /**\n> @@ -445,6 +447,26 @@ void Stream::createBuffers(unsigned int count)\n>  \t\treturn;\n>  \n>  \tbufferPool_.createBuffers(count);\n> +\n> +\t/* Streams with internal memory usage do not need buffer mapping. */\n> +\tif (memoryType_ == InternalMemory)\n> +\t\treturn;\n> +\n> +\t/*\n> +\t * Prepare for buffer mapping by queuing all buffers from the internal\n> +\t * pool. Each external buffer presented by application will be mapped\n> +\t * on an internal one.\n> +\t */\n> +\tmappableBuffers_.clear();\n> +\tfor (Buffer &buffer : bufferPool_.buffers()) {\n> +\t\t/* Reserve all planes to support mapping multiplanar buffers. */\n> +\t\tbuffer.planes().clear();\n> +\t\t/* \\todo: I would use VIDEO_MAX_PLANES but that's V4L2 stuff.. */\n> +\t\tfor (unsigned int i = 0; i < 3; ++i)\n> +\t\t\tbuffer.planes().emplace_back();\n\nJust a small one:\n\nCould buffer.planes().resize(3) be better here?\n\n\n> +\n> +\t\tmappableBuffers_.push_back(&buffer);\n> +\t}\n>  }\n>  \n>  /**\n> @@ -457,6 +479,50 @@ void Stream::destroyBuffers()\n>  \tcreateBuffers(0);\n>  }\n>  \n> +/**\n> + * \\brief Map the buffer an application has associated with a Request to an\n> + * internl one\n> + *\n> + * \\todo Rewrite documentation\n> + * If the Stream uses external memory, we need to map the externally\n> + * provided buffer to an internal one, trying to keep a best effort\n> + * association based on the Buffer's last usage time.\n> + * External and internal buffers are associated by using the dmabuf\n> + * fds as key.\n> + */\n> +Buffer *Stream::mapBuffer(Buffer *requestBuffer)\n> +{\n> +\t/*\n> +\t * \\todo Multiplane APIs have one fd per plane, the key should be\n> +\t * hashed using all the planes fds.\n> +\t */\n> +\tunsigned int key = requestBuffer->planes()[0].dmabuf();\n> +\n> +\t/* If the buffer has already been mapped, just return it. */\n> +\tauto mapped = bufferMaps_.find(key);\n> +\tif (mapped != bufferMaps_.end())\n> +\t\treturn mapped->second;\n> +\n> +\t/*\n> +\t * Remove the last recently used buffer from the circular list and\n> +\t * use it for mapping.\n> +\t */\n> +\tauto mappable = mappableBuffers_.begin();\n> +\tBuffer *buffer = *mappable;\n> +\tmappableBuffers_.erase(mappable);\n> +\tmappableBuffers_.push_back(buffer);\n> +\n> +\t/* \\todo: Support multiplanar external buffers. */\n> +\tbuffer->planes()[0].setDmabuf(key, 0);\n> +\n> +\t/* Pipeline handlers use request_ at buffer completion time. */\n> +\tbuffer->request_ = requestBuffer->request();\n> +\n> +\tbufferMaps_[key] = buffer;\n> +\n> +\treturn buffer;\n> +}\n> +\n>  /**\n>   * \\var Stream::bufferPool_\n>   * \\brief The pool of buffers associated with the stream\n>","headers":{"Return-Path":"<kieran.bingham@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 5610C60B2D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  3 Jul 2019 12:47:09 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AB88D24B;\n\tWed,  3 Jul 2019 12:47:08 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1562150828;\n\tbh=yWRnKcGbvmZTBv11RN26jpC6dN+53XXFgPbXNAUY4jE=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=c/gMJfupkNgP4S8K3I2AbnPqfA+60MHgwhsxZFaIBJ8T4FINwgUAUnRbqIntsn7j/\n\tq+HCsXmNR3ouohqSBv0pUJIJXfXk7/1vrHCpe0JKnk4aduOJ76x4O1qaiajPB+YwwM\n\tcF27hFM50+K2NoyF9irT6+cKHi8y0IQ2dLxPWIGw=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Jacopo Mondi <jacopo@jmondi.org>, libcamera-devel@lists.libcamera.org","References":"<20190630181049.9548-1-jacopo@jmondi.org>\n\t<20190630181049.9548-7-jacopo@jmondi.org>","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":"<fc4ce14c-cc0f-12bf-c4a4-260fa96a4e1b@ideasonboard.com>","Date":"Wed, 3 Jul 2019 11:47:05 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.1","MIME-Version":"1.0","In-Reply-To":"<20190630181049.9548-7-jacopo@jmondi.org>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [RFC 6/8] libcamera: stream: Add operation to\n\tmap buffers","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":"Wed, 03 Jul 2019 10:47:09 -0000"}}]