[libcamera-devel] [PATCH v4 18/32] libcamera: v4l2_videodevice: Add FrameBuffer interface
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Jan 12 14:41:54 CET 2020
Hi Niklas,
Thank you for the patch.
On Sun, Jan 12, 2020 at 02:01:58AM +0100, Niklas Söderlund wrote:
> 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 throughout 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 <niklas.soderlund at ragnatech.se>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> * Changes since v2
> - s/todo:/todo/
> - Style updates
> - Update documentation
> - Remove buf.length > check in V4L2VideoDevice::dequeueFrameBuffer()
> - Set length for multi planer output buffers in queueBuffer()
>
> * 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 | 313 ++++++++++++++++++++++-
> 2 files changed, 315 insertions(+), 10 deletions(-)
>
> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
> index 9d22754a39a75621..09967d3c6ae59044 100644
> --- a/src/libcamera/include/v4l2_videodevice.h
> +++ b/src/libcamera/include/v4l2_videodevice.h
> @@ -184,11 +184,17 @@ public:
>
> int exportBuffers(BufferPool *pool);
> int importBuffers(BufferPool *pool);
> + int exportBuffers(unsigned int count,
> + std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> + int importBuffers(unsigned int count);
> int releaseBuffers();
>
> int queueBuffer(Buffer *buffer);
> std::vector<std::unique_ptr<Buffer>> queueAllBuffers();
> Signal<Buffer *> bufferReady;
> + int queueBuffer(FrameBuffer *buffer);
> + /* todo Rename to bufferReady when the Buffer version is removed */
> + Signal<FrameBuffer *> frameBufferReady;
>
> int streamOn();
> int streamOff();
> @@ -219,10 +225,13 @@ private:
> int requestBuffers(unsigned int count);
> int createPlane(BufferMemory *buffer, unsigned int index,
> unsigned int plane, unsigned int length);
> + std::unique_ptr<FrameBuffer> createFrameBuffer(const struct v4l2_buffer &buf);
> FileDescriptor 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_;
>
> @@ -230,7 +239,10 @@ private:
> enum v4l2_memory memoryType_;
>
> BufferPool *bufferPool_;
> + V4L2BufferCache *cache_;
> std::map<unsigned int, Buffer *> queuedBuffers_;
> + /* todo Rename to queuedBuffers_ when the Buffer version is removed */
> + std::map<unsigned int, FrameBuffer *> queuedFrameBuffers_;
>
> EventNotifier *fdEvent_;
> };
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 84c45dbcb85c8638..ed5b2377b806f19c 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -410,7 +410,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
> @@ -1075,6 +1076,94 @@ 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<std::unique_ptr<FrameBuffer>> *buffers)
> +{
> + if (cache_) {
> + LOG(V4L2, Error) << "Buffers 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<FrameBuffer> 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<FrameBuffer>
> +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<FrameBuffer::Plane> planes;
> + for (unsigned int nplane = 0; nplane < numPlanes; nplane++) {
> + FileDescriptor fd = exportDmabufFd(buf.index, nplane);
> + if (fd.fd() < 0)
How about
if (!fd.isValid())
The rest looks good, please keep my R-b.
> + return nullptr;
> +
> + FrameBuffer::Plane plane;
> + plane.fd = std::move(fd);
> + plane.length = multiPlanar ?
> + buf.m.planes[nplane].length : buf.length;
> +
> + planes.push_back(std::move(plane));
> + }
> +
> + return utils::make_unique<FrameBuffer>(std::move(planes));
> +}
> +
> FileDescriptor V4L2VideoDevice::exportDmabufFd(unsigned int index,
> unsigned int plane)
> {
> @@ -1096,14 +1185,41 @@ FileDescriptor V4L2VideoDevice::exportDmabufFd(unsigned int index,
> return FileDescriptor(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) << "Buffers 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);
> }
> @@ -1222,6 +1338,90 @@ std::vector<std::unique_ptr<Buffer>> V4L2VideoDevice::queueAllBuffers()
> return buffers;
> }
>
> +/**
> + * \brief Queue a buffer to 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.
> + *
> + * The best available V4L2 buffer is picked for \a buffer using the V4L2 buffer
> + * cache.
> + *
> + * \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<FrameBuffer::Plane> &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;
> + v4l2Planes[nplane].length = buffer->planes()[nplane].length;
> + nplane++;
> + }
> + } 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
> *
> @@ -1281,17 +1481,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. */
> - bufferReady.emit(buffer);
> + /* 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 Rename 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_;
> +
> + 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) {
> + 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;
> }
>
> /**
> @@ -1299,6 +1579,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
> @@ -1321,7 +1606,7 @@ 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,
> + * immediately dequeued with their status set to FrameMetadata::FrameCancelled,
> * and the bufferReady signal is emitted for them. The order in which those
> * buffers are dequeued is not specified.
> *
> @@ -1348,7 +1633,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;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list