From 66d4929c7348b36e977eb20bfd8641f2c70f051e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20S=C3=B6derlund?= Date: Wed, 18 Dec 2019 17:38:38 +0100 Subject: libcamera: v4l2_videodevice: Remove Buffer interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 | 21 +- 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 | 339 ++----------------------------- test/v4l2_videodevice/buffer_sharing.cpp | 4 +- test/v4l2_videodevice/capture_async.cpp | 2 +- test/v4l2_videodevice/v4l2_m2mdevice.cpp | 4 +- 9 files changed, 33 insertions(+), 355 deletions(-) diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h index 09967d3c..e4d35ab3 100644 --- a/src/libcamera/include/v4l2_videodevice.h +++ b/src/libcamera/include/v4l2_videodevice.h @@ -182,19 +182,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(); @@ -223,26 +217,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); + std::unique_ptr createBuffer(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(); + FrameBuffer *dequeueBuffer(); 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 065f5d98..1ea4d938 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -903,13 +903,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 d669d2de..da6e079f 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -950,9 +950,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 66380e29..67d29b79 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -352,7 +352,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 5e0f5c63..4cfdb5ae 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -437,7 +437,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 dd120129..66adf7b2 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -410,8 +410,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 @@ -970,112 +969,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) -{ - LOG(V4L2, Debug) - << "Buffer " << index - << " plane " << planeIndex - << ": length=" << length; - - FileDescriptor fd = exportDmabufFd(index, planeIndex); - if (!fd.isValid()) - return -EINVAL; - - FrameBuffer::Plane plane; - plane.fd = fd; - plane.length = length; - buffer->planes().push_back(plane); - - 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 @@ -1114,7 +1007,7 @@ int V4L2VideoDevice::exportBuffers(unsigned int count, goto err_buf; } - std::unique_ptr buffer = createFrameBuffer(buf); + std::unique_ptr buffer = createBuffer(buf); if (!buffer) { LOG(V4L2, Error) << "Unable to create buffer"; ret = -EINVAL; @@ -1137,7 +1030,7 @@ err_buf: } std::unique_ptr -V4L2VideoDevice::createFrameBuffer(const struct v4l2_buffer &buf) +V4L2VideoDevice::createBuffer(const struct v4l2_buffer &buf) { const bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type); const unsigned int numPlanes = multiPlanar ? buf.length : 1; @@ -1217,127 +1110,12 @@ int V4L2VideoDevice::releaseBuffers() { LOG(V4L2, Debug) << "Releasing buffers"; - bufferPool_ = nullptr; delete cache_; cache_ = nullptr; return requestBuffers(0); } -/** - * \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(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(); - - if (!metadata.planes.empty()) - 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 to the video device * \param[in] buffer The buffer to be queued @@ -1414,66 +1192,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 @@ -1486,30 +1212,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 = dequeueBuffer(); + if (!buffer) + return; - /* Notify anyone listening to the device. */ - bufferReady.emit(buffer); - } + /* Notify anyone listening to the device. */ + bufferReady.emit(buffer); } /** @@ -1518,11 +1226,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 Rename 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::dequeueBuffer() { struct v4l2_buffer buf = {}; struct v4l2_plane planes[VIDEO_MAX_PLANES] = {}; @@ -1549,11 +1255,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 @@ -1576,11 +1282,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 */ @@ -1625,23 +1326,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 6acb06a2..fefa969a 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 a57abed3..6a103a03 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 43b99c4f..203afc4f 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())) { -- cgit v1.2.1