[libcamera-devel] [PATCH v3 10/11] libcamera: pipeline: simple: Add simple format converter

[Kieran Bingham]

Hi Laurent,

On 20/03/2020 01:48, Laurent Pinchart wrote:
> The simple format converter supports V4L2 M2M devices that convert pixel
> formats.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> Changes since v2:
> 
> - Rebase on top of V4L2PixelFormat

Not much I can spot in this one, and I'm pleased to see the V4L2 M2M
device abstraction being useful/used more ;-)

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> ---
>  src/libcamera/pipeline/simple/converter.cpp | 209 ++++++++++++++++++++
>  src/libcamera/pipeline/simple/converter.h   |  60 ++++++
>  src/libcamera/pipeline/simple/meson.build   |   1 +
>  3 files changed, 270 insertions(+)
>  create mode 100644 src/libcamera/pipeline/simple/converter.cpp
>  create mode 100644 src/libcamera/pipeline/simple/converter.h
> 
> diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
> new file mode 100644
> index 000000000000..3025c3dea809
> --- /dev/null
> +++ b/src/libcamera/pipeline/simple/converter.cpp
> @@ -0,0 +1,209 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Laurent Pinchart
> + *
> + * converter.cpp - Format converter for simple pipeline handler
> + */
> +
> +#include "converter.h"
> +
> +#include <algorithm>
> +
> +#include <libcamera/buffer.h>
> +#include <libcamera/geometry.h>
> +#include <libcamera/signal.h>
> +
> +#include "log.h"
> +#include "media_device.h"
> +#include "v4l2_videodevice.h"
> +
> +namespace libcamera {
> +
> +LOG_DECLARE_CATEGORY(SimplePipeline);
> +
> +SimpleConverter::SimpleConverter(MediaDevice *media)
> +	: m2m_(nullptr)
> +{
> +	/*
> +	 * Locate the video node. There's no need to validate the pipeline
> +	 * further, the caller guarantees that this is a V4L2 mem2mem device.
> +	 */
> +	const std::vector<MediaEntity *> &entities = media->entities();
> +	auto it = std::find_if(entities.begin(), entities.end(),
> +			       [](MediaEntity *entity) {
> +				       return entity->function() == MEDIA_ENT_F_IO_V4L;
> +			       });
> +	if (it == entities.end())
> +		return;
> +
> +	m2m_ = new V4L2M2MDevice((*it)->deviceNode());
> +
> +	m2m_->output()->bufferReady.connect(this, &SimpleConverter::outputBufferReady);
> +	m2m_->capture()->bufferReady.connect(this, &SimpleConverter::captureBufferReady);
> +}
> +
> +SimpleConverter::~SimpleConverter()
> +{
> +	delete m2m_;
> +}
> +
> +int SimpleConverter::open()
> +{
> +	if (!m2m_)
> +		return -ENODEV;
> +
> +	return m2m_->open();
> +}
> +
> +void SimpleConverter::close()
> +{
> +	if (m2m_)
> +		m2m_->close();
> +}
> +
> +std::vector<PixelFormat> SimpleConverter::formats(PixelFormat input)
> +{
> +	if (!m2m_)
> +		return {};
> +

/* Set the given PixelFormat on the output queue, to determine the
conversion capabilities at the capture queue. */

(where /queue/{node,device,something else}/)

> +	V4L2DeviceFormat format;
> +	format.fourcc = m2m_->output()->toV4L2PixelFormat(input);
> +	format.size = { 1, 1 };
> +
> +	int ret = m2m_->output()->setFormat(&format);
> +	if (ret < 0) {
> +		LOG(SimplePipeline, Error)
> +			<< "Failed to set format: " << strerror(-ret);
> +		return {};
> +	}
> +
> +	std::vector<PixelFormat> pixelFormats;
> +
> +	for (const auto &format : m2m_->capture()->formats()) {
> +		PixelFormat pixelFormat = m2m_->capture()->toPixelFormat(format.first);
> +		if (pixelFormat)
> +			pixelFormats.push_back(pixelFormat);
> +	}
> +
> +	return pixelFormats;
> +}
> +
> +int SimpleConverter::configure(PixelFormat inputFormat,
> +			       PixelFormat outputFormat, const Size &size)
> +{
> +	V4L2DeviceFormat format;
> +	int ret;
> +
> +	V4L2PixelFormat videoFormat = m2m_->output()->toV4L2PixelFormat(inputFormat);
> +	format.fourcc = videoFormat;
> +	format.size = size;
> +
> +	ret = m2m_->output()->setFormat(&format);
> +	if (ret < 0) {
> +		LOG(SimplePipeline, Error)
> +			<< "Failed to set input format: " << strerror(-ret);
> +		return ret;
> +	}
> +
> +	if (format.fourcc != videoFormat || format.size != size) {
> +		LOG(SimplePipeline, Error)
> +			<< "Input format not supported";
> +		return -EINVAL;
> +	}
> +
> +	videoFormat = m2m_->capture()->toV4L2PixelFormat(outputFormat);
> +	format.fourcc = videoFormat;
> +
> +	ret = m2m_->capture()->setFormat(&format);
> +	if (ret < 0) {
> +		LOG(SimplePipeline, Error)
> +			<< "Failed to set output format: " << strerror(-ret);
> +		return ret;
> +	}
> +
> +	if (format.fourcc != videoFormat || format.size != size) {
> +		LOG(SimplePipeline, Error)
> +			<< "Output format not supported";
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +int SimpleConverter::exportBuffers(unsigned int count,
> +				   std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +{
> +	return m2m_->capture()->exportBuffers(count, buffers);
> +}
> +
> +int SimpleConverter::start(unsigned int count)
> +{
> +	int ret = m2m_->output()->importBuffers(count);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = m2m_->capture()->importBuffers(count);
> +	if (ret < 0) {
> +		stop();
> +		return ret;
> +	}
> +
> +	ret = m2m_->output()->streamOn();
> +	if (ret < 0) {
> +		stop();
> +		return ret;
> +	}
> +
> +	ret = m2m_->capture()->streamOn();
> +	if (ret < 0) {
> +		stop();
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +void SimpleConverter::stop()
> +{
> +	m2m_->capture()->streamOff();
> +	m2m_->output()->streamOff();
> +	m2m_->capture()->releaseBuffers();
> +	m2m_->output()->releaseBuffers();
> +}
> +
> +int SimpleConverter::queueBuffers(FrameBuffer *input, FrameBuffer *output)
> +{
> +	int ret = m2m_->output()->queueBuffer(input);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = m2m_->capture()->queueBuffer(output);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +void SimpleConverter::captureBufferReady(FrameBuffer *buffer)
> +{

Do we need any kind of locking between these two BufferReady slots which
essentially interact with each other? or is the queue 'safe' enough
 (or perhaps are we guaranteed to process both in the same thread?)

> +	if (!outputDoneQueue_.empty()) {
> +		FrameBuffer *other = outputDoneQueue_.front();
> +		outputDoneQueue_.pop();
> +		bufferReady.emit(other, buffer);
> +	} else {
> +		captureDoneQueue_.push(buffer);
> +	}
> +}
> +
> +void SimpleConverter::outputBufferReady(FrameBuffer *buffer)
> +{
> +	if (!captureDoneQueue_.empty()) {
> +		FrameBuffer *other = captureDoneQueue_.front();
> +		captureDoneQueue_.pop();
> +		bufferReady.emit(buffer, other);
> +	} else {
> +		outputDoneQueue_.push(buffer);
> +	}
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h
> new file mode 100644
> index 000000000000..a33071fa8578
> --- /dev/null
> +++ b/src/libcamera/pipeline/simple/converter.h
> @@ -0,0 +1,60 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Laurent Pinchart
> + *
> + * converter.h - Format converter for simple pipeline handler
> + */
> +
> +#ifndef __LIBCAMERA_PIPELINE_SIMPLE_CONVERTER_H__
> +#define __LIBCAMERA_PIPELINE_SIMPLE_CONVERTER_H__
> +
> +#include <memory>
> +#include <queue>
> +#include <vector>
> +
> +#include <libcamera/pixelformats.h>
> +#include <libcamera/signal.h>
> +
> +namespace libcamera {
> +
> +class FrameBuffer;
> +class MediaDevice;
> +struct Size;
> +class V4L2M2MDevice;
> +
> +class SimpleConverter
> +{
> +public:
> +	SimpleConverter(MediaDevice *media);
> +	~SimpleConverter();
> +
> +	int open();
> +	void close();
> +
> +	std::vector<PixelFormat> formats(PixelFormat input);
> +
> +	int configure(PixelFormat inputFormat, PixelFormat outputFormat,
> +		      const Size &size);
> +	int exportBuffers(unsigned int count,
> +			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> +
> +	int start(unsigned int count);
> +	void stop();
> +
> +	int queueBuffers(FrameBuffer *input, FrameBuffer *output);
> +
> +	Signal<FrameBuffer *, FrameBuffer *> bufferReady;
> +
> +private:
> +	void captureBufferReady(FrameBuffer *buffer);
> +	void outputBufferReady(FrameBuffer *buffer);
> +
> +	V4L2M2MDevice *m2m_;
> +
> +	std::queue<FrameBuffer *> captureDoneQueue_;
> +	std::queue<FrameBuffer *> outputDoneQueue_;
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_PIPELINE_SIMPLE_CONVERTER_H__ */
> diff --git a/src/libcamera/pipeline/simple/meson.build b/src/libcamera/pipeline/simple/meson.build
> index 4945a3e173cf..8372f24e3788 100644
> --- a/src/libcamera/pipeline/simple/meson.build
> +++ b/src/libcamera/pipeline/simple/meson.build
> @@ -1,3 +1,4 @@
>  libcamera_sources += files([
> +    'converter.cpp',
>      'simple.cpp',
>  ])
> 

-- 
Regards
--
Kieran