Message ID | 20190713172351.25452-13-laurent.pinchart@ideasonboard.com |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
Hi Laurent, thanks for the update On Sat, Jul 13, 2019 at 08:23:47PM +0300, Laurent Pinchart wrote: > From: Jacopo Mondi <jacopo@jmondi.org> > > In addition to referencing buffer memory by index, add support to > referencing it using dmabuf file descriptors. This will be used to > reference buffer memory allocated outside of libcamera and import it. > > The dmabuf file descriptors are stored in an array in the Buffer class, > and a new Stream::createBuffer() overload is added to construct a buffer > from dmabuf file descriptor. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/buffer.h | 3 +++ > include/libcamera/stream.h | 1 + > src/libcamera/buffer.cpp | 13 ++++++++++- > src/libcamera/request.cpp | 5 ++++ > src/libcamera/stream.cpp | 47 +++++++++++++++++++++++++++++++++++++- > 5 files changed, 67 insertions(+), 2 deletions(-) > > diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h > index f5ba6207bcef..f8569a6b67f3 100644 > --- a/include/libcamera/buffer.h > +++ b/include/libcamera/buffer.h > @@ -7,6 +7,7 @@ > #ifndef __LIBCAMERA_BUFFER_H__ > #define __LIBCAMERA_BUFFER_H__ > > +#include <array> > #include <stdint.h> > #include <vector> > > @@ -75,6 +76,7 @@ public: > Buffer &operator=(const Buffer &) = delete; > > unsigned int index() const { return index_; } > + const std::array<int, 3> &dmabufs() const { return dmabuf_; } > > unsigned int bytesused() const { return bytesused_; } > uint64_t timestamp() const { return timestamp_; } > @@ -95,6 +97,7 @@ private: > void setRequest(Request *request) { request_ = request; } > > unsigned int index_; > + std::array<int, 3> dmabuf_; > > unsigned int bytesused_; > uint64_t timestamp_; > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h > index 08eb8cc7d5c7..1883d9e9b9c5 100644 > --- a/include/libcamera/stream.h > +++ b/include/libcamera/stream.h > @@ -75,6 +75,7 @@ public: > Stream(); > > std::unique_ptr<Buffer> createBuffer(unsigned int index); > + std::unique_ptr<Buffer> createBuffer(const std::array<int, 3> &fds); > > BufferPool &bufferPool() { return bufferPool_; } > std::vector<BufferMemory> &buffers() { return bufferPool_.buffers(); } > diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp > index ecbf25246a55..99358633a088 100644 > --- a/src/libcamera/buffer.cpp > +++ b/src/libcamera/buffer.cpp > @@ -269,7 +269,8 @@ void BufferPool::destroyBuffers() > * for a stream with Stream::createBuffer(). > */ > Buffer::Buffer(unsigned int index, const Buffer *metadata) > - : index_(index), status_(Buffer::BufferSuccess), request_(nullptr), > + : index_(index), dmabuf_({ -1, -1, -1 }), > + status_(Buffer::BufferSuccess), request_(nullptr), > stream_(nullptr) > { > if (metadata) { > @@ -289,6 +290,16 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata) > * \return The buffer index > */ > > +/** > + * \fn Buffer::dmabufs() > + * \brief Retrieve the dmabuf file descriptors for all buffer planes > + * > + * The dmabufs array contains one dmabuf file descriptor per plane. Unused > + * entries are set to -1. > + * > + * \return The dmabuf file descriptors > + */ > + > /** > * \fn Buffer::bytesused() > * \brief Retrieve the number of bytes occupied by the data in the buffer > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp > index 19131472710b..ee2158fc7a9c 100644 > --- a/src/libcamera/request.cpp > +++ b/src/libcamera/request.cpp > @@ -106,10 +106,15 @@ Request::~Request() > * > * \return 0 on success or a negative error code otherwise > * \retval -EEXIST The request already contains a buffer for the stream > + * \retval -EINVAL The buffer does not reference a valid Stream > */ > int Request::addBuffer(std::unique_ptr<Buffer> buffer) > { > Stream *stream = buffer->stream(); > + if (!stream) { > + LOG(Request, Error) << "Invalid stream reference"; > + return -EINVAL; > + } > > auto it = bufferMap_.find(stream); > if (it != bufferMap_.end()) { > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp > index 94aa4810f6b9..e6aa1b643a37 100644 > --- a/src/libcamera/stream.cpp > +++ b/src/libcamera/stream.cpp > @@ -424,17 +424,26 @@ Stream::Stream() > } > > /** > - * \brief Create a Buffer instance > + * \brief Create a Buffer instance referencing the memory buffer \a index > * \param[in] index The desired buffer index > * > * This method creates a Buffer instance that references a BufferMemory from > * the stream's buffers pool by its \a index. The index shall be lower than the > * number of buffers in the pool. > * > + * This method is only valid for streams that use the InternalMemory type. It > + * will return a null pointer when called on streams using the ExternalMemory > + * type. > + * > * \return A newly created Buffer on success or nullptr otherwise > */ > std::unique_ptr<Buffer> Stream::createBuffer(unsigned int index) > { > + if (memoryType_ != InternalMemory) { > + LOG(Stream, Error) << "Invalid stream memory type"; > + return nullptr; > + } > + > if (index >= bufferPool_.count()) { > LOG(Stream, Error) << "Invalid buffer index " << index; > return nullptr; > @@ -447,6 +456,42 @@ std::unique_ptr<Buffer> Stream::createBuffer(unsigned int index) > return std::unique_ptr<Buffer>(buffer); > } > > +/** > + * \brief Create a Buffer instance that represents a memory area identified by > + * dmabuf file descriptors > + * \param[in] fds The dmabuf file descriptors for each plane > + * > + * This method creates a Buffer instance that references buffer memory > + * allocated outside of libcamera through dmabuf file descriptors. The \a > + * dmabuf array shall contain a file descriptor for each plane in the buffer, > + * and unused entries shall be set to -1. > + * > + * The buffer is created without a valid index, as it does not yet map to any of > + * the stream's BufferMemory instances. An index will be assigned at the time > + * the buffer is queued to the camera in a request. Applications may thus > + * create any number of Buffer instances, providing that no more than the > + * number of buffers allocated for the stream are queued at any given time. > + * > + * This method is only valid for streams that use the InternalMemory type. It > + * will return a null pointer when called on streams using the ExternalMemory > + * type. It's actually the other way around! Thanks j > + * > + * \return A newly created Buffer on success or nullptr otherwise > + */ > +std::unique_ptr<Buffer> Stream::createBuffer(const std::array<int, 3> &fds) > +{ > + if (memoryType_ != ExternalMemory) { > + LOG(Stream, Error) << "Invalid stream memory type"; > + return nullptr; > + } > + > + Buffer *buffer = new Buffer(); > + buffer->dmabuf_ = fds; > + buffer->stream_ = this; > + > + return std::unique_ptr<Buffer>(buffer); > +} > + > /** > * \fn Stream::bufferPool() > * \brief Retrieve the buffer pool for the stream > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo and Laurent, Thanks for your patch. On 2019-07-13 20:23:47 +0300, Laurent Pinchart wrote: > From: Jacopo Mondi <jacopo@jmondi.org> > > In addition to referencing buffer memory by index, add support to > referencing it using dmabuf file descriptors. This will be used to > reference buffer memory allocated outside of libcamera and import it. > > The dmabuf file descriptors are stored in an array in the Buffer class, > and a new Stream::createBuffer() overload is added to construct a buffer > from dmabuf file descriptor. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > include/libcamera/buffer.h | 3 +++ > include/libcamera/stream.h | 1 + > src/libcamera/buffer.cpp | 13 ++++++++++- > src/libcamera/request.cpp | 5 ++++ > src/libcamera/stream.cpp | 47 +++++++++++++++++++++++++++++++++++++- > 5 files changed, 67 insertions(+), 2 deletions(-) > > diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h > index f5ba6207bcef..f8569a6b67f3 100644 > --- a/include/libcamera/buffer.h > +++ b/include/libcamera/buffer.h > @@ -7,6 +7,7 @@ > #ifndef __LIBCAMERA_BUFFER_H__ > #define __LIBCAMERA_BUFFER_H__ > > +#include <array> > #include <stdint.h> > #include <vector> > > @@ -75,6 +76,7 @@ public: > Buffer &operator=(const Buffer &) = delete; > > unsigned int index() const { return index_; } > + const std::array<int, 3> &dmabufs() const { return dmabuf_; } > > unsigned int bytesused() const { return bytesused_; } > uint64_t timestamp() const { return timestamp_; } > @@ -95,6 +97,7 @@ private: > void setRequest(Request *request) { request_ = request; } > > unsigned int index_; > + std::array<int, 3> dmabuf_; > > unsigned int bytesused_; > uint64_t timestamp_; > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h > index 08eb8cc7d5c7..1883d9e9b9c5 100644 > --- a/include/libcamera/stream.h > +++ b/include/libcamera/stream.h > @@ -75,6 +75,7 @@ public: > Stream(); > > std::unique_ptr<Buffer> createBuffer(unsigned int index); > + std::unique_ptr<Buffer> createBuffer(const std::array<int, 3> &fds); > > BufferPool &bufferPool() { return bufferPool_; } > std::vector<BufferMemory> &buffers() { return bufferPool_.buffers(); } > diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp > index ecbf25246a55..99358633a088 100644 > --- a/src/libcamera/buffer.cpp > +++ b/src/libcamera/buffer.cpp > @@ -269,7 +269,8 @@ void BufferPool::destroyBuffers() > * for a stream with Stream::createBuffer(). > */ > Buffer::Buffer(unsigned int index, const Buffer *metadata) > - : index_(index), status_(Buffer::BufferSuccess), request_(nullptr), > + : index_(index), dmabuf_({ -1, -1, -1 }), > + status_(Buffer::BufferSuccess), request_(nullptr), > stream_(nullptr) > { > if (metadata) { > @@ -289,6 +290,16 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata) > * \return The buffer index > */ > > +/** > + * \fn Buffer::dmabufs() > + * \brief Retrieve the dmabuf file descriptors for all buffer planes > + * > + * The dmabufs array contains one dmabuf file descriptor per plane. Unused > + * entries are set to -1. > + * > + * \return The dmabuf file descriptors > + */ > + > /** > * \fn Buffer::bytesused() > * \brief Retrieve the number of bytes occupied by the data in the buffer > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp > index 19131472710b..ee2158fc7a9c 100644 > --- a/src/libcamera/request.cpp > +++ b/src/libcamera/request.cpp > @@ -106,10 +106,15 @@ Request::~Request() > * > * \return 0 on success or a negative error code otherwise > * \retval -EEXIST The request already contains a buffer for the stream > + * \retval -EINVAL The buffer does not reference a valid Stream > */ > int Request::addBuffer(std::unique_ptr<Buffer> buffer) > { > Stream *stream = buffer->stream(); > + if (!stream) { > + LOG(Request, Error) << "Invalid stream reference"; > + return -EINVAL; > + } > > auto it = bufferMap_.find(stream); > if (it != bufferMap_.end()) { > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp > index 94aa4810f6b9..e6aa1b643a37 100644 > --- a/src/libcamera/stream.cpp > +++ b/src/libcamera/stream.cpp > @@ -424,17 +424,26 @@ Stream::Stream() > } > > /** > - * \brief Create a Buffer instance > + * \brief Create a Buffer instance referencing the memory buffer \a index > * \param[in] index The desired buffer index > * > * This method creates a Buffer instance that references a BufferMemory from > * the stream's buffers pool by its \a index. The index shall be lower than the > * number of buffers in the pool. > * > + * This method is only valid for streams that use the InternalMemory type. It > + * will return a null pointer when called on streams using the ExternalMemory > + * type. > + * > * \return A newly created Buffer on success or nullptr otherwise > */ > std::unique_ptr<Buffer> Stream::createBuffer(unsigned int index) > { > + if (memoryType_ != InternalMemory) { > + LOG(Stream, Error) << "Invalid stream memory type"; > + return nullptr; > + } > + > if (index >= bufferPool_.count()) { > LOG(Stream, Error) << "Invalid buffer index " << index; > return nullptr; > @@ -447,6 +456,42 @@ std::unique_ptr<Buffer> Stream::createBuffer(unsigned int index) > return std::unique_ptr<Buffer>(buffer); > } > > +/** > + * \brief Create a Buffer instance that represents a memory area identified by > + * dmabuf file descriptors > + * \param[in] fds The dmabuf file descriptors for each plane > + * > + * This method creates a Buffer instance that references buffer memory > + * allocated outside of libcamera through dmabuf file descriptors. The \a > + * dmabuf array shall contain a file descriptor for each plane in the buffer, > + * and unused entries shall be set to -1. > + * > + * The buffer is created without a valid index, as it does not yet map to any of > + * the stream's BufferMemory instances. An index will be assigned at the time > + * the buffer is queued to the camera in a request. Applications may thus > + * create any number of Buffer instances, providing that no more than the > + * number of buffers allocated for the stream are queued at any given time. > + * > + * This method is only valid for streams that use the InternalMemory type. It > + * will return a null pointer when called on streams using the ExternalMemory > + * type. > + * > + * \return A newly created Buffer on success or nullptr otherwise > + */ > +std::unique_ptr<Buffer> Stream::createBuffer(const std::array<int, 3> &fds) > +{ > + if (memoryType_ != ExternalMemory) { > + LOG(Stream, Error) << "Invalid stream memory type"; > + return nullptr; > + } > + > + Buffer *buffer = new Buffer(); > + buffer->dmabuf_ = fds; > + buffer->stream_ = this; > + > + return std::unique_ptr<Buffer>(buffer); > +} > + > /** > * \fn Stream::bufferPool() > * \brief Retrieve the buffer pool for the stream > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index f5ba6207bcef..f8569a6b67f3 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -7,6 +7,7 @@ #ifndef __LIBCAMERA_BUFFER_H__ #define __LIBCAMERA_BUFFER_H__ +#include <array> #include <stdint.h> #include <vector> @@ -75,6 +76,7 @@ public: Buffer &operator=(const Buffer &) = delete; unsigned int index() const { return index_; } + const std::array<int, 3> &dmabufs() const { return dmabuf_; } unsigned int bytesused() const { return bytesused_; } uint64_t timestamp() const { return timestamp_; } @@ -95,6 +97,7 @@ private: void setRequest(Request *request) { request_ = request; } unsigned int index_; + std::array<int, 3> dmabuf_; unsigned int bytesused_; uint64_t timestamp_; diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index 08eb8cc7d5c7..1883d9e9b9c5 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -75,6 +75,7 @@ public: Stream(); std::unique_ptr<Buffer> createBuffer(unsigned int index); + std::unique_ptr<Buffer> createBuffer(const std::array<int, 3> &fds); BufferPool &bufferPool() { return bufferPool_; } std::vector<BufferMemory> &buffers() { return bufferPool_.buffers(); } diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index ecbf25246a55..99358633a088 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -269,7 +269,8 @@ void BufferPool::destroyBuffers() * for a stream with Stream::createBuffer(). */ Buffer::Buffer(unsigned int index, const Buffer *metadata) - : index_(index), status_(Buffer::BufferSuccess), request_(nullptr), + : index_(index), dmabuf_({ -1, -1, -1 }), + status_(Buffer::BufferSuccess), request_(nullptr), stream_(nullptr) { if (metadata) { @@ -289,6 +290,16 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata) * \return The buffer index */ +/** + * \fn Buffer::dmabufs() + * \brief Retrieve the dmabuf file descriptors for all buffer planes + * + * The dmabufs array contains one dmabuf file descriptor per plane. Unused + * entries are set to -1. + * + * \return The dmabuf file descriptors + */ + /** * \fn Buffer::bytesused() * \brief Retrieve the number of bytes occupied by the data in the buffer diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 19131472710b..ee2158fc7a9c 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -106,10 +106,15 @@ Request::~Request() * * \return 0 on success or a negative error code otherwise * \retval -EEXIST The request already contains a buffer for the stream + * \retval -EINVAL The buffer does not reference a valid Stream */ int Request::addBuffer(std::unique_ptr<Buffer> buffer) { Stream *stream = buffer->stream(); + if (!stream) { + LOG(Request, Error) << "Invalid stream reference"; + return -EINVAL; + } auto it = bufferMap_.find(stream); if (it != bufferMap_.end()) { diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index 94aa4810f6b9..e6aa1b643a37 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -424,17 +424,26 @@ Stream::Stream() } /** - * \brief Create a Buffer instance + * \brief Create a Buffer instance referencing the memory buffer \a index * \param[in] index The desired buffer index * * This method creates a Buffer instance that references a BufferMemory from * the stream's buffers pool by its \a index. The index shall be lower than the * number of buffers in the pool. * + * This method is only valid for streams that use the InternalMemory type. It + * will return a null pointer when called on streams using the ExternalMemory + * type. + * * \return A newly created Buffer on success or nullptr otherwise */ std::unique_ptr<Buffer> Stream::createBuffer(unsigned int index) { + if (memoryType_ != InternalMemory) { + LOG(Stream, Error) << "Invalid stream memory type"; + return nullptr; + } + if (index >= bufferPool_.count()) { LOG(Stream, Error) << "Invalid buffer index " << index; return nullptr; @@ -447,6 +456,42 @@ std::unique_ptr<Buffer> Stream::createBuffer(unsigned int index) return std::unique_ptr<Buffer>(buffer); } +/** + * \brief Create a Buffer instance that represents a memory area identified by + * dmabuf file descriptors + * \param[in] fds The dmabuf file descriptors for each plane + * + * This method creates a Buffer instance that references buffer memory + * allocated outside of libcamera through dmabuf file descriptors. The \a + * dmabuf array shall contain a file descriptor for each plane in the buffer, + * and unused entries shall be set to -1. + * + * The buffer is created without a valid index, as it does not yet map to any of + * the stream's BufferMemory instances. An index will be assigned at the time + * the buffer is queued to the camera in a request. Applications may thus + * create any number of Buffer instances, providing that no more than the + * number of buffers allocated for the stream are queued at any given time. + * + * This method is only valid for streams that use the InternalMemory type. It + * will return a null pointer when called on streams using the ExternalMemory + * type. + * + * \return A newly created Buffer on success or nullptr otherwise + */ +std::unique_ptr<Buffer> Stream::createBuffer(const std::array<int, 3> &fds) +{ + if (memoryType_ != ExternalMemory) { + LOG(Stream, Error) << "Invalid stream memory type"; + return nullptr; + } + + Buffer *buffer = new Buffer(); + buffer->dmabuf_ = fds; + buffer->stream_ = this; + + return std::unique_ptr<Buffer>(buffer); +} + /** * \fn Stream::bufferPool() * \brief Retrieve the buffer pool for the stream