| Message ID | 20190704225334.26170-7-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thanks for your patch. On 2019-07-05 00:53:31 +0200, Jacopo Mondi wrote: > Add and operation to map external buffers provided by applications in > a Request to the Stream's internal buffers used by the pipeline handlers > to interact with the video device. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > include/libcamera/buffer.h | 1 + > include/libcamera/stream.h | 6 ++ > src/libcamera/stream.cpp | 116 +++++++++++++++++++++++++++++++++++++ > 3 files changed, 123 insertions(+) > > diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h > index 260a62e9e77e..d5d3dc90a096 100644 > --- a/include/libcamera/buffer.h > +++ b/include/libcamera/buffer.h > @@ -59,6 +59,7 @@ private: > friend class BufferPool; > friend class PipelineHandler; > friend class Request; > + friend class Stream; I understand this is needed now so no problem. But for each new friend gained now is one more friend we have to try and design away later ;-) > friend class V4L2VideoDevice; > > void cancel(); > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h > index 796f1aff2602..74415062cbdd 100644 > --- a/include/libcamera/stream.h > +++ b/include/libcamera/stream.h > @@ -85,6 +85,8 @@ public: > const StreamConfiguration &configuration() const { return configuration_; } > MemoryType memoryType() const { return memoryType_; } > > + Buffer *mapBuffer(Buffer *applicationBuffer); > + > protected: > friend class Camera; > > @@ -94,6 +96,10 @@ protected: > BufferPool bufferPool_; > StreamConfiguration configuration_; > MemoryType memoryType_; > + > +private: > + std::vector<Buffer *> mappableBuffers_; > + std::map<unsigned int, Buffer *> bufferMap_; > }; > > } /* namespace libcamera */ > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp > index 97e0f429c9fb..4585c0db77a4 100644 > --- a/src/libcamera/stream.cpp > +++ b/src/libcamera/stream.cpp > @@ -13,6 +13,8 @@ > #include <iomanip> > #include <sstream> > > +#include <libcamera/request.h> > + > #include "log.h" > > /** > @@ -470,6 +472,26 @@ void Stream::createBuffers(unsigned int count) > return; > > bufferPool_.createBuffers(count); > + > + /* Streams with internal memory usage do not need buffer mapping. */ > + if (memoryType_ == InternalMemory) > + return; > + > + /* > + * Prepare for buffer mapping by queuing all buffers from the internal > + * pool. Each external buffer presented by application will be mapped > + * on an internal one. > + */ > + mappableBuffers_.clear(); > + for (Buffer &buffer : bufferPool_.buffers()) { > + /* > + * Reserve all planes to support mapping multiplanar buffers. > + * \todo I would use VIDEO_MAX_PLANES but it's a V4L2 thing. > + */ > + buffer.planes().resize(3); > + > + mappableBuffers_.push_back(&buffer); > + } > } > > /** > @@ -480,6 +502,100 @@ void Stream::destroyBuffers() > bufferPool_.destroyBuffers(); > } > > +/** > + * \brief Map an application provided buffer to a stream buffer > + * \param applicationBuffer The application provided buffer > + * > + * If a Stream has been configured to use application provided buffers a > + * mapping between the external buffers and the internal ones, which are > + * actually used to interface with the video device, is required. > + * > + * The most commonly used mechanism to perform zero-copy memory sharing > + * on Linux-based system is dmabuf, which allows user-space applications to > + * share buffers by exchanging dmabuf generated file descriptors. This > + * operations assumes that all application provided buffers have each of their > + * used memory planes exported as dmabuf file descriptor, to copy them in > + * the buffer to be then queued on the video device by pipeline handlers. > + * > + * Perform mapping by maintaining a cache in a map associating the dmabuf file > + * descriptor of the application provided buffer to one of the stream's internal > + * buffers to provide pipeline handlers the buffer to use to interact with video > + * devices. > + * > + * Once the buffer completes, the mapping should be reverted to return to > + * the application the buffer it first provided here. > + * > + * \return The stream buffer which maps to the application provided buffer > + */ > +Buffer *Stream::mapBuffer(Buffer *applicationBuffer) > +{ > + unsigned int key = applicationBuffer->planes()[0].dmabuf(); > + > + /* > + * Buffer hit: the application buffer has already been mapped, return > + * the assigned stream buffer > + */ > + auto mapped = bufferMap_.find(key); > + if (mapped != bufferMap_.end()) { > + Buffer *buffer = mapped->second; > + > + /* > + * Keep the mappableBuffers_ vector warm: move the hit buffer to > + * the vector end as on a buffer miss buffers are below evicted > + * from the vector head. > + */ > + auto it = std::find(mappableBuffers_.begin(), > + mappableBuffers_.end(), buffer); > + > + ASSERT(it != mappableBuffers_.end()); > + std::rotate(it, it + 1, mappableBuffers_.end()); This don't seem to be right. The it found will indeed be pushed to the back, but the buffer which was at the end will be swapped to the location it as at. Is not correct behavior here to delete the it position and insert it at the end? I assume the idea is to the closer to end() a buffer is, the "warmer" it is. So this swapping could lead to non optimal performance. > + > + return buffer; > + } > + > + /* > + * Buffer miss: assign to the application buffer the stream buffer > + * at mappableBuffers_ begin, then move it to the end. > + */ > + Buffer *buffer = *(mappableBuffers_.begin()); > + std::rotate(mappableBuffers_.begin(), mappableBuffers_.begin() + 1, > + mappableBuffers_.end()); > + > + > + /* Remove the [key, buffer] entry buffer from the buffer map */ > + auto deadBuf = std::find_if(bufferMap_.begin(), bufferMap_.end(), > + [&](std::pair<const unsigned int, Buffer *> &map) { > + return bufferMap_[map.first] == buffer; > + }); > + if (deadBuf != bufferMap_.end()) > + bufferMap_.erase(deadBuf); > + > + /* > + * Assign the buffer by copying the dmabuf file descriptors from the > + * application provided buffer. > + */ > + for (unsigned int i = 0; i < applicationBuffer->planes().size(); ++i) { > + int fd = applicationBuffer->planes()[i].dmabuf(); > + > + /* > + * The ARC camera stack seems to report more planes that the s/that/then/ > + * ones it actually uses. s/it// > + */ > + if (fd < 0) > + break; > + > + buffer->planes()[i].setDmabuf(fd, 0); > + } > + > + /* Pipeline handlers use request_ at buffer completion time. */ > + buffer->request_ = applicationBuffer->request(); > + > + /* And finally, store the mapping for later re-use and return it. */ > + bufferMap_[key] = buffer; > + > + return buffer; > +} > + > /** > * \var Stream::bufferPool_ > * \brief The pool of buffers associated with the stream > -- > 2.21.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On 06/07/2019 13:00, Niklas Söderlund wrote: > Hi Jacopo, > > Thanks for your patch. > > On 2019-07-05 00:53:31 +0200, Jacopo Mondi wrote: >> Add and operation to map external buffers provided by applications in >> a Request to the Stream's internal buffers used by the pipeline handlers >> to interact with the video device. >> >> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> >> --- >> include/libcamera/buffer.h | 1 + >> include/libcamera/stream.h | 6 ++ >> src/libcamera/stream.cpp | 116 +++++++++++++++++++++++++++++++++++++ >> 3 files changed, 123 insertions(+) >> >> diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h >> index 260a62e9e77e..d5d3dc90a096 100644 >> --- a/include/libcamera/buffer.h >> +++ b/include/libcamera/buffer.h >> @@ -59,6 +59,7 @@ private: >> friend class BufferPool; >> friend class PipelineHandler; >> friend class Request; >> + friend class Stream; > > I understand this is needed now so no problem. But for each new friend > gained now is one more friend we have to try and design away later ;-) Can we handle any mapping/LRU requirements in the bufferPool class instead? The Bufferpool could have a map instead of the stream? which then removes the friend requirements ... > >> friend class V4L2VideoDevice; >> >> void cancel(); >> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h >> index 796f1aff2602..74415062cbdd 100644 >> --- a/include/libcamera/stream.h >> +++ b/include/libcamera/stream.h >> @@ -85,6 +85,8 @@ public: >> const StreamConfiguration &configuration() const { return configuration_; } >> MemoryType memoryType() const { return memoryType_; } >> >> + Buffer *mapBuffer(Buffer *applicationBuffer); >> + >> protected: >> friend class Camera; >> >> @@ -94,6 +96,10 @@ protected: >> BufferPool bufferPool_; >> StreamConfiguration configuration_; >> MemoryType memoryType_; >> + >> +private: >> + std::vector<Buffer *> mappableBuffers_; >> + std::map<unsigned int, Buffer *> bufferMap_; >> }; >> >> } /* namespace libcamera */ >> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp >> index 97e0f429c9fb..4585c0db77a4 100644 >> --- a/src/libcamera/stream.cpp >> +++ b/src/libcamera/stream.cpp >> @@ -13,6 +13,8 @@ >> #include <iomanip> >> #include <sstream> >> >> +#include <libcamera/request.h> >> + >> #include "log.h" >> >> /** >> @@ -470,6 +472,26 @@ void Stream::createBuffers(unsigned int count) >> return; >> >> bufferPool_.createBuffers(count); >> + >> + /* Streams with internal memory usage do not need buffer mapping. */ >> + if (memoryType_ == InternalMemory) >> + return; >> + >> + /* >> + * Prepare for buffer mapping by queuing all buffers from the internal >> + * pool. Each external buffer presented by application will be mapped >> + * on an internal one. >> + */ >> + mappableBuffers_.clear(); >> + for (Buffer &buffer : bufferPool_.buffers()) { >> + /* >> + * Reserve all planes to support mapping multiplanar buffers. >> + * \todo I would use VIDEO_MAX_PLANES but it's a V4L2 thing. >> + */ >> + buffer.planes().resize(3); >> + >> + mappableBuffers_.push_back(&buffer); >> + } >> } >> >> /** >> @@ -480,6 +502,100 @@ void Stream::destroyBuffers() >> bufferPool_.destroyBuffers(); >> } >> >> +/** >> + * \brief Map an application provided buffer to a stream buffer >> + * \param applicationBuffer The application provided buffer >> + * >> + * If a Stream has been configured to use application provided buffers a >> + * mapping between the external buffers and the internal ones, which are >> + * actually used to interface with the video device, is required. >> + * >> + * The most commonly used mechanism to perform zero-copy memory sharing >> + * on Linux-based system is dmabuf, which allows user-space applications to >> + * share buffers by exchanging dmabuf generated file descriptors. This >> + * operations assumes that all application provided buffers have each of their >> + * used memory planes exported as dmabuf file descriptor, to copy them in >> + * the buffer to be then queued on the video device by pipeline handlers. >> + * >> + * Perform mapping by maintaining a cache in a map associating the dmabuf file >> + * descriptor of the application provided buffer to one of the stream's internal >> + * buffers to provide pipeline handlers the buffer to use to interact with video >> + * devices. >> + * >> + * Once the buffer completes, the mapping should be reverted to return to >> + * the application the buffer it first provided here. >> + * >> + * \return The stream buffer which maps to the application provided buffer >> + */ >> +Buffer *Stream::mapBuffer(Buffer *applicationBuffer) >> +{ >> + unsigned int key = applicationBuffer->planes()[0].dmabuf(); >> + >> + /* >> + * Buffer hit: the application buffer has already been mapped, return >> + * the assigned stream buffer >> + */ >> + auto mapped = bufferMap_.find(key); >> + if (mapped != bufferMap_.end()) { >> + Buffer *buffer = mapped->second; >> + >> + /* >> + * Keep the mappableBuffers_ vector warm: move the hit buffer to >> + * the vector end as on a buffer miss buffers are below evicted >> + * from the vector head. >> + */ >> + auto it = std::find(mappableBuffers_.begin(), >> + mappableBuffers_.end(), buffer); >> + >> + ASSERT(it != mappableBuffers_.end()); >> + std::rotate(it, it + 1, mappableBuffers_.end()); > > This don't seem to be right. > > The it found will indeed be pushed to the back, but the buffer which was > at the end will be swapped to the location it as at. Is not correct > behavior here to delete the it position and insert it at the end? > > I assume the idea is to the closer to end() a buffer is, the "warmer" it > is. So this swapping could lead to non optimal performance. > >> + >> + return buffer; >> + } >> + >> + /* >> + * Buffer miss: assign to the application buffer the stream buffer >> + * at mappableBuffers_ begin, then move it to the end. >> + */ >> + Buffer *buffer = *(mappableBuffers_.begin()); >> + std::rotate(mappableBuffers_.begin(), mappableBuffers_.begin() + 1, >> + mappableBuffers_.end()); >> + >> + >> + /* Remove the [key, buffer] entry buffer from the buffer map */ >> + auto deadBuf = std::find_if(bufferMap_.begin(), bufferMap_.end(), >> + [&](std::pair<const unsigned int, Buffer *> &map) { >> + return bufferMap_[map.first] == buffer; >> + }); >> + if (deadBuf != bufferMap_.end()) >> + bufferMap_.erase(deadBuf); >> + >> + /* >> + * Assign the buffer by copying the dmabuf file descriptors from the >> + * application provided buffer. >> + */ >> + for (unsigned int i = 0; i < applicationBuffer->planes().size(); ++i) { >> + int fd = applicationBuffer->planes()[i].dmabuf(); >> + >> + /* >> + * The ARC camera stack seems to report more planes that the > > s/that/then/ s/s/that/then//s/that/than// :D ok that's unreadable: How about : s/that/than/ :D > >> + * ones it actually uses. > > s/it// the 'it' pronoun is needed, so it should stay. >> + */ >> + if (fd < 0) >> + break; >> + >> + buffer->planes()[i].setDmabuf(fd, 0); >> + } >> + >> + /* Pipeline handlers use request_ at buffer completion time. */ >> + buffer->request_ = applicationBuffer->request(); >> + >> + /* And finally, store the mapping for later re-use and return it. */ >> + bufferMap_[key] = buffer; >> + >> + return buffer; >> +} >> + >> /** >> * \var Stream::bufferPool_ >> * \brief The pool of buffers associated with the stream >> -- >> 2.21.0 >> >> _______________________________________________ >> libcamera-devel mailing list >> libcamera-devel@lists.libcamera.org >> https://lists.libcamera.org/listinfo/libcamera-devel >
On 2019-07-08 12:39:31 +0100, Kieran Bingham wrote: > > > > s/that/then/ > > s/s/that/then//s/that/than// :D ok that's unreadable: > > How about : s/that/than/ How about s@s/that/then/@s/that/than/@ would that make it more readable? ;-P
Hi Niklas, On Sat, Jul 06, 2019 at 02:00:03PM +0200, Niklas Söderlund wrote: > Hi Jacopo, > > Thanks for your patch. > > On 2019-07-05 00:53:31 +0200, Jacopo Mondi wrote: > > Add and operation to map external buffers provided by applications in > > a Request to the Stream's internal buffers used by the pipeline handlers > > to interact with the video device. > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > include/libcamera/buffer.h | 1 + > > include/libcamera/stream.h | 6 ++ > > src/libcamera/stream.cpp | 116 +++++++++++++++++++++++++++++++++++++ > > 3 files changed, 123 insertions(+) > > > > diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h > > index 260a62e9e77e..d5d3dc90a096 100644 > > --- a/include/libcamera/buffer.h > > +++ b/include/libcamera/buffer.h > > @@ -59,6 +59,7 @@ private: > > friend class BufferPool; > > friend class PipelineHandler; > > friend class Request; > > + friend class Stream; > > I understand this is needed now so no problem. But for each new friend > gained now is one more friend we have to try and design away later ;-) > > > friend class V4L2VideoDevice; > > > > void cancel(); > > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h > > index 796f1aff2602..74415062cbdd 100644 > > --- a/include/libcamera/stream.h > > +++ b/include/libcamera/stream.h > > @@ -85,6 +85,8 @@ public: > > const StreamConfiguration &configuration() const { return configuration_; } > > MemoryType memoryType() const { return memoryType_; } > > > > + Buffer *mapBuffer(Buffer *applicationBuffer); > > + > > protected: > > friend class Camera; > > > > @@ -94,6 +96,10 @@ protected: > > BufferPool bufferPool_; > > StreamConfiguration configuration_; > > MemoryType memoryType_; > > + > > +private: > > + std::vector<Buffer *> mappableBuffers_; > > + std::map<unsigned int, Buffer *> bufferMap_; > > }; > > > > } /* namespace libcamera */ > > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp > > index 97e0f429c9fb..4585c0db77a4 100644 > > --- a/src/libcamera/stream.cpp > > +++ b/src/libcamera/stream.cpp > > @@ -13,6 +13,8 @@ > > #include <iomanip> > > #include <sstream> > > > > +#include <libcamera/request.h> > > + > > #include "log.h" > > > > /** > > @@ -470,6 +472,26 @@ void Stream::createBuffers(unsigned int count) > > return; > > > > bufferPool_.createBuffers(count); > > + > > + /* Streams with internal memory usage do not need buffer mapping. */ > > + if (memoryType_ == InternalMemory) > > + return; > > + > > + /* > > + * Prepare for buffer mapping by queuing all buffers from the internal > > + * pool. Each external buffer presented by application will be mapped > > + * on an internal one. > > + */ > > + mappableBuffers_.clear(); > > + for (Buffer &buffer : bufferPool_.buffers()) { > > + /* > > + * Reserve all planes to support mapping multiplanar buffers. > > + * \todo I would use VIDEO_MAX_PLANES but it's a V4L2 thing. > > + */ > > + buffer.planes().resize(3); > > + > > + mappableBuffers_.push_back(&buffer); > > + } > > } > > > > /** > > @@ -480,6 +502,100 @@ void Stream::destroyBuffers() > > bufferPool_.destroyBuffers(); > > } > > > > +/** > > + * \brief Map an application provided buffer to a stream buffer > > + * \param applicationBuffer The application provided buffer > > + * > > + * If a Stream has been configured to use application provided buffers a > > + * mapping between the external buffers and the internal ones, which are > > + * actually used to interface with the video device, is required. > > + * > > + * The most commonly used mechanism to perform zero-copy memory sharing > > + * on Linux-based system is dmabuf, which allows user-space applications to > > + * share buffers by exchanging dmabuf generated file descriptors. This > > + * operations assumes that all application provided buffers have each of their > > + * used memory planes exported as dmabuf file descriptor, to copy them in > > + * the buffer to be then queued on the video device by pipeline handlers. > > + * > > + * Perform mapping by maintaining a cache in a map associating the dmabuf file > > + * descriptor of the application provided buffer to one of the stream's internal > > + * buffers to provide pipeline handlers the buffer to use to interact with video > > + * devices. > > + * > > + * Once the buffer completes, the mapping should be reverted to return to > > + * the application the buffer it first provided here. > > + * > > + * \return The stream buffer which maps to the application provided buffer > > + */ > > +Buffer *Stream::mapBuffer(Buffer *applicationBuffer) > > +{ > > + unsigned int key = applicationBuffer->planes()[0].dmabuf(); > > + > > + /* > > + * Buffer hit: the application buffer has already been mapped, return > > + * the assigned stream buffer > > + */ > > + auto mapped = bufferMap_.find(key); > > + if (mapped != bufferMap_.end()) { > > + Buffer *buffer = mapped->second; > > + > > + /* > > + * Keep the mappableBuffers_ vector warm: move the hit buffer to > > + * the vector end as on a buffer miss buffers are below evicted > > + * from the vector head. > > + */ > > + auto it = std::find(mappableBuffers_.begin(), > > + mappableBuffers_.end(), buffer); > > + > > + ASSERT(it != mappableBuffers_.end()); > > + std::rotate(it, it + 1, mappableBuffers_.end()); > > This don't seem to be right. > > The it found will indeed be pushed to the back, but the buffer which was > at the end will be swapped to the location it as at. Is not correct > behavior here to delete the it position and insert it at the end? Note that the iterator returned by end() point to the past-the-end position of the vector https://en.cppreference.com/w/cpp/container/vector/end So swapping an element with it, it's safe. To make sure, you can run this very simple test program too: https://paste.debian.net/1090903/ Whiche gives you: 1234 Moving 2 to vector end 1342 As you can see, 2 does not get swapped with 4, but moved after it instead. > > I assume the idea is to the closer to end() a buffer is, the "warmer" it > is. So this swapping could lead to non optimal performance. It's the other way around actually. Removing and re-inserting causes the vector to be relocated, and it should be avoided as much as possible. > > > + > > + return buffer; > > + } > > + > > + /* > > + * Buffer miss: assign to the application buffer the stream buffer > > + * at mappableBuffers_ begin, then move it to the end. > > + */ > > + Buffer *buffer = *(mappableBuffers_.begin()); > > + std::rotate(mappableBuffers_.begin(), mappableBuffers_.begin() + 1, > > + mappableBuffers_.end()); > > + > > + > > + /* Remove the [key, buffer] entry buffer from the buffer map */ > > + auto deadBuf = std::find_if(bufferMap_.begin(), bufferMap_.end(), > > + [&](std::pair<const unsigned int, Buffer *> &map) { > > + return bufferMap_[map.first] == buffer; > > + }); > > + if (deadBuf != bufferMap_.end()) > > + bufferMap_.erase(deadBuf); > > + > > + /* > > + * Assign the buffer by copying the dmabuf file descriptors from the > > + * application provided buffer. > > + */ > > + for (unsigned int i = 0; i < applicationBuffer->planes().size(); ++i) { > > + int fd = applicationBuffer->planes()[i].dmabuf(); > > + > > + /* > > + * The ARC camera stack seems to report more planes that the > > s/that/then/ > > > + * ones it actually uses. > > s/it// > > > + */ > > + if (fd < 0) > > + break; > > + > > + buffer->planes()[i].setDmabuf(fd, 0); > > + } > > + > > + /* Pipeline handlers use request_ at buffer completion time. */ > > + buffer->request_ = applicationBuffer->request(); > > + > > + /* And finally, store the mapping for later re-use and return it. */ > > + bufferMap_[key] = buffer; > > + > > + return buffer; > > +} > > + > > /** > > * \var Stream::bufferPool_ > > * \brief The pool of buffers associated with the stream > > -- > > 2.21.0 > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel > > -- > Regards, > Niklas Söderlund
diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 260a62e9e77e..d5d3dc90a096 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -59,6 +59,7 @@ private: friend class BufferPool; friend class PipelineHandler; friend class Request; + friend class Stream; friend class V4L2VideoDevice; void cancel(); diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index 796f1aff2602..74415062cbdd 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -85,6 +85,8 @@ public: const StreamConfiguration &configuration() const { return configuration_; } MemoryType memoryType() const { return memoryType_; } + Buffer *mapBuffer(Buffer *applicationBuffer); + protected: friend class Camera; @@ -94,6 +96,10 @@ protected: BufferPool bufferPool_; StreamConfiguration configuration_; MemoryType memoryType_; + +private: + std::vector<Buffer *> mappableBuffers_; + std::map<unsigned int, Buffer *> bufferMap_; }; } /* namespace libcamera */ diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index 97e0f429c9fb..4585c0db77a4 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -13,6 +13,8 @@ #include <iomanip> #include <sstream> +#include <libcamera/request.h> + #include "log.h" /** @@ -470,6 +472,26 @@ void Stream::createBuffers(unsigned int count) return; bufferPool_.createBuffers(count); + + /* Streams with internal memory usage do not need buffer mapping. */ + if (memoryType_ == InternalMemory) + return; + + /* + * Prepare for buffer mapping by queuing all buffers from the internal + * pool. Each external buffer presented by application will be mapped + * on an internal one. + */ + mappableBuffers_.clear(); + for (Buffer &buffer : bufferPool_.buffers()) { + /* + * Reserve all planes to support mapping multiplanar buffers. + * \todo I would use VIDEO_MAX_PLANES but it's a V4L2 thing. + */ + buffer.planes().resize(3); + + mappableBuffers_.push_back(&buffer); + } } /** @@ -480,6 +502,100 @@ void Stream::destroyBuffers() bufferPool_.destroyBuffers(); } +/** + * \brief Map an application provided buffer to a stream buffer + * \param applicationBuffer The application provided buffer + * + * If a Stream has been configured to use application provided buffers a + * mapping between the external buffers and the internal ones, which are + * actually used to interface with the video device, is required. + * + * The most commonly used mechanism to perform zero-copy memory sharing + * on Linux-based system is dmabuf, which allows user-space applications to + * share buffers by exchanging dmabuf generated file descriptors. This + * operations assumes that all application provided buffers have each of their + * used memory planes exported as dmabuf file descriptor, to copy them in + * the buffer to be then queued on the video device by pipeline handlers. + * + * Perform mapping by maintaining a cache in a map associating the dmabuf file + * descriptor of the application provided buffer to one of the stream's internal + * buffers to provide pipeline handlers the buffer to use to interact with video + * devices. + * + * Once the buffer completes, the mapping should be reverted to return to + * the application the buffer it first provided here. + * + * \return The stream buffer which maps to the application provided buffer + */ +Buffer *Stream::mapBuffer(Buffer *applicationBuffer) +{ + unsigned int key = applicationBuffer->planes()[0].dmabuf(); + + /* + * Buffer hit: the application buffer has already been mapped, return + * the assigned stream buffer + */ + auto mapped = bufferMap_.find(key); + if (mapped != bufferMap_.end()) { + Buffer *buffer = mapped->second; + + /* + * Keep the mappableBuffers_ vector warm: move the hit buffer to + * the vector end as on a buffer miss buffers are below evicted + * from the vector head. + */ + auto it = std::find(mappableBuffers_.begin(), + mappableBuffers_.end(), buffer); + + ASSERT(it != mappableBuffers_.end()); + std::rotate(it, it + 1, mappableBuffers_.end()); + + return buffer; + } + + /* + * Buffer miss: assign to the application buffer the stream buffer + * at mappableBuffers_ begin, then move it to the end. + */ + Buffer *buffer = *(mappableBuffers_.begin()); + std::rotate(mappableBuffers_.begin(), mappableBuffers_.begin() + 1, + mappableBuffers_.end()); + + + /* Remove the [key, buffer] entry buffer from the buffer map */ + auto deadBuf = std::find_if(bufferMap_.begin(), bufferMap_.end(), + [&](std::pair<const unsigned int, Buffer *> &map) { + return bufferMap_[map.first] == buffer; + }); + if (deadBuf != bufferMap_.end()) + bufferMap_.erase(deadBuf); + + /* + * Assign the buffer by copying the dmabuf file descriptors from the + * application provided buffer. + */ + for (unsigned int i = 0; i < applicationBuffer->planes().size(); ++i) { + int fd = applicationBuffer->planes()[i].dmabuf(); + + /* + * The ARC camera stack seems to report more planes that the + * ones it actually uses. + */ + if (fd < 0) + break; + + buffer->planes()[i].setDmabuf(fd, 0); + } + + /* Pipeline handlers use request_ at buffer completion time. */ + buffer->request_ = applicationBuffer->request(); + + /* And finally, store the mapping for later re-use and return it. */ + bufferMap_[key] = buffer; + + return buffer; +} + /** * \var Stream::bufferPool_ * \brief The pool of buffers associated with the stream
Add and operation to map external buffers provided by applications in a Request to the Stream's internal buffers used by the pipeline handlers to interact with the video device. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- include/libcamera/buffer.h | 1 + include/libcamera/stream.h | 6 ++ src/libcamera/stream.cpp | 116 +++++++++++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+)