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())) {