From patchwork Mon Dec 30 12:04:46 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2451 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3AAE86046B for ; Mon, 30 Dec 2019 13:05:50 +0100 (CET) X-Halon-ID: b7b22477-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id b7b22477-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:05:49 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:04:46 +0100 Message-Id: <20191230120510.938333-2-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 01/25] libcamera: buffer: Add FrameMetadata container for metadata information X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:05:50 -0000 With the introduction of FrameBuffer objects, the metadata information related to captured frames will not be stored directly in the frame buffer object. Add a new FrameMetadata class to hold this information. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- * Changes since v1: - Rename BufferInfo to FrameMetadata - Rename prefix for BufferInfo::Status s/Buffer/Frame/ - Make FrameMetadata a struct with public data members instead of a class with accessors methods. - Dropped FrameMetadata::update() - Documentation updates --- include/libcamera/buffer.h | 17 +++++++++++ src/libcamera/buffer.cpp | 62 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 80602124f24be4a0..0b95e41a2dd205b2 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -16,6 +16,23 @@ namespace libcamera { class Request; class Stream; +struct FrameMetadata { + enum Status { + FrameSuccess, + FrameError, + FrameCancelled, + }; + + struct Plane { + unsigned int bytesused; + }; + + Status status; + unsigned int sequence; + uint64_t timestamp; + std::vector planes; +}; + class Plane final { public: diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 960eeb8f7d193ddd..7673b96f29728a24 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -23,6 +23,68 @@ namespace libcamera { LOG_DEFINE_CATEGORY(Buffer) +/** + * \struct FrameMetadata + * \brief Metadata related to a captured frame + * + * The FrameMetadata structure stores all metadata related to a captured frame, + * as stored in a FrameBuffer, such as capture status, timestamp or memory size. + */ + +/** + * \enum FrameMetadata::Status + * \brief Define the frame completion status + * \var FrameMetadata::FrameSuccess + * The frame has been captured with success and contains valid data and metadata + * \var FrameMetadata::FrameError + * The frame has been captured with an error and doesn't contain valid metadata + * \var FrameMetadata::FrameCancelled + * The frame has been cancelled and doesn't contain valid metadata + */ + +/** + * \struct FrameMetadata::Plane + * \brief Per-plane frame metadata + * + * Frames are stored in memory in one or multiple planes. The FrameInfo::Plane + * structure stores per-plane metadata for each plane. + */ + +/** + * \var FrameMetadata::Plane::bytesused + * \brief Number of bytes occupied by the data in the plane + */ + +/** + * \var FrameMetadata::status + * \brief Status of the frame + * + * Values in the container are only valid when the status is + * FrameMetadata::FrameSuccess. + */ + +/** + * \var FrameMetadata::sequence + * \brief Frame sequence number + * + * The sequence number is a monotonically increasing number assigned to the + * buffer processed by the stream. Gaps in the sequence numbers indicate + * dropped frames. + */ + +/** + * \var FrameMetadata::timestamp + * \brief Time when the frame was captured + * + * The timestamp is expressed as a number of nanoseconds relative to the system + * clock since an unspecified time point. + */ + +/** + * \var FrameMetadata::planes + * \brief Array holding plane specific metadata + */ + /** * \class Plane * \brief A memory region to store a single plane of a frame From patchwork Mon Dec 30 12:04:47 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2452 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E76FC6046C for ; Mon, 30 Dec 2019 13:05:50 +0100 (CET) X-Halon-ID: b81dca40-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id b81dca40-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:05:49 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:04:47 +0100 Message-Id: <20191230120510.938333-3-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 02/25] libcamera: buffer: Add FrameBuffer interface X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:05:51 -0000 Add a new FrameBuffer class to describe memory used to store frames. This change just adds the new interface, future patches will migrate all parts of libcamera to use this and replace the old Buffer interface. This change needs to specify the const keyword for Plane::length() as to not get Doxygen confused with FrameBuffer::Plane::length added in this change. Signed-off-by: Niklas Söderlund --- Changes since v1: - Rename FrameBuffer::info() to FrameBuffer::metadata() - Fixup commit message - Reorder method order - Rewrite documentation - Add operator==() and operator=!() for FrameBuffer::Plane --- include/libcamera/buffer.h | 32 +++++++++ src/libcamera/buffer.cpp | 132 ++++++++++++++++++++++++++++++++++++- 2 files changed, 163 insertions(+), 1 deletion(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 0b95e41a2dd205b2..862031123b4b510c 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -11,6 +11,8 @@ #include #include +#include + namespace libcamera { class Request; @@ -33,6 +35,36 @@ struct FrameMetadata { std::vector planes; }; +class FrameBuffer final +{ +public: + struct Plane { + FileDescriptor fd; + unsigned int length; + }; + + FrameBuffer(const std::vector &planes = {}, unsigned int cookie = 0); + + const std::vector &planes() const { return planes_; } + + Request *request() const { return request_; } + const FrameMetadata &metadata() const { return metadata_; }; + + unsigned int cookie() const { return cookie_; } + void setCookie(unsigned int cookie) { cookie_ = cookie; } + +private: + friend class Request; /* Needed to update request_. */ + friend class V4L2VideoDevice; /* Needed to update metadata_. */ + + std::vector planes_; + + Request *request_; + FrameMetadata metadata_; + + unsigned int cookie_; +}; + class Plane final { public: diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 7673b96f29728a24..7c78b9b8f1e90aa5 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -229,7 +229,7 @@ void *Plane::mem() } /** - * \fn Plane::length() + * \fn Plane::length() const * \brief Retrieve the length of the memory region * \return The length of the memory region */ @@ -462,4 +462,134 @@ void Buffer::cancel() * The intended callers are Request::prepare() and Request::completeBuffer(). */ +/** + * \class FrameBuffer + * \brief A buffer handle and dynamic metadata + * + * The FrameBuffer class is the primary interface for applications, IPAs and + * pipelines to interact with capture memory. It contains all static and + * dynamic information to handle the whole life-cycle of creating, queueing, + * capturing and consumption of a single frame capture. + * + * The static information describes the one or more memory planes which makes + * up the complete frame. The static information needs to be available at + * creation of the FrameBuffer and needs to be expressed as a valid dmabuf file + * descriptor and length. + * + * The dynamic data, content of the memory planes and the metadata, is updated + * each time the buffer is successfully processed by a camera, by queueing it as + * part of a request. The dynamic data is only valid after the cameras buffer + * complete signal have been raised and until the FrameBuffer is queued once + * more to the camera as part of a new request. + * + * The creator of a FrameBuffer (applications, IPA or pipeline) may associate + * an integer cookie with the instance at any time to aid in its usage by the + * object creator. This cookie is transparent to libcamera core and shall only + * be set and consumed by the user. This supplements the Request objects cookie. + */ + +/** + * \struct FrameBuffer::Plane + * \brief A description of a memory plane + * + * A memory plane is described by a dmabuf file descriptor and a plane length. + * + * Applications or IPAs can use this information to map the planes memory and + * access its content. The mapper of the memory is responsible for unmaping + * the memory before the FrameBuffer::Plane object is destroyed. + * + * \todo Once we have a Kernel API which can express offsets within a plane + * this structure shall be extended to contain this information. + */ + +/** + * \var FrameBuffer::Plane::fd + * \brief The dmabuf file descriptor + */ + +/** + * \var FrameBuffer::Plane::length + * + * The length value is only used if the memory of the plane needs to be mapped. + * Therefor a length of zero is valid for planes allocated outside libcamera and + * queued to a Camera as part of a Request, libcamera will never poke inside + * that memory and thus do not need to know its length. + * + * All planes created by pipeline handlers however need to have a correct length + * set as IPAs and other parts of libcamera core might wish to memory map the + * plane. + * + * \brief The plane length + */ + +/** + * \brief Create a libcamera FrameBuffer object from an array of planes + * \param[in] planes Planes that makes up the FrameBuffer + * \param[in] cookie Integer cookie + * + * Create a FrameBuffer object from an array of static plane descriptions. The + * creator may close the file descriptors in the plane description after the + * FrameBuffer have been created. + */ +FrameBuffer::FrameBuffer(const std::vector &planes, unsigned int cookie) + : request_(nullptr), cookie_(cookie) +{ + /* Clone all file descriptors. */ + planes_ = planes; +} + +/** + * \fn FrameBuffer::planes() + * \brief Retrieve the static plane descriptions + * \return Array of plane descriptions + */ + +/** + * \fn FrameBuffer::request() + * \brief Retrieve the request this buffer belongs to + * + * The intended callers of this method are buffer completion handlers that + * need to associate a buffer to the request it belongs to. + * + * A Buffer is associated to a request by Request::prepare() and the + * association is valid until the buffer completes. The returned request + * pointer is valid only during that interval. + * + * \return The Request the Buffer belongs to, or nullptr if the buffer is + * either completed or not associated with a request + */ + +/** + * \fn FrameBuffer::metadata() + * \brief Retrieve the dynamic metadata + * \return Dynamic metadata for the frame + */ + +/** + * \fn FrameBuffer::cookie() + * \brief Retrieve the integer cookie + * + * The cookie shall only be read by the creator of the FrameBuffer. The cookie + * is transparent to libcamera core. + * + * \return Integer cookie + */ + +/** + * \fn FrameBuffer::setCookie() + * \brief Set the integer cookie + * \param[in] cookie Cookie to set + * + * The cookie shall only be set by the creator of the FrameBuffer. The cookie + * is transparent to libcamera core. + */ + +/** + * \var FrameBuffer::request_ + * \brief The request this frame belongs to + * + * This member is intended to be set by Request::prepare() and + * Request::completeBuffer(). + */ + } /* namespace libcamera */ From patchwork Mon Dec 30 12:04:48 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2453 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1337A605D8 for ; Mon, 30 Dec 2019 13:05:52 +0100 (CET) X-Halon-ID: b895d447-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id b895d447-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:05:50 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:04:48 +0100 Message-Id: <20191230120510.938333-4-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 03/25] ipa: Switch to FrameBuffer interface X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:05:53 -0000 Switch the IPA interfaces and implementations to use the Framebuffer interface. - The IPA interface is switch to use the simpler FrameBuffer::Plane container when carrying dmabuf descriptions (fd and length) over the pipeline/IPA boundary. - The RkISP1 IPA implementation takes advantage of the new simpler and safer (better control over file descriptors) FrameBuffer interface. The biggest change can be observed in the test case where it is demonstrated that FrameBuffer objects only carry the necessary number of planes and not a fixed number. As a consequence of this the test needs to verify the file descriptors it injects and not depend on the library to handle uninitialized fields. Signed-off-by: Niklas Söderlund --- include/ipa/ipa_interface.h | 2 +- src/ipa/libipa/ipa_interface_wrapper.cpp | 9 ++---- src/ipa/rkisp1/rkisp1.cpp | 30 +++++++++++++---- src/libcamera/ipa_context_wrapper.cpp | 8 ++--- src/libcamera/ipa_interface.cpp | 7 ++-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 +++++-- test/ipa/ipa_wrappers_test.cpp | 41 +++++++++++++----------- 7 files changed, 66 insertions(+), 43 deletions(-) diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h index 0ea53d120fe10acf..229d1124b19ec1c9 100644 --- a/include/ipa/ipa_interface.h +++ b/include/ipa/ipa_interface.h @@ -105,7 +105,7 @@ struct IPAStream { struct IPABuffer { unsigned int id; - BufferMemory memory; + std::vector planes; }; struct IPAOperationData { diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp index 6a389dfa714ab535..3628a785dc3e63f4 100644 --- a/src/ipa/libipa/ipa_interface_wrapper.cpp +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp @@ -144,17 +144,14 @@ void IPAInterfaceWrapper::map_buffers(struct ipa_context *_ctx, for (unsigned int i = 0; i < num_buffers; ++i) { const struct ipa_buffer &_buffer = _buffers[i]; IPABuffer &buffer = buffers[i]; - std::vector &planes = buffer.memory.planes(); + std::vector &planes = buffer.planes; buffer.id = _buffer.id; planes.resize(_buffer.num_planes); for (unsigned int j = 0; j < _buffer.num_planes; ++j) { - if (_buffer.planes[j].dmabuf != -1) - planes[j].setDmabuf(_buffer.planes[j].dmabuf, - _buffer.planes[j].length); - /** \todo Create a Dmabuf class to implement RAII. */ - ::close(_buffer.planes[j].dmabuf); + planes[j].fd = FileDescriptor(_buffer.planes[j].dmabuf); + planes[j].length = _buffer.planes[j].length; } } diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 744a16ae5b9aa420..63776563b149f7ed 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include @@ -48,7 +49,8 @@ private: void setControls(unsigned int frame); void metadataReady(unsigned int frame, unsigned int aeState); - std::map bufferInfo_; + std::map buffers_; + std::map buffersMemory_; ControlInfoMap ctrls_; @@ -102,15 +104,29 @@ void IPARkISP1::configure(const std::map &streamConfig, void IPARkISP1::mapBuffers(const std::vector &buffers) { for (const IPABuffer &buffer : buffers) { - bufferInfo_[buffer.id] = buffer.memory; - bufferInfo_[buffer.id].planes()[0].mem(); + buffers_.emplace(buffer.id, FrameBuffer(buffer.planes)); + buffersMemory_[buffer.id] = mmap(NULL, + buffers_[buffer.id].planes()[0].length, + PROT_READ | PROT_WRITE, + MAP_SHARED, + buffers_[buffer.id].planes()[0].fd.fd(), + 0); + + if (buffersMemory_[buffer.id] == MAP_FAILED) { + int ret = -errno; + LOG(IPARkISP1, Fatal) << "Failed to mmap buffer: " + << strerror(-ret); + } } } void IPARkISP1::unmapBuffers(const std::vector &ids) { - for (unsigned int id : ids) - bufferInfo_.erase(id); + for (unsigned int id : ids) { + munmap(buffersMemory_[id], buffers_[id].planes()[0].length); + buffersMemory_.erase(id); + buffers_.erase(id); + } } void IPARkISP1::processEvent(const IPAOperationData &event) @@ -121,7 +137,7 @@ void IPARkISP1::processEvent(const IPAOperationData &event) unsigned int bufferId = event.data[1]; const rkisp1_stat_buffer *stats = - static_cast(bufferInfo_[bufferId].planes()[0].mem()); + static_cast(buffersMemory_[bufferId]); updateStatistics(frame, stats); break; @@ -131,7 +147,7 @@ void IPARkISP1::processEvent(const IPAOperationData &event) unsigned int bufferId = event.data[1]; rkisp1_isp_params_cfg *params = - static_cast(bufferInfo_[bufferId].planes()[0].mem()); + static_cast(buffersMemory_[bufferId]); queueRequest(frame, params, event.controls[0]); break; diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp index 9603fdac87150aa0..946a2fd8cab1782a 100644 --- a/src/libcamera/ipa_context_wrapper.cpp +++ b/src/libcamera/ipa_context_wrapper.cpp @@ -149,15 +149,15 @@ void IPAContextWrapper::mapBuffers(const std::vector &buffers) for (unsigned int i = 0; i < buffers.size(); ++i) { struct ipa_buffer &c_buffer = c_buffers[i]; const IPABuffer &buffer = buffers[i]; - const std::vector &planes = buffer.memory.planes(); + const std::vector &planes = buffer.planes; c_buffer.id = buffer.id; c_buffer.num_planes = planes.size(); for (unsigned int j = 0; j < planes.size(); ++j) { - const Plane &plane = planes[j]; - c_buffer.planes[j].dmabuf = plane.dmabuf(); - c_buffer.planes[j].length = plane.length(); + const FrameBuffer::Plane &plane = planes[j]; + c_buffer.planes[j].dmabuf = plane.fd.fd(); + c_buffer.planes[j].length = plane.length; } } diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp index ee3e3622f39ae85f..2f86087ee4aa414f 100644 --- a/src/libcamera/ipa_interface.cpp +++ b/src/libcamera/ipa_interface.cpp @@ -334,11 +334,10 @@ namespace libcamera { */ /** - * \var IPABuffer::memory - * \brief The buffer memory description + * \var IPABuffer::planes + * \brief The buffer planes description * - * The memory field stores the dmabuf handle and size for each plane of the - * buffer. + * Stores the dmabuf handle and length for each plane of the buffer. */ /** diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 7e41222e3d01a475..d7ee95ded0f76027 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -687,14 +687,22 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera, } for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) { + FrameBuffer::Plane plane; + plane.fd = FileDescriptor(paramPool_.buffers()[i].planes()[0].dmabuf()); + plane.length = paramPool_.buffers()[i].planes()[0].length(); + data->ipaBuffers_.push_back({ .id = RKISP1_PARAM_BASE | i, - .memory = paramPool_.buffers()[i] }); + .planes = { plane } }); paramBuffers_.push(new Buffer(i)); } for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) { + FrameBuffer::Plane plane; + plane.fd = FileDescriptor(statPool_.buffers()[i].planes()[0].dmabuf()); + plane.length = statPool_.buffers()[i].planes()[0].length(); + data->ipaBuffers_.push_back({ .id = RKISP1_STAT_BASE | i, - .memory = statPool_.buffers()[i] }); + .planes = { plane } }); statBuffers_.push(new Buffer(i)); } diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp index a1e34ad52317f47b..e711e4fe318d8179 100644 --- a/test/ipa/ipa_wrappers_test.cpp +++ b/test/ipa/ipa_wrappers_test.cpp @@ -115,28 +115,28 @@ public: return report(Op_mapBuffers, TestFail); } - if (buffers[0].memory.planes().size() != 3 || - buffers[1].memory.planes().size() != 3) { + if (buffers[0].planes.size() != 3 || + buffers[1].planes.size() != 3) { cerr << "mapBuffers(): Invalid number of planes" << endl; return report(Op_mapBuffers, TestFail); } - if (buffers[0].memory.planes()[0].length() != 4096 || - buffers[0].memory.planes()[1].length() != 0 || - buffers[0].memory.planes()[2].length() != 0 || - buffers[0].memory.planes()[0].length() != 4096 || - buffers[1].memory.planes()[1].length() != 4096 || - buffers[1].memory.planes()[2].length() != 0) { + if (buffers[0].planes[0].length != 4096 || + buffers[0].planes[1].length != 0 || + buffers[0].planes[2].length != 0 || + buffers[0].planes[0].length != 4096 || + buffers[1].planes[1].length != 4096 || + buffers[1].planes[2].length != 0) { cerr << "mapBuffers(): Invalid length" << endl; return report(Op_mapBuffers, TestFail); } - if (buffers[0].memory.planes()[0].dmabuf() == -1 || - buffers[0].memory.planes()[1].dmabuf() != -1 || - buffers[0].memory.planes()[2].dmabuf() != -1 || - buffers[0].memory.planes()[0].dmabuf() == -1 || - buffers[1].memory.planes()[1].dmabuf() == -1 || - buffers[1].memory.planes()[2].dmabuf() != -1) { + if (buffers[0].planes[0].fd.fd() == -1 || + buffers[0].planes[1].fd.fd() != -1 || + buffers[0].planes[2].fd.fd() != -1 || + buffers[0].planes[0].fd.fd() == -1 || + buffers[1].planes[1].fd.fd() == -1 || + buffers[1].planes[2].fd.fd() != -1) { cerr << "mapBuffers(): Invalid dmabuf" << endl; return report(Op_mapBuffers, TestFail); } @@ -287,13 +287,16 @@ protected: /* Test mapBuffers(). */ std::vector buffers(2); - buffers[0].memory.planes().resize(3); + buffers[0].planes.resize(3); buffers[0].id = 10; - buffers[0].memory.planes()[0].setDmabuf(fd_, 4096); + buffers[0].planes[0].fd = FileDescriptor(fd_); + buffers[0].planes[0].length = 4096; buffers[1].id = 11; - buffers[1].memory.planes().resize(3); - buffers[1].memory.planes()[0].setDmabuf(fd_, 4096); - buffers[1].memory.planes()[1].setDmabuf(fd_, 4096); + buffers[1].planes.resize(3); + buffers[1].planes[0].fd = FileDescriptor(fd_); + buffers[1].planes[0].length = 4096; + buffers[1].planes[1].fd = FileDescriptor(fd_); + buffers[1].planes[1].length = 4096; ret = INVOKE(mapBuffers, buffers); if (ret == TestFail) From patchwork Mon Dec 30 12:04:49 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2454 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id CB17E6046B for ; Mon, 30 Dec 2019 13:05:52 +0100 (CET) X-Halon-ID: b9353e40-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id b9353e40-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:05:51 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:04:49 +0100 Message-Id: <20191230120510.938333-5-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 04/25] libcamera: buffer: Switch from Plane to FrameBuffer::Plane X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:05:53 -0000 It is not libcameras responsibility to handle memory mappings. Switch from the soon to be removed Plane class which deals with memory mappings to FrameBuffer::Plane which just describes it. This makes the transition to the full FrameBuffer easier. As the full FrameBuffer interface have not yet spread to all parts of libcamera core it is hard to create efficient caching of memory mappings in the qcam application. This will be fixed in a later patch, for now the dmabuf is mapped and unmapped each time it is seen by the application. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- include/libcamera/buffer.h | 6 +++--- src/cam/buffer_writer.cpp | 10 +++++++--- src/libcamera/buffer.cpp | 4 ++-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 ++++---- src/libcamera/stream.cpp | 6 ++++-- src/libcamera/v4l2_videodevice.cpp | 13 +++++++------ src/qcam/main_window.cpp | 14 ++++++++++---- test/camera/buffer_import.cpp | 2 +- 8 files changed, 38 insertions(+), 25 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 862031123b4b510c..71633492e4752efb 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -89,11 +89,11 @@ private: class BufferMemory final { public: - const std::vector &planes() const { return planes_; } - std::vector &planes() { return planes_; } + const std::vector &planes() const { return planes_; } + std::vector &planes() { return planes_; } private: - std::vector planes_; + std::vector planes_; }; class BufferPool final diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp index c33e99c5f8173db8..3e84068e66bb4dd7 100644 --- a/src/cam/buffer_writer.cpp +++ b/src/cam/buffer_writer.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include "buffer_writer.h" @@ -43,9 +44,10 @@ int BufferWriter::write(Buffer *buffer, const std::string &streamName) return -errno; BufferMemory *mem = buffer->mem(); - for (Plane &plane : mem->planes()) { - void *data = plane.mem(); - unsigned int length = plane.length(); + for (const FrameBuffer::Plane &plane : mem->planes()) { + void *data = mmap(NULL, plane.length, PROT_READ, MAP_SHARED, + plane.fd.fd(), 0); + unsigned int length = plane.length; ret = ::write(fd, data, length); if (ret < 0) { @@ -59,6 +61,8 @@ int BufferWriter::write(Buffer *buffer, const std::string &streamName) << length << std::endl; break; } + + munmap(data, length); } close(fd); diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 7c78b9b8f1e90aa5..fde0b33511c64c9e 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -246,13 +246,13 @@ void *Plane::mem() /** * \fn BufferMemory::planes() const * \brief Retrieve the planes within the buffer - * \return A const reference to a vector holding all Planes within the buffer + * \return A const reference to a vector holding all planes within the buffer */ /** * \fn BufferMemory::planes() * \brief Retrieve the planes within the buffer - * \return A reference to a vector holding all Planes within the buffer + * \return A reference to a vector holding all planes within the buffer */ /** diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index d7ee95ded0f76027..bb652d0da9c6df52 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -688,8 +688,8 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera, for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) { FrameBuffer::Plane plane; - plane.fd = FileDescriptor(paramPool_.buffers()[i].planes()[0].dmabuf()); - plane.length = paramPool_.buffers()[i].planes()[0].length(); + plane.fd = FileDescriptor(paramPool_.buffers()[i].planes()[0].fd.fd()); + plane.length = paramPool_.buffers()[i].planes()[0].length; data->ipaBuffers_.push_back({ .id = RKISP1_PARAM_BASE | i, .planes = { plane } }); @@ -698,8 +698,8 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera, for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) { FrameBuffer::Plane plane; - plane.fd = FileDescriptor(statPool_.buffers()[i].planes()[0].dmabuf()); - plane.length = statPool_.buffers()[i].planes()[0].length(); + plane.fd = FileDescriptor(statPool_.buffers()[i].planes()[0].fd.fd()); + plane.length = statPool_.buffers()[i].planes()[0].length; data->ipaBuffers_.push_back({ .id = RKISP1_STAT_BASE | i, .planes = { plane } }); diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index 45f31ae1e2daeb53..a6adc0de5da40063 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -577,8 +577,10 @@ int Stream::mapBuffer(const Buffer *buffer) if (dmabufs[i] == -1) break; - mem->planes().emplace_back(); - mem->planes().back().setDmabuf(dmabufs[i], 0); + FrameBuffer::Plane plane; + plane.fd = FileDescriptor(dmabufs[i]); + plane.length = 0; + mem->planes().emplace_back(plane); } /* Remove the buffer from the cache and return its index. */ diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 13e023237dab0daf..f3f5303b7f470f63 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -921,9 +921,10 @@ int V4L2VideoDevice::createPlane(BufferMemory *buffer, unsigned int index, return ret; } - buffer->planes().emplace_back(); - Plane &plane = buffer->planes().back(); - plane.setDmabuf(expbuf.fd, length); + FrameBuffer::Plane plane; + plane.fd = FileDescriptor(expbuf.fd); + plane.length = length; + buffer->planes().emplace_back(plane); ::close(expbuf.fd); return 0; @@ -986,14 +987,14 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer) bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); BufferMemory *mem = &bufferPool_->buffers()[buf.index]; - const std::vector &planes = mem->planes(); + const std::vector &planes = mem->planes(); if (buf.memory == V4L2_MEMORY_DMABUF) { if (multiPlanar) { for (unsigned int p = 0; p < planes.size(); ++p) - v4l2Planes[p].m.fd = planes[p].dmabuf(); + v4l2Planes[p].m.fd = planes[p].fd.fd(); } else { - buf.m.fd = planes[0].dmabuf(); + buf.m.fd = planes[0].fd.fd(); } } diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 0c7ca61ac12ec41c..11fb67a509e973f5 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -296,13 +297,18 @@ void MainWindow::requestComplete(Request *request) int MainWindow::display(Buffer *buffer) { - BufferMemory *mem = buffer->mem(); - if (mem->planes().size() != 1) + if (buffer->mem()->planes().size() != 1) return -EINVAL; - Plane &plane = mem->planes().front(); - unsigned char *raw = static_cast(plane.mem()); + /* \todo: Once the FrameBuffer is done cache mapped memory. */ + const FrameBuffer::Plane &plane = buffer->mem()->planes().front(); + void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED, + plane.fd.fd(), 0); + + unsigned char *raw = static_cast(memory); viewfinder_->display(raw, buffer->bytesused()); + munmap(memory, plane.length); + return 0; } diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index 3efe02704c02f691..171540edd96f9fca 100644 --- a/test/camera/buffer_import.cpp +++ b/test/camera/buffer_import.cpp @@ -178,7 +178,7 @@ private: uint64_t cookie = index; BufferMemory &mem = pool_.buffers()[index]; - int dmabuf = mem.planes()[0].dmabuf(); + int dmabuf = mem.planes()[0].fd.fd(); requestReady.emit(cookie, dmabuf); From patchwork Mon Dec 30 12:04:50 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2455 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A3F596046B for ; Mon, 30 Dec 2019 13:05:53 +0100 (CET) X-Halon-ID: b9b1ae93-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id b9b1ae93-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:05:52 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:04:50 +0100 Message-Id: <20191230120510.938333-6-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 05/25] libcamera: buffers: Remove Plane class X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:05:53 -0000 There are no users left of the Plane class, drop it. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- include/libcamera/buffer.h | 21 ------ src/libcamera/buffer.cpp | 149 ------------------------------------- 2 files changed, 170 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 71633492e4752efb..2d8885e9f3a1234f 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -65,27 +65,6 @@ private: unsigned int cookie_; }; -class Plane final -{ -public: - Plane(); - ~Plane(); - - int dmabuf() const { return fd_; } - int setDmabuf(int fd, unsigned int length); - - void *mem(); - unsigned int length() const { return length_; } - -private: - int mmap(); - int munmap(); - - int fd_; - unsigned int length_; - void *mem_; -}; - class BufferMemory final { public: diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index fde0b33511c64c9e..3ea982306843b116 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -85,155 +85,6 @@ LOG_DEFINE_CATEGORY(Buffer) * \brief Array holding plane specific metadata */ -/** - * \class Plane - * \brief A memory region to store a single plane of a frame - * - * Planar pixel formats use multiple memory regions to store planes - * corresponding to the different colour components of a frame. The Plane class - * tracks the specific details of a memory region used to store a single plane - * for a given frame and provides the means to access the memory, both for the - * application and for DMA. A Buffer then contains one or multiple planes - * depending on its pixel format. - * - * To support DMA access, planes are associated with dmabuf objects represented - * by file handles. Each plane carries a dmabuf file handle and an offset within - * the buffer. Those file handles may refer to the same dmabuf object, depending - * on whether the devices accessing the memory regions composing the image - * support non-contiguous DMA to planes ore require DMA-contiguous memory. - * - * To support CPU access, planes carry the CPU address of their backing memory. - * Similarly to the dmabuf file handles, the CPU addresses for planes composing - * an image may or may not be contiguous. - */ - -Plane::Plane() - : fd_(-1), length_(0), mem_(0) -{ -} - -Plane::~Plane() -{ - munmap(); - - if (fd_ != -1) - close(fd_); -} - -/** - * \fn Plane::dmabuf() - * \brief Get the dmabuf file handle backing the buffer - */ - -/** - * \brief Set the dmabuf file handle backing the buffer - * \param[in] fd The dmabuf file handle - * \param[in] length The size of the memory region - * - * The \a fd dmabuf file handle is duplicated and stored. The caller may close - * the original file handle. - * - * \return 0 on success or a negative error code otherwise - */ -int Plane::setDmabuf(int fd, unsigned int length) -{ - if (fd < 0) { - LOG(Buffer, Error) << "Invalid dmabuf fd provided"; - return -EINVAL; - } - - if (fd_ != -1) { - close(fd_); - fd_ = -1; - } - - fd_ = dup(fd); - if (fd_ == -1) { - int ret = -errno; - LOG(Buffer, Error) - << "Failed to duplicate dmabuf: " << strerror(-ret); - return ret; - } - - length_ = length; - - return 0; -} - -/** - * \brief Map the plane memory data to a CPU accessible address - * - * The file descriptor to map the memory from must be set by a call to - * setDmaBuf() before calling this function. - * - * \sa setDmaBuf() - * - * \return 0 on success or a negative error code otherwise - */ -int Plane::mmap() -{ - void *map; - - if (mem_) - return 0; - - map = ::mmap(NULL, length_, PROT_READ | PROT_WRITE, MAP_SHARED, fd_, 0); - if (map == MAP_FAILED) { - int ret = -errno; - LOG(Buffer, Error) - << "Failed to mmap plane: " << strerror(-ret); - return ret; - } - - mem_ = map; - - return 0; -} - -/** - * \brief Unmap any existing CPU accessible mapping - * - * Unmap the memory mapped by an earlier call to mmap(). - * - * \return 0 on success or a negative error code otherwise - */ -int Plane::munmap() -{ - int ret = 0; - - if (mem_) - ret = ::munmap(mem_, length_); - - if (ret) { - ret = -errno; - LOG(Buffer, Warning) - << "Failed to unmap plane: " << strerror(-ret); - } else { - mem_ = 0; - } - - return ret; -} - -/** - * \fn Plane::mem() - * \brief Retrieve the CPU accessible memory address of the Plane - * \return The CPU accessible memory address on success or nullptr otherwise. - */ -void *Plane::mem() -{ - if (!mem_) - mmap(); - - return mem_; -} - -/** - * \fn Plane::length() const - * \brief Retrieve the length of the memory region - * \return The length of the memory region - */ - /** * \class BufferMemory * \brief A memory buffer to store an image From patchwork Mon Dec 30 12:04:51 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2456 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 65810605E8 for ; Mon, 30 Dec 2019 13:05:54 +0100 (CET) X-Halon-ID: ba3349c3-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id ba3349c3-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:05:53 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:04:51 +0100 Message-Id: <20191230120510.938333-7-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 06/25] libcamera: buffer: Drop private function setRequest() X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:05:55 -0000 There is no need to have a private helper function to access a private data member when a friend statement is needed anyhow. Remove the helper function to simplify the code and make it clear that a private member of Buffer is accessed. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- include/libcamera/buffer.h | 2 -- src/libcamera/buffer.cpp | 8 ++++---- src/libcamera/request.cpp | 4 ++-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 2d8885e9f3a1234f..8dd9f91272291648 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -123,8 +123,6 @@ private: void cancel(); - void setRequest(Request *request) { request_ = request; } - unsigned int index_; std::array dmabuf_; BufferMemory *mem_; diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 3ea982306843b116..4a77f20751408b7c 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -279,7 +279,6 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata) * * \return The Request the Buffer belongs to, or nullptr if the buffer is * either completed or not associated with a request - * \sa Buffer::setRequest() */ /** @@ -307,10 +306,11 @@ void Buffer::cancel() } /** - * \fn Buffer::setRequest() - * \brief Set the request this buffer belongs to + * \var Buffer::request_ + * \brief The request this buffer belongs to * - * The intended callers are Request::prepare() and Request::completeBuffer(). + * This member is intended to be set by Request::prepare() and + * Request::completeBuffer(). */ /** diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index c14ed1a4d3ce55d0..c2854dc2e8caab2e 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -220,7 +220,7 @@ int Request::prepare() for (auto const &pair : bufferMap_) { Buffer *buffer = pair.second; - buffer->setRequest(this); + buffer->request_ = this; pending_.insert(buffer); } @@ -258,7 +258,7 @@ bool Request::completeBuffer(Buffer *buffer) int ret = pending_.erase(buffer); ASSERT(ret == 1); - buffer->setRequest(nullptr); + buffer->request_ = nullptr; if (buffer->status() == Buffer::BufferCancelled) cancelled_ = true; From patchwork Mon Dec 30 12:04:52 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2457 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1E2936046B for ; Mon, 30 Dec 2019 13:05:55 +0100 (CET) X-Halon-ID: baa976e1-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id baa976e1-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:05:54 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:04:52 +0100 Message-Id: <20191230120510.938333-8-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 07/25] libcamera: v4l2_videodevice: Align which type variable is used in queueBuffer() X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:05:55 -0000 Reading V4L2VideoDevice::queueBuffer() is confusing since buf.type is first set to bufferType_ but then both variables are used in V4L2 macros to operate based on which type of buffer is being processed. Align on only using buf.type since it has the most existing users. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- src/libcamera/v4l2_videodevice.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index f3f5303b7f470f63..2e3f6d00d4aae6ff 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1003,7 +1003,7 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer) buf.m.planes = v4l2Planes; } - if (V4L2_TYPE_IS_OUTPUT(bufferType_)) { + if (V4L2_TYPE_IS_OUTPUT(buf.type)) { buf.bytesused = buffer->bytesused_; buf.sequence = buffer->sequence_; buf.timestamp.tv_sec = buffer->timestamp_ / 1000000000; From patchwork Mon Dec 30 12:04:53 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2458 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id EB26E6046F for ; Mon, 30 Dec 2019 13:05:55 +0100 (CET) X-Halon-ID: bb11bd33-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id bb11bd33-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:05:55 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:04:53 +0100 Message-Id: <20191230120510.938333-9-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 08/25] libcamera: v4l2_videodevice: Extract exportDmabufFd() X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:05:56 -0000 The part in createPlane() that exports a dma buffer from a video device will be used directly by the FrameBuffer interface. Break it out to a separate function. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- * Changes since v1 - Rename exportDmaBuffer() to exportDmabufFd() --- src/libcamera/include/v4l2_videodevice.h | 1 + src/libcamera/v4l2_videodevice.cpp | 41 +++++++++++++++--------- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index fdf11b3a6ec9b702..1f52fe0120831fa3 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -179,6 +179,7 @@ private: int requestBuffers(unsigned int count); int createPlane(BufferMemory *buffer, unsigned int index, unsigned int plane, unsigned int length); + int exportDmabufFd(unsigned int index, unsigned int plane); Buffer *dequeueBuffer(); void bufferAvailable(EventNotifier *notifier); diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 2e3f6d00d4aae6ff..7d585ce03ed9a45a 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -901,31 +901,22 @@ int V4L2VideoDevice::exportBuffers(BufferPool *pool) int V4L2VideoDevice::createPlane(BufferMemory *buffer, unsigned int index, unsigned int planeIndex, unsigned int length) { - struct v4l2_exportbuffer expbuf = {}; - int ret; + int fd; LOG(V4L2, Debug) << "Buffer " << index << " plane " << planeIndex << ": length=" << length; - expbuf.type = bufferType_; - expbuf.index = index; - expbuf.plane = planeIndex; - expbuf.flags = O_RDWR; - - ret = ioctl(VIDIOC_EXPBUF, &expbuf); - if (ret < 0) { - LOG(V4L2, Error) - << "Failed to export buffer: " << strerror(-ret); - return ret; - } + fd = exportDmabufFd(index, planeIndex); + if (fd < 0) + return fd; FrameBuffer::Plane plane; - plane.fd = FileDescriptor(expbuf.fd); + plane.fd = FileDescriptor(fd); plane.length = length; buffer->planes().emplace_back(plane); - ::close(expbuf.fd); + ::close(fd); return 0; } @@ -951,6 +942,26 @@ int V4L2VideoDevice::importBuffers(BufferPool *pool) return 0; } +int V4L2VideoDevice::exportDmabufFd(unsigned int index, unsigned int plane) +{ + struct v4l2_exportbuffer expbuf = {}; + int ret; + + expbuf.type = bufferType_; + expbuf.index = index; + expbuf.plane = plane; + expbuf.flags = O_RDWR; + + ret = ioctl(VIDIOC_EXPBUF, &expbuf); + if (ret < 0) { + LOG(V4L2, Error) + << "Failed to export buffer: " << strerror(-ret); + return ret; + } + + return expbuf.fd; +} + /** * \brief Release all internally allocated buffers */ From patchwork Mon Dec 30 12:04:54 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2459 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C3193605DA for ; Mon, 30 Dec 2019 13:05:56 +0100 (CET) X-Halon-ID: bb8e7506-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id bb8e7506-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:05:55 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:04:54 +0100 Message-Id: <20191230120510.938333-10-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 09/25] libcamera: request: In addBuffer() do not fetch stream from Buffer X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:05:57 -0000 In the FrameBuffer interface the stream will not be available from the buffer object as the buffer might be allocated externally. The application needs to explicitly state which stream the buffer is being added for to the request. Extend the addBuffer() function to get this information explicitly from the caller. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- include/libcamera/request.h | 2 +- src/android/camera_device.cpp | 2 +- src/cam/capture.cpp | 4 ++-- src/libcamera/request.cpp | 4 ++-- src/qcam/main_window.cpp | 4 ++-- test/camera/buffer_import.cpp | 2 +- test/camera/capture.cpp | 4 ++-- test/camera/statemachine.cpp | 2 +- 8 files changed, 12 insertions(+), 12 deletions(-) diff --git a/include/libcamera/request.h b/include/libcamera/request.h index 2d5a5964e99eb75f..a8708010ec1247a2 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -39,7 +39,7 @@ public: ControlList &controls() { return *controls_; } ControlList &metadata() { return *metadata_; } const std::map &buffers() const { return bufferMap_; } - int addBuffer(std::unique_ptr buffer); + int addBuffer(Stream *stream, std::unique_ptr buffer); Buffer *findBuffer(Stream *stream) const; uint64_t cookie() const { return cookie_; } diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 065e0292e186c2ad..09588c16a6301649 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -754,7 +754,7 @@ void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reque Request *request = camera_->createRequest(reinterpret_cast(descriptor)); - request->addBuffer(std::move(buffer)); + request->addBuffer(stream, std::move(buffer)); int ret = camera_->queueRequest(request); if (ret) { diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index 4b65b1d0a2dbed35..1a4dbe7ce4a15a2d 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -95,7 +95,7 @@ int Capture::capture(EventLoop *loop) Stream *stream = cfg.stream(); std::unique_ptr buffer = stream->createBuffer(i); - ret = request->addBuffer(std::move(buffer)); + ret = request->addBuffer(stream, std::move(buffer)); if (ret < 0) { std::cerr << "Can't set buffer for request" << std::endl; @@ -185,7 +185,7 @@ void Capture::requestComplete(Request *request) return; } - request->addBuffer(std::move(newBuffer)); + request->addBuffer(stream, std::move(newBuffer)); } camera_->queueRequest(request); diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index c2854dc2e8caab2e..3b49e52510918eee 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -113,6 +113,7 @@ Request::~Request() /** * \brief Store a Buffer with its associated Stream in the Request + * \param[in] stream The stream the buffer belongs to * \param[in] buffer The Buffer to store in the request * * Ownership of the buffer is passed to the request. It will be deleted when @@ -125,9 +126,8 @@ Request::~Request() * \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) +int Request::addBuffer(Stream *stream, std::unique_ptr buffer) { - Stream *stream = buffer->stream(); if (!stream) { LOG(Request, Error) << "Invalid stream reference"; return -EINVAL; diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 11fb67a509e973f5..5dec9252898100db 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -191,7 +191,7 @@ int MainWindow::startCapture() goto error; } - ret = request->addBuffer(std::move(buffer)); + ret = request->addBuffer(stream, std::move(buffer)); if (ret < 0) { std::cerr << "Can't set buffer for request" << std::endl; goto error; @@ -289,7 +289,7 @@ void MainWindow::requestComplete(Request *request) return; } - request->addBuffer(std::move(newBuffer)); + request->addBuffer(stream, std::move(newBuffer)); } camera_->queueRequest(request); diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index 171540edd96f9fca..e5c010d81b8d6e0e 100644 --- a/test/camera/buffer_import.cpp +++ b/test/camera/buffer_import.cpp @@ -268,7 +268,7 @@ public: Request *request = camera_->createRequest(cookie); std::unique_ptr buffer = stream_->createBuffer({ dmabuf, -1, -1 }); - request->addBuffer(move(buffer)); + request->addBuffer(stream_, move(buffer)); camera_->queueRequest(request); } diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index 7cb76038f557d461..ca1ebe419946dd4d 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -49,7 +49,7 @@ protected: std::unique_ptr newBuffer = stream->createBuffer(buffer->index()); request = camera_->createRequest(); - request->addBuffer(std::move(newBuffer)); + request->addBuffer(stream, std::move(newBuffer)); camera_->queueRequest(request); } @@ -101,7 +101,7 @@ protected: return TestFail; } - if (request->addBuffer(std::move(buffer))) { + if (request->addBuffer(stream, std::move(buffer))) { cout << "Failed to associating buffer with request" << endl; return TestFail; } diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index afa13ba77b0b7fe0..f627b8f37422350e 100644 --- a/test/camera/statemachine.cpp +++ b/test/camera/statemachine.cpp @@ -219,7 +219,7 @@ protected: Stream *stream = *camera_->streams().begin(); std::unique_ptr buffer = stream->createBuffer(0); - if (request->addBuffer(std::move(buffer))) + if (request->addBuffer(stream, std::move(buffer))) return TestFail; if (camera_->queueRequest(request)) From patchwork Mon Dec 30 12:04:55 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2460 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E9E906046C for ; Mon, 30 Dec 2019 13:05:57 +0100 (CET) X-Halon-ID: bc182cdf-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id bc182cdf-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:05:56 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:04:55 +0100 Message-Id: <20191230120510.938333-11-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 10/25] libcamera: buffer: Move captured metadata to FrameMetadata X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:05:58 -0000 Move the metadata retrieved when dequeuing a V4L2 buffer into a FrameMetadata object. This is done as a step to migrate to the FrameBuffer interface as the functions added to Buffer around FrameMetadata match the ones in FrameBuffer. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- * Changes since v1 - Rework to match FrameMetadata being a struct instead of a class - Align statements broken over multiple lines - Spiffy up bytesused printing in cam - Merged with patch who removes the old fields in Buffer. --- include/libcamera/buffer.h | 16 +---- src/android/camera_device.cpp | 4 +- src/cam/buffer_writer.cpp | 2 +- src/cam/capture.cpp | 13 +++- src/libcamera/buffer.cpp | 75 ++++++------------------ src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 +-- src/libcamera/request.cpp | 2 +- src/libcamera/v4l2_videodevice.cpp | 24 ++++---- src/qcam/main_window.cpp | 13 ++-- test/camera/buffer_import.cpp | 2 +- test/camera/capture.cpp | 2 +- test/v4l2_videodevice/buffer_sharing.cpp | 8 ++- 13 files changed, 70 insertions(+), 103 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 8dd9f91272291648..8bd61f786748af5f 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -93,12 +93,6 @@ private: class Buffer final { public: - enum Status { - BufferSuccess, - BufferError, - BufferCancelled, - }; - Buffer(unsigned int index = -1, const Buffer *metadata = nullptr); Buffer(const Buffer &) = delete; Buffer &operator=(const Buffer &) = delete; @@ -107,11 +101,8 @@ public: const std::array &dmabufs() const { return dmabuf_; } BufferMemory *mem() { return mem_; } - unsigned int bytesused() const { return bytesused_; } - uint64_t timestamp() const { return timestamp_; } - unsigned int sequence() const { return sequence_; } + const FrameMetadata &metadata() const { return metadata_; }; - Status status() const { return status_; } Request *request() const { return request_; } Stream *stream() const { return stream_; } @@ -127,11 +118,8 @@ private: std::array dmabuf_; BufferMemory *mem_; - unsigned int bytesused_; - uint64_t timestamp_; - unsigned int sequence_; + FrameMetadata metadata_; - Status status_; Request *request_; Stream *stream_; }; diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 09588c16a6301649..ebe91ea8af4f6436 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -803,11 +803,11 @@ void CameraDevice::requestComplete(Request *request) if (status == CAMERA3_BUFFER_STATUS_OK) { notifyShutter(descriptor->frameNumber, - libcameraBuffer->timestamp()); + libcameraBuffer->metadata().timestamp); captureResult.partial_result = 1; resultMetadata = getResultMetadata(descriptor->frameNumber, - libcameraBuffer->timestamp()); + libcameraBuffer->metadata().timestamp); captureResult.result = resultMetadata->get(); } diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp index 3e84068e66bb4dd7..7c58a1f50829f290 100644 --- a/src/cam/buffer_writer.cpp +++ b/src/cam/buffer_writer.cpp @@ -33,7 +33,7 @@ int BufferWriter::write(Buffer *buffer, const std::string &streamName) if (pos != std::string::npos) { std::stringstream ss; ss << streamName << "-" << std::setw(6) - << std::setfill('0') << buffer->sequence(); + << std::setfill('0') << buffer->metadata().sequence; filename.replace(pos, 1, ss.str()); } diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index 1a4dbe7ce4a15a2d..da942f56118983bd 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -154,9 +154,18 @@ void Capture::requestComplete(Request *request) Buffer *buffer = it->second; const std::string &name = streamName_[stream]; + const FrameMetadata &metadata = buffer->metadata(); + info << " " << name - << " seq: " << std::setw(6) << std::setfill('0') << buffer->sequence() - << " bytesused: " << buffer->bytesused(); + << " seq: " << std::setw(6) << std::setfill('0') << metadata.sequence + << " bytesused: "; + + unsigned int nplane = 0; + for (const FrameMetadata::Plane &plane : metadata.planes) { + info << plane.bytesused; + if (++nplane < metadata.planes.size()) + info << "/"; + } if (writer_) writer_->write(buffer, name); diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 4a77f20751408b7c..686d0412bf54b2c1 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -166,20 +166,6 @@ void BufferPool::destroyBuffers() * deleted automatically after the request complete handler returns. */ -/** - * \enum Buffer::Status - * Buffer completion status - * \var Buffer::BufferSuccess - * The buffer has completed with success and contains valid data. All its other - * metadata (such as bytesused(), timestamp() or sequence() number) are valid. - * \var Buffer::BufferError - * The buffer has completed with an error and doesn't contain valid data. Its - * other metadata are valid. - * \var Buffer::BufferCancelled - * The buffer has been cancelled due to capture stop. Its other metadata are - * invalid and shall not be used. - */ - /** * \brief Construct a buffer not associated with any stream * @@ -188,18 +174,19 @@ void BufferPool::destroyBuffers() * for a stream with Stream::createBuffer(). */ Buffer::Buffer(unsigned int index, const Buffer *metadata) - : index_(index), dmabuf_({ -1, -1, -1 }), - status_(Buffer::BufferSuccess), request_(nullptr), + : index_(index), dmabuf_({ -1, -1, -1 }), request_(nullptr), stream_(nullptr) { + metadata_.status = FrameMetadata::FrameSuccess; + if (metadata) { - bytesused_ = metadata->bytesused_; - sequence_ = metadata->sequence_; - timestamp_ = metadata->timestamp_; + metadata_.sequence = metadata->metadata().sequence; + metadata_.timestamp = metadata->metadata().timestamp; + metadata_.planes = metadata->metadata().planes; } else { - bytesused_ = 0; - sequence_ = 0; - timestamp_ = 0; + metadata_.sequence = 0; + metadata_.timestamp = 0; + metadata_.planes = { { 0 } }; } } @@ -231,39 +218,13 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata) */ /** - * \fn Buffer::bytesused() - * \brief Retrieve the number of bytes occupied by the data in the buffer - * \return Number of bytes occupied in the buffer - */ - -/** - * \fn Buffer::timestamp() - * \brief Retrieve the time when the buffer was processed - * - * The timestamp is expressed as a number of nanoseconds since the epoch. - * - * \return Timestamp when the buffer was processed - */ - -/** - * \fn Buffer::sequence() - * \brief Retrieve the buffer sequence number - * - * The sequence number is a monotonically increasing number assigned to the - * buffer processed by the stream. Gaps in the sequence numbers indicate - * dropped frames. - * - * \return Sequence number of the buffer - */ - -/** - * \fn Buffer::status() - * \brief Retrieve the buffer status + * \fn Buffer::metadata() + * \brief Retrieve the buffer metadata * - * The buffer status reports whether the buffer has completed successfully - * (BufferSuccess) or if an error occurred (BufferError). + * The buffer metadata is update every time the buffer contained are changed, + * for example when it is dequeued from hardware. * - * \return The buffer status + * \return Metadata for the buffer */ /** @@ -299,10 +260,10 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata) */ void Buffer::cancel() { - bytesused_ = 0; - timestamp_ = 0; - sequence_ = 0; - status_ = BufferCancelled; + metadata_.status = FrameMetadata::FrameCancelled; + metadata_.sequence = 0; + metadata_.timestamp = 0; + metadata_.planes = {}; } /** diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 6d8c3fada127310e..34fc792977d151be 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -928,7 +928,7 @@ int PipelineHandlerIPU3::registerCameras() void IPU3CameraData::imguInputBufferReady(Buffer *buffer) { /* \todo Handle buffer failures when state is set to BufferError. */ - if (buffer->status() == Buffer::BufferCancelled) + if (buffer->metadata().status == FrameMetadata::FrameCancelled) return; cio2_.output_->queueBuffer(buffer); @@ -962,7 +962,7 @@ void IPU3CameraData::imguOutputBufferReady(Buffer *buffer) void IPU3CameraData::cio2BufferReady(Buffer *buffer) { /* \todo Handle buffer failures when state is set to BufferError. */ - if (buffer->status() == Buffer::BufferCancelled) + if (buffer->metadata().status == FrameMetadata::FrameCancelled) return; imgu_->input_->queueBuffer(buffer); diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index bb652d0da9c6df52..46df871a51105ee4 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -100,10 +100,10 @@ public: ASSERT(frameOffset(SOE) == 0); utils::time_point soe = std::chrono::time_point() - + std::chrono::nanoseconds(buffer->timestamp()) + + std::chrono::nanoseconds(buffer->metadata().timestamp) + timeOffset(SOE); - notifyStartOfExposure(buffer->sequence(), soe); + notifyStartOfExposure(buffer->metadata().sequence, soe); } void setDelay(unsigned int type, int frame, int msdelay) @@ -1002,8 +1002,8 @@ void PipelineHandlerRkISP1::bufferReady(Buffer *buffer) data->timeline_.bufferReady(buffer); - if (data->frame_ <= buffer->sequence()) - data->frame_ = buffer->sequence() + 1; + if (data->frame_ <= buffer->metadata().sequence) + data->frame_ = buffer->metadata().sequence + 1; completeBuffer(activeCamera_, request, buffer); tryCompleteRequest(request); diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index 3b49e52510918eee..b17a6c2278361f00 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -260,7 +260,7 @@ bool Request::completeBuffer(Buffer *buffer) buffer->request_ = nullptr; - if (buffer->status() == Buffer::BufferCancelled) + if (buffer->metadata().status == FrameMetadata::FrameCancelled) cancelled_ = true; return !hasPendingBuffers(); diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 7d585ce03ed9a45a..51112d197068cf0a 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1015,10 +1015,12 @@ int V4L2VideoDevice::queueBuffer(Buffer *buffer) } if (V4L2_TYPE_IS_OUTPUT(buf.type)) { - buf.bytesused = buffer->bytesused_; - buf.sequence = buffer->sequence_; - buf.timestamp.tv_sec = buffer->timestamp_ / 1000000000; - buf.timestamp.tv_usec = (buffer->timestamp_ / 1000) % 1000000; + const FrameMetadata &metadata = buffer->metadata(); + + buf.bytesused = metadata.planes[0].bytesused; + buf.sequence = metadata.sequence; + buf.timestamp.tv_sec = metadata.timestamp / 1000000000; + buf.timestamp.tv_usec = (metadata.timestamp / 1000) % 1000000; } LOG(V4L2, Debug) << "Queueing buffer " << buf.index; @@ -1125,12 +1127,14 @@ Buffer *V4L2VideoDevice::dequeueBuffer() fdEvent_->setEnabled(false); buffer->index_ = buf.index; - buffer->bytesused_ = buf.bytesused; - buffer->timestamp_ = buf.timestamp.tv_sec * 1000000000ULL - + buf.timestamp.tv_usec * 1000ULL; - buffer->sequence_ = buf.sequence; - buffer->status_ = buf.flags & V4L2_BUF_FLAG_ERROR - ? Buffer::BufferError : Buffer::BufferSuccess; + + buffer->metadata_.status = buf.flags & V4L2_BUF_FLAG_ERROR + ? FrameMetadata::FrameError + : FrameMetadata::FrameSuccess; + buffer->metadata_.sequence = buf.sequence; + buffer->metadata_.timestamp = buf.timestamp.tv_sec * 1000000000ULL + + buf.timestamp.tv_usec * 1000ULL; + buffer->metadata_.planes = { { buf.bytesused } }; return buffer; } diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 5dec9252898100db..47793702b9aa0ee9 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -259,14 +259,15 @@ void MainWindow::requestComplete(Request *request) framesCaptured_++; Buffer *buffer = buffers.begin()->second; + const FrameMetadata &metadata = buffer->metadata(); - double fps = buffer->timestamp() - lastBufferTime_; + double fps = metadata.timestamp - lastBufferTime_; fps = lastBufferTime_ && fps ? 1000000000.0 / fps : 0.0; - lastBufferTime_ = buffer->timestamp(); + lastBufferTime_ = metadata.timestamp; - std::cout << "seq: " << std::setw(6) << std::setfill('0') << buffer->sequence() - << " bytesused: " << buffer->bytesused() - << " timestamp: " << buffer->timestamp() + std::cout << "seq: " << std::setw(6) << std::setfill('0') << metadata.sequence + << " bytesused: " << metadata.planes[0].bytesused + << " timestamp: " << metadata.timestamp << " fps: " << std::fixed << std::setprecision(2) << fps << std::endl; @@ -306,7 +307,7 @@ int MainWindow::display(Buffer *buffer) plane.fd.fd(), 0); unsigned char *raw = static_cast(memory); - viewfinder_->display(raw, buffer->bytesused()); + viewfinder_->display(raw, buffer->metadata().planes[0].bytesused); munmap(memory, plane.length); diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index e5c010d81b8d6e0e..3ba6ce9690f29329 100644 --- a/test/camera/buffer_import.cpp +++ b/test/camera/buffer_import.cpp @@ -275,7 +275,7 @@ public: protected: void bufferComplete(Request *request, Buffer *buffer) { - if (buffer->status() != Buffer::BufferSuccess) + if (buffer->metadata().status != FrameMetadata::FrameSuccess) return; unsigned int index = buffer->index(); diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index ca1ebe419946dd4d..0d9ffc476650f414 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -28,7 +28,7 @@ protected: void bufferComplete(Request *request, Buffer *buffer) { - if (buffer->status() != Buffer::BufferSuccess) + if (buffer->metadata().status != FrameMetadata::FrameSuccess) return; completeBuffersCount_++; diff --git a/test/v4l2_videodevice/buffer_sharing.cpp b/test/v4l2_videodevice/buffer_sharing.cpp index 3a56862cb2b77d38..fe48b2e98fdada8d 100644 --- a/test/v4l2_videodevice/buffer_sharing.cpp +++ b/test/v4l2_videodevice/buffer_sharing.cpp @@ -92,9 +92,11 @@ protected: void captureBufferReady(Buffer *buffer) { + const FrameMetadata &metadata = buffer->metadata(); + std::cout << "Received capture buffer" << std::endl; - if (buffer->status() != Buffer::BufferSuccess) + if (metadata.status != FrameMetadata::FrameSuccess) return; output_->queueBuffer(buffer); @@ -103,9 +105,11 @@ protected: void outputBufferReady(Buffer *buffer) { + const FrameMetadata &metadata = buffer->metadata(); + std::cout << "Received output buffer" << std::endl; - if (buffer->status() != Buffer::BufferSuccess) + if (metadata.status != FrameMetadata::FrameSuccess) return; capture_->queueBuffer(buffer); From patchwork Mon Dec 30 12:04:56 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2461 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A29716046B for ; Mon, 30 Dec 2019 13:05:58 +0100 (CET) X-Halon-ID: bcc20cc2-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id bcc20cc2-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:05:57 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:04:56 +0100 Message-Id: <20191230120510.938333-12-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 11/25] libcamera: v4l2_videodevice: Add V4L2BufferCache to deal with index mapping X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:05:58 -0000 In preparation for the FrameBuffer interface add a class that will deal with keeping the cache between dmafds and V4L2 video device buffer indexes. This initial implement ensures that no hot association is lost while its eviction strategy could be improved in the future. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- * Changes since v1 - fetch() take a const reference instead of a const pointer - Rename argument for V4L2BufferCache() - Rename V4L2BufferCache::CacheInfo to V4L2BufferCache::Entry. - Turn V4L2BufferCache::Entry into a class with constructors for foo.emplace_back(). - Rename V4L2BufferCache::fetch() to V4L2BufferCache::get() - Make use of operator== - Large updates of documentation. --- src/libcamera/include/v4l2_videodevice.h | 58 +++++++++++++- src/libcamera/v4l2_videodevice.cpp | 98 +++++++++++++++++++++++- 2 files changed, 152 insertions(+), 4 deletions(-) diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index 1f52fe0120831fa3..9fc2082b2e7f7219 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -11,7 +11,9 @@ #include #include +#include +#include #include #include #include @@ -22,9 +24,6 @@ namespace libcamera { -class Buffer; -class BufferMemory; -class BufferPool; class EventNotifier; class MediaDevice; class MediaEntity; @@ -105,6 +104,59 @@ struct V4L2Capability final : v4l2_capability { } }; +class V4L2BufferCache +{ +public: + V4L2BufferCache(unsigned int numEntries); + V4L2BufferCache(const std::vector> &buffers); + + int get(const FrameBuffer &buffer); + void put(unsigned int index); + +private: + class Entry + { + public: + Entry(bool free, const std::vector &planes) + : free(free) + { + for (const FrameBuffer::Plane &plane : planes) + last.emplace_back(plane); + } + + bool operator==(const std::vector &planes) + { + if (last.size() != planes.size()) + return false; + + for (unsigned int i = 0; i < planes.size(); i++) + if (last[i].fd != planes[i].fd.fd() || + last[i].length != planes[i].length) + return false; + return true; + } + + bool free; + + private: + class Plane + { + public: + Plane(const FrameBuffer::Plane &plane) + : fd(plane.fd.fd()), length(plane.length) + { + } + + int fd; + unsigned int length; + }; + + std::vector last; + }; + + std::vector cache_; +}; + class V4L2DeviceFormat { public: diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 51112d197068cf0a..c9e17c44bfe17c3b 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -19,7 +19,6 @@ #include -#include #include #include "log.h" @@ -134,6 +133,103 @@ LOG_DECLARE_CATEGORY(V4L2) * \return True if the video device provides Streaming I/O IOCTLs */ +/** + * \class V4L2BufferCache + * \brief Hot cache of associations between V4L2 buffer indexes and FrameBuffer + * + * When importing buffers, V4L2 performs lazy mapping of dmabuf instances at + * VIDIOC_QBUF (or VIDIOC_PREPARE_BUF) time and keeps the mapping associated + * with the V4L2 buffer, as identified by its index. If the same V4L2 buffer is + * then reused and queued with different dmabufs, the old dmabufs will be + * unmapped and the new ones mapped. To keep this process efficient, it is + * crucial to consistently use the same V4L2 buffer for given dmabufs through + * the whole duration of a capture cycle. + * + * The V4L2BufferCache class keeps a map of previous dmabufs to V4L2 buffer + * index associations to help selecting V4L2 buffers. It tracks, for every + * entry, if the V4L2 buffer is in use, and offers lookup of the best free V4L2 + * buffer for a set of dmabufs. + */ + +/** + * \brief Create an empty cache with \a numEntries entries + * \param[in] numEntries Number of entries to reserve in the cache + * + * Create a cache with \a numEntries entries all marked as unused. The entries + * will be populated as the cache is used. This is typically used to implement + * buffer import, with buffers added to the cache as they are queued. + */ +V4L2BufferCache::V4L2BufferCache(unsigned int numEntries) +{ + cache_.resize(numEntries, Entry(true, {})); +} + +/** + * \brief Create a pre-populated cache + * \param[in] buffers Array of buffers to pre-populated with + * + * Create a cache pre-populated with \a buffers. This is typically used to + * implement buffer export, with all buffers added to the cache when they are + * allocated. + */ +V4L2BufferCache::V4L2BufferCache(const std::vector> &buffers) +{ + for (const std::unique_ptr &buffer : buffers) + cache_.emplace_back(true, buffer->planes()); +} + +/** + * \brief Find the best V4L2 buffer for a FrameBuffer + * \param[in] buffer The FrameBuffer + * + * Find the best V4L2 buffer index to be used for the FrameBuffer \a buffer + * based on previous mappings of frame buffers to V4L2 buffers. If a free V4L2 + * buffer previously used with the same dmabufs as \a buffer is found in the + * cache, return its index. Otherwise return the index of the first free V4L2 + * buffer and record its association with the dmabufs of \a buffer. + * + * \return The index of the best V4L2 buffer, or -ENOENT if no free V4L2 buffer + * is available + */ +int V4L2BufferCache::get(const FrameBuffer &buffer) +{ + const std::vector &planes = buffer.planes(); + int use = -1; + + for (unsigned int index = 0; index < cache_.size(); index++) { + const Entry &entry = cache_[index]; + + if (!entry.free) + continue; + + if (use < 0) + use = index; + + /* Try to find a cache hit by comparing the planes. */ + if (cache_[index] == planes) { + use = index; + break; + } + } + + if (use < 0) + return -ENOENT; + + cache_[use] = Entry(false, planes); + + return use; +} + +/** + * \brief Mark buffer \a index as free in the cache + * \param[in] index The V4L2 buffer index + */ +void V4L2BufferCache::put(unsigned int index) +{ + ASSERT(index < cache_.size()); + cache_[index].free = true; +} + /** * \class V4L2DeviceFormat * \brief The V4L2 video device image format and sizes From patchwork Mon Dec 30 12:04:57 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2462 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D4110605F3 for ; Mon, 30 Dec 2019 13:05:59 +0100 (CET) X-Halon-ID: bd39d2d2-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id bd39d2d2-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:05:58 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:04:57 +0100 Message-Id: <20191230120510.938333-13-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 12/25] libcamera: v4l2_videodevice: Add FrameBuffer interface X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:06:00 -0000 Add a new interface in parallel with the existing Buffer implementation to also support FrameBuffer. The reason it's added in parallel is to aid in the migration from Buffer to FrameBuffer thru out libcamera. With this change discrete parts of libcamera can be migrated and tested independently. As the new interface is added in parallel there are some oddities in this change which will be undone in a follow up patch once libcamera have migrated away from the Buffer interface. - There is a nasty hack in V4L2VideoDevice::bufferAvailable(). It is needed to allow both interfaces to exist and function at the same time. The idea is if buffers are allocated using the FrameBuffer interface V4L2VideoDevice::cache_ is set and we know to call the FrameBuffer 'buffer ready' signal, and likewise if it's not to call the Buffer variant. - There is some code duplication between the two interfaces as they aim to solve the same thing in slightly different ways. As all Buffer related code is soon to be removed no effort to create code sharing between them have been made. - Some function and variables which can't be distinguished by their argument types have been given a frameBuffer prefix instead of a buffer prefix. They are clearly documented in the code and will be renamed to the correct buffer prefix when the Buffer interface is removed. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- * Changes since v1 - Rename allocateBuffers() to exportBuffers() - Rename externalBuffers() to importBuffers() - Rename createBuffer() to createFrameBuffer() - Reworked createFrameBuffer() to take advantage of the FileDescriptor() - Fix up a lot of documentation and small code beautification fixes. - Merged all FrameBuffer interface additions into this patch, some where done in the really huge 27/30 patch. --- src/libcamera/include/v4l2_videodevice.h | 12 + src/libcamera/v4l2_videodevice.cpp | 312 ++++++++++++++++++++++- 2 files changed, 313 insertions(+), 11 deletions(-) diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index 9fc2082b2e7f7219..97a79e56c647e7a6 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -196,11 +196,17 @@ public: int exportBuffers(BufferPool *pool); int importBuffers(BufferPool *pool); + int exportBuffers(unsigned int count, + std::vector> *buffers); + int importBuffers(unsigned int count); int releaseBuffers(); int queueBuffer(Buffer *buffer); std::vector> queueAllBuffers(); Signal bufferReady; + int queueBuffer(FrameBuffer *buffer); + /* todo: Rename to bufferReady when the Buffer version is removed */ + Signal frameBufferReady; int streamOn(); int streamOff(); @@ -231,10 +237,13 @@ private: int requestBuffers(unsigned int count); int createPlane(BufferMemory *buffer, unsigned int index, unsigned int plane, unsigned int length); + std::unique_ptr createFrameBuffer(const struct v4l2_buffer &buf); int exportDmabufFd(unsigned int index, unsigned int plane); Buffer *dequeueBuffer(); void bufferAvailable(EventNotifier *notifier); + /* todo: Rename to dequeueBuffer() when the Buffer version is removed */ + FrameBuffer *dequeueFrameBuffer(); V4L2Capability caps_; @@ -242,7 +251,10 @@ private: enum v4l2_memory memoryType_; BufferPool *bufferPool_; + V4L2BufferCache *cache_; std::map queuedBuffers_; + /* todo: Rename to queuedBuffers_ when the Buffer version is removed */ + std::map queuedFrameBuffers_; EventNotifier *fdEvent_; }; diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index c9e17c44bfe17c3b..b380f08c1e880427 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -370,7 +370,8 @@ const std::string V4L2DeviceFormat::toString() const * \param[in] deviceNode The file-system path to the video device node */ V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode) - : V4L2Device(deviceNode), bufferPool_(nullptr), fdEvent_(nullptr) + : V4L2Device(deviceNode), bufferPool_(nullptr), cache_(nullptr), + fdEvent_(nullptr) { /* * We default to an MMAP based CAPTURE video device, however this will @@ -1038,6 +1039,95 @@ int V4L2VideoDevice::importBuffers(BufferPool *pool) return 0; } +/** + * \brief Allocate buffers from the video device + * \param[in] count Number of buffers to allocate + * \param[out] buffers Vector to store allocated buffers + * \return 0 on success or a negative error code otherwise + */ +int V4L2VideoDevice::exportBuffers(unsigned int count, + std::vector> *buffers) +{ + if (cache_) { + LOG(V4L2, Error) << "FrameBuffers already allocated"; + return -EINVAL; + } + + memoryType_ = V4L2_MEMORY_MMAP; + + int ret = requestBuffers(count); + if (ret < 0) + return ret; + + for (unsigned i = 0; i < count; ++i) { + struct v4l2_buffer buf = {}; + struct v4l2_plane planes[VIDEO_MAX_PLANES] = {}; + + buf.index = i; + buf.type = bufferType_; + buf.memory = memoryType_; + buf.length = ARRAY_SIZE(planes); + buf.m.planes = planes; + + ret = ioctl(VIDIOC_QUERYBUF, &buf); + if (ret < 0) { + LOG(V4L2, Error) + << "Unable to query buffer " << i << ": " + << strerror(-ret); + goto err_buf; + } + + std::unique_ptr buffer = createFrameBuffer(buf); + if (!buffer) { + LOG(V4L2, Error) << "Unable to create buffer"; + ret = -EINVAL; + goto err_buf; + } + + buffers->push_back(std::move(buffer)); + } + + cache_ = new V4L2BufferCache(*buffers); + + return count; + +err_buf: + requestBuffers(0); + + buffers->clear(); + + return ret; +} + +std::unique_ptr V4L2VideoDevice::createFrameBuffer(const struct v4l2_buffer &buf) +{ + const bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); + const unsigned int numPlanes = multiPlanar ? buf.length : 1; + + if (numPlanes == 0 || numPlanes > VIDEO_MAX_PLANES) { + LOG(V4L2, Error) << "Invalid number of planes"; + return nullptr; + } + + std::vector planes; + for (unsigned int nplane = 0; nplane < numPlanes; nplane++) { + int fd = exportDmabufFd(buf.index, nplane); + if (fd < 0) + return nullptr; + + FrameBuffer::Plane plane; + plane.fd = FileDescriptor(fd); + plane.length = multiPlanar ? + buf.m.planes[nplane].length : buf.length; + + planes.push_back(plane); + + ::close(fd); + } + + return utils::make_unique(planes); +} + int V4L2VideoDevice::exportDmabufFd(unsigned int index, unsigned int plane) { struct v4l2_exportbuffer expbuf = {}; @@ -1058,14 +1148,41 @@ int V4L2VideoDevice::exportDmabufFd(unsigned int index, unsigned int plane) return expbuf.fd; } +/** + * \brief Prepare the device to import \a count buffers + * \param[in] count Number of buffers to prepare to import + * \return 0 on success or a negative error code otherwise + */ +int V4L2VideoDevice::importBuffers(unsigned int count) +{ + if (cache_) { + LOG(V4L2, Error) << "FrameBuffers already allocated"; + return -EINVAL; + } + + memoryType_ = V4L2_MEMORY_DMABUF; + + int ret = requestBuffers(count); + if (ret) + return ret; + + cache_ = new V4L2BufferCache(count); + + LOG(V4L2, Debug) << "Prepared to import " << count << " buffers"; + + return 0; +} + /** * \brief Release all internally allocated buffers */ int V4L2VideoDevice::releaseBuffers() { - LOG(V4L2, Debug) << "Releasing bufferPool"; + LOG(V4L2, Debug) << "Releasing buffers"; bufferPool_ = nullptr; + delete cache_; + cache_ = nullptr; return requestBuffers(0); } @@ -1079,6 +1196,8 @@ int V4L2VideoDevice::releaseBuffers() * will be processed by the device. Once the device has finished processing the * buffer, it will be available for dequeue. * + * The best available V4L2 buffer is picked for \a buffer using the V4L2 cache. + * * \return 0 on success or a negative error code otherwise */ int V4L2VideoDevice::queueBuffer(Buffer *buffer) @@ -1183,6 +1302,84 @@ std::vector> V4L2VideoDevice::queueAllBuffers() return buffers; } +/** + * \brief Queue a buffer into the video device + * \param[in] buffer The buffer to be queued + * + * For capture video devices the \a buffer will be filled with data by the + * device. For output video devices the \a buffer shall contain valid data and + * will be processed by the device. Once the device has finished processing the + * buffer, it will be available for dequeue. + * + * \return 0 on success or a negative error code otherwise + */ +int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) +{ + struct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {}; + struct v4l2_buffer buf = {}; + int ret; + + ret = cache_->get(*buffer); + if (ret < 0) + return ret; + + buf.index = ret; + buf.type = bufferType_; + buf.memory = memoryType_; + buf.field = V4L2_FIELD_NONE; + + bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); + const std::vector &planes = buffer->planes(); + + if (buf.memory == V4L2_MEMORY_DMABUF) { + if (multiPlanar) { + for (unsigned int p = 0; p < planes.size(); ++p) + v4l2Planes[p].m.fd = planes[p].fd.fd(); + } else { + buf.m.fd = planes[0].fd.fd(); + } + } + + if (multiPlanar) { + buf.length = planes.size(); + buf.m.planes = v4l2Planes; + } + + if (V4L2_TYPE_IS_OUTPUT(buf.type)) { + const FrameMetadata &metadata = buffer->metadata(); + + if (multiPlanar) { + unsigned int nplane = 0; + for (const FrameMetadata::Plane &plane : metadata.planes) + v4l2Planes[nplane++].bytesused = plane.bytesused; + } else { + if (metadata.planes.size()) + buf.bytesused = metadata.planes[0].bytesused; + } + + buf.sequence = metadata.sequence; + buf.timestamp.tv_sec = metadata.timestamp / 1000000000; + buf.timestamp.tv_usec = (metadata.timestamp / 1000) % 1000000; + } + + LOG(V4L2, Debug) << "Queueing buffer " << buf.index; + + ret = ioctl(VIDIOC_QBUF, &buf); + if (ret < 0) { + LOG(V4L2, Error) + << "Failed to queue buffer " << buf.index << ": " + << strerror(-ret); + return ret; + } + + if (queuedFrameBuffers_.empty()) + fdEvent_->setEnabled(true); + + queuedFrameBuffers_[buf.index] = buffer; + + return 0; +} + /** * \brief Dequeue the next available buffer from the video device * @@ -1242,17 +1439,97 @@ Buffer *V4L2VideoDevice::dequeueBuffer() * When this slot is called, a Buffer has become available from the device, and * will be emitted through the bufferReady Signal. * - * For Capture video devices the Buffer will contain valid data. - * For Output video devices the Buffer can be considered empty. + * For Capture video devices the FrameBuffer will contain valid data. + * For Output video devices the FrameBuffer can be considered empty. */ void V4L2VideoDevice::bufferAvailable(EventNotifier *notifier) { - Buffer *buffer = dequeueBuffer(); - if (!buffer) - return; + /** + * This is a hack which allows both Buffer and FrameBuffer interfaces + * to work with the same code base. This allows different parts of + * libcamera to migrate to FrameBuffer in different patches. + * + * If we have a cache_ we know FrameBuffer is in use. + * + * \todo: Remove this hack when the Buffer interface is removed. + */ + if (cache_) { + FrameBuffer *buffer = dequeueFrameBuffer(); + if (!buffer) + return; + + /* Notify anyone listening to the device. */ + frameBufferReady.emit(buffer); + } else { + Buffer *buffer = dequeueBuffer(); + if (!buffer) + return; + + /* Notify anyone listening to the device. */ + bufferReady.emit(buffer); + } +} + +/** + * \brief Dequeue the next available buffer from the video device + * + * This method dequeues the next available buffer from the device. If no buffer + * is available to be dequeued it will return nullptr immediately. + * + * \todo: Reanme to dequeueBuffer() once the FrameBuffer transition is complete + * + * \return A pointer to the dequeued buffer on success, or nullptr otherwise + */ +FrameBuffer *V4L2VideoDevice::dequeueFrameBuffer() +{ + struct v4l2_buffer buf = {}; + struct v4l2_plane planes[VIDEO_MAX_PLANES] = {}; + int ret; + + buf.type = bufferType_; + buf.memory = memoryType_; - /* Notify anyone listening to the device. */ - bufferReady.emit(buffer); + bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); + + if (multiPlanar) { + buf.length = VIDEO_MAX_PLANES; + buf.m.planes = planes; + } + + ret = ioctl(VIDIOC_DQBUF, &buf); + if (ret < 0) { + LOG(V4L2, Error) + << "Failed to dequeue buffer: " << strerror(-ret); + return nullptr; + } + + LOG(V4L2, Debug) << "Dequeuing buffer " << buf.index; + + cache_->put(buf.index); + + auto it = queuedFrameBuffers_.find(buf.index); + FrameBuffer *buffer = it->second; + queuedFrameBuffers_.erase(it); + + if (queuedFrameBuffers_.empty()) + fdEvent_->setEnabled(false); + + buffer->metadata_.status = buf.flags & V4L2_BUF_FLAG_ERROR + ? FrameMetadata::FrameError + : FrameMetadata::FrameSuccess; + buffer->metadata_.sequence = buf.sequence; + buffer->metadata_.timestamp = buf.timestamp.tv_sec * 1000000000ULL + + buf.timestamp.tv_usec * 1000ULL; + + buffer->metadata_.planes.clear(); + if (multiPlanar && buf.length > 1) { + for (unsigned int nplane = 0; nplane < buf.length; nplane++) + buffer->metadata_.planes.push_back({ planes[nplane].bytesused }); + } else { + buffer->metadata_.planes.push_back({ buf.bytesused }); + } + + return buffer; } /** @@ -1260,6 +1537,11 @@ void V4L2VideoDevice::bufferAvailable(EventNotifier *notifier) * \brief A Signal emitted when a buffer completes */ +/** + * \var V4L2VideoDevice::frameBufferReady + * \brief A Signal emitted when a framebuffer completes + */ + /** * \brief Start the video stream * \return 0 on success or a negative error code otherwise @@ -1281,8 +1563,8 @@ int V4L2VideoDevice::streamOn() /** * \brief Stop the video stream * - * Buffers that are still queued when the video stream is stopped are - * immediately dequeued with their status set to Buffer::BufferError, + * FrameBuffers that are still queued when the video stream is stopped are + * immediately dequeued with their status set to BufferMetadata::BufferCancelled, * and the bufferReady signal is emitted for them. The order in which those * buffers are dequeued is not specified. * @@ -1309,7 +1591,15 @@ int V4L2VideoDevice::streamOff() bufferReady.emit(buffer); } + for (auto it : queuedFrameBuffers_) { + FrameBuffer *buffer = it.second; + + buffer->metadata_.status = FrameMetadata::FrameCancelled; + frameBufferReady.emit(buffer); + } + queuedBuffers_.clear(); + queuedFrameBuffers_.clear(); fdEvent_->setEnabled(false); return 0; From patchwork Mon Dec 30 12:04:58 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2463 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A8E326046C for ; Mon, 30 Dec 2019 13:06:00 +0100 (CET) X-Halon-ID: bdea377f-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id bdea377f-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:05:59 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:04:58 +0100 Message-Id: <20191230120510.938333-14-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 13/25] test: v4l2_videodevice: Switch to FrameBuffer interface X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:06:00 -0000 The V4L2VideoDevice class can now operate using a FrameBuffer interface, switch all test cases to use it. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- test/v4l2_videodevice/buffer_sharing.cpp | 30 ++++++------- test/v4l2_videodevice/capture_async.cpp | 20 ++++----- test/v4l2_videodevice/request_buffers.cpp | 11 +---- test/v4l2_videodevice/stream_on_off.cpp | 6 +-- test/v4l2_videodevice/v4l2_m2mdevice.cpp | 44 ++++++++----------- test/v4l2_videodevice/v4l2_videodevice_test.h | 2 +- 6 files changed, 48 insertions(+), 65 deletions(-) diff --git a/test/v4l2_videodevice/buffer_sharing.cpp b/test/v4l2_videodevice/buffer_sharing.cpp index fe48b2e98fdada8d..6acb06a24b47f653 100644 --- a/test/v4l2_videodevice/buffer_sharing.cpp +++ b/test/v4l2_videodevice/buffer_sharing.cpp @@ -73,16 +73,14 @@ protected: return TestFail; } - pool_.createBuffers(bufferCount); - - ret = capture_->exportBuffers(&pool_); - if (ret) { - std::cout << "Failed to export buffers" << std::endl; + ret = capture_->exportBuffers(bufferCount, &buffers_); + if (ret < 0) { + std::cout << "Failed to allocate buffers" << std::endl; return TestFail; } - ret = output_->importBuffers(&pool_); - if (ret) { + ret = output_->importBuffers(bufferCount); + if (ret < 0) { std::cout << "Failed to import buffers" << std::endl; return TestFail; } @@ -90,7 +88,7 @@ protected: return 0; } - void captureBufferReady(Buffer *buffer) + void captureBufferReady(FrameBuffer *buffer) { const FrameMetadata &metadata = buffer->metadata(); @@ -103,7 +101,7 @@ protected: framesCaptured_++; } - void outputBufferReady(Buffer *buffer) + void outputBufferReady(FrameBuffer *buffer) { const FrameMetadata &metadata = buffer->metadata(); @@ -122,13 +120,15 @@ protected: Timer timeout; int ret; - capture_->bufferReady.connect(this, &BufferSharingTest::captureBufferReady); - output_->bufferReady.connect(this, &BufferSharingTest::outputBufferReady); + capture_->frameBufferReady.connect(this, &BufferSharingTest::captureBufferReady); + output_->frameBufferReady.connect(this, &BufferSharingTest::outputBufferReady); - std::vector> buffers; - buffers = capture_->queueAllBuffers(); - if (buffers.empty()) - return TestFail; + for (const std::unique_ptr &buffer : buffers_) { + if (capture_->queueBuffer(buffer.get())) { + std::cout << "Failed to queue buffer" << std::endl; + return TestFail; + } + } ret = capture_->streamOn(); if (ret) { diff --git a/test/v4l2_videodevice/capture_async.cpp b/test/v4l2_videodevice/capture_async.cpp index f62bbd837b213a0a..a57abed3bd0debc1 100644 --- a/test/v4l2_videodevice/capture_async.cpp +++ b/test/v4l2_videodevice/capture_async.cpp @@ -20,7 +20,7 @@ public: CaptureAsyncTest() : V4L2VideoDeviceTest("vimc", "Raw Capture 0"), frames(0) {} - void receiveBuffer(Buffer *buffer) + void receiveBuffer(FrameBuffer *buffer) { std::cout << "Buffer received" << std::endl; frames++; @@ -38,18 +38,18 @@ protected: Timer timeout; int ret; - pool_.createBuffers(bufferCount); - - ret = capture_->exportBuffers(&pool_); - if (ret) + ret = capture_->exportBuffers(bufferCount, &buffers_); + if (ret < 0) return TestFail; - capture_->bufferReady.connect(this, &CaptureAsyncTest::receiveBuffer); + capture_->frameBufferReady.connect(this, &CaptureAsyncTest::receiveBuffer); - std::vector> buffers; - buffers = capture_->queueAllBuffers(); - if (buffers.empty()) - return TestFail; + for (const std::unique_ptr &buffer : buffers_) { + if (capture_->queueBuffer(buffer.get())) { + std::cout << "Failed to queue buffer" << std::endl; + return TestFail; + } + } ret = capture_->streamOn(); if (ret) diff --git a/test/v4l2_videodevice/request_buffers.cpp b/test/v4l2_videodevice/request_buffers.cpp index c4aedf7b3cd61e80..1dd65b05da430e63 100644 --- a/test/v4l2_videodevice/request_buffers.cpp +++ b/test/v4l2_videodevice/request_buffers.cpp @@ -16,17 +16,10 @@ public: protected: int run() { - /* - * TODO: - * Test invalid requests - * Test different buffer allocations - */ const unsigned int bufferCount = 8; - pool_.createBuffers(bufferCount); - - int ret = capture_->exportBuffers(&pool_); - if (ret) + int ret = capture_->exportBuffers(bufferCount, &buffers_); + if (ret != bufferCount) return TestFail; return TestPass; diff --git a/test/v4l2_videodevice/stream_on_off.cpp b/test/v4l2_videodevice/stream_on_off.cpp index 7664adc4c1f07046..552df0963633ae4b 100644 --- a/test/v4l2_videodevice/stream_on_off.cpp +++ b/test/v4l2_videodevice/stream_on_off.cpp @@ -17,10 +17,8 @@ protected: { const unsigned int bufferCount = 8; - pool_.createBuffers(bufferCount); - - int ret = capture_->exportBuffers(&pool_); - if (ret) + int ret = capture_->exportBuffers(bufferCount, &buffers_); + if (ret < 0) return TestFail; ret = capture_->streamOn(); diff --git a/test/v4l2_videodevice/v4l2_m2mdevice.cpp b/test/v4l2_videodevice/v4l2_m2mdevice.cpp index 442bcac5dc49cc59..43b99c4f7ea9bf26 100644 --- a/test/v4l2_videodevice/v4l2_m2mdevice.cpp +++ b/test/v4l2_videodevice/v4l2_m2mdevice.cpp @@ -29,7 +29,7 @@ public: { } - void outputBufferComplete(Buffer *buffer) + void outputBufferComplete(FrameBuffer *buffer) { cout << "Received output buffer" << endl; @@ -39,7 +39,7 @@ public: vim2m_->output()->queueBuffer(buffer); } - void receiveCaptureBuffer(Buffer *buffer) + void receiveCaptureBuffer(FrameBuffer *buffer) { cout << "Received capture buffer" << endl; @@ -112,39 +112,31 @@ protected: return TestFail; } - capturePool_.createBuffers(bufferCount); - outputPool_.createBuffers(bufferCount); - - ret = capture->exportBuffers(&capturePool_); - if (ret) { + ret = capture->exportBuffers(bufferCount, &captureBuffers_); + if (ret < 0) { cerr << "Failed to export Capture Buffers" << endl; return TestFail; } - ret = output->exportBuffers(&outputPool_); - if (ret) { + ret = output->exportBuffers(bufferCount, &outputBuffers_); + if (ret < 0) { cerr << "Failed to export Output Buffers" << endl; return TestFail; } - capture->bufferReady.connect(this, &V4L2M2MDeviceTest::receiveCaptureBuffer); - output->bufferReady.connect(this, &V4L2M2MDeviceTest::outputBufferComplete); + capture->frameBufferReady.connect(this, &V4L2M2MDeviceTest::receiveCaptureBuffer); + output->frameBufferReady.connect(this, &V4L2M2MDeviceTest::outputBufferComplete); - std::vector> captureBuffers; - captureBuffers = capture->queueAllBuffers(); - if (captureBuffers.empty()) { - cerr << "Failed to queue all Capture Buffers" << endl; - return TestFail; + for (const std::unique_ptr &buffer : captureBuffers_) { + if (capture->queueBuffer(buffer.get())) { + std::cout << "Failed to queue capture buffer" << std::endl; + return TestFail; + } } - /* We can't "queueAllBuffers()" on an output device, so we do it manually */ - std::vector> outputBuffers; - for (unsigned int i = 0; i < outputPool_.count(); ++i) { - Buffer *buffer = new Buffer(i); - outputBuffers.emplace_back(buffer); - ret = output->queueBuffer(buffer); - if (ret) { - cerr << "Failed to queue output buffer" << i << endl; + for (const std::unique_ptr &buffer : outputBuffers_) { + if (output->queueBuffer(buffer.get())) { + std::cout << "Failed to queue output buffer" << std::endl; return TestFail; } } @@ -202,8 +194,8 @@ private: std::shared_ptr media_; V4L2M2MDevice *vim2m_; - BufferPool capturePool_; - BufferPool outputPool_; + std::vector> captureBuffers_; + std::vector> outputBuffers_; unsigned int outputFrames_; unsigned int captureFrames_; diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.h b/test/v4l2_videodevice/v4l2_videodevice_test.h index 34dd231c6d9d108d..9acaceb84fe0a12f 100644 --- a/test/v4l2_videodevice/v4l2_videodevice_test.h +++ b/test/v4l2_videodevice/v4l2_videodevice_test.h @@ -41,7 +41,7 @@ protected: CameraSensor *sensor_; V4L2Subdevice *debayer_; V4L2VideoDevice *capture_; - BufferPool pool_; + std::vector> buffers_; }; #endif /* __LIBCAMERA_V4L2_DEVICE_TEST_H_ */ From patchwork Mon Dec 30 12:04:59 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2464 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E3EB66046C for ; Mon, 30 Dec 2019 13:06:01 +0100 (CET) X-Halon-ID: be69b947-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id be69b947-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:06:00 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:04:59 +0100 Message-Id: <20191230120510.938333-15-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 14/25] test: camera: buffer_import: Update to FrameBuffer restrictions X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:06:02 -0000 With the FrameBuffer interface the V4L2 buffer indexes are not visible outside the V4L2VideoDevice so it's not possible to to test that the expected indexes are used when using external buffers (allocated directly from a V4L2 video device) with a Camera. Rewrite the test to this limitation. The idea of the test stays the same, test that buffers allocated from a V4L2 video device (vivid) can be imported and used on a Camera (vimc). As an added bonus the rewrite makes us of the FrameBuffer interface for the allocation of buffers at the source (vivid) and imports them to the Camera which still uses the Buffer interface internally. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- test/camera/buffer_import.cpp | 390 ++++++++++------------------------ 1 file changed, 110 insertions(+), 280 deletions(-) diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index 3ba6ce9690f29329..6559a89ce914a183 100644 --- a/test/camera/buffer_import.cpp +++ b/test/camera/buffer_import.cpp @@ -22,19 +22,32 @@ using namespace libcamera; -/* Keep SINK_BUFFER_COUNT > CAMERA_BUFFER_COUNT + 1 */ -static constexpr unsigned int SINK_BUFFER_COUNT = 8; -static constexpr unsigned int CAMERA_BUFFER_COUNT = 4; +namespace { -class FrameSink +/* A provider of external buffers, suitable for import by a Camera. */ +class BufferSource { public: - FrameSink() + BufferSource() : video_(nullptr) { } - int init() + ~BufferSource() + { + if (video_) { + video_->releaseBuffers(); + video_->close(); + } + + delete video_; + video_ = nullptr; + + if (media_) + media_->release(); + } + + int allocate(const StreamConfiguration &config) { int ret; @@ -72,187 +85,27 @@ public: return TestFail; /* Configure the format. */ - ret = video_->getFormat(&format_); + V4L2DeviceFormat format; + ret = video_->getFormat(&format); if (ret) { std::cout << "Failed to get format on output device" << std::endl; return ret; } - format_.size.width = 1920; - format_.size.height = 1080; - format_.fourcc = V4L2_PIX_FMT_RGB24; - format_.planesCount = 1; - format_.planes[0].size = 1920 * 1080 * 3; - format_.planes[0].bpl = 1920 * 3; - - if (video_->setFormat(&format_)) { - cleanup(); - return TestFail; - } - - /* Export the buffers to a pool. */ - pool_.createBuffers(SINK_BUFFER_COUNT); - ret = video_->exportBuffers(&pool_); - if (ret) { - std::cout << "Failed to export buffers" << std::endl; - cleanup(); + format.size = config.size; + format.fourcc = V4L2VideoDevice::toV4L2Fourcc(config.pixelFormat, false); + if (video_->setFormat(&format)) return TestFail; - } - - /* Only use the first CAMERA_BUFFER_COUNT buffers to start with. */ - availableBuffers_.resize(CAMERA_BUFFER_COUNT); - std::iota(availableBuffers_.begin(), availableBuffers_.end(), 0); - - /* Connect the buffer ready signal. */ - video_->bufferReady.connect(this, &FrameSink::bufferComplete); - - return TestPass; - } - - void cleanup() - { - if (video_) { - video_->streamOff(); - video_->releaseBuffers(); - video_->close(); - - delete video_; - video_ = nullptr; - } - if (media_) - media_->release(); + return video_->exportBuffers(config.bufferCount, &buffers_); } - int start() - { - requestsCount_ = 0; - done_ = false; - - int ret = video_->streamOn(); - if (ret < 0) - return ret; - - /* Queue all the initial requests. */ - for (unsigned int index = 0; index < CAMERA_BUFFER_COUNT; ++index) - queueRequest(index); - - return 0; - } - - int stop() - { - return video_->streamOff(); - } - - void requestComplete(uint64_t cookie, const Buffer *metadata) - { - unsigned int index = cookie; - - Buffer *buffer = new Buffer(index, metadata); - int ret = video_->queueBuffer(buffer); - if (ret < 0) - std::cout << "Failed to queue buffer to sink" << std::endl; - } - - bool done() const { return done_; } - - PixelFormat format() const - { - return video_->toPixelFormat(format_.fourcc); - } - - const Size &size() const - { - return format_.size; - } - - Signal requestReady; + const std::vector> &buffers() { return buffers_; }; private: - void queueRequest(unsigned int index) - { - auto it = std::find(availableBuffers_.begin(), - availableBuffers_.end(), index); - availableBuffers_.erase(it); - - uint64_t cookie = index; - BufferMemory &mem = pool_.buffers()[index]; - int dmabuf = mem.planes()[0].fd.fd(); - - requestReady.emit(cookie, dmabuf); - - requestsCount_++; - } - - void bufferComplete(Buffer *buffer) - { - availableBuffers_.push_back(buffer->index()); - - /* - * Pick the buffer for the next request among the available - * buffers. - * - * For the first 20 frames, select the buffer that has just - * completed to keep the mapping of dmabuf fds to buffers - * unchanged in the camera. - * - * For the next 20 frames, cycle randomly over the available - * buffers. The mapping should still be kept unchanged, as the - * camera should map using the cached fds. - * - * For the last 20 frames, cycles through all buffers, which - * should trash the mappings. - */ - unsigned int index = buffer->index(); - delete buffer; - - std::cout << "Completed buffer, request=" << requestsCount_ - << ", available buffers=" << availableBuffers_.size() - << std::endl; - - if (requestsCount_ >= 60) { - if (availableBuffers_.size() == SINK_BUFFER_COUNT) - done_ = true; - return; - } - - if (requestsCount_ == 40) { - /* Add the remaining of the buffers. */ - for (unsigned int i = CAMERA_BUFFER_COUNT; - i < SINK_BUFFER_COUNT; ++i) - availableBuffers_.push_back(i); - } - - if (requestsCount_ >= 20) { - /* - * Wait until we have enough buffers to make this - * meaningful. Preferably half of the camera buffers, - * but no less than 2 in any case. - */ - const unsigned int min_pool_size = - std::min(CAMERA_BUFFER_COUNT / 2, 2U); - if (availableBuffers_.size() < min_pool_size) - return; - - /* Pick a buffer at random. */ - unsigned int pos = random_() % availableBuffers_.size(); - index = availableBuffers_[pos]; - } - - queueRequest(index); - } - std::shared_ptr media_; V4L2VideoDevice *video_; - BufferPool pool_; - V4L2DeviceFormat format_; - - unsigned int requestsCount_; - std::vector availableBuffers_; - std::random_device random_; - - bool done_; + std::vector> buffers_; }; class BufferImportTest : public CameraTest, public Test @@ -263,175 +116,152 @@ public: { } - void queueRequest(uint64_t cookie, int dmabuf) - { - Request *request = camera_->createRequest(cookie); - - std::unique_ptr buffer = stream_->createBuffer({ dmabuf, -1, -1 }); - request->addBuffer(stream_, move(buffer)); - camera_->queueRequest(request); - } - protected: void bufferComplete(Request *request, Buffer *buffer) { if (buffer->metadata().status != FrameMetadata::FrameSuccess) return; - unsigned int index = buffer->index(); - int dmabuf = buffer->dmabufs()[0]; - - /* Record dmabuf to index remappings. */ - bool remapped = false; - if (bufferMappings_.find(index) != bufferMappings_.end()) { - if (bufferMappings_[index] != dmabuf) - remapped = true; - } + completeBuffersCount_++; + } - std::cout << "Completed request " << framesCaptured_ - << ": dmabuf fd " << dmabuf - << " -> index " << index - << " (" << (remapped ? 'R' : '-') << ")" - << std::endl; + void requestComplete(Request *request) + { + if (request->status() != Request::RequestComplete) + return; - if (remapped) - bufferRemappings_.push_back(framesCaptured_); + const std::map &buffers = request->buffers(); - bufferMappings_[index] = dmabuf; - framesCaptured_++; + completeRequestsCount_++; - sink_.requestComplete(request->cookie(), buffer); + /* Create a new request. */ + Stream *stream = buffers.begin()->first; + int dmabuf = buffers.begin()->second->dmabufs()[0]; + std::unique_ptr buffer = stream->createBuffer({ dmabuf, -1, -1 }); - if (framesCaptured_ == 60) - sink_.stop(); + request = camera_->createRequest(); + request->addBuffer(stream, std::move(buffer)); + camera_->queueRequest(request); } - int initCamera() + int init() override { - if (camera_->acquire()) { - std::cout << "Failed to acquire the camera" << std::endl; - return TestFail; - } + if (status_ != TestPass) + return status_; - /* - * Configure the Stream to work with externally allocated - * buffers by setting the memoryType to ExternalMemory. - */ - std::unique_ptr config; - config = camera_->generateConfiguration({ StreamRole::VideoRecording }); - if (!config || config->size() != 1) { - std::cout << "Failed to generate configuration" << std::endl; + config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); + if (!config_ || config_->size() != 1) { + std::cout << "Failed to generate default configuration" << std::endl; return TestFail; } - StreamConfiguration &cfg = config->at(0); - cfg.size = sink_.size(); - cfg.pixelFormat = sink_.format(); - cfg.bufferCount = CAMERA_BUFFER_COUNT; + StreamConfiguration &cfg = config_->at(0); cfg.memoryType = ExternalMemory; - if (camera_->configure(config.get())) { - std::cout << "Failed to set configuration" << std::endl; + return TestPass; + } + + int run() override + { + StreamConfiguration &cfg = config_->at(0); + + if (camera_->acquire()) { + std::cout << "Failed to acquire the camera" << std::endl; return TestFail; } - stream_ = cfg.stream(); + if (camera_->configure(config_.get())) { + std::cout << "Failed to set default configuration" << std::endl; + return TestFail; + } - /* Allocate buffers. */ if (camera_->allocateBuffers()) { std::cout << "Failed to allocate buffers" << std::endl; return TestFail; } - /* Connect the buffer completed signal. */ - camera_->bufferCompleted.connect(this, &BufferImportTest::bufferComplete); + Stream *stream = cfg.stream(); - return TestPass; - } + BufferSource source; + int ret = source.allocate(cfg); + if (ret < 0) + return TestFail; - int init() - { - if (status_ != TestPass) - return status_; + std::vector requests; + for (const std::unique_ptr &framebuffer : source.buffers()) { + int dmabuf = framebuffer->planes()[0].fd.fd(); - int ret = sink_.init(); - if (ret != TestPass) { - cleanup(); - return ret; - } + Request *request = camera_->createRequest(); + if (!request) { + std::cout << "Failed to create request" << std::endl; + return TestFail; + } - ret = initCamera(); - if (ret != TestPass) { - cleanup(); - return ret; - } + std::unique_ptr buffer = stream->createBuffer({ dmabuf, -1, -1 }); + if (request->addBuffer(stream, std::move(buffer))) { + std::cout << "Failed to associating buffer with request" << std::endl; + return TestFail; + } - sink_.requestReady.connect(this, &BufferImportTest::queueRequest); - return TestPass; - } + requests.push_back(request); + } - int run() - { - int ret; + completeRequestsCount_ = 0; + completeBuffersCount_ = 0; - framesCaptured_ = 0; + camera_->bufferCompleted.connect(this, &BufferImportTest::bufferComplete); + camera_->requestCompleted.connect(this, &BufferImportTest::requestComplete); if (camera_->start()) { std::cout << "Failed to start camera" << std::endl; return TestFail; } - ret = sink_.start(); - if (ret < 0) { - std::cout << "Failed to start sink" << std::endl; - return TestFail; + for (Request *request : requests) { + if (camera_->queueRequest(request)) { + std::cout << "Failed to queue request" << std::endl; + return TestFail; + } } EventDispatcher *dispatcher = cm_->eventDispatcher(); Timer timer; - timer.start(5000); - while (timer.isRunning() && !sink_.done()) + timer.start(1000); + while (timer.isRunning()) dispatcher->processEvents(); - std::cout << framesCaptured_ << " frames captured, " - << bufferRemappings_.size() << " buffers remapped" - << std::endl; + if (completeRequestsCount_ <= cfg.bufferCount * 2) { + std::cout << "Failed to capture enough frames (got " + << completeRequestsCount_ << " expected at least " + << cfg.bufferCount * 2 << ")" << std::endl; + return TestFail; + } - if (framesCaptured_ < 60) { - std::cout << "Too few frames captured" << std::endl; + if (completeRequestsCount_ != completeBuffersCount_) { + std::cout << "Number of completed buffers and requests differ" << std::endl; return TestFail; } - if (bufferRemappings_.empty()) { - std::cout << "No buffer remappings" << std::endl; + if (camera_->stop()) { + std::cout << "Failed to stop camera" << std::endl; return TestFail; } - if (bufferRemappings_[0] < 40) { - std::cout << "Early buffer remapping" << std::endl; + if (camera_->freeBuffers()) { + std::cout << "Failed to free buffers" << std::endl; return TestFail; } return TestPass; } - void cleanup() - { - sink_.cleanup(); - - camera_->stop(); - camera_->freeBuffers(); - } - private: - Stream *stream_; - - std::map bufferMappings_; - std::vector bufferRemappings_; - unsigned int framesCaptured_; - - FrameSink sink_; + unsigned int completeBuffersCount_; + unsigned int completeRequestsCount_; + std::unique_ptr config_; }; +} /* namespace */ + TEST_REGISTER(BufferImportTest); From patchwork Mon Dec 30 12:05:00 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2465 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 0F6B66046A for ; Mon, 30 Dec 2019 13:06:03 +0100 (CET) X-Halon-ID: bf1d973d-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id bf1d973d-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:06:01 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:05:00 +0100 Message-Id: <20191230120510.938333-16-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 15/25] libcamera: pipeline: rkisp1: Destroy frame information before completing request X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:06:03 -0000 The FrameBuffer interface will allow reuse of FrameBuffers form the request completion handler. For this reason the pipeline must destroy its cached information freeing the statistics and parameters buffer used to allow them to be reused directly. Signed-off-by: Niklas Söderlund --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 46df871a51105ee4..9cd0ab3ad88b35cc 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -989,9 +989,9 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request) if (!info->paramDequeued) return; - completeRequest(activeCamera_, request); - data->frameInfo_.destroy(info->frame); + + completeRequest(activeCamera_, request); } void PipelineHandlerRkISP1::bufferReady(Buffer *buffer) From patchwork Mon Dec 30 12:05:01 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2466 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 00B656046A for ; Mon, 30 Dec 2019 13:06:03 +0100 (CET) X-Halon-ID: bfce7828-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id bfce7828-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:06:02 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:05:01 +0100 Message-Id: <20191230120510.938333-17-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 16/25] libcamera: pipeline: rkisp1: Switch to FrameBuffer interface for stat and param X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:06:04 -0000 The V4L2VideoDevice class can now operate using a FrameBuffer interface, switch the RkISP1 statistics and parameters buffer to use it. We can not yet convert the application facing buffers yet. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 122 ++++++++++++----------- 1 file changed, 66 insertions(+), 56 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 9cd0ab3ad88b35cc..ea581aaaa85088cd 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -32,9 +32,6 @@ #include "v4l2_subdevice.h" #include "v4l2_videodevice.h" -#define RKISP1_PARAM_BASE 0x100 -#define RKISP1_STAT_BASE 0x200 - namespace libcamera { LOG_DEFINE_CATEGORY(RkISP1) @@ -52,8 +49,8 @@ struct RkISP1FrameInfo { unsigned int frame; Request *request; - Buffer *paramBuffer; - Buffer *statBuffer; + FrameBuffer *paramBuffer; + FrameBuffer *statBuffer; Buffer *videoBuffer; bool paramFilled; @@ -71,6 +68,7 @@ public: RkISP1FrameInfo *find(unsigned int frame); RkISP1FrameInfo *find(Buffer *buffer); + RkISP1FrameInfo *find(FrameBuffer *buffer); RkISP1FrameInfo *find(Request *request); private: @@ -152,6 +150,7 @@ public: const V4L2SubdeviceFormat &sensorFormat() { return sensorFormat_; } + private: static constexpr unsigned int RKISP1_BUFFER_COUNT = 4; @@ -203,8 +202,8 @@ private: int createCamera(MediaEntity *sensor); void tryCompleteRequest(Request *request); void bufferReady(Buffer *buffer); - void paramReady(Buffer *buffer); - void statReady(Buffer *buffer); + void paramReady(FrameBuffer *buffer); + void statReady(FrameBuffer *buffer); MediaDevice *media_; V4L2Subdevice *dphy_; @@ -213,11 +212,10 @@ private: V4L2VideoDevice *param_; V4L2VideoDevice *stat_; - BufferPool paramPool_; - BufferPool statPool_; - - std::queue paramBuffers_; - std::queue statBuffers_; + std::vector> paramBuffersMemory; + std::vector> statBuffersMemory; + std::queue paramBuffers_; + std::queue statBuffers_; Camera *activeCamera_; }; @@ -233,13 +231,13 @@ RkISP1FrameInfo *RkISP1Frames::create(unsigned int frame, Request *request, Stre LOG(RkISP1, Error) << "Parameters buffer underrun"; return nullptr; } - Buffer *paramBuffer = pipe_->paramBuffers_.front(); + FrameBuffer *paramBuffer = pipe_->paramBuffers_.front(); if (pipe_->statBuffers_.empty()) { LOG(RkISP1, Error) << "Statisitc buffer underrun"; return nullptr; } - Buffer *statBuffer = pipe_->statBuffers_.front(); + FrameBuffer *statBuffer = pipe_->statBuffers_.front(); Buffer *videoBuffer = request->findBuffer(stream); if (!videoBuffer) { @@ -299,9 +297,21 @@ RkISP1FrameInfo *RkISP1Frames::find(Buffer *buffer) for (auto &itInfo : frameInfo_) { RkISP1FrameInfo *info = itInfo.second; + if (info->videoBuffer == buffer) + return info; + } + + LOG(RkISP1, Error) << "Can't locate info from buffer"; + return nullptr; +} + +RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer) +{ + for (auto &itInfo : frameInfo_) { + RkISP1FrameInfo *info = itInfo.second; + if (info->paramBuffer == buffer || - info->statBuffer == buffer || - info->videoBuffer == buffer) + info->statBuffer == buffer) return info; } @@ -660,6 +670,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera, const std::set &streams) { RkISP1CameraData *data = cameraData(camera); + unsigned int count = 1; Stream *stream = *streams.begin(); int ret; @@ -671,43 +682,41 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera, if (ret) return ret; - paramPool_.createBuffers(stream->configuration().bufferCount + 1); - ret = param_->exportBuffers(¶mPool_); - if (ret) { - video_->releaseBuffers(); - return ret; - } + unsigned int maxBuffers = 0; + for (const Stream *s : camera->streams()) + maxBuffers = std::max(maxBuffers, s->configuration().bufferCount); - statPool_.createBuffers(stream->configuration().bufferCount + 1); - ret = stat_->exportBuffers(&statPool_); - if (ret) { - param_->releaseBuffers(); - video_->releaseBuffers(); - return ret; - } + ret = param_->exportBuffers(maxBuffers, ¶mBuffersMemory); + if (ret < 0) + goto err; - for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) { - FrameBuffer::Plane plane; - plane.fd = FileDescriptor(paramPool_.buffers()[i].planes()[0].fd.fd()); - plane.length = paramPool_.buffers()[i].planes()[0].length; + ret = stat_->exportBuffers(maxBuffers, &statBuffersMemory); + if (ret < 0) + goto err; - data->ipaBuffers_.push_back({ .id = RKISP1_PARAM_BASE | i, - .planes = { plane } }); - paramBuffers_.push(new Buffer(i)); + for (std::unique_ptr &buffer : paramBuffersMemory) { + buffer->setCookie(count++); + data->ipaBuffers_.push_back({ .id = buffer->cookie(), + .planes = buffer->planes() }); + paramBuffers_.push(buffer.get()); } - for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) { - FrameBuffer::Plane plane; - plane.fd = FileDescriptor(statPool_.buffers()[i].planes()[0].fd.fd()); - plane.length = statPool_.buffers()[i].planes()[0].length; - - data->ipaBuffers_.push_back({ .id = RKISP1_STAT_BASE | i, - .planes = { plane } }); - statBuffers_.push(new Buffer(i)); + for (std::unique_ptr &buffer : statBuffersMemory) { + buffer->setCookie(count++); + data->ipaBuffers_.push_back({ .id = buffer->cookie(), + .planes = buffer->planes() }); + statBuffers_.push(buffer.get()); } data->ipa_->mapBuffers(data->ipaBuffers_); + return 0; + +err: + paramBuffersMemory.clear(); + statBuffersMemory.clear(); + video_->releaseBuffers(); + return ret; } @@ -716,15 +725,14 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera, { RkISP1CameraData *data = cameraData(camera); - while (!statBuffers_.empty()) { - delete statBuffers_.front(); + while (!statBuffers_.empty()) statBuffers_.pop(); - } - while (!paramBuffers_.empty()) { - delete paramBuffers_.front(); + while (!paramBuffers_.empty()) paramBuffers_.pop(); - } + + paramBuffersMemory.clear(); + statBuffersMemory.clear(); std::vector ids; for (IPABuffer &ipabuf : data->ipaBuffers_) @@ -829,9 +837,11 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, if (!info) return -ENOENT; + unsigned int paramid = info->paramBuffer->cookie(); + IPAOperationData op; op.operation = RKISP1_IPA_EVENT_QUEUE_REQUEST; - op.data = { data->frame_, RKISP1_PARAM_BASE | info->paramBuffer->index() }; + op.data = { data->frame_, paramid }; op.controls = { request->controls() }; data->ipa_->processEvent(op); @@ -946,8 +956,8 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) return false; video_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady); - stat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady); - param_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady); + stat_->frameBufferReady.connect(this, &PipelineHandlerRkISP1::statReady); + param_->frameBufferReady.connect(this, &PipelineHandlerRkISP1::paramReady); /* Configure default links. */ if (initLinks() < 0) { @@ -1009,7 +1019,7 @@ void PipelineHandlerRkISP1::bufferReady(Buffer *buffer) tryCompleteRequest(request); } -void PipelineHandlerRkISP1::paramReady(Buffer *buffer) +void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer) { ASSERT(activeCamera_); RkISP1CameraData *data = cameraData(activeCamera_); @@ -1020,7 +1030,7 @@ void PipelineHandlerRkISP1::paramReady(Buffer *buffer) tryCompleteRequest(info->request); } -void PipelineHandlerRkISP1::statReady(Buffer *buffer) +void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer) { ASSERT(activeCamera_); RkISP1CameraData *data = cameraData(activeCamera_); @@ -1030,7 +1040,7 @@ void PipelineHandlerRkISP1::statReady(Buffer *buffer) return; unsigned int frame = info->frame; - unsigned int statid = RKISP1_STAT_BASE | info->statBuffer->index(); + unsigned int statid = info->statBuffer->cookie(); IPAOperationData op; op.operation = RKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER; From patchwork Mon Dec 30 12:05:02 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2467 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C23F56046B for ; Mon, 30 Dec 2019 13:06:04 +0100 (CET) X-Halon-ID: c060cc51-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id c060cc51-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:06:03 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:05:02 +0100 Message-Id: <20191230120510.938333-18-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 17/25] libcamera: pipeline: ipu3: Switch to FrameBuffer interface for cio2 and stat X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:06:05 -0000 The V4L2VideoDevice class can now operate using a FrameBuffer interface, switch the IPU3 CIO2 and statistic buffer to use it. We can not yet convert the application facing buffers yet. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/pipeline/ipu3/ipu3.cpp | 103 ++++++++++++--------------- 1 file changed, 44 insertions(+), 59 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 34fc792977d151be..030ba2b6a0df2e0b 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -45,6 +45,7 @@ public: unsigned int pad; std::string name; BufferPool *pool; + std::vector> buffers; }; ImgUDevice() @@ -70,7 +71,6 @@ public: int configureOutput(ImgUOutput *output, const StreamConfiguration &cfg); - int importInputBuffers(BufferPool *pool); int importOutputBuffers(ImgUOutput *output, BufferPool *pool); int exportOutputBuffers(ImgUOutput *output, BufferPool *pool); void freeBuffers(); @@ -95,7 +95,6 @@ public: /* \todo Add param video device for 3A tuning */ BufferPool vfPool_; - BufferPool statPool_; BufferPool outPool_; }; @@ -120,10 +119,10 @@ public: int configure(const Size &size, V4L2DeviceFormat *outputFormat); - BufferPool *exportBuffers(); + int allocateBuffers(); void freeBuffers(); - int start(std::vector> *buffers); + int start(); int stop(); static int mediaBusToFormat(unsigned int code); @@ -132,7 +131,8 @@ public: V4L2Subdevice *csi2_; CameraSensor *sensor_; - BufferPool pool_; +private: + std::vector> buffers_; }; class IPU3Stream : public Stream @@ -157,16 +157,14 @@ public: } void imguOutputBufferReady(Buffer *buffer); - void imguInputBufferReady(Buffer *buffer); - void cio2BufferReady(Buffer *buffer); + void imguInputBufferReady(FrameBuffer *buffer); + void cio2BufferReady(FrameBuffer *buffer); CIO2Device cio2_; ImgUDevice *imgu_; IPU3Stream outStream_; IPU3Stream vfStream_; - - std::vector> rawBuffers_; }; class IPU3CameraConfiguration : public CameraConfiguration @@ -638,24 +636,28 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, int ret; /* Share buffers between CIO2 output and ImgU input. */ - BufferPool *pool = cio2->exportBuffers(); - if (!pool) - return -ENOMEM; + ret = cio2->allocateBuffers(); + if (ret < 0) + return ret; - ret = imgu->importInputBuffers(pool); - if (ret) + bufferCount = ret; + + ret = imgu->input_->importBuffers(bufferCount); + if (ret) { + LOG(IPU3, Error) << "Failed to import ImgU input buffers"; goto error; + } /* * Use for the stat's internal pool the same number of buffer as * for the input pool. * \todo To be revised when we'll actually use the stat node. */ - bufferCount = pool->count(); - imgu->stat_.pool->createBuffers(bufferCount); - ret = imgu->exportOutputBuffers(&imgu->stat_, imgu->stat_.pool); - if (ret) + ret = imgu->stat_.dev->exportBuffers(bufferCount, &imgu->stat_.buffers); + if (ret < 0) { + LOG(IPU3, Error) << "Failed to allocate ImgU stat buffers"; goto error; + } /* Allocate buffers for each active stream. */ for (Stream *s : streams) { @@ -722,7 +724,7 @@ int PipelineHandlerIPU3::start(Camera *camera) * Start the ImgU video devices, buffers will be queued to the * ImgU output and viewfinder when requests will be queued. */ - ret = cio2->start(&data->rawBuffers_); + ret = cio2->start(); if (ret) goto error; @@ -738,7 +740,6 @@ int PipelineHandlerIPU3::start(Camera *camera) error: LOG(IPU3, Error) << "Failed to start camera " << camera->name(); - data->rawBuffers_.clear(); return ret; } @@ -752,8 +753,6 @@ void PipelineHandlerIPU3::stop(Camera *camera) if (ret) LOG(IPU3, Warning) << "Failed to stop camera " << camera->name(); - - data->rawBuffers_.clear(); } int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request) @@ -885,9 +884,9 @@ int PipelineHandlerIPU3::registerCameras() * associated ImgU input where they get processed and * returned through the ImgU main and secondary outputs. */ - data->cio2_.output_->bufferReady.connect(data.get(), + data->cio2_.output_->frameBufferReady.connect(data.get(), &IPU3CameraData::cio2BufferReady); - data->imgu_->input_->bufferReady.connect(data.get(), + data->imgu_->input_->frameBufferReady.connect(data.get(), &IPU3CameraData::imguInputBufferReady); data->imgu_->output_.dev->bufferReady.connect(data.get(), &IPU3CameraData::imguOutputBufferReady); @@ -925,7 +924,7 @@ int PipelineHandlerIPU3::registerCameras() * Buffers completed from the ImgU input are immediately queued back to the * CIO2 unit to continue frame capture. */ -void IPU3CameraData::imguInputBufferReady(Buffer *buffer) +void IPU3CameraData::imguInputBufferReady(FrameBuffer *buffer) { /* \todo Handle buffer failures when state is set to BufferError. */ if (buffer->metadata().status == FrameMetadata::FrameCancelled) @@ -959,7 +958,7 @@ void IPU3CameraData::imguOutputBufferReady(Buffer *buffer) * Buffers completed from the CIO2 are immediately queued to the ImgU unit * for further processing. */ -void IPU3CameraData::cio2BufferReady(Buffer *buffer) +void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer) { /* \todo Handle buffer failures when state is set to BufferError. */ if (buffer->metadata().status == FrameMetadata::FrameCancelled) @@ -1034,7 +1033,6 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index) stat_.pad = PAD_STAT; stat_.name = "stat"; - stat_.pool = &statPool_; return 0; } @@ -1134,22 +1132,6 @@ int ImgUDevice::configureOutput(ImgUOutput *output, return 0; } -/** - * \brief Import buffers from \a pool into the ImgU input - * \param[in] pool The buffer pool to import - * \return 0 on success or a negative error code otherwise - */ -int ImgUDevice::importInputBuffers(BufferPool *pool) -{ - int ret = input_->importBuffers(pool); - if (ret) { - LOG(IPU3, Error) << "Failed to import ImgU input buffers"; - return ret; - } - - return 0; -} - /** * \brief Export buffers from \a output to the provided \a pool * \param[in] output The ImgU output device @@ -1448,37 +1430,40 @@ int CIO2Device::configure(const Size &size, } /** - * \brief Allocate CIO2 memory buffers and export them in a BufferPool + * \brief Allocate frame buffers for the CIO2 output * - * Allocate memory buffers in the CIO2 video device and export them to - * a buffer pool that can be imported by another device. + * Allocate frame buffers in the CIO2 video device to be used to capture frames + * from the CIO2 output. The buffers are stored in the CIO2Device::buffers_ + * vector. * - * \return The buffer pool with export buffers on success or nullptr otherwise + * \return Number of buffers allocated or negative error code */ -BufferPool *CIO2Device::exportBuffers() +int CIO2Device::allocateBuffers() { - pool_.createBuffers(CIO2_BUFFER_COUNT); - - int ret = output_->exportBuffers(&pool_); - if (ret) { + int ret = output_->exportBuffers(CIO2_BUFFER_COUNT, &buffers_); + if (ret < 0) LOG(IPU3, Error) << "Failed to export CIO2 buffers"; - return nullptr; - } - return &pool_; + return ret; } void CIO2Device::freeBuffers() { + buffers_.clear(); + if (output_->releaseBuffers()) LOG(IPU3, Error) << "Failed to release CIO2 buffers"; } -int CIO2Device::start(std::vector> *buffers) +int CIO2Device::start() { - *buffers = output_->queueAllBuffers(); - if (buffers->empty()) - return -EINVAL; + for (const std::unique_ptr &buffer : buffers_) { + int ret = output_->queueBuffer(buffer.get()); + if (ret) { + LOG(IPU3, Error) << "Failed to queue CIO2 buffer"; + return ret; + } + } return output_->streamOn(); } From patchwork Mon Dec 30 12:05:03 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2468 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 03CC16046A for ; Mon, 30 Dec 2019 13:06:05 +0100 (CET) X-Halon-ID: c0e1eaf5-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id c0e1eaf5-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:06:04 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:05:03 +0100 Message-Id: <20191230120510.938333-19-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 18/25] libcamera: pipelines: Add FrameBuffer handlers X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:06:06 -0000 Extend the pipeline handlers to support the FrameBuffer API with three new methods to handle allocation, importing and freeing of buffers. The new methods will replace allocateBuffers() and freeBuffers(). The FrameBuffer API will use the methods on a stream level and either allocate or import buffers for each active stream controlled from the Camera class and an upcoming FrameBufferAllocator helper. With this new API the implementation in pipeline handlers can be made simpler as all streams don't need to be handled in allocateBuffers(). Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/include/pipeline_handler.h | 7 +++ src/libcamera/pipeline/ipu3/ipu3.cpp | 34 +++++++++++++ src/libcamera/pipeline/rkisp1/rkisp1.cpp | 25 ++++++++++ src/libcamera/pipeline/uvcvideo.cpp | 31 ++++++++++++ src/libcamera/pipeline/vimc.cpp | 31 ++++++++++++ src/libcamera/pipeline_handler.cpp | 62 ++++++++++++++++++++++++ 6 files changed, 190 insertions(+) diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index f3622631022d87b9..520d3fccaaa130cc 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -69,6 +69,13 @@ public: const StreamRoles &roles) = 0; virtual int configure(Camera *camera, CameraConfiguration *config) = 0; + virtual int allocateFrameBuffers(Camera *camera, Stream *stream, + unsigned int count, + std::vector> *buffers) = 0; + virtual int importFrameBuffers(Camera *camera, Stream *stream, + unsigned int count) = 0; + virtual void freeFrameBuffers(Camera *camera, Stream *stream) = 0; + virtual int allocateBuffers(Camera *camera, const std::set &streams) = 0; virtual int freeBuffers(Camera *camera, diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 030ba2b6a0df2e0b..f9bddcc88523301f 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -210,6 +210,13 @@ public: const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; + int allocateFrameBuffers(Camera *camera, Stream *stream, + unsigned int count, + std::vector> *buffers) override; + int importFrameBuffers(Camera *camera, Stream *stream, + unsigned int count) override; + void freeFrameBuffers(Camera *camera, Stream *stream) override; + int allocateBuffers(Camera *camera, const std::set &streams) override; int freeBuffers(Camera *camera, @@ -616,6 +623,33 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) return 0; } +int PipelineHandlerIPU3::allocateFrameBuffers(Camera *camera, Stream *stream, + unsigned int count, + std::vector> *buffers) +{ + IPU3Stream *ipu3stream = static_cast(stream); + V4L2VideoDevice *video = ipu3stream->device_->dev; + + return video->exportBuffers(count, buffers); +} + +int PipelineHandlerIPU3::importFrameBuffers(Camera *camera, Stream *stream, + unsigned int count) +{ + IPU3Stream *ipu3stream = static_cast(stream); + V4L2VideoDevice *video = ipu3stream->device_->dev; + + return video->importBuffers(count); +} + +void PipelineHandlerIPU3::freeFrameBuffers(Camera *camera, Stream *stream) +{ + IPU3Stream *ipu3stream = static_cast(stream); + V4L2VideoDevice *video = ipu3stream->device_->dev; + + video->releaseBuffers(); +} + /** * \todo Clarify if 'viewfinder' and 'stat' nodes have to be set up and * started even if not in use. As of now, if not properly configured and diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index ea581aaaa85088cd..e77925f6f9deff08 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -175,6 +175,13 @@ public: const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; + int allocateFrameBuffers(Camera *camera, Stream *stream, + unsigned int count, + std::vector> *buffers) override; + int importFrameBuffers(Camera *camera, Stream *stream, + unsigned int count) override; + void freeFrameBuffers(Camera *camera, Stream *stream) override; + int allocateBuffers(Camera *camera, const std::set &streams) override; int freeBuffers(Camera *camera, @@ -666,6 +673,24 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) return 0; } +int PipelineHandlerRkISP1::allocateFrameBuffers(Camera *camera, Stream *stream, + unsigned int count, + std::vector> *buffers) +{ + return video_->exportBuffers(count, buffers); +} + +int PipelineHandlerRkISP1::importFrameBuffers(Camera *camera, Stream *stream, + unsigned int count) +{ + return video_->importBuffers(count); +} + +void PipelineHandlerRkISP1::freeFrameBuffers(Camera *camera, Stream *stream) +{ + video_->releaseBuffers(); +} + int PipelineHandlerRkISP1::allocateBuffers(Camera *camera, const std::set &streams) { diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 3043366bee38bcfc..655c5e6d96a696d6 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -64,6 +64,13 @@ public: const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; + int allocateFrameBuffers(Camera *camera, Stream *stream, + unsigned int count, + std::vector> *buffers) override; + int importFrameBuffers(Camera *camera, Stream *stream, + unsigned int count) override; + void freeFrameBuffers(Camera *camera, Stream *stream) override; + int allocateBuffers(Camera *camera, const std::set &streams) override; int freeBuffers(Camera *camera, @@ -192,6 +199,30 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config) return 0; } +int PipelineHandlerUVC::allocateFrameBuffers(Camera *camera, Stream *stream, + unsigned int count, + std::vector> *buffers) +{ + UVCCameraData *data = cameraData(camera); + + return data->video_->exportBuffers(count, buffers); +} + +int PipelineHandlerUVC::importFrameBuffers(Camera *camera, Stream *stream, + unsigned int count) +{ + UVCCameraData *data = cameraData(camera); + + return data->video_->importBuffers(count); +} + +void PipelineHandlerUVC::freeFrameBuffers(Camera *camera, Stream *stream) +{ + UVCCameraData *data = cameraData(camera); + + data->video_->releaseBuffers(); +} + int PipelineHandlerUVC::allocateBuffers(Camera *camera, const std::set &streams) { diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 292900bcf8d1b359..02235ffb0515982f 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -82,6 +82,13 @@ public: const StreamRoles &roles) override; int configure(Camera *camera, CameraConfiguration *config) override; + int allocateFrameBuffers(Camera *camera, Stream *stream, + unsigned int count, + std::vector> *buffers) override; + int importFrameBuffers(Camera *camera, Stream *stream, + unsigned int count) override; + void freeFrameBuffers(Camera *camera, Stream *stream) override; + int allocateBuffers(Camera *camera, const std::set &streams) override; int freeBuffers(Camera *camera, @@ -259,6 +266,30 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config) return 0; } +int PipelineHandlerVimc::allocateFrameBuffers(Camera *camera, Stream *stream, + unsigned int count, + std::vector> *buffers) +{ + VimcCameraData *data = cameraData(camera); + + return data->video_->exportBuffers(count, buffers); +} + +int PipelineHandlerVimc::importFrameBuffers(Camera *camera, Stream *stream, + unsigned int count) +{ + VimcCameraData *data = cameraData(camera); + + return data->video_->importBuffers(count); +} + +void PipelineHandlerVimc::freeFrameBuffers(Camera *camera, Stream *stream) +{ + VimcCameraData *data = cameraData(camera); + + data->video_->releaseBuffers(); +} + int PipelineHandlerVimc::allocateBuffers(Camera *camera, const std::set &streams) { diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 5badf31ce793edf8..fb69ca8e97216e6c 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -287,6 +287,68 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera) * \return 0 on success or a negative error code otherwise */ +/** + * \fn PipelineHandler::allocateFrameBuffers() + * \brief Allocate buffers for \a stream + * \param[in] camera The camera to allocate buffers on + * \param[in] stream The stream to allocate buffers for + * \param[in] count Number of buffers to allocate + * \param[out] buffers Array of buffers successfully allocated + * + * Allocate buffers for \a stream using the resources associated with the stream + * inside the pipelinehandler. + * + * The method could be called multiple times to operate on different streams. + * It is not allowed to call allocateFrameBuffers() and importFrameBuffers() + * on the same \a stream as they are mutually exclusive operations. The + * pipelinehandler must guarantee that all streams exposed as part of the Camera + * can be used simultaneously with all combinations of the two. + * + * This is a helper which may be used by libcamera helper classes to allocate + * buffers from the stream itself. The allocated buffers may then be treated + * in the same way as if they where externally allocated. + * + * The only intended caller is the FrameBufferAllocator helper. + * + * \return 0 on success or a negative error code otherwise + */ + +/** + * \fn PipelineHandler::importFrameBuffers() + * \brief Prepare \a stream to use external buffers + * \param[in] camera The camera to prepare for import on + * \param[in] stream The stream to prepare for import + * \param[in] count Number of buffers to import + * + * Prepare the \a stream to use \a count number of buffers allocated outside + * the pipelinehandler. + * + * The method could be called multiple times to operate on different streams. + * It is not allowed to call allocateFrameBuffers() and importFrameBuffers() + * on the same \a stream as they are mutually exclusive operations. The + * pipelinehandler must guarantee that all streams exposed as part of the Camera + * can be used simultaneously with all combinations of the two. + * + * This is a helper which may be used by libcamera helper classes to import + * buffers to the \a stream from external sources. + * + * The only intended caller is the FrameBufferAllocator helper. + * + * \return 0 on success or a negative error code otherwise + */ + +/** + * \fn PipelineHandler::freeFrameBuffers() + * \brief Free buffers allocated from the stram + * \param[in] camera The camera to free buffers on + * \param[in] stream The stream to free buffers for + * + * This is a helper that frees buffers and resources allocated using + * allocateFrameBuffers() or importFrameBuffers(). + * + * The only intended caller is the FrameBufferAllocator helper. + */ + /** * \fn PipelineHandler::allocateBuffers() * \brief Allocate buffers for a stream From patchwork Mon Dec 30 12:05:04 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2469 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E57A760609 for ; Mon, 30 Dec 2019 13:06:06 +0100 (CET) X-Halon-ID: c18951e0-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id c18951e0-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:06:05 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:05:04 +0100 Message-Id: <20191230120510.938333-20-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 19/25] libcamera: allocator: Add FrameBufferAllocator to help applications allocate buffers X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:06:07 -0000 The FrameBuffer interface is based on the idea that all buffers are allocated externally to libcamera and are only used by it. This is meant to create a simpler API centered around usage of buffers, regardless of where they come from. Linux however lacks a centralized allocator at the moment, and not all users of libcamera are expected to use another device that could provide suitable buffers for the camera. This patch thus adds a helper class to allocate buffers internally in libcamera, in a way that matches the needs of the FrameBuffer-based API. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- * Changes since v1 - Update commit message - Rename class to FrameBufferAllocator - Rename source files framebuffer_allocator.{cpp,h} - Update include headers - Update documentation - Check for double allocation for the same stream in allocate() - Add interactions with Camera::allocator_ and enforce buffers may only be allocated when the camera is in the configured state. --- include/libcamera/camera.h | 5 + include/libcamera/framebuffer_allocator.h | 47 +++++ include/libcamera/meson.build | 1 + src/libcamera/camera.cpp | 15 +- src/libcamera/framebuffer_allocator.cpp | 213 ++++++++++++++++++++++ src/libcamera/meson.build | 1 + 6 files changed, 281 insertions(+), 1 deletion(-) create mode 100644 include/libcamera/framebuffer_allocator.h create mode 100644 src/libcamera/framebuffer_allocator.cpp diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index ef6a37bb142c83a6..f2827438871189a1 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -20,6 +20,7 @@ namespace libcamera { class Buffer; +class FrameBufferAllocator; class PipelineHandler; class Request; @@ -126,6 +127,10 @@ private: bool disconnected_; State state_; + + /* Needed to update allocator_ and read state_. */ + friend class FrameBufferAllocator; + FrameBufferAllocator *allocator_; }; } /* namespace libcamera */ diff --git a/include/libcamera/framebuffer_allocator.h b/include/libcamera/framebuffer_allocator.h new file mode 100644 index 0000000000000000..75952762884e44ac --- /dev/null +++ b/include/libcamera/framebuffer_allocator.h @@ -0,0 +1,47 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * framebuffer_allocator.h - FrameBuffer allocator + */ +#ifndef __LIBCAMERA_FRAMEBUFFER_ALLOCATOR_H__ +#define __LIBCAMERA_FRAMEBUFFER_ALLOCATOR_H__ + +#include +#include +#include +#include + +namespace libcamera { + +class Camera; +class FrameBuffer; +class Stream; + +class FrameBufferAllocator +{ +public: + static FrameBufferAllocator *create(std::shared_ptr camera); + + FrameBufferAllocator(const Camera &) = delete; + FrameBufferAllocator &operator=(const Camera &) = delete; + + ~FrameBufferAllocator(); + + int allocate(Stream *stream); + int release(Stream *stream); + + const std::unordered_set &streams() const { return streams_; } + const std::vector> &buffers(Stream *stream) const; + +private: + FrameBufferAllocator(std::shared_ptr camera); + + std::shared_ptr camera_; + std::unordered_set streams_; + std::map>> buffers_; +}; + +} /* namespace libcamera */ + +#endif /* __LIBCAMERA_FRAMEBUFFER_ALLOCATOR_H__ */ diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build index 543e6773cc5158a0..8db217bb782c1443 100644 --- a/include/libcamera/meson.build +++ b/include/libcamera/meson.build @@ -7,6 +7,7 @@ libcamera_api = files([ 'event_dispatcher.h', 'event_notifier.h', 'file_descriptor.h', + 'framebuffer_allocator.h', 'geometry.h', 'logging.h', 'object.h', diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index e810fb725d81350d..222c6811d5698f0f 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -9,6 +9,7 @@ #include +#include #include #include @@ -405,7 +406,7 @@ const std::string &Camera::name() const Camera::Camera(PipelineHandler *pipe, const std::string &name) : pipe_(pipe->shared_from_this()), name_(name), disconnected_(false), - state_(CameraAvailable) + state_(CameraAvailable), allocator_(nullptr) { } @@ -541,6 +542,12 @@ int Camera::release() if (!stateBetween(CameraAvailable, CameraConfigured)) return -EBUSY; + if (allocator_) { + LOG(Camera, Error) + << "Allocator must be deleted before camera can be released"; + return -EBUSY; + } + pipe_->unlock(); state_ = CameraAvailable; @@ -649,6 +656,12 @@ int Camera::configure(CameraConfiguration *config) if (!stateBetween(CameraAcquired, CameraConfigured)) return -EACCES; + if (allocator_ && allocator_->streams().size()) { + LOG(Camera, Error) + << "Allocator must be deleted before camera can be reconfigured"; + return -EBUSY; + } + if (config->validate() != CameraConfiguration::Valid) { LOG(Camera, Error) << "Can't configure camera with invalid configuration"; diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp new file mode 100644 index 0000000000000000..98b649ecf6053790 --- /dev/null +++ b/src/libcamera/framebuffer_allocator.cpp @@ -0,0 +1,213 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * framebuffer_allocator.cpp - FrameBuffer allocator + */ + +#include + +#include + +#include +#include +#include + +#include "log.h" +#include "pipeline_handler.h" + +/** + * \file framebuffer_allocator.h + * \brief FrameBuffer allocator + */ + +namespace libcamera { + +LOG_DEFINE_CATEGORY(Allocator) + +/** + * \class FrameBufferAllocator + * \brief FrameBuffer allocator for applications + * + * The libcamera API is designed to consume buffers provided by applications as + * FrameBuffer instances. This makes libcamera a user of buffers exported by + * other devices (such as displays or video encoders), or allocated from an + * external allocator (such as ION on Android platforms). In some situations, + * applications do not have any mean to allocate or get hold of suitable + * buffers, for instance when no other device is involved, on Linux platforms + * that lack a centralized allocator. The FrameBufferAllocator class provides a + * buffer allocator that can be used in these situations. + * + * Applications create a framebuffer allocator for a Camera, and use it to + * allocate buffers for streams of a CameraConfiguration with allocate(). They + * control which streams to allocate buffers for, and can thus use external + * buffers for a subset of the streams if desired. + * + * Buffers are deleted for a stream with release(), and destroying the allocator + * automatically deletes all allocated buffers. Applications own the buffers + * allocated by the FrameBufferAllocator and are responsible for ensuring the + * buffers are not deleted while they are in use (part of a Request that has + * been queued and hasn't completed yet). + * + * Usage of the FrameBufferAllocator is optional, if all buffers for a camera + * are provided externally applications shall not use this class. + */ + +/** + * \brief Create a framebuffer allocator instance + * \param[in] camera The camera the allocator serves + * + * The caller is responsible for deleting the allocator before the camera is + * released. + * + * \return A pointer to the newly created allocator object or nullptr on error + */ +FrameBufferAllocator * +FrameBufferAllocator::create(std::shared_ptr camera) +{ + if (camera->allocator_) { + LOG(Allocator, Error) << "Camera already have an allocator"; + return nullptr; + } + + FrameBufferAllocator *allocator = new FrameBufferAllocator(camera); + + camera->allocator_ = allocator; + + return allocator; +} + +/** + * \brief Allocate buffers for a stream with a configuration + * \param[in] stream The stream to allocate buffers for + * + * Allocate buffers suitable for capturing frames from the \a stream. Upon + * successful allocation, the allocated buffers can be retrieved with the + * buffers() method. + * + * \return 0 on success or a negative error code otherwise + * \retval -EACCES The camera is not in a state where buffers can be allocated + * \retval -EINVAL The allocator do not handle the \a stream + * \retval -EBUSY Buffers are already allocated for the \a stream + */ +int FrameBufferAllocator::allocate(Stream *stream) +{ + if (camera_->state_ != Camera::CameraConfigured && camera_->state_ != Camera::CameraPrepared) { + LOG(Allocator, Error) + << "Camera must be in the configured state to allocate buffers"; + return -EACCES; + } + + auto iter = camera_->streams().find(stream); + if (iter == camera_->streams().end()) { + LOG(Allocator, Error) + << "Stream does not belong to " << camera_->name(); + return -EINVAL; + } + + if (buffers_.count(stream)) { + LOG(Allocator, Error) << "Buffers already allocated for stream"; + return -EBUSY; + } + + unsigned int bufferCount = stream->configuration().bufferCount; + int ret = camera_->pipe_->allocateFrameBuffers(camera_.get(), stream, + bufferCount, + &buffers_[stream]); + if (ret) + return ret; + + streams_.insert(stream); + + return 0; +} + +/** + * \brief Free buffers previously allocated for a \a stream + * \param[in] stream The stream to free buffers for + * + * Free buffers allocated with allocate(). + * + * This invalidates the buffers returned by buffers(). + * + * \return 0 on success or a negative error code otherwise + * \retval -EACCES The camera is not in a state where buffers can be freed + * \retval -EINVAL The allocator do not handle the \a stream + */ +int FrameBufferAllocator::release(Stream *stream) +{ + if (camera_->state_ != Camera::CameraConfigured && camera_->state_ != Camera::CameraPrepared) { + LOG(Allocator, Error) + << "Camera must be in the configured state to free buffers"; + return -EACCES; + } + + auto iter = buffers_.find(stream); + if (iter == buffers_.end()) + return -EINVAL; + + std::vector> &buffers = iter->second; + + buffers.clear(); + + camera_->pipe_->freeFrameBuffers(camera_.get(), stream); + + buffers_.erase(iter); + + streams_.erase(stream); + + return 0; +} + +/** + * \fn FrameBufferAllocator::streams() + * \brief Retrieve the streams the allocator handles + * \return The streams the allocator handles + */ + +/** + * \brief Retrieve the buffers allocated for a \a stream + * \param[in] stream The stream to retrive buffers for + * + * This method shall only be called after successfully allocating buffers for + * \a stream with allocate(). The returned buffers are valid until release() is + * called for the same stream or the destruction of the FrameBufferAllocator + * instance. + * + * \return The buffers allocated for the \a stream + */ +const std::vector> +&FrameBufferAllocator::buffers(Stream *stream) const +{ + static std::vector> empty; + + auto iter = buffers_.find(stream); + if (iter == buffers_.end()) + return empty; + + return iter->second; +} + +/** + * \brief Create a FrameBufferAllocator serving a camera + * \param[in] camera The camera + */ + +FrameBufferAllocator::FrameBufferAllocator(std::shared_ptr camera) + : camera_(camera) +{ +} + +FrameBufferAllocator::~FrameBufferAllocator() +{ + for (auto &value : buffers_) { + Stream *stream = value.first; + camera_->pipe_->freeFrameBuffers(camera_.get(), stream); + } + + buffers_.clear(); + + camera_->allocator_ = nullptr; +} + +} /* namespace libcamera */ diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index 722c5bc15afe52ef..68d89559b290befd 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -16,6 +16,7 @@ libcamera_sources = files([ 'event_notifier.cpp', 'file_descriptor.cpp', 'formats.cpp', + 'framebuffer_allocator.cpp', 'geometry.cpp', 'ipa_context_wrapper.cpp', 'ipa_controls.cpp', From patchwork Mon Dec 30 12:05:05 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2470 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6A69F6046A for ; Mon, 30 Dec 2019 13:06:09 +0100 (CET) X-Halon-ID: c22764a7-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id c22764a7-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:06:06 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:05:05 +0100 Message-Id: <20191230120510.938333-21-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 20/25] libcamera: Switch to FrameBuffer interface X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:06:09 -0000 Switch to the FrameBuffer interface where all buffers are treated as external buffers and are allocated outside the camera. Applications allocating buffers using libcamera are switched to use the FrameBufferAllocator helper. A follow up changes to this one will finalize the transition to the new FrameBuffer interface by removing code that is left unused after this change. Signed-off-by: Niklas Söderlund --- include/libcamera/camera.h | 4 +- include/libcamera/request.h | 14 +-- src/android/camera_device.cpp | 29 ++++-- src/cam/buffer_writer.cpp | 5 +- src/cam/buffer_writer.h | 3 +- src/cam/capture.cpp | 42 ++++---- src/cam/capture.h | 5 +- src/libcamera/camera.cpp | 49 +++------ src/libcamera/include/pipeline_handler.h | 5 +- src/libcamera/pipeline/ipu3/ipu3.cpp | 124 +++++++---------------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 41 ++------ src/libcamera/pipeline/uvcvideo.cpp | 22 ++-- src/libcamera/pipeline/vimc.cpp | 22 ++-- src/libcamera/pipeline_handler.cpp | 2 +- src/libcamera/request.cpp | 29 +++--- src/qcam/main_window.cpp | 44 ++++---- src/qcam/main_window.h | 6 +- test/camera/buffer_import.cpp | 19 ++-- test/camera/capture.cpp | 38 ++++--- test/camera/statemachine.cpp | 12 ++- 20 files changed, 209 insertions(+), 306 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index f2827438871189a1..20f48faed62da7a7 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -19,7 +19,7 @@ namespace libcamera { -class Buffer; +class FrameBuffer; class FrameBufferAllocator; class PipelineHandler; class Request; @@ -78,7 +78,7 @@ public: const std::string &name() const; - Signal bufferCompleted; + Signal bufferCompleted; Signal requestCompleted; Signal disconnected; diff --git a/include/libcamera/request.h b/include/libcamera/request.h index a8708010ec1247a2..02c0ef5f6b028bd4 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -17,9 +17,9 @@ namespace libcamera { -class Buffer; class Camera; class CameraControlValidator; +class FrameBuffer; class Stream; class Request @@ -38,9 +38,9 @@ public: ControlList &controls() { return *controls_; } ControlList &metadata() { return *metadata_; } - const std::map &buffers() const { return bufferMap_; } - int addBuffer(Stream *stream, std::unique_ptr buffer); - Buffer *findBuffer(Stream *stream) const; + const std::map &buffers() const { return bufferMap_; } + int addBuffer(Stream *stream, FrameBuffer *buffer); + FrameBuffer *findBuffer(Stream *stream) const; uint64_t cookie() const { return cookie_; } Status status() const { return status_; } @@ -54,14 +54,14 @@ private: int prepare(); void complete(); - bool completeBuffer(Buffer *buffer); + bool completeBuffer(FrameBuffer *buffer); Camera *camera_; CameraControlValidator *validator_; ControlList *controls_; ControlList *metadata_; - std::map bufferMap_; - std::unordered_set pending_; + std::map bufferMap_; + std::unordered_set pending_; const uint64_t cookie_; Status status_; diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index ebe91ea8af4f6436..21417aba19135de5 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -739,13 +739,19 @@ void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reque * and (currently) only supported request buffer. */ const buffer_handle_t camera3Handle = *camera3Buffers[0].buffer; - std::array fds = { - camera3Handle->data[0], - camera3Handle->data[1], - camera3Handle->data[2], - }; - std::unique_ptr buffer = stream->createBuffer(fds); + std::vector planes; + for (int i = 0; i < 3; i++) { + planes[i].fd = FileDescriptor(camera3Handle->data[i]); + /* + * Setting length to zero here is OK as the length is only used + * to map the memory of the plane. Libcamera do not need to poke + * at the memory content queued by the HAL. + */ + planes[i].length = 0; + } + + FrameBuffer *buffer = new FrameBuffer(planes); if (!buffer) { LOG(HAL, Error) << "Failed to create buffer"; delete descriptor; @@ -754,7 +760,7 @@ void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reque Request *request = camera_->createRequest(reinterpret_cast(descriptor)); - request->addBuffer(stream, std::move(buffer)); + request->addBuffer(stream, buffer); int ret = camera_->queueRequest(request); if (ret) { @@ -771,8 +777,8 @@ error: void CameraDevice::requestComplete(Request *request) { - const std::map &buffers = request->buffers(); - Buffer *libcameraBuffer = buffers.begin()->second; + const std::map &buffers = request->buffers(); + FrameBuffer *buffer = buffers.begin()->second; camera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK; std::unique_ptr resultMetadata; @@ -803,11 +809,11 @@ void CameraDevice::requestComplete(Request *request) if (status == CAMERA3_BUFFER_STATUS_OK) { notifyShutter(descriptor->frameNumber, - libcameraBuffer->metadata().timestamp); + buffer->metadata().timestamp); captureResult.partial_result = 1; resultMetadata = getResultMetadata(descriptor->frameNumber, - libcameraBuffer->metadata().timestamp); + buffer->metadata().timestamp); captureResult.result = resultMetadata->get(); } @@ -825,6 +831,7 @@ void CameraDevice::requestComplete(Request *request) callbacks_->process_capture_result(callbacks_, &captureResult); delete descriptor; + delete buffer; } void CameraDevice::notifyShutter(uint32_t frameNumber, uint64_t timestamp) diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp index 7c58a1f50829f290..320ae3332a4db4a6 100644 --- a/src/cam/buffer_writer.cpp +++ b/src/cam/buffer_writer.cpp @@ -22,7 +22,7 @@ BufferWriter::BufferWriter(const std::string &pattern) { } -int BufferWriter::write(Buffer *buffer, const std::string &streamName) +int BufferWriter::write(FrameBuffer *buffer, const std::string &streamName) { std::string filename; size_t pos; @@ -43,8 +43,7 @@ int BufferWriter::write(Buffer *buffer, const std::string &streamName) if (fd == -1) return -errno; - BufferMemory *mem = buffer->mem(); - for (const FrameBuffer::Plane &plane : mem->planes()) { + for (const FrameBuffer::Plane &plane : buffer->planes()) { void *data = mmap(NULL, plane.length, PROT_READ, MAP_SHARED, plane.fd.fd(), 0); unsigned int length = plane.length; diff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h index 7bf785d1e83235ff..5917a7dfb5e28106 100644 --- a/src/cam/buffer_writer.h +++ b/src/cam/buffer_writer.h @@ -16,7 +16,8 @@ class BufferWriter public: BufferWriter(const std::string &pattern = "frame-#.bin"); - int write(libcamera::Buffer *buffer, const std::string &streamName); + int write(libcamera::FrameBuffer *buffer, + const std::string &streamName); private: std::string pattern_; diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index da942f56118983bd..dd078eb0ae4a2c62 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -57,7 +57,10 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options) writer_ = new BufferWriter(); } - ret = capture(loop); + + FrameBufferAllocator *allocator = FrameBufferAllocator::create(camera_); + + ret = capture(loop, allocator); if (options.isSet(OptFile)) { delete writer_; @@ -66,17 +69,27 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options) camera_->freeBuffers(); + delete allocator; + return ret; } -int Capture::capture(EventLoop *loop) +int Capture::capture(EventLoop *loop, FrameBufferAllocator *allocator) { int ret; /* Identify the stream with the least number of buffers. */ unsigned int nbuffers = UINT_MAX; - for (StreamConfiguration &cfg : *config_) - nbuffers = std::min(nbuffers, cfg.bufferCount); + for (StreamConfiguration &cfg : *config_) { + ret = allocator->allocate(cfg.stream()); + if (ret < 0) { + std::cerr << "Can't allocate buffers" << std::endl; + return -ENOMEM; + } + + unsigned int allocated = allocator->buffers(cfg.stream()).size(); + nbuffers = std::min(nbuffers, allocated); + } /* * TODO: make cam tool smarter to support still capture by for @@ -93,9 +106,11 @@ int Capture::capture(EventLoop *loop) for (StreamConfiguration &cfg : *config_) { Stream *stream = cfg.stream(); - std::unique_ptr buffer = stream->createBuffer(i); + const std::vector> &buffers = + allocator->buffers(stream); + const std::unique_ptr &buffer = buffers[i]; - ret = request->addBuffer(stream, std::move(buffer)); + ret = request->addBuffer(stream, buffer.get()); if (ret < 0) { std::cerr << "Can't set buffer for request" << std::endl; @@ -138,7 +153,7 @@ void Capture::requestComplete(Request *request) if (request->status() == Request::RequestCancelled) return; - const std::map &buffers = request->buffers(); + const std::map &buffers = request->buffers(); std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now(); double fps = std::chrono::duration_cast(now - last_).count(); @@ -151,7 +166,7 @@ void Capture::requestComplete(Request *request) for (auto it = buffers.begin(); it != buffers.end(); ++it) { Stream *stream = it->first; - Buffer *buffer = it->second; + FrameBuffer *buffer = it->second; const std::string &name = streamName_[stream]; const FrameMetadata &metadata = buffer->metadata(); @@ -185,16 +200,9 @@ void Capture::requestComplete(Request *request) for (auto it = buffers.begin(); it != buffers.end(); ++it) { Stream *stream = it->first; - Buffer *buffer = it->second; - unsigned int index = buffer->index(); - - std::unique_ptr newBuffer = stream->createBuffer(index); - if (!newBuffer) { - std::cerr << "Can't create buffer" << std::endl; - return; - } + FrameBuffer *buffer = it->second; - request->addBuffer(stream, std::move(newBuffer)); + request->addBuffer(stream, buffer); } camera_->queueRequest(request); diff --git a/src/cam/capture.h b/src/cam/capture.h index c692d48918f2de1d..9bca5661070efcf5 100644 --- a/src/cam/capture.h +++ b/src/cam/capture.h @@ -10,7 +10,9 @@ #include #include +#include #include +#include #include #include @@ -26,7 +28,8 @@ public: int run(EventLoop *loop, const OptionsParser::Options &options); private: - int capture(EventLoop *loop); + int capture(EventLoop *loop, + libcamera::FrameBufferAllocator *allocator); void requestComplete(libcamera::Request *request); diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 222c6811d5698f0f..5ea663ef85720f2e 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -691,12 +691,6 @@ int Camera::configure(CameraConfiguration *config) stream->configuration_ = cfg; activeStreams_.insert(stream); - - /* - * Allocate buffer objects in the pool. - * Memory will be allocated and assigned later. - */ - stream->createBuffers(cfg.memoryType, cfg.bufferCount); } state_ = CameraConfigured; @@ -752,14 +746,6 @@ int Camera::freeBuffers() if (!stateIs(CameraPrepared)) return -EACCES; - for (Stream *stream : activeStreams_) { - /* - * All mappings must be destroyed before buffers can be freed - * by the V4L2 device that has allocated them. - */ - stream->destroyBuffers(); - } - state_ = CameraConfigured; return pipe_->freeBuffers(this, activeStreams_); @@ -825,24 +811,11 @@ int Camera::queueRequest(Request *request) for (auto const &it : request->buffers()) { Stream *stream = it.first; - Buffer *buffer = it.second; if (activeStreams_.find(stream) == activeStreams_.end()) { LOG(Camera, Error) << "Invalid request"; return -EINVAL; } - - if (stream->memoryType() == ExternalMemory) { - int index = stream->mapBuffer(buffer); - if (index < 0) { - LOG(Camera, Error) << "No buffer memory available"; - return -ENOMEM; - } - - buffer->index_ = index; - } - - buffer->mem_ = &stream->buffers()[buffer->index_]; } int ret = request->prepare(); @@ -877,6 +850,14 @@ int Camera::start() LOG(Camera, Debug) << "Starting capture"; + for (Stream * stream : activeStreams_) { + if (allocator_ && !allocator_->buffers(stream).empty()) + continue; + + unsigned int bufferCount = stream->configuration_.bufferCount; + pipe_->importFrameBuffers(this, stream, bufferCount); + } + int ret = pipe_->start(this); if (ret) return ret; @@ -912,6 +893,13 @@ int Camera::stop() pipe_->stop(this); + for (Stream * stream : activeStreams_) { + if (allocator_ && !allocator_->buffers(stream).empty()) + continue; + + pipe_->freeFrameBuffers(this, stream); + } + return 0; } @@ -925,13 +913,6 @@ int Camera::stop() */ void Camera::requestComplete(Request *request) { - for (auto it : request->buffers()) { - Stream *stream = it.first; - Buffer *buffer = it.second; - if (stream->memoryType() == ExternalMemory) - stream->unmapBuffer(buffer); - } - requestCompleted.emit(request); delete request; } diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 520d3fccaaa130cc..20b866c4a511387a 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -20,12 +20,12 @@ namespace libcamera { -class Buffer; class Camera; class CameraConfiguration; class CameraManager; class DeviceEnumerator; class DeviceMatch; +class FrameBuffer; class MediaDevice; class PipelineHandler; class Request; @@ -86,7 +86,8 @@ public: int queueRequest(Camera *camera, Request *request); - bool completeBuffer(Camera *camera, Request *request, Buffer *buffer); + bool completeBuffer(Camera *camera, Request *request, + FrameBuffer *buffer); void completeRequest(Camera *camera, Request *request); const char *name() const { return name_; } diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index f9bddcc88523301f..2f3ba2bf10f2f177 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -31,6 +31,8 @@ namespace libcamera { LOG_DEFINE_CATEGORY(IPU3) +class IPU3CameraData; + class ImgUDevice { public: @@ -44,7 +46,6 @@ public: V4L2VideoDevice *dev; unsigned int pad; std::string name; - BufferPool *pool; std::vector> buffers; }; @@ -71,9 +72,7 @@ public: int configureOutput(ImgUOutput *output, const StreamConfiguration &cfg); - int importOutputBuffers(ImgUOutput *output, BufferPool *pool); - int exportOutputBuffers(ImgUOutput *output, BufferPool *pool); - void freeBuffers(); + void freeBuffers(IPU3CameraData *data); int start(); int stop(); @@ -93,9 +92,6 @@ public: ImgUOutput viewfinder_; ImgUOutput stat_; /* \todo Add param video device for 3A tuning */ - - BufferPool vfPool_; - BufferPool outPool_; }; class CIO2Device @@ -156,7 +152,7 @@ public: { } - void imguOutputBufferReady(Buffer *buffer); + void imguOutputBufferReady(FrameBuffer *buffer); void imguInputBufferReady(FrameBuffer *buffer); void cio2BufferReady(FrameBuffer *buffer); @@ -693,39 +689,30 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, goto error; } - /* Allocate buffers for each active stream. */ - for (Stream *s : streams) { - IPU3Stream *stream = static_cast(s); - ImgUDevice::ImgUOutput *dev = stream->device_; - - if (stream->memoryType() == InternalMemory) - ret = imgu->exportOutputBuffers(dev, &stream->bufferPool()); - else - ret = imgu->importOutputBuffers(dev, &stream->bufferPool()); - if (ret) - goto error; - } - /* * Allocate buffers also on non-active outputs; use the same number * of buffers as the active ones. */ if (!outStream->active_) { - bufferCount = vfStream->configuration().bufferCount; - outStream->device_->pool->createBuffers(bufferCount); - ret = imgu->exportOutputBuffers(outStream->device_, - outStream->device_->pool); - if (ret) + ImgUDevice::ImgUOutput *output = outStream->device_; + + ret = output->dev->exportBuffers(bufferCount, &output->buffers); + if (ret < 0) { + LOG(IPU3, Error) << "Failed to allocate ImgU " + << output->name << " buffers"; goto error; + } } if (!vfStream->active_) { - bufferCount = outStream->configuration().bufferCount; - vfStream->device_->pool->createBuffers(bufferCount); - ret = imgu->exportOutputBuffers(vfStream->device_, - vfStream->device_->pool); - if (ret) + ImgUDevice::ImgUOutput *output = vfStream->device_; + + ret = output->dev->exportBuffers(bufferCount, &output->buffers); + if (ret < 0) { + LOG(IPU3, Error) << "Failed to allocate ImgU " + << output->name << " buffers"; goto error; + } } return 0; @@ -742,7 +729,7 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera, IPU3CameraData *data = cameraData(camera); data->cio2_.freeBuffers(); - data->imgu_->freeBuffers(); + data->imgu_->freeBuffers(data); return 0; } @@ -795,7 +782,7 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request) for (auto it : request->buffers()) { IPU3Stream *stream = static_cast(it.first); - Buffer *buffer = it.second; + FrameBuffer *buffer = it.second; int ret = stream->device_->dev->queueBuffer(buffer); if (ret < 0) @@ -922,9 +909,9 @@ int PipelineHandlerIPU3::registerCameras() &IPU3CameraData::cio2BufferReady); data->imgu_->input_->frameBufferReady.connect(data.get(), &IPU3CameraData::imguInputBufferReady); - data->imgu_->output_.dev->bufferReady.connect(data.get(), + data->imgu_->output_.dev->frameBufferReady.connect(data.get(), &IPU3CameraData::imguOutputBufferReady); - data->imgu_->viewfinder_.dev->bufferReady.connect(data.get(), + data->imgu_->viewfinder_.dev->frameBufferReady.connect(data.get(), &IPU3CameraData::imguOutputBufferReady); /* Create and register the Camera instance. */ @@ -973,7 +960,7 @@ void IPU3CameraData::imguInputBufferReady(FrameBuffer *buffer) * * Buffers completed from the ImgU output are directed to the application. */ -void IPU3CameraData::imguOutputBufferReady(Buffer *buffer) +void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer) { Request *request = buffer->request(); @@ -1048,7 +1035,6 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index) output_.pad = PAD_OUTPUT; output_.name = "output"; - output_.pool = &outPool_; viewfinder_.dev = V4L2VideoDevice::fromEntityName(media, name_ + " viewfinder"); @@ -1058,7 +1044,6 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index) viewfinder_.pad = PAD_VF; viewfinder_.name = "viewfinder"; - viewfinder_.pool = &vfPool_; stat_.dev = V4L2VideoDevice::fromEntityName(media, name_ + " 3a stat"); ret = stat_.dev->open(); @@ -1166,69 +1151,28 @@ int ImgUDevice::configureOutput(ImgUOutput *output, return 0; } -/** - * \brief Export buffers from \a output to the provided \a pool - * \param[in] output The ImgU output device - * \param[in] pool The buffer pool where to export buffers - * - * Export memory buffers reserved in the video device memory associated with - * \a output id to the buffer pool provided as argument. - * - * \return 0 on success or a negative error code otherwise - */ -int ImgUDevice::exportOutputBuffers(ImgUOutput *output, BufferPool *pool) -{ - int ret = output->dev->exportBuffers(pool); - if (ret) { - LOG(IPU3, Error) << "Failed to export ImgU " - << output->name << " buffers"; - return ret; - } - - return 0; -} - -/** - * \brief Reserve buffers in \a output from the provided \a pool - * \param[in] output The ImgU output device - * \param[in] pool The buffer pool used to reserve buffers in \a output - * - * Reserve a number of buffers equal to the number of buffers in \a pool - * in the \a output device. - * - * \return 0 on success or a negative error code otherwise - */ -int ImgUDevice::importOutputBuffers(ImgUOutput *output, BufferPool *pool) -{ - int ret = output->dev->importBuffers(pool); - if (ret) { - LOG(IPU3, Error) - << "Failed to import buffer in " << output->name - << " ImgU device"; - return ret; - } - - return 0; -} - /** * \brief Release buffers for all the ImgU video devices */ -void ImgUDevice::freeBuffers() +void ImgUDevice::freeBuffers(IPU3CameraData *data) { int ret; - ret = output_.dev->releaseBuffers(); - if (ret) - LOG(IPU3, Error) << "Failed to release ImgU output buffers"; + if (!data->outStream_.active_) { + ret = output_.dev->releaseBuffers(); + if (ret) + LOG(IPU3, Error) << "Failed to release ImgU output buffers"; + } ret = stat_.dev->releaseBuffers(); if (ret) LOG(IPU3, Error) << "Failed to release ImgU stat buffers"; - ret = viewfinder_.dev->releaseBuffers(); - if (ret) - LOG(IPU3, Error) << "Failed to release ImgU viewfinder buffers"; + if (!data->vfStream_.active_) { + ret = viewfinder_.dev->releaseBuffers(); + if (ret) + LOG(IPU3, Error) << "Failed to release ImgU viewfinder buffers"; + } ret = input_->releaseBuffers(); if (ret) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index e77925f6f9deff08..cc9c9ff961e948dc 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -51,7 +51,7 @@ struct RkISP1FrameInfo { FrameBuffer *paramBuffer; FrameBuffer *statBuffer; - Buffer *videoBuffer; + FrameBuffer *videoBuffer; bool paramFilled; bool paramDequeued; @@ -67,7 +67,6 @@ public: int destroy(unsigned int frame); RkISP1FrameInfo *find(unsigned int frame); - RkISP1FrameInfo *find(Buffer *buffer); RkISP1FrameInfo *find(FrameBuffer *buffer); RkISP1FrameInfo *find(Request *request); @@ -87,7 +86,7 @@ public: setDelay(QueueBuffers, -1, 10); } - void bufferReady(Buffer *buffer) + void bufferReady(FrameBuffer *buffer) { /* * Calculate SOE by taking the end of DMA set by the kernel and applying @@ -208,7 +207,7 @@ private: int initLinks(); int createCamera(MediaEntity *sensor); void tryCompleteRequest(Request *request); - void bufferReady(Buffer *buffer); + void bufferReady(FrameBuffer *buffer); void paramReady(FrameBuffer *buffer); void statReady(FrameBuffer *buffer); @@ -246,7 +245,7 @@ RkISP1FrameInfo *RkISP1Frames::create(unsigned int frame, Request *request, Stre } FrameBuffer *statBuffer = pipe_->statBuffers_.front(); - Buffer *videoBuffer = request->findBuffer(stream); + FrameBuffer *videoBuffer = request->findBuffer(stream); if (!videoBuffer) { LOG(RkISP1, Error) << "Attempt to queue request with invalid stream"; @@ -299,26 +298,14 @@ RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame) return nullptr; } -RkISP1FrameInfo *RkISP1Frames::find(Buffer *buffer) -{ - for (auto &itInfo : frameInfo_) { - RkISP1FrameInfo *info = itInfo.second; - - if (info->videoBuffer == buffer) - return info; - } - - LOG(RkISP1, Error) << "Can't locate info from buffer"; - return nullptr; -} - RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer) { for (auto &itInfo : frameInfo_) { RkISP1FrameInfo *info = itInfo.second; if (info->paramBuffer == buffer || - info->statBuffer == buffer) + info->statBuffer == buffer || + info->videoBuffer == buffer) return info; } @@ -696,17 +683,8 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera, { RkISP1CameraData *data = cameraData(camera); unsigned int count = 1; - Stream *stream = *streams.begin(); int ret; - if (stream->memoryType() == InternalMemory) - ret = video_->exportBuffers(&stream->bufferPool()); - else - ret = video_->importBuffers(&stream->bufferPool()); - - if (ret) - return ret; - unsigned int maxBuffers = 0; for (const Stream *s : camera->streams()) maxBuffers = std::max(maxBuffers, s->configuration().bufferCount); @@ -772,9 +750,6 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera, if (stat_->releaseBuffers()) LOG(RkISP1, Error) << "Failed to release stat buffers"; - if (video_->releaseBuffers()) - LOG(RkISP1, Error) << "Failed to release video buffers"; - return 0; } @@ -980,7 +955,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) if (param_->open() < 0) return false; - video_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady); + video_->frameBufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady); stat_->frameBufferReady.connect(this, &PipelineHandlerRkISP1::statReady); param_->frameBufferReady.connect(this, &PipelineHandlerRkISP1::paramReady); @@ -1029,7 +1004,7 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request) completeRequest(activeCamera_, request); } -void PipelineHandlerRkISP1::bufferReady(Buffer *buffer) +void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer) { ASSERT(activeCamera_); RkISP1CameraData *data = cameraData(activeCamera_); diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 655c5e6d96a696d6..c29bad707b464bcd 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -41,7 +41,7 @@ public: } int init(MediaEntity *entity); - void bufferReady(Buffer *buffer); + void bufferReady(FrameBuffer *buffer); V4L2VideoDevice *video_; Stream stream_; @@ -226,23 +226,13 @@ void PipelineHandlerUVC::freeFrameBuffers(Camera *camera, Stream *stream) int PipelineHandlerUVC::allocateBuffers(Camera *camera, const std::set &streams) { - UVCCameraData *data = cameraData(camera); - Stream *stream = *streams.begin(); - const StreamConfiguration &cfg = stream->configuration(); - - LOG(UVC, Debug) << "Requesting " << cfg.bufferCount << " buffers"; - - if (stream->memoryType() == InternalMemory) - return data->video_->exportBuffers(&stream->bufferPool()); - else - return data->video_->importBuffers(&stream->bufferPool()); + return 0; } int PipelineHandlerUVC::freeBuffers(Camera *camera, const std::set &streams) { - UVCCameraData *data = cameraData(camera); - return data->video_->releaseBuffers(); + return 0; } int PipelineHandlerUVC::start(Camera *camera) @@ -296,7 +286,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request) { UVCCameraData *data = cameraData(camera); - Buffer *buffer = request->findBuffer(&data->stream_); + FrameBuffer *buffer = request->findBuffer(&data->stream_); if (!buffer) { LOG(UVC, Error) << "Attempt to queue request with invalid stream"; @@ -361,7 +351,7 @@ int UVCCameraData::init(MediaEntity *entity) if (ret) return ret; - video_->bufferReady.connect(this, &UVCCameraData::bufferReady); + video_->frameBufferReady.connect(this, &UVCCameraData::bufferReady); /* Initialise the supported controls. */ const ControlInfoMap &controls = video_->controls(); @@ -401,7 +391,7 @@ int UVCCameraData::init(MediaEntity *entity) return 0; } -void UVCCameraData::bufferReady(Buffer *buffer) +void UVCCameraData::bufferReady(FrameBuffer *buffer) { Request *request = buffer->request(); diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 02235ffb0515982f..33fec6520240b328 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -55,7 +55,7 @@ public: } int init(MediaDevice *media); - void bufferReady(Buffer *buffer); + void bufferReady(FrameBuffer *buffer); CameraSensor *sensor_; V4L2Subdevice *debayer_; @@ -293,23 +293,13 @@ void PipelineHandlerVimc::freeFrameBuffers(Camera *camera, Stream *stream) int PipelineHandlerVimc::allocateBuffers(Camera *camera, const std::set &streams) { - VimcCameraData *data = cameraData(camera); - Stream *stream = *streams.begin(); - const StreamConfiguration &cfg = stream->configuration(); - - LOG(VIMC, Debug) << "Requesting " << cfg.bufferCount << " buffers"; - - if (stream->memoryType() == InternalMemory) - return data->video_->exportBuffers(&stream->bufferPool()); - else - return data->video_->importBuffers(&stream->bufferPool()); + return 0; } int PipelineHandlerVimc::freeBuffers(Camera *camera, const std::set &streams) { - VimcCameraData *data = cameraData(camera); - return data->video_->releaseBuffers(); + return 0; } int PipelineHandlerVimc::start(Camera *camera) @@ -357,7 +347,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) int PipelineHandlerVimc::queueRequestDevice(Camera *camera, Request *request) { VimcCameraData *data = cameraData(camera); - Buffer *buffer = request->findBuffer(&data->stream_); + FrameBuffer *buffer = request->findBuffer(&data->stream_); if (!buffer) { LOG(VIMC, Error) << "Attempt to queue request with invalid stream"; @@ -449,7 +439,7 @@ int VimcCameraData::init(MediaDevice *media) if (video_->open()) return -ENODEV; - video_->bufferReady.connect(this, &VimcCameraData::bufferReady); + video_->frameBufferReady.connect(this, &VimcCameraData::bufferReady); raw_ = new V4L2VideoDevice(media->getEntityByName("Raw Capture 1")); if (raw_->open()) @@ -486,7 +476,7 @@ int VimcCameraData::init(MediaDevice *media) return 0; } -void VimcCameraData::bufferReady(Buffer *buffer) +void VimcCameraData::bufferReady(FrameBuffer *buffer) { Request *request = buffer->request(); diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index fb69ca8e97216e6c..11286d31d4e54213 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -460,7 +460,7 @@ int PipelineHandler::queueRequest(Camera *camera, Request *request) * otherwise */ bool PipelineHandler::completeBuffer(Camera *camera, Request *request, - Buffer *buffer) + FrameBuffer *buffer) { camera->bufferCompleted.emit(request, buffer); return request->completeBuffer(buffer); diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index b17a6c2278361f00..4ac0d9684e5be801 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -75,11 +75,6 @@ Request::Request(Camera *camera, uint64_t cookie) Request::~Request() { - for (auto it : bufferMap_) { - Buffer *buffer = it.second; - delete buffer; - } - delete metadata_; delete controls_; delete validator_; @@ -106,18 +101,18 @@ Request::~Request() * \brief Retrieve the request's streams to buffers map * * Return a reference to the map that associates each Stream part of the - * request to the Buffer the Stream output should be directed to. + * request to the FrameBuffer the Stream output should be directed to. * - * \return The map of Stream to Buffer + * \return The map of Stream to FrameBuffer */ /** - * \brief Store a Buffer with its associated Stream in the Request + * \brief Store a FrameBuffer with its associated Stream in the Request * \param[in] stream The stream the buffer belongs to - * \param[in] buffer The Buffer to store in the request + * \param[in] buffer The FrameBuffer to store in the request * - * Ownership of the buffer is passed to the request. It will be deleted when - * the request is destroyed after completing. + * Ownership of the buffer is passed to the request. Owner will be returned to + * the user when Request complete callback is called. * * A request can only contain one buffer per stream. If a buffer has already * been added to the request for the same stream, this method returns -EEXIST. @@ -126,7 +121,7 @@ Request::~Request() * \retval -EEXIST The request already contains a buffer for the stream * \retval -EINVAL The buffer does not reference a valid Stream */ -int Request::addBuffer(Stream *stream, std::unique_ptr buffer) +int Request::addBuffer(Stream *stream, FrameBuffer *buffer) { if (!stream) { LOG(Request, Error) << "Invalid stream reference"; @@ -135,11 +130,11 @@ int Request::addBuffer(Stream *stream, std::unique_ptr buffer) auto it = bufferMap_.find(stream); if (it != bufferMap_.end()) { - LOG(Request, Error) << "Buffer already set for stream"; + LOG(Request, Error) << "FrameBuffer already set for stream"; return -EEXIST; } - bufferMap_[stream] = buffer.release(); + bufferMap_[stream] = buffer; return 0; } @@ -159,7 +154,7 @@ int Request::addBuffer(Stream *stream, std::unique_ptr buffer) * \return The buffer associated with the stream, or nullptr if the stream is * not part of this request */ -Buffer *Request::findBuffer(Stream *stream) const +FrameBuffer *Request::findBuffer(Stream *stream) const { auto it = bufferMap_.find(stream); if (it == bufferMap_.end()) @@ -219,7 +214,7 @@ int Request::prepare() } for (auto const &pair : bufferMap_) { - Buffer *buffer = pair.second; + FrameBuffer *buffer = pair.second; buffer->request_ = this; pending_.insert(buffer); } @@ -253,7 +248,7 @@ void Request::complete() * \return True if all buffers contained in the request have completed, false * otherwise */ -bool Request::completeBuffer(Buffer *buffer) +bool Request::completeBuffer(FrameBuffer *buffer) { int ret = pending_.erase(buffer); ASSERT(ret == 1); diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 47793702b9aa0ee9..e53ed2801291f2c5 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -23,7 +23,7 @@ using namespace libcamera; MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options) - : options_(options), isCapturing_(false) + : options_(options), allocator_(nullptr), isCapturing_(false) { int ret; @@ -37,8 +37,10 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options) adjustSize(); ret = openCamera(cm); - if (!ret) + if (!ret) { + allocator_ = FrameBufferAllocator::create(camera_); ret = startCapture(); + } if (ret < 0) QTimer::singleShot(0, QCoreApplication::instance(), @@ -49,6 +51,7 @@ MainWindow::~MainWindow() { if (camera_) { stopCapture(); + delete allocator_; camera_->release(); camera_.reset(); } @@ -176,8 +179,14 @@ int MainWindow::startCapture() return ret; } + ret = allocator_->allocate(stream); + if (ret < 0) { + std::cerr << "Failed to allocate capture buffers" << std::endl; + return ret; + } + std::vector requests; - for (unsigned int i = 0; i < cfg.bufferCount; ++i) { + for (const std::unique_ptr &buffer : allocator_->buffers(stream)) { Request *request = camera_->createRequest(); if (!request) { std::cerr << "Can't create request" << std::endl; @@ -185,13 +194,7 @@ int MainWindow::startCapture() goto error; } - std::unique_ptr buffer = stream->createBuffer(i); - if (!buffer) { - std::cerr << "Can't create buffer " << i << std::endl; - goto error; - } - - ret = request->addBuffer(stream, std::move(buffer)); + ret = request->addBuffer(stream, buffer.get()); if (ret < 0) { std::cerr << "Can't set buffer for request" << std::endl; goto error; @@ -254,11 +257,11 @@ void MainWindow::requestComplete(Request *request) if (request->status() == Request::RequestCancelled) return; - const std::map &buffers = request->buffers(); + const std::map &buffers = request->buffers(); framesCaptured_++; - Buffer *buffer = buffers.begin()->second; + FrameBuffer *buffer = buffers.begin()->second; const FrameMetadata &metadata = buffer->metadata(); double fps = metadata.timestamp - lastBufferTime_; @@ -281,28 +284,21 @@ void MainWindow::requestComplete(Request *request) for (auto it = buffers.begin(); it != buffers.end(); ++it) { Stream *stream = it->first; - Buffer *buffer = it->second; - unsigned int index = buffer->index(); - - std::unique_ptr newBuffer = stream->createBuffer(index); - if (!newBuffer) { - std::cerr << "Can't create buffer" << std::endl; - return; - } + FrameBuffer *buffer = it->second; - request->addBuffer(stream, std::move(newBuffer)); + request->addBuffer(stream, buffer); } camera_->queueRequest(request); } -int MainWindow::display(Buffer *buffer) +int MainWindow::display(FrameBuffer *buffer) { - if (buffer->mem()->planes().size() != 1) + if (buffer->planes().size() != 1) return -EINVAL; /* \todo: Once the FrameBuffer is done cache mapped memory. */ - const FrameBuffer::Plane &plane = buffer->mem()->planes().front(); + const FrameBuffer::Plane &plane = buffer->planes().front(); void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED, plane.fd.fd(), 0); diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h index 0786e915ec512255..05cde4ceab5f7ea1 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -14,8 +14,10 @@ #include #include +#include #include #include +#include #include #include "../cam/options.h" @@ -49,7 +51,7 @@ private: void stopCapture(); void requestComplete(Request *request); - int display(Buffer *buffer); + int display(FrameBuffer *buffer); QString title_; QTimer titleTimer_; @@ -57,6 +59,8 @@ private: const OptionsParser::Options &options_; std::shared_ptr camera_; + FrameBufferAllocator *allocator_; + bool isCapturing_; std::unique_ptr config_; diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index 6559a89ce914a183..63e5fb100e49f958 100644 --- a/test/camera/buffer_import.cpp +++ b/test/camera/buffer_import.cpp @@ -117,7 +117,7 @@ public: } protected: - void bufferComplete(Request *request, Buffer *buffer) + void bufferComplete(Request *request, FrameBuffer *buffer) { if (buffer->metadata().status != FrameMetadata::FrameSuccess) return; @@ -130,17 +130,16 @@ protected: if (request->status() != Request::RequestComplete) return; - const std::map &buffers = request->buffers(); + const std::map &buffers = request->buffers(); completeRequestsCount_++; /* Create a new request. */ Stream *stream = buffers.begin()->first; - int dmabuf = buffers.begin()->second->dmabufs()[0]; - std::unique_ptr buffer = stream->createBuffer({ dmabuf, -1, -1 }); + FrameBuffer *buffer = buffers.begin()->second; request = camera_->createRequest(); - request->addBuffer(stream, std::move(buffer)); + request->addBuffer(stream, buffer); camera_->queueRequest(request); } @@ -155,9 +154,6 @@ protected: return TestFail; } - StreamConfiguration &cfg = config_->at(0); - cfg.memoryType = ExternalMemory; - return TestPass; } @@ -188,17 +184,14 @@ protected: return TestFail; std::vector requests; - for (const std::unique_ptr &framebuffer : source.buffers()) { - int dmabuf = framebuffer->planes()[0].fd.fd(); - + for (const std::unique_ptr &buffer : source.buffers()) { Request *request = camera_->createRequest(); if (!request) { std::cout << "Failed to create request" << std::endl; return TestFail; } - std::unique_ptr buffer = stream->createBuffer({ dmabuf, -1, -1 }); - if (request->addBuffer(stream, std::move(buffer))) { + if (request->addBuffer(stream, buffer.get())) { std::cout << "Failed to associating buffer with request" << std::endl; return TestFail; } diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index 0d9ffc476650f414..de879ee4eb1420a6 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -26,7 +26,7 @@ protected: unsigned int completeBuffersCount_; unsigned int completeRequestsCount_; - void bufferComplete(Request *request, Buffer *buffer) + void bufferComplete(Request *request, FrameBuffer *buffer) { if (buffer->metadata().status != FrameMetadata::FrameSuccess) return; @@ -39,17 +39,16 @@ protected: if (request->status() != Request::RequestComplete) return; - const std::map &buffers = request->buffers(); + const std::map &buffers = request->buffers(); completeRequestsCount_++; /* Create a new request. */ Stream *stream = buffers.begin()->first; - Buffer *buffer = buffers.begin()->second; - std::unique_ptr newBuffer = stream->createBuffer(buffer->index()); + FrameBuffer *buffer = buffers.begin()->second; request = camera_->createRequest(); - request->addBuffer(stream, std::move(newBuffer)); + request->addBuffer(stream, buffer); camera_->queueRequest(request); } @@ -64,9 +63,16 @@ protected: return TestFail; } + allocator_ = FrameBufferAllocator::create(camera_); + return TestPass; } + void cleanup() override + { + delete allocator_; + } + int run() override { StreamConfiguration &cfg = config_->at(0); @@ -87,21 +93,20 @@ protected: } Stream *stream = cfg.stream(); + + int ret = allocator_->allocate(stream); + if (ret < 0) + return TestFail; + std::vector requests; - for (unsigned int i = 0; i < cfg.bufferCount; ++i) { + for (const std::unique_ptr &buffer : allocator_->buffers(stream)) { Request *request = camera_->createRequest(); if (!request) { cout << "Failed to create request" << endl; return TestFail; } - std::unique_ptr buffer = stream->createBuffer(i); - if (!buffer) { - cout << "Failed to create buffer " << i << endl; - return TestFail; - } - - if (request->addBuffer(stream, std::move(buffer))) { + if (request->addBuffer(stream, buffer.get())) { cout << "Failed to associating buffer with request" << endl; return TestFail; } @@ -134,10 +139,12 @@ protected: while (timer.isRunning()) dispatcher->processEvents(); - if (completeRequestsCount_ <= cfg.bufferCount * 2) { + unsigned int nbuffers = allocator_->buffers(stream).size(); + + if (completeRequestsCount_ <= nbuffers * 2) { cout << "Failed to capture enough frames (got " << completeRequestsCount_ << " expected at least " - << cfg.bufferCount * 2 << ")" << endl; + << nbuffers * 2 << ")" << endl; return TestFail; } @@ -160,6 +167,7 @@ protected: } std::unique_ptr config_; + FrameBufferAllocator *allocator_; }; } /* namespace */ diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index f627b8f37422350e..f3a7ca7c32a5ec97 100644 --- a/test/camera/statemachine.cpp +++ b/test/camera/statemachine.cpp @@ -185,6 +185,12 @@ protected: if (camera_->allocateBuffers()) return TestFail; + /* Use internally allocated buffers. */ + allocator_ = FrameBufferAllocator::create(camera_); + Stream *stream = *camera_->streams().begin(); + if (allocator_->allocate(stream) < 0) + return TestFail; + if (camera_->start()) return TestFail; @@ -218,8 +224,7 @@ protected: return TestFail; Stream *stream = *camera_->streams().begin(); - std::unique_ptr buffer = stream->createBuffer(0); - if (request->addBuffer(stream, std::move(buffer))) + if (request->addBuffer(stream, allocator_->buffers(stream)[0].get())) return TestFail; if (camera_->queueRequest(request)) @@ -229,6 +234,8 @@ protected: if (camera_->stop()) return TestFail; + delete allocator_; + if (camera_->freeBuffers()) return TestFail; @@ -283,6 +290,7 @@ protected: } std::unique_ptr defconf_; + FrameBufferAllocator *allocator_; }; } /* namespace */ From patchwork Mon Dec 30 12:05:06 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2471 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C0E79605D6 for ; Mon, 30 Dec 2019 13:06:10 +0100 (CET) X-Halon-ID: c399f91a-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id c399f91a-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:06:09 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:05:06 +0100 Message-Id: <20191230120510.938333-22-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 21/25] libcamera: v4l2_videodevice: Remove Buffer interface X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:06:11 -0000 The Buffer interface is no longer in use and can be removed. While doing so clean up the two odd names (dequeueFrameBuffer() and queuedFrameBuffers_) that had to be used when adding the FrameBuffer interface. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/include/v4l2_videodevice.h | 19 +- src/libcamera/pipeline/ipu3/ipu3.cpp | 8 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 6 +- src/libcamera/pipeline/uvcvideo.cpp | 2 +- src/libcamera/pipeline/vimc.cpp | 2 +- src/libcamera/v4l2_videodevice.cpp | 337 +---------------------- test/v4l2_videodevice/buffer_sharing.cpp | 4 +- test/v4l2_videodevice/capture_async.cpp | 2 +- test/v4l2_videodevice/v4l2_m2mdevice.cpp | 4 +- 9 files changed, 30 insertions(+), 354 deletions(-) diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index 97a79e56c647e7a6..ba03a087f918c2a7 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -194,19 +194,13 @@ public: int setFormat(V4L2DeviceFormat *format); ImageFormats formats(); - int exportBuffers(BufferPool *pool); - int importBuffers(BufferPool *pool); int exportBuffers(unsigned int count, std::vector> *buffers); int importBuffers(unsigned int count); int releaseBuffers(); - int queueBuffer(Buffer *buffer); - std::vector> queueAllBuffers(); - Signal bufferReady; int queueBuffer(FrameBuffer *buffer); - /* todo: Rename to bufferReady when the Buffer version is removed */ - Signal frameBufferReady; + Signal bufferReady; int streamOn(); int streamOff(); @@ -235,26 +229,19 @@ private: std::vector enumSizes(unsigned int pixelFormat); int requestBuffers(unsigned int count); - int createPlane(BufferMemory *buffer, unsigned int index, - unsigned int plane, unsigned int length); std::unique_ptr createFrameBuffer(const struct v4l2_buffer &buf); int exportDmabufFd(unsigned int index, unsigned int plane); - Buffer *dequeueBuffer(); void bufferAvailable(EventNotifier *notifier); - /* todo: Rename to dequeueBuffer() when the Buffer version is removed */ - FrameBuffer *dequeueFrameBuffer(); + FrameBuffer *dequeuBuffer(); V4L2Capability caps_; enum v4l2_buf_type bufferType_; enum v4l2_memory memoryType_; - BufferPool *bufferPool_; V4L2BufferCache *cache_; - std::map queuedBuffers_; - /* todo: Rename to queuedBuffers_ when the Buffer version is removed */ - std::map queuedFrameBuffers_; + std::map queuedBuffers_; EventNotifier *fdEvent_; }; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 2f3ba2bf10f2f177..ea02a4e5f7f04718 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -905,13 +905,13 @@ int PipelineHandlerIPU3::registerCameras() * associated ImgU input where they get processed and * returned through the ImgU main and secondary outputs. */ - data->cio2_.output_->frameBufferReady.connect(data.get(), + data->cio2_.output_->bufferReady.connect(data.get(), &IPU3CameraData::cio2BufferReady); - data->imgu_->input_->frameBufferReady.connect(data.get(), + data->imgu_->input_->bufferReady.connect(data.get(), &IPU3CameraData::imguInputBufferReady); - data->imgu_->output_.dev->frameBufferReady.connect(data.get(), + data->imgu_->output_.dev->bufferReady.connect(data.get(), &IPU3CameraData::imguOutputBufferReady); - data->imgu_->viewfinder_.dev->frameBufferReady.connect(data.get(), + data->imgu_->viewfinder_.dev->bufferReady.connect(data.get(), &IPU3CameraData::imguOutputBufferReady); /* Create and register the Camera instance. */ diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index cc9c9ff961e948dc..7ed729fb7974d6ea 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -955,9 +955,9 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) if (param_->open() < 0) return false; - video_->frameBufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady); - stat_->frameBufferReady.connect(this, &PipelineHandlerRkISP1::statReady); - param_->frameBufferReady.connect(this, &PipelineHandlerRkISP1::paramReady); + video_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady); + stat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady); + param_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady); /* Configure default links. */ if (initLinks() < 0) { diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index c29bad707b464bcd..1eb809914f9aa0dc 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -351,7 +351,7 @@ int UVCCameraData::init(MediaEntity *entity) if (ret) return ret; - video_->frameBufferReady.connect(this, &UVCCameraData::bufferReady); + video_->bufferReady.connect(this, &UVCCameraData::bufferReady); /* Initialise the supported controls. */ const ControlInfoMap &controls = video_->controls(); diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 33fec6520240b328..13a7fc6cb5294eec 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -439,7 +439,7 @@ int VimcCameraData::init(MediaDevice *media) if (video_->open()) return -ENODEV; - video_->frameBufferReady.connect(this, &VimcCameraData::bufferReady); + video_->bufferReady.connect(this, &VimcCameraData::bufferReady); raw_ = new V4L2VideoDevice(media->getEntityByName("Raw Capture 1")); if (raw_->open()) diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index b380f08c1e880427..2aeaabf3c6ca1733 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -370,8 +370,7 @@ const std::string V4L2DeviceFormat::toString() const * \param[in] deviceNode The file-system path to the video device node */ V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode) - : V4L2Device(deviceNode), bufferPool_(nullptr), cache_(nullptr), - fdEvent_(nullptr) + : V4L2Device(deviceNode), cache_(nullptr), fdEvent_(nullptr) { /* * We default to an MMAP based CAPTURE video device, however this will @@ -930,115 +929,6 @@ int V4L2VideoDevice::requestBuffers(unsigned int count) return 0; } -/** - * \brief Request buffers to be allocated from the video device and stored in - * the buffer pool provided. - * \param[out] pool BufferPool to populate with buffers - * \return 0 on success or a negative error code otherwise - */ -int V4L2VideoDevice::exportBuffers(BufferPool *pool) -{ - unsigned int i; - int ret; - - memoryType_ = V4L2_MEMORY_MMAP; - - ret = requestBuffers(pool->count()); - if (ret) - return ret; - - /* Map the buffers. */ - for (i = 0; i < pool->count(); ++i) { - struct v4l2_plane planes[VIDEO_MAX_PLANES] = {}; - struct v4l2_buffer buf = {}; - BufferMemory &buffer = pool->buffers()[i]; - - buf.index = i; - buf.type = bufferType_; - buf.memory = memoryType_; - buf.length = VIDEO_MAX_PLANES; - buf.m.planes = planes; - - ret = ioctl(VIDIOC_QUERYBUF, &buf); - if (ret < 0) { - LOG(V4L2, Error) - << "Unable to query buffer " << i << ": " - << strerror(-ret); - break; - } - - if (V4L2_TYPE_IS_MULTIPLANAR(buf.type)) { - for (unsigned int p = 0; p < buf.length; ++p) { - ret = createPlane(&buffer, i, p, - buf.m.planes[p].length); - if (ret) - break; - } - } else { - ret = createPlane(&buffer, i, 0, buf.length); - } - - if (ret) { - LOG(V4L2, Error) << "Failed to create plane"; - break; - } - } - - if (ret) { - requestBuffers(0); - pool->destroyBuffers(); - return ret; - } - - bufferPool_ = pool; - - return 0; -} - -int V4L2VideoDevice::createPlane(BufferMemory *buffer, unsigned int index, - unsigned int planeIndex, unsigned int length) -{ - int fd; - - LOG(V4L2, Debug) - << "Buffer " << index - << " plane " << planeIndex - << ": length=" << length; - - fd = exportDmabufFd(index, planeIndex); - if (fd < 0) - return fd; - - FrameBuffer::Plane plane; - plane.fd = FileDescriptor(fd); - plane.length = length; - buffer->planes().emplace_back(plane); - ::close(fd); - - return 0; -} - -/** - * \brief Import the externally allocated \a pool of buffers - * \param[in] pool BufferPool of buffers to import - * \return 0 on success or a negative error code otherwise - */ -int V4L2VideoDevice::importBuffers(BufferPool *pool) -{ - int ret; - - memoryType_ = V4L2_MEMORY_DMABUF; - - ret = requestBuffers(pool->count()); - if (ret) - return ret; - - LOG(V4L2, Debug) << "provided pool of " << pool->count() << " buffers"; - bufferPool_ = pool; - - return 0; -} - /** * \brief Allocate buffers from the video device * \param[in] count Number of buffers to allocate @@ -1180,7 +1070,6 @@ int V4L2VideoDevice::releaseBuffers() { LOG(V4L2, Debug) << "Releasing buffers"; - bufferPool_ = nullptr; delete cache_; cache_ = nullptr; @@ -1200,119 +1089,6 @@ int V4L2VideoDevice::releaseBuffers() * * \return 0 on success or a negative error code otherwise */ -int V4L2VideoDevice::queueBuffer(Buffer *buffer) -{ - struct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {}; - struct v4l2_buffer buf = {}; - int ret; - - buf.index = buffer->index(); - buf.type = bufferType_; - buf.memory = memoryType_; - buf.field = V4L2_FIELD_NONE; - - bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); - BufferMemory *mem = &bufferPool_->buffers()[buf.index]; - const std::vector &planes = mem->planes(); - - if (buf.memory == V4L2_MEMORY_DMABUF) { - if (multiPlanar) { - for (unsigned int p = 0; p < planes.size(); ++p) - v4l2Planes[p].m.fd = planes[p].fd.fd(); - } else { - buf.m.fd = planes[0].fd.fd(); - } - } - - if (multiPlanar) { - buf.length = planes.size(); - buf.m.planes = v4l2Planes; - } - - if (V4L2_TYPE_IS_OUTPUT(buf.type)) { - const FrameMetadata &metadata = buffer->metadata(); - - buf.bytesused = metadata.planes[0].bytesused; - buf.sequence = metadata.sequence; - buf.timestamp.tv_sec = metadata.timestamp / 1000000000; - buf.timestamp.tv_usec = (metadata.timestamp / 1000) % 1000000; - } - - LOG(V4L2, Debug) << "Queueing buffer " << buf.index; - - ret = ioctl(VIDIOC_QBUF, &buf); - if (ret < 0) { - LOG(V4L2, Error) - << "Failed to queue buffer " << buf.index << ": " - << strerror(-ret); - return ret; - } - - if (queuedBuffers_.empty()) - fdEvent_->setEnabled(true); - - queuedBuffers_[buf.index] = buffer; - - return 0; -} - -/** - * \brief Queue all buffers into the video device - * - * When starting video capture users of the video device often need to queue - * all allocated buffers to the device. This helper method simplifies the - * implementation of the user by queuing all buffers and returning a vector of - * Buffer instances for each queued buffer. - * - * This method is meant to be used with video capture devices internal to a - * pipeline handler, such as ISP statistics capture devices, or raw CSI-2 - * receivers. For video capture devices facing applications, buffers shall - * instead be queued when requests are received, and for video output devices, - * buffers shall be queued when frames are ready to be output. - * - * The caller shall ensure that the returned buffers vector remains valid until - * all the queued buffers are dequeued, either during capture, or by stopping - * the video device. - * - * Calling this method on an output device or on a device that has buffers - * already queued is an error and will return an empty vector. - * - * \return A vector of queued buffers, which will be empty if an error occurs - */ -std::vector> V4L2VideoDevice::queueAllBuffers() -{ - int ret; - - if (!queuedBuffers_.empty()) - return {}; - - if (V4L2_TYPE_IS_OUTPUT(bufferType_)) - return {}; - - std::vector> buffers; - - for (unsigned int i = 0; i < bufferPool_->count(); ++i) { - Buffer *buffer = new Buffer(i); - buffers.emplace_back(buffer); - ret = queueBuffer(buffer); - if (ret) - return {}; - } - - return buffers; -} - -/** - * \brief Queue a buffer into the video device - * \param[in] buffer The buffer to be queued - * - * For capture video devices the \a buffer will be filled with data by the - * device. For output video devices the \a buffer shall contain valid data and - * will be processed by the device. Once the device has finished processing the - * buffer, it will be available for dequeue. - * - * \return 0 on success or a negative error code otherwise - */ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) { struct v4l2_plane v4l2Planes[VIDEO_MAX_PLANES] = {}; @@ -1372,66 +1148,14 @@ int V4L2VideoDevice::queueBuffer(FrameBuffer *buffer) return ret; } - if (queuedFrameBuffers_.empty()) + if (queuedBuffers_.empty()) fdEvent_->setEnabled(true); - queuedFrameBuffers_[buf.index] = buffer; + queuedBuffers_[buf.index] = buffer; return 0; } -/** - * \brief Dequeue the next available buffer from the video device - * - * This method dequeues the next available buffer from the device. If no buffer - * is available to be dequeued it will return nullptr immediately. - * - * \return A pointer to the dequeued buffer on success, or nullptr otherwise - */ -Buffer *V4L2VideoDevice::dequeueBuffer() -{ - struct v4l2_buffer buf = {}; - struct v4l2_plane planes[VIDEO_MAX_PLANES] = {}; - int ret; - - buf.type = bufferType_; - buf.memory = memoryType_; - - if (V4L2_TYPE_IS_MULTIPLANAR(buf.type)) { - buf.length = VIDEO_MAX_PLANES; - buf.m.planes = planes; - } - - ret = ioctl(VIDIOC_DQBUF, &buf); - if (ret < 0) { - LOG(V4L2, Error) - << "Failed to dequeue buffer: " << strerror(-ret); - return nullptr; - } - - LOG(V4L2, Debug) << "Dequeuing buffer " << buf.index; - ASSERT(buf.index < bufferPool_->count()); - - auto it = queuedBuffers_.find(buf.index); - Buffer *buffer = it->second; - queuedBuffers_.erase(it); - - if (queuedBuffers_.empty()) - fdEvent_->setEnabled(false); - - buffer->index_ = buf.index; - - buffer->metadata_.status = buf.flags & V4L2_BUF_FLAG_ERROR - ? FrameMetadata::FrameError - : FrameMetadata::FrameSuccess; - buffer->metadata_.sequence = buf.sequence; - buffer->metadata_.timestamp = buf.timestamp.tv_sec * 1000000000ULL - + buf.timestamp.tv_usec * 1000ULL; - buffer->metadata_.planes = { { buf.bytesused } }; - - return buffer; -} - /** * \brief Slot to handle completed buffer events from the V4L2 video device * \param[in] notifier The event notifier @@ -1444,30 +1168,12 @@ Buffer *V4L2VideoDevice::dequeueBuffer() */ void V4L2VideoDevice::bufferAvailable(EventNotifier *notifier) { - /** - * This is a hack which allows both Buffer and FrameBuffer interfaces - * to work with the same code base. This allows different parts of - * libcamera to migrate to FrameBuffer in different patches. - * - * If we have a cache_ we know FrameBuffer is in use. - * - * \todo: Remove this hack when the Buffer interface is removed. - */ - if (cache_) { - FrameBuffer *buffer = dequeueFrameBuffer(); - if (!buffer) - return; - - /* Notify anyone listening to the device. */ - frameBufferReady.emit(buffer); - } else { - Buffer *buffer = dequeueBuffer(); - if (!buffer) - return; + FrameBuffer *buffer = dequeuBuffer(); + if (!buffer) + return; - /* Notify anyone listening to the device. */ - bufferReady.emit(buffer); - } + /* Notify anyone listening to the device. */ + bufferReady.emit(buffer); } /** @@ -1476,11 +1182,9 @@ void V4L2VideoDevice::bufferAvailable(EventNotifier *notifier) * This method dequeues the next available buffer from the device. If no buffer * is available to be dequeued it will return nullptr immediately. * - * \todo: Reanme to dequeueBuffer() once the FrameBuffer transition is complete - * * \return A pointer to the dequeued buffer on success, or nullptr otherwise */ -FrameBuffer *V4L2VideoDevice::dequeueFrameBuffer() +FrameBuffer *V4L2VideoDevice::dequeuBuffer() { struct v4l2_buffer buf = {}; struct v4l2_plane planes[VIDEO_MAX_PLANES] = {}; @@ -1507,11 +1211,11 @@ FrameBuffer *V4L2VideoDevice::dequeueFrameBuffer() cache_->put(buf.index); - auto it = queuedFrameBuffers_.find(buf.index); + auto it = queuedBuffers_.find(buf.index); FrameBuffer *buffer = it->second; - queuedFrameBuffers_.erase(it); + queuedBuffers_.erase(it); - if (queuedFrameBuffers_.empty()) + if (queuedBuffers_.empty()) fdEvent_->setEnabled(false); buffer->metadata_.status = buf.flags & V4L2_BUF_FLAG_ERROR @@ -1534,11 +1238,6 @@ FrameBuffer *V4L2VideoDevice::dequeueFrameBuffer() /** * \var V4L2VideoDevice::bufferReady - * \brief A Signal emitted when a buffer completes - */ - -/** - * \var V4L2VideoDevice::frameBufferReady * \brief A Signal emitted when a framebuffer completes */ @@ -1583,23 +1282,13 @@ int V4L2VideoDevice::streamOff() /* Send back all queued buffers. */ for (auto it : queuedBuffers_) { - unsigned int index = it.first; - Buffer *buffer = it.second; - - buffer->index_ = index; - buffer->cancel(); - bufferReady.emit(buffer); - } - - for (auto it : queuedFrameBuffers_) { FrameBuffer *buffer = it.second; buffer->metadata_.status = FrameMetadata::FrameCancelled; - frameBufferReady.emit(buffer); + bufferReady.emit(buffer); } queuedBuffers_.clear(); - queuedFrameBuffers_.clear(); fdEvent_->setEnabled(false); return 0; diff --git a/test/v4l2_videodevice/buffer_sharing.cpp b/test/v4l2_videodevice/buffer_sharing.cpp index 6acb06a24b47f653..fefa969a5f3926a2 100644 --- a/test/v4l2_videodevice/buffer_sharing.cpp +++ b/test/v4l2_videodevice/buffer_sharing.cpp @@ -120,8 +120,8 @@ protected: Timer timeout; int ret; - capture_->frameBufferReady.connect(this, &BufferSharingTest::captureBufferReady); - output_->frameBufferReady.connect(this, &BufferSharingTest::outputBufferReady); + capture_->bufferReady.connect(this, &BufferSharingTest::captureBufferReady); + output_->bufferReady.connect(this, &BufferSharingTest::outputBufferReady); for (const std::unique_ptr &buffer : buffers_) { if (capture_->queueBuffer(buffer.get())) { diff --git a/test/v4l2_videodevice/capture_async.cpp b/test/v4l2_videodevice/capture_async.cpp index a57abed3bd0debc1..6a103a035f3d4635 100644 --- a/test/v4l2_videodevice/capture_async.cpp +++ b/test/v4l2_videodevice/capture_async.cpp @@ -42,7 +42,7 @@ protected: if (ret < 0) return TestFail; - capture_->frameBufferReady.connect(this, &CaptureAsyncTest::receiveBuffer); + capture_->bufferReady.connect(this, &CaptureAsyncTest::receiveBuffer); for (const std::unique_ptr &buffer : buffers_) { if (capture_->queueBuffer(buffer.get())) { diff --git a/test/v4l2_videodevice/v4l2_m2mdevice.cpp b/test/v4l2_videodevice/v4l2_m2mdevice.cpp index 43b99c4f7ea9bf26..203afc4fc0339e24 100644 --- a/test/v4l2_videodevice/v4l2_m2mdevice.cpp +++ b/test/v4l2_videodevice/v4l2_m2mdevice.cpp @@ -124,8 +124,8 @@ protected: return TestFail; } - capture->frameBufferReady.connect(this, &V4L2M2MDeviceTest::receiveCaptureBuffer); - output->frameBufferReady.connect(this, &V4L2M2MDeviceTest::outputBufferComplete); + capture->bufferReady.connect(this, &V4L2M2MDeviceTest::receiveCaptureBuffer); + output->bufferReady.connect(this, &V4L2M2MDeviceTest::outputBufferComplete); for (const std::unique_ptr &buffer : captureBuffers_) { if (capture->queueBuffer(buffer.get())) { From patchwork Mon Dec 30 12:05:07 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2472 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 57AFF6046A for ; Mon, 30 Dec 2019 13:06:12 +0100 (CET) X-Halon-ID: c47032a7-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id c47032a7-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:06:10 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:05:07 +0100 Message-Id: <20191230120510.938333-23-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 22/25] libcamera: Remove dead code after switch to FrameBuffer X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:06:12 -0000 Delete all dead code after switching to the FrameBuffer interface. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- include/libcamera/buffer.h | 61 --------- include/libcamera/stream.h | 23 ---- src/android/camera_device.cpp | 1 - src/libcamera/buffer.cpp | 189 ------------------------- src/libcamera/stream.cpp | 250 +--------------------------------- 5 files changed, 2 insertions(+), 522 deletions(-) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 8bd61f786748af5f..9d4443cf56d4b60b 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -7,7 +7,6 @@ #ifndef __LIBCAMERA_BUFFER_H__ #define __LIBCAMERA_BUFFER_H__ -#include #include #include @@ -16,7 +15,6 @@ namespace libcamera { class Request; -class Stream; struct FrameMetadata { enum Status { @@ -65,65 +63,6 @@ private: unsigned int cookie_; }; -class BufferMemory final -{ -public: - const std::vector &planes() const { return planes_; } - std::vector &planes() { return planes_; } - -private: - std::vector planes_; -}; - -class BufferPool final -{ -public: - ~BufferPool(); - - void createBuffers(unsigned int count); - void destroyBuffers(); - - unsigned int count() const { return buffers_.size(); } - std::vector &buffers() { return buffers_; } - -private: - std::vector buffers_; -}; - -class Buffer final -{ -public: - Buffer(unsigned int index = -1, const Buffer *metadata = nullptr); - Buffer(const Buffer &) = delete; - Buffer &operator=(const Buffer &) = delete; - - unsigned int index() const { return index_; } - const std::array &dmabufs() const { return dmabuf_; } - BufferMemory *mem() { return mem_; } - - const FrameMetadata &metadata() const { return metadata_; }; - - Request *request() const { return request_; } - Stream *stream() const { return stream_; } - -private: - friend class Camera; - friend class Request; - friend class Stream; - friend class V4L2VideoDevice; - - void cancel(); - - unsigned int index_; - std::array dmabuf_; - BufferMemory *mem_; - - FrameMetadata metadata_; - - Request *request_; - Stream *stream_; -}; - } /* namespace libcamera */ #endif /* __LIBCAMERA_BUFFER_H__ */ diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index a404eccf34d9c93b..29a8030dff71d58f 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -36,11 +36,6 @@ private: std::map> formats_; }; -enum MemoryType { - InternalMemory, - ExternalMemory, -}; - struct StreamConfiguration { StreamConfiguration(); StreamConfiguration(const StreamFormats &formats); @@ -48,7 +43,6 @@ struct StreamConfiguration { PixelFormat pixelFormat; Size size; - MemoryType memoryType; unsigned int bufferCount; Stream *stream() const { return stream_; } @@ -75,29 +69,12 @@ class Stream public: Stream(); - std::unique_ptr createBuffer(unsigned int index); - std::unique_ptr createBuffer(const std::array &fds); - - BufferPool &bufferPool() { return bufferPool_; } - std::vector &buffers() { return bufferPool_.buffers(); } const StreamConfiguration &configuration() const { return configuration_; } - MemoryType memoryType() const { return memoryType_; } protected: friend class Camera; - int mapBuffer(const Buffer *buffer); - void unmapBuffer(const Buffer *buffer); - - void createBuffers(MemoryType memory, unsigned int count); - void destroyBuffers(); - - BufferPool bufferPool_; StreamConfiguration configuration_; - MemoryType memoryType_; - -private: - std::vector, unsigned int>> bufferCache_; }; } /* namespace libcamera */ diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 21417aba19135de5..cf2cd2ea9d1aa619 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -641,7 +641,6 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list) StreamConfiguration *streamConfiguration = &config_->at(0); streamConfiguration->size.width = camera3Stream->width; streamConfiguration->size.height = camera3Stream->height; - streamConfiguration->memoryType = ExternalMemory; /* * \todo We'll need to translate from Android defined pixel format codes diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 686d0412bf54b2c1..7200a24d4b90cc88 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -85,195 +85,6 @@ LOG_DEFINE_CATEGORY(Buffer) * \brief Array holding plane specific metadata */ -/** - * \class BufferMemory - * \brief A memory buffer to store an image - * - * The BufferMemory class represents the memory buffers used to store full frame - * images, which may contain multiple separate memory Plane objects if the - * image format is multi-planar. - */ - -/** - * \fn BufferMemory::planes() const - * \brief Retrieve the planes within the buffer - * \return A const reference to a vector holding all planes within the buffer - */ - -/** - * \fn BufferMemory::planes() - * \brief Retrieve the planes within the buffer - * \return A reference to a vector holding all planes within the buffer - */ - -/** - * \class BufferPool - * \brief A pool of buffers - * - * The BufferPool class groups together a collection of Buffers to store frames. - * The buffers must be exported by a device before they can be imported into - * another device for further use. - */ - -BufferPool::~BufferPool() -{ - destroyBuffers(); -} - -/** - * \brief Create buffers in the Pool - * \param[in] count The number of buffers to create - */ -void BufferPool::createBuffers(unsigned int count) -{ - buffers_.resize(count); -} - -/** - * \brief Release all buffers from pool - * - * If no buffers have been created or if buffers have already been released no - * operation is performed. - */ -void BufferPool::destroyBuffers() -{ - buffers_.resize(0); -} - -/** - * \fn BufferPool::count() - * \brief Retrieve the number of buffers contained within the pool - * \return The number of buffers contained in the pool - */ - -/** - * \fn BufferPool::buffers() - * \brief Retrieve all the buffers in the pool - * \return A vector containing all the buffers in the pool. - */ - -/** - * \class Buffer - * \brief A buffer handle and dynamic metadata - * - * The Buffer class references a buffer memory and associates dynamic metadata - * related to the frame contained in the buffer. It allows referencing buffer - * memory through a single interface regardless of whether the memory is - * allocated internally in libcamera or provided externally through dmabuf. - * - * Buffer instances are allocated dynamically for a stream through - * Stream::createBuffer(), added to a request with Request::addBuffer() and - * deleted automatically after the request complete handler returns. - */ - -/** - * \brief Construct a buffer not associated with any stream - * - * This method constructs an orphaned buffer not associated with any stream. It - * is not meant to be called by applications, they should instead create buffers - * for a stream with Stream::createBuffer(). - */ -Buffer::Buffer(unsigned int index, const Buffer *metadata) - : index_(index), dmabuf_({ -1, -1, -1 }), request_(nullptr), - stream_(nullptr) -{ - metadata_.status = FrameMetadata::FrameSuccess; - - if (metadata) { - metadata_.sequence = metadata->metadata().sequence; - metadata_.timestamp = metadata->metadata().timestamp; - metadata_.planes = metadata->metadata().planes; - } else { - metadata_.sequence = 0; - metadata_.timestamp = 0; - metadata_.planes = { { 0 } }; - } -} - -/** - * \fn Buffer::index() - * \brief Retrieve the Buffer index - * \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::mem() - * \brief Retrieve the BufferMemory this buffer is associated with - * - * The association between the buffer and a BufferMemory instance is valid from - * the time the request containing this buffer is queued to a camera to the end - * of that request's completion handler. - * - * \return The BufferMemory this buffer is associated with - */ - -/** - * \fn Buffer::metadata() - * \brief Retrieve the buffer metadata - * - * The buffer metadata is update every time the buffer contained are changed, - * for example when it is dequeued from hardware. - * - * \return Metadata for the buffer - */ - -/** - * \fn Buffer::request() - * \brief Retrieve the request this buffer belongs to - * - * The intended callers of this method are buffer completion handlers that - * need to associate a buffer to the request it belongs to. - * - * A Buffer is associated to a request by Request::prepare() and the - * association is valid until the buffer completes. The returned request - * pointer is valid only during that interval. - * - * \return The Request the Buffer belongs to, or nullptr if the buffer is - * either completed or not associated with a request - */ - -/** - * \fn Buffer::stream() - * \brief Retrieve the stream this buffer is associated with - * - * A Buffer is associated to the stream that created it with - * Stream::createBuffer() and the association is valid until the buffer is - * destroyed. Buffer instances that are created directly are not associated - * with any stream. - * - * \return The Stream the Buffer is associated with, or nullptr if the buffer - * is not associated with a stream - */ - -/** - * \brief Mark a buffer as cancel by setting its status to BufferCancelled - */ -void Buffer::cancel() -{ - metadata_.status = FrameMetadata::FrameCancelled; - metadata_.sequence = 0; - metadata_.timestamp = 0; - metadata_.planes = {}; -} - -/** - * \var Buffer::request_ - * \brief The request this buffer belongs to - * - * This member is intended to be set by Request::prepare() and - * Request::completeBuffer(). - */ - /** * \class FrameBuffer * \brief A buffer handle and dynamic metadata diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index a6adc0de5da40063..13789e9eb344f95c 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -266,17 +266,6 @@ SizeRange StreamFormats::range(PixelFormat pixelformat) const return range; } -/** - * \enum MemoryType - * \brief Define the memory type used by a Stream - * \var MemoryType::InternalMemory - * The Stream uses memory allocated internally by the library and exported to - * applications. - * \var MemoryType::ExternalMemory - * The Stream uses memory allocated externally by application and imported in - * the library. - */ - /** * \struct StreamConfiguration * \brief Configuration parameters for a stream @@ -290,7 +279,7 @@ SizeRange StreamFormats::range(PixelFormat pixelformat) const * handlers provied StreamFormats. */ StreamConfiguration::StreamConfiguration() - : pixelFormat(0), memoryType(InternalMemory), stream_(nullptr) + : pixelFormat(0), stream_(nullptr) { } @@ -298,8 +287,7 @@ StreamConfiguration::StreamConfiguration() * \brief Construct a configuration with stream formats */ StreamConfiguration::StreamConfiguration(const StreamFormats &formats) - : pixelFormat(0), memoryType(InternalMemory), stream_(nullptr), - formats_(formats) + : pixelFormat(0), stream_(nullptr), formats_(formats) { } @@ -313,11 +301,6 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats) * \brief Stream pixel format */ -/** - * \var StreamConfiguration::memoryType - * \brief The memory type the stream shall use - */ - /** * \var StreamConfiguration::bufferCount * \brief Requested number of buffers to allocate for the stream @@ -420,236 +403,12 @@ Stream::Stream() { } -/** - * \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 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; - } - - Buffer *buffer = new Buffer(); - buffer->index_ = index; - buffer->stream_ = this; - - return std::unique_ptr(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 ExternalMemory type. It - * will return a null pointer when called on streams using the InternalMemory - * type. - * - * \sa Stream::mapBuffer() - * - * \return A newly created Buffer on success or nullptr otherwise - */ -std::unique_ptr Stream::createBuffer(const std::array &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); -} - -/** - * \fn Stream::bufferPool() - * \brief Retrieve the buffer pool for the stream - * - * The buffer pool handles the memory buffers used to store frames for the - * stream. It is initially created empty and shall be populated with - * buffers before being used. - * - * \return A reference to the buffer pool - */ - -/** - * \fn Stream::buffers() - * \brief Retrieve the memory buffers in the Stream's buffer pool - * \return The list of stream's memory buffers - */ - /** * \fn Stream::configuration() * \brief Retrieve the active configuration of the stream * \return The active configuration of the stream */ -/** - * \fn Stream::memoryType() - * \brief Retrieve the stream memory type - * \return The memory type used by the stream - */ - -/** - * \brief Map a Buffer to a buffer memory index - * \param[in] buffer The buffer to map to a buffer memory index - * - * Streams configured to use externally allocated memory need to maintain a - * best-effort association between the memory area the \a buffer represents - * and the associated buffer memory in the Stream's pool. - * - * The buffer memory to use, once the \a buffer reaches the video device, - * is selected using the index assigned to the \a buffer and to minimize - * relocations in the V4L2 back-end, this operation provides a best-effort - * caching mechanism that associates to the dmabuf file descriptors contained - * in the \a buffer the index of the buffer memory that was lastly queued with - * those file descriptors set. - * - * If the Stream uses internally allocated memory, the index of the memory - * buffer to use will match the one request at Stream::createBuffer(unsigned int) - * time, and no mapping is thus required. - * - * \return The buffer memory index for the buffer on success, or a negative - * error code otherwise - * \retval -ENOMEM No buffer memory was available to map the buffer - */ -int Stream::mapBuffer(const Buffer *buffer) -{ - ASSERT(memoryType_ == ExternalMemory); - - if (bufferCache_.empty()) - return -ENOMEM; - - const std::array &dmabufs = buffer->dmabufs(); - - /* - * Try to find a previously mapped buffer in the cache. If we miss, use - * the oldest entry in the cache. - */ - auto map = std::find_if(bufferCache_.begin(), bufferCache_.end(), - [&](std::pair, unsigned int> &entry) { - return entry.first == dmabufs; - }); - if (map == bufferCache_.end()) - map = bufferCache_.begin(); - - /* - * Update the dmabuf file descriptors of the entry. We can't assume that - * identical file descriptor numbers refer to the same dmabuf object as - * it may have been closed and its file descriptor reused. We thus need - * to update the plane's internally cached mmap()ed memory. - */ - unsigned int index = map->second; - BufferMemory *mem = &bufferPool_.buffers()[index]; - mem->planes().clear(); - - for (unsigned int i = 0; i < dmabufs.size(); ++i) { - if (dmabufs[i] == -1) - break; - - FrameBuffer::Plane plane; - plane.fd = FileDescriptor(dmabufs[i]); - plane.length = 0; - mem->planes().emplace_back(plane); - } - - /* Remove the buffer from the cache and return its index. */ - bufferCache_.erase(map); - return index; -} - -/** - * \brief Unmap a Buffer from its buffer memory - * \param[in] buffer The buffer to unmap - * - * This method releases the buffer memory entry that was mapped by mapBuffer(), - * making it available for new mappings. - */ -void Stream::unmapBuffer(const Buffer *buffer) -{ - ASSERT(memoryType_ == ExternalMemory); - - bufferCache_.emplace_back(buffer->dmabufs(), buffer->index()); -} - -/** - * \brief Create buffers for the stream - * \param[in] count The number of buffers to create - * \param[in] memory The stream memory type - * - * Create \a count empty buffers in the Stream's buffer pool. - */ -void Stream::createBuffers(MemoryType memory, unsigned int count) -{ - destroyBuffers(); - if (count == 0) - return; - - memoryType_ = memory; - bufferPool_.createBuffers(count); - - /* Streams with internal memory usage do not need buffer mapping. */ - if (memoryType_ == InternalMemory) - return; - - /* - * Prepare for buffer mapping by adding all buffer memory entries to the - * cache. - */ - bufferCache_.clear(); - for (unsigned int i = 0; i < bufferPool_.count(); ++i) - bufferCache_.emplace_back(std::array{ -1, -1, -1 }, i); -} - -/** - * \brief Destroy buffers in the stream - * - * If no buffers have been created or if buffers have already been destroyed no - * operation is performed. - */ -void Stream::destroyBuffers() -{ - bufferPool_.destroyBuffers(); -} - -/** - * \var Stream::bufferPool_ - * \brief The pool of buffers associated with the stream - * - * The stream buffer pool is populated by the Camera class after a successful - * stream configuration. - */ - /** * \var Stream::configuration_ * \brief The stream configuration @@ -659,9 +418,4 @@ void Stream::destroyBuffers() * next call to Camera::configure() regardless of if it includes the stream. */ -/** - * \var Stream::memoryType_ - * \brief The stream memory type - */ - } /* namespace libcamera */ From patchwork Mon Dec 30 12:05:08 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2473 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id F00896046A for ; Mon, 30 Dec 2019 13:06:12 +0100 (CET) X-Halon-ID: c56016c4-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id c56016c4-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:06:12 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:05:08 +0100 Message-Id: <20191230120510.938333-24-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 23/25] qcam: Cache buffer memory mapping X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:06:13 -0000 With the buffer allocator in use it's possible to cache the dmabuf memory mappings when starting the camera instead of mapping and unmapping them each time. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/qcam/main_window.cpp | 28 ++++++++++++++++++++++------ src/qcam/main_window.h | 1 + 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index e53ed2801291f2c5..2008931075b96260 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -201,6 +201,13 @@ int MainWindow::startCapture() } requests.push_back(request); + + /* Cache buffer memory mapping. */ + const FrameBuffer::Plane &plane = buffer->planes().front(); + void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED, + plane.fd.fd(), 0); + mappedBuffers_[plane.fd.fd()] = + std::make_pair(memory, plane.length); } titleTimer_.start(2000); @@ -230,6 +237,13 @@ error: for (Request *request : requests) delete request; + for (auto &itr : mappedBuffers_) { + void *memory = itr.second.first; + unsigned int length = itr.second.second; + munmap(memory, length); + } + mappedBuffers_.clear(); + camera_->freeBuffers(); return ret; } @@ -243,6 +257,13 @@ void MainWindow::stopCapture() if (ret) std::cout << "Failed to stop capture" << std::endl; + for (auto &itr : mappedBuffers_) { + void *memory = itr.second.first; + unsigned int length = itr.second.second; + munmap(memory, length); + } + mappedBuffers_.clear(); + camera_->freeBuffers(); isCapturing_ = false; @@ -297,15 +318,10 @@ int MainWindow::display(FrameBuffer *buffer) if (buffer->planes().size() != 1) return -EINVAL; - /* \todo: Once the FrameBuffer is done cache mapped memory. */ const FrameBuffer::Plane &plane = buffer->planes().front(); - void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED, - plane.fd.fd(), 0); - + void *memory = mappedBuffers_[plane.fd.fd()].first; unsigned char *raw = static_cast(memory); viewfinder_->display(raw, buffer->metadata().planes[0].bytesused); - munmap(memory, plane.length); - return 0; } diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h index 05cde4ceab5f7ea1..04fb9e3ea869c3fb 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -71,6 +71,7 @@ private: uint32_t framesCaptured_; ViewFinder *viewfinder_; + std::map> mappedBuffers_; }; #endif /* __QCAM_MAIN_WINDOW__ */ From patchwork Mon Dec 30 12:05:09 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2474 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 08994605EE for ; Mon, 30 Dec 2019 13:06:13 +0100 (CET) X-Halon-ID: c5b48ad4-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id c5b48ad4-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:06:12 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:05:09 +0100 Message-Id: <20191230120510.938333-25-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 24/25] libcamera: pipeline: Remove explicit buffer handling X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:06:14 -0000 With the FrameBuffer interface in place there is no need for the Camera to call into the specific pipelines allocation and freeing of buffers as it no longer needs to be synchronized with buffer allocation by the application. Remove the function prototypes in the pipeline handler base class and fold the functionality in the pipelines start() and stop() functions where needed. A follow up patch will remove the now no-op Camera::allocateBuffers() and Camera::freeBuffers(). Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/camera.cpp | 8 +------ src/libcamera/include/pipeline_handler.h | 5 ---- src/libcamera/pipeline/ipu3/ipu3.cpp | 24 ++++++++++++-------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 24 ++++++++++++-------- src/libcamera/pipeline/uvcvideo.cpp | 17 -------------- src/libcamera/pipeline/vimc.cpp | 17 -------------- src/libcamera/pipeline_handler.cpp | 29 ------------------------ 7 files changed, 30 insertions(+), 94 deletions(-) diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 5ea663ef85720f2e..dba537ef0da61b44 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -722,12 +722,6 @@ int Camera::allocateBuffers() return -EINVAL; } - int ret = pipe_->allocateBuffers(this, activeStreams_); - if (ret) { - LOG(Camera, Error) << "Failed to allocate buffers"; - return ret; - } - state_ = CameraPrepared; return 0; @@ -748,7 +742,7 @@ int Camera::freeBuffers() state_ = CameraConfigured; - return pipe_->freeBuffers(this, activeStreams_); + return 0; } /** diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 20b866c4a511387a..6cee976a6ce1939d 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -76,11 +76,6 @@ public: unsigned int count) = 0; virtual void freeFrameBuffers(Camera *camera, Stream *stream) = 0; - virtual int allocateBuffers(Camera *camera, - const std::set &streams) = 0; - virtual int freeBuffers(Camera *camera, - const std::set &streams) = 0; - virtual int start(Camera *camera) = 0; virtual void stop(Camera *camera) = 0; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index ea02a4e5f7f04718..4f837ebc68730883 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -213,11 +213,6 @@ public: unsigned int count) override; void freeFrameBuffers(Camera *camera, Stream *stream) override; - int allocateBuffers(Camera *camera, - const std::set &streams) override; - int freeBuffers(Camera *camera, - const std::set &streams) override; - int start(Camera *camera) override; void stop(Camera *camera) override; @@ -234,6 +229,9 @@ private: int registerCameras(); + int allocateBuffers(Camera *camera); + int freeBuffers(Camera *camera); + ImgUDevice imgu0_; ImgUDevice imgu1_; MediaDevice *cio2MediaDev_; @@ -654,8 +652,7 @@ void PipelineHandlerIPU3::freeFrameBuffers(Camera *camera, Stream *stream) * In order to be able to start the 'viewfinder' and 'stat' nodes, we need * memory to be reserved. */ -int PipelineHandlerIPU3::allocateBuffers(Camera *camera, - const std::set &streams) +int PipelineHandlerIPU3::allocateBuffers(Camera *camera) { IPU3CameraData *data = cameraData(camera); IPU3Stream *outStream = &data->outStream_; @@ -718,13 +715,12 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, return 0; error: - freeBuffers(camera, streams); + freeBuffers(camera); return ret; } -int PipelineHandlerIPU3::freeBuffers(Camera *camera, - const std::set &streams) +int PipelineHandlerIPU3::freeBuffers(Camera *camera) { IPU3CameraData *data = cameraData(camera); @@ -741,6 +737,11 @@ int PipelineHandlerIPU3::start(Camera *camera) ImgUDevice *imgu = data->imgu_; int ret; + /* Allocate buffers for internal pipeline usage. */ + ret = allocateBuffers(camera); + if (ret) + return ret; + /* * Start the ImgU video devices, buffers will be queued to the * ImgU output and viewfinder when requests will be queued. @@ -759,6 +760,7 @@ int PipelineHandlerIPU3::start(Camera *camera) return 0; error: + freeBuffers(camera); LOG(IPU3, Error) << "Failed to start camera " << camera->name(); return ret; @@ -774,6 +776,8 @@ void PipelineHandlerIPU3::stop(Camera *camera) if (ret) LOG(IPU3, Warning) << "Failed to stop camera " << camera->name(); + + freeBuffers(camera); } int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 7ed729fb7974d6ea..20d549f6dba649a5 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -181,11 +181,6 @@ public: unsigned int count) override; void freeFrameBuffers(Camera *camera, Stream *stream) override; - int allocateBuffers(Camera *camera, - const std::set &streams) override; - int freeBuffers(Camera *camera, - const std::set &streams) override; - int start(Camera *camera) override; void stop(Camera *camera) override; @@ -211,6 +206,9 @@ private: void paramReady(FrameBuffer *buffer); void statReady(FrameBuffer *buffer); + int allocateBuffers(Camera *camera); + int freeBuffers(Camera *camera); + MediaDevice *media_; V4L2Subdevice *dphy_; V4L2Subdevice *isp_; @@ -678,8 +676,7 @@ void PipelineHandlerRkISP1::freeFrameBuffers(Camera *camera, Stream *stream) video_->releaseBuffers(); } -int PipelineHandlerRkISP1::allocateBuffers(Camera *camera, - const std::set &streams) +int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) { RkISP1CameraData *data = cameraData(camera); unsigned int count = 1; @@ -723,8 +720,7 @@ err: return ret; } -int PipelineHandlerRkISP1::freeBuffers(Camera *camera, - const std::set &streams) +int PipelineHandlerRkISP1::freeBuffers(Camera *camera) { RkISP1CameraData *data = cameraData(camera); @@ -758,10 +754,16 @@ int PipelineHandlerRkISP1::start(Camera *camera) RkISP1CameraData *data = cameraData(camera); int ret; + /* Allocate buffers for internal pipeline usage. */ + ret = allocateBuffers(camera); + if (ret) + return ret; + data->frame_ = 0; ret = param_->streamOn(); if (ret) { + freeBuffers(camera); LOG(RkISP1, Error) << "Failed to start parameters " << camera->name(); return ret; @@ -770,6 +772,7 @@ int PipelineHandlerRkISP1::start(Camera *camera) ret = stat_->streamOn(); if (ret) { param_->streamOff(); + freeBuffers(camera); LOG(RkISP1, Error) << "Failed to start statistics " << camera->name(); return ret; @@ -779,6 +782,7 @@ int PipelineHandlerRkISP1::start(Camera *camera) if (ret) { param_->streamOff(); stat_->streamOff(); + freeBuffers(camera); LOG(RkISP1, Error) << "Failed to start camera " << camera->name(); @@ -823,6 +827,8 @@ void PipelineHandlerRkISP1::stop(Camera *camera) data->timeline_.reset(); + freeBuffers(camera); + activeCamera_ = nullptr; } diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 1eb809914f9aa0dc..d972d1bbdb113b95 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -71,11 +71,6 @@ public: unsigned int count) override; void freeFrameBuffers(Camera *camera, Stream *stream) override; - int allocateBuffers(Camera *camera, - const std::set &streams) override; - int freeBuffers(Camera *camera, - const std::set &streams) override; - int start(Camera *camera) override; void stop(Camera *camera) override; @@ -223,18 +218,6 @@ void PipelineHandlerUVC::freeFrameBuffers(Camera *camera, Stream *stream) data->video_->releaseBuffers(); } -int PipelineHandlerUVC::allocateBuffers(Camera *camera, - const std::set &streams) -{ - return 0; -} - -int PipelineHandlerUVC::freeBuffers(Camera *camera, - const std::set &streams) -{ - return 0; -} - int PipelineHandlerUVC::start(Camera *camera) { UVCCameraData *data = cameraData(camera); diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 13a7fc6cb5294eec..215d50a70256773e 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -89,11 +89,6 @@ public: unsigned int count) override; void freeFrameBuffers(Camera *camera, Stream *stream) override; - int allocateBuffers(Camera *camera, - const std::set &streams) override; - int freeBuffers(Camera *camera, - const std::set &streams) override; - int start(Camera *camera) override; void stop(Camera *camera) override; @@ -290,18 +285,6 @@ void PipelineHandlerVimc::freeFrameBuffers(Camera *camera, Stream *stream) data->video_->releaseBuffers(); } -int PipelineHandlerVimc::allocateBuffers(Camera *camera, - const std::set &streams) -{ - return 0; -} - -int PipelineHandlerVimc::freeBuffers(Camera *camera, - const std::set &streams) -{ - return 0; -} - int PipelineHandlerVimc::start(Camera *camera) { VimcCameraData *data = cameraData(camera); diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 11286d31d4e54213..19c9b6fe6e7fb03e 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -349,35 +349,6 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera) * The only intended caller is the FrameBufferAllocator helper. */ -/** - * \fn PipelineHandler::allocateBuffers() - * \brief Allocate buffers for a stream - * \param[in] camera The camera the \a stream belongs to - * \param[in] streams The set of streams to allocate buffers for - * - * This method allocates buffers internally in the pipeline handler for each - * stream in the \a streams buffer set, and associates them with the stream's - * buffer pool. - * - * The intended caller of this method is the Camera class. - * - * \return 0 on success or a negative error code otherwise - */ - -/** - * \fn PipelineHandler::freeBuffers() - * \brief Free all buffers associated with a stream - * \param[in] camera The camera the \a stream belongs to - * \param[in] streams The set of streams to free buffers from - * - * After a capture session has been stopped all buffers associated with each - * stream shall be freed. - * - * The intended caller of this method is the Camera class. - * - * \return 0 on success or a negative error code otherwise - */ - /** * \fn PipelineHandler::start() * \brief Start capturing from a group of streams From patchwork Mon Dec 30 12:05:10 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 2475 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5636E605DC for ; Mon, 30 Dec 2019 13:06:15 +0100 (CET) X-Halon-ID: c652affe-2afc-11ea-a00b-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2fd0.dip0.t-ipconnect.de [79.202.47.208]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id c652affe-2afc-11ea-a00b-005056917a89; Mon, 30 Dec 2019 13:06:13 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Mon, 30 Dec 2019 13:05:10 +0100 Message-Id: <20191230120510.938333-26-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.24.1 In-Reply-To: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> References: <20191230120510.938333-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 25/25] libcamera: camera: Remove the prepared state X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Dec 2019 12:06:15 -0000 With the FrameBuffer rework completed there is no reason to keep the camera prepared state around as buffer allocations are now decoupled from the camera state. Remove the camera state simplifying the API. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- include/libcamera/camera.h | 4 - src/android/camera_device.cpp | 11 +-- src/cam/capture.cpp | 8 -- src/libcamera/camera.cpp | 100 +++++------------------- src/libcamera/framebuffer_allocator.cpp | 4 +- src/qcam/main_window.cpp | 9 --- test/camera/buffer_import.cpp | 10 --- test/camera/capture.cpp | 10 --- test/camera/statemachine.cpp | 83 -------------------- 9 files changed, 22 insertions(+), 217 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 20f48faed62da7a7..0a04a01f3aca76e1 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -91,9 +91,6 @@ public: std::unique_ptr generateConfiguration(const StreamRoles &roles); int configure(CameraConfiguration *config); - int allocateBuffers(); - int freeBuffers(); - Request *createRequest(uint64_t cookie = 0); int queueRequest(Request *request); @@ -105,7 +102,6 @@ private: CameraAvailable, CameraAcquired, CameraConfigured, - CameraPrepared, CameraRunning, }; diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index cf2cd2ea9d1aa619..4a3e41b54d9ca867 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -77,8 +77,6 @@ int CameraDevice::open() void CameraDevice::close() { camera_->stop(); - - camera_->freeBuffers(); camera_->release(); running_ = false; @@ -690,16 +688,9 @@ void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reque /* Start the camera if that's the first request we handle. */ if (!running_) { - int ret = camera_->allocateBuffers(); - if (ret) { - LOG(HAL, Error) << "Failed to allocate buffers"; - return; - } - - ret = camera_->start(); + int ret = camera_->start(); if (ret) { LOG(HAL, Error) << "Failed to start camera"; - camera_->freeBuffers(); return; } diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp index dd078eb0ae4a2c62..247d988f5c738470 100644 --- a/src/cam/capture.cpp +++ b/src/cam/capture.cpp @@ -42,12 +42,6 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options) return ret; } - ret = camera_->allocateBuffers(); - if (ret) { - std::cerr << "Failed to allocate buffers" << std::endl; - return ret; - } - camera_->requestCompleted.connect(this, &Capture::requestComplete); if (options.isSet(OptFile)) { @@ -67,8 +61,6 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options) writer_ = nullptr; } - camera_->freeBuffers(); - delete allocator; return ret; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index dba537ef0da61b44..5564aff78405fef2 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -275,15 +275,13 @@ std::size_t CameraConfiguration::size() const * \section camera_operation Operating the Camera * * An application needs to perform a sequence of operations on a camera before - * it is ready to process requests. The camera needs to be acquired, configured - * and resources allocated or imported to prepare the camera for capture. Once - * started the camera can process requests until it is stopped. When an - * application is done with a camera all resources allocated need to be freed - * and the camera released. + * it is ready to process requests. The camera needs to be acquired and + * configured to prepare the camera for capture. Once started the camera can + * process requests until it is stopped. When an application is done with a + * camera, the camera needs to be released. * * An application may start and stop a camera multiple times as long as it is - * not released. The camera may also be reconfigured provided that all - * resources allocated are freed prior to the reconfiguration. + * not released. The camera may also be reconfigured. * * \subsection Camera States * @@ -297,7 +295,6 @@ std::size_t CameraConfiguration::size() const * node [shape = doublecircle ]; Available; * node [shape = circle ]; Acquired; * node [shape = circle ]; Configured; - * node [shape = circle ]; Prepared; * node [shape = circle ]; Running; * * Available -> Available [label = "release()"]; @@ -307,14 +304,10 @@ std::size_t CameraConfiguration::size() const * Acquired -> Configured [label = "configure()"]; * * Configured -> Available [label = "release()"]; - * Configured -> Configured [label = "configure()"]; - * Configured -> Prepared [label = "allocateBuffers()"]; + * Configured -> Configured [label = "configure(), createRequest()"]; + * Configured -> Running [label = "start()"]; * - * Prepared -> Configured [label = "freeBuffers()"]; - * Prepared -> Prepared [label = "createRequest()"]; - * Prepared -> Running [label = "start()"]; - * - * Running -> Prepared [label = "stop()"]; + * Running -> Configured [label = "stop()"]; * Running -> Running [label = "createRequest(), queueRequest()"]; * } * \enddot @@ -330,19 +323,14 @@ std::size_t CameraConfiguration::size() const * Configured state. * * \subsubsection Configured - * The camera is configured and ready for the application to prepare it with - * resources. The camera may be reconfigured multiple times until resources - * are provided and the state progresses to Prepared. - * - * \subsubsection Prepared - * The camera has been configured and provided with resources and is ready to be - * started. The application may free the camera's resources to get back to the - * Configured state or start() it to progress to the Running state. + * The camera is configured and ready to be started. The application may + * release() the camera and to get back to the Available state or start() + * it to progress to the Running state. * * \subsubsection Running * The camera is running and ready to process requests queued by the * application. The camera remains in this state until it is stopped and moved - * to the Prepared state. + * to the Configured state. */ /** @@ -420,7 +408,6 @@ static const char *const camera_state_names[] = { "Available", "Acquired", "Configured", - "Prepared", "Running", }; @@ -465,8 +452,6 @@ bool Camera::stateIs(State state) const * * \todo Deal with pending requests if the camera is disconnected in a * running state. - * \todo Update comment about Running state when importing buffers as well as - * allocating them are supported. */ void Camera::disconnect() { @@ -474,11 +459,11 @@ void Camera::disconnect() /* * If the camera was running when the hardware was removed force the - * state to Prepared to allow applications to call freeBuffers() and - * release() before deleting the camera. + * state to Configured state to allow applications to free resources + * and call release() before deleting the camera. */ if (state_ == CameraRunning) - state_ = CameraPrepared; + state_ = CameraConfigured; disconnected_ = true; disconnected.emit(this); @@ -698,53 +683,6 @@ int Camera::configure(CameraConfiguration *config) return 0; } -/** - * \brief Allocate buffers for all configured streams - * - * This function affects the state of the camera, see \ref camera_operation. - * - * \return 0 on success or a negative error code otherwise - * \retval -ENODEV The camera has been disconnected from the system - * \retval -EACCES The camera is not in a state where buffers can be allocated - * \retval -EINVAL The configuration is not valid - */ -int Camera::allocateBuffers() -{ - if (disconnected_) - return -ENODEV; - - if (!stateIs(CameraConfigured)) - return -EACCES; - - if (activeStreams_.empty()) { - LOG(Camera, Error) - << "Can't allocate buffers without streams"; - return -EINVAL; - } - - state_ = CameraPrepared; - - return 0; -} - -/** - * \brief Release all buffers from allocated pools in each stream - * - * This function affects the state of the camera, see \ref camera_operation. - * - * \return 0 on success or a negative error code otherwise - * \retval -EACCES The camera is not in a state where buffers can be freed - */ -int Camera::freeBuffers() -{ - if (!stateIs(CameraPrepared)) - return -EACCES; - - state_ = CameraConfigured; - - return 0; -} - /** * \brief Create a request object for the camera * \param[in] cookie Opaque cookie for application use @@ -760,14 +698,14 @@ int Camera::freeBuffers() * The ownership of the returned request is passed to the caller, which is * responsible for either queueing the request or deleting it. * - * This function shall only be called when the camera is in the Prepared + * This function shall only be called when the camera is in the Configured * or Running state, see \ref camera_operation. * * \return A pointer to the newly created request, or nullptr on error */ Request *Camera::createRequest(uint64_t cookie) { - if (disconnected_ || !stateBetween(CameraPrepared, CameraRunning)) + if (disconnected_ || !stateBetween(CameraConfigured, CameraRunning)) return nullptr; return new Request(this, cookie); @@ -839,7 +777,7 @@ int Camera::start() if (disconnected_) return -ENODEV; - if (!stateIs(CameraPrepared)) + if (!stateIs(CameraConfigured)) return -EACCES; LOG(Camera, Debug) << "Starting capture"; @@ -883,7 +821,7 @@ int Camera::stop() LOG(Camera, Debug) << "Stopping capture"; - state_ = CameraPrepared; + state_ = CameraConfigured; pipe_->stop(this); diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp index 98b649ecf6053790..6c45ccfffa985873 100644 --- a/src/libcamera/framebuffer_allocator.cpp +++ b/src/libcamera/framebuffer_allocator.cpp @@ -92,7 +92,7 @@ FrameBufferAllocator::create(std::shared_ptr camera) */ int FrameBufferAllocator::allocate(Stream *stream) { - if (camera_->state_ != Camera::CameraConfigured && camera_->state_ != Camera::CameraPrepared) { + if (camera_->state_ != Camera::CameraConfigured) { LOG(Allocator, Error) << "Camera must be in the configured state to allocate buffers"; return -EACCES; @@ -136,7 +136,7 @@ int FrameBufferAllocator::allocate(Stream *stream) */ int FrameBufferAllocator::release(Stream *stream) { - if (camera_->state_ != Camera::CameraConfigured && camera_->state_ != Camera::CameraPrepared) { + if (camera_->state_ != Camera::CameraConfigured) { LOG(Allocator, Error) << "Camera must be in the configured state to free buffers"; return -EACCES; diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 2008931075b96260..982d08bb3b01007a 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -172,13 +172,6 @@ int MainWindow::startCapture() adjustSize(); - ret = camera_->allocateBuffers(); - if (ret) { - std::cerr << "Failed to allocate buffers" - << std::endl; - return ret; - } - ret = allocator_->allocate(stream); if (ret < 0) { std::cerr << "Failed to allocate capture buffers" << std::endl; @@ -244,7 +237,6 @@ error: } mappedBuffers_.clear(); - camera_->freeBuffers(); return ret; } @@ -264,7 +256,6 @@ void MainWindow::stopCapture() } mappedBuffers_.clear(); - camera_->freeBuffers(); isCapturing_ = false; config_.reset(); diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index 63e5fb100e49f958..922d9933658b22d1 100644 --- a/test/camera/buffer_import.cpp +++ b/test/camera/buffer_import.cpp @@ -171,11 +171,6 @@ protected: return TestFail; } - if (camera_->allocateBuffers()) { - std::cout << "Failed to allocate buffers" << std::endl; - return TestFail; - } - Stream *stream = cfg.stream(); BufferSource source; @@ -241,11 +236,6 @@ protected: return TestFail; } - if (camera_->freeBuffers()) { - std::cout << "Failed to free buffers" << std::endl; - return TestFail; - } - return TestPass; } diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index de879ee4eb1420a6..b304d59c1c2aa9e2 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -87,11 +87,6 @@ protected: return TestFail; } - if (camera_->allocateBuffers()) { - cout << "Failed to allocate buffers" << endl; - return TestFail; - } - Stream *stream = cfg.stream(); int ret = allocator_->allocate(stream); @@ -158,11 +153,6 @@ protected: return TestFail; } - if (camera_->freeBuffers()) { - cout << "Failed to free buffers" << endl; - return TestFail; - } - return TestPass; } diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index f3a7ca7c32a5ec97..20541b3e4752dc81 100644 --- a/test/camera/statemachine.cpp +++ b/test/camera/statemachine.cpp @@ -29,12 +29,6 @@ protected: if (camera_->configure(defconf_.get()) != -EACCES) return TestFail; - if (camera_->allocateBuffers() != -EACCES) - return TestFail; - - if (camera_->freeBuffers() != -EACCES) - return TestFail; - if (camera_->createRequest()) return TestFail; @@ -65,12 +59,6 @@ protected: if (camera_->acquire() != -EBUSY) return TestFail; - if (camera_->allocateBuffers() != -EACCES) - return TestFail; - - if (camera_->freeBuffers() != -EACCES) - return TestFail; - if (camera_->createRequest()) return TestFail; @@ -103,57 +91,6 @@ protected: if (camera_->acquire() != -EBUSY) return TestFail; - if (camera_->freeBuffers() != -EACCES) - return TestFail; - - if (camera_->createRequest()) - return TestFail; - - Request request(camera_.get()); - if (camera_->queueRequest(&request) != -EACCES) - return TestFail; - - if (camera_->start() != -EACCES) - return TestFail; - - if (camera_->stop() != -EACCES) - return TestFail; - - /* Test operations which should pass. */ - if (camera_->configure(defconf_.get())) - return TestFail; - - /* Test valid state transitions, end in Prepared state. */ - if (camera_->release()) - return TestFail; - - if (camera_->acquire()) - return TestFail; - - if (camera_->configure(defconf_.get())) - return TestFail; - - if (camera_->allocateBuffers()) - return TestFail; - - return TestPass; - } - - int testPrepared() - { - /* Test operations which should fail. */ - if (camera_->acquire() != -EBUSY) - return TestFail; - - if (camera_->release() != -EBUSY) - return TestFail; - - if (camera_->configure(defconf_.get()) != -EACCES) - return TestFail; - - if (camera_->allocateBuffers() != -EACCES) - return TestFail; - Request request1(camera_.get()); if (camera_->queueRequest(&request1) != -EACCES) return TestFail; @@ -170,9 +107,6 @@ protected: delete request2; /* Test valid state transitions, end in Running state. */ - if (camera_->freeBuffers()) - return TestFail; - if (camera_->release()) return TestFail; @@ -182,9 +116,6 @@ protected: if (camera_->configure(defconf_.get())) return TestFail; - if (camera_->allocateBuffers()) - return TestFail; - /* Use internally allocated buffers. */ allocator_ = FrameBufferAllocator::create(camera_); Stream *stream = *camera_->streams().begin(); @@ -209,12 +140,6 @@ protected: if (camera_->configure(defconf_.get()) != -EACCES) return TestFail; - if (camera_->allocateBuffers() != -EACCES) - return TestFail; - - if (camera_->freeBuffers() != -EACCES) - return TestFail; - if (camera_->start() != -EACCES) return TestFail; @@ -236,9 +161,6 @@ protected: delete allocator_; - if (camera_->freeBuffers()) - return TestFail; - if (camera_->release()) return TestFail; @@ -276,11 +198,6 @@ protected: return TestFail; } - if (testPrepared() != TestPass) { - cout << "State machine in Prepared state failed" << endl; - return TestFail; - } - if (testRuning() != TestPass) { cout << "State machine in Running state failed" << endl; return TestFail;