libcamera: v4l2_videodevice: Remove Buffer interface

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>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

6 files changed, 28 insertions, 350 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<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();
@@ -223,26 +217,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);
+	std::unique_ptr<FrameBuffer> 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<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 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
@@ -971,112 +970,6 @@ int V4L2VideoDevice::requestBuffers(unsigned int count)
 }
 
 /**
- * \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
  * \param[out] buffers Vector to store allocated buffers
@@ -1114,7 +1007,7 @@ int V4L2VideoDevice::exportBuffers(unsigned int count,
 			goto err_buf;
 		}
 
-		std::unique_ptr<FrameBuffer> buffer = createFrameBuffer(buf);
+		std::unique_ptr<FrameBuffer> buffer = createBuffer(buf);
 		if (!buffer) {
 			LOG(V4L2, Error) << "Unable to create buffer";
 			ret = -EINVAL;
@@ -1137,7 +1030,7 @@ err_buf:
 }
 
 std::unique_ptr<FrameBuffer>
-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,7 +1110,6 @@ int V4L2VideoDevice::releaseBuffers()
 {
 	LOG(V4L2, Debug) << "Releasing buffers";
 
-	bufferPool_ = nullptr;
 	delete cache_;
 	cache_ = nullptr;
 
@@ -1225,120 +1117,6 @@ int V4L2VideoDevice::releaseBuffers()
 }
 
 /**
- * \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<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();
-
-		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<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 to the video device
  * \param[in] buffer The buffer to be queued
  *
@@ -1414,67 +1192,15 @@ 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;