| Message ID | 20191230120510.938333-22-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, Thank you for the patch. On Mon, Dec 30, 2019 at 01:05:06PM +0100, Niklas Söderlund wrote: > 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 <niklas.soderlund@ragnatech.se> > --- > 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<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; > + Signal<FrameBuffer *> bufferReady; > > int streamOn(); > int streamOff(); > @@ -235,26 +229,19 @@ private: > std::vector<SizeRange> 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<FrameBuffer> createFrameBuffer(const struct v4l2_buffer &buf); Should this be renamed to createBuffer() ? > 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(); s/dequeu/dequeue/ Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > V4L2Capability caps_; > > enum v4l2_buf_type bufferType_; > 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_; > + std::map<unsigned int, FrameBuffer *> 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<FrameBuffer::Plane> &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<std::unique_ptr<Buffer>> V4L2VideoDevice::queueAllBuffers() > -{ > - int ret; > - > - if (!queuedBuffers_.empty()) > - return {}; > - > - if (V4L2_TYPE_IS_OUTPUT(bufferType_)) > - return {}; > - > - std::vector<std::unique_ptr<Buffer>> 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<FrameBuffer> &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<FrameBuffer> &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<FrameBuffer> &buffer : captureBuffers_) { > if (capture->queueBuffer(buffer.get())) {
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<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; + Signal<FrameBuffer *> bufferReady; int streamOn(); int streamOff(); @@ -235,26 +229,19 @@ private: std::vector<SizeRange> 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<FrameBuffer> 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<unsigned int, Buffer *> queuedBuffers_; - /* todo: Rename to queuedBuffers_ when the Buffer version is removed */ - std::map<unsigned int, FrameBuffer *> queuedFrameBuffers_; + std::map<unsigned int, FrameBuffer *> 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<FrameBuffer::Plane> &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<std::unique_ptr<Buffer>> V4L2VideoDevice::queueAllBuffers() -{ - int ret; - - if (!queuedBuffers_.empty()) - return {}; - - if (V4L2_TYPE_IS_OUTPUT(bufferType_)) - return {}; - - std::vector<std::unique_ptr<Buffer>> 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<FrameBuffer> &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<FrameBuffer> &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<FrameBuffer> &buffer : captureBuffers_) { if (capture->queueBuffer(buffer.get())) {
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 <niklas.soderlund@ragnatech.se> --- 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(-)