[libcamera-devel] [PATCH v2 1/2] libcamera: Declare generic converter interface

[Jacopo Mondi]

Hi Xavier

On Thu, Nov 17, 2022 at 06:50:25PM +0100, Xavier Roumegue (OSS) wrote:
> Hi Jacopo,
>
> On 11/17/22 10:29, Jacopo Mondi wrote:
> > Hi Xavier
> >
> > On Tue, Nov 15, 2022 at 01:59:33PM +0100, Xavier Roumegue wrote:
> > > Declare a converter Abstract Base Class intended to provide generic
> > > interfaces to hardware offering size and format conversion services on
> > > streams. This is mainly based on the public interfaces of the current
> > > converter class implementation found in the simple pipeline handler.
> > >
> > > The main change is the introduction of loadConfiguration() function
> > > which can be used by the concrete implementation to load hardware
> > > specific runtime parameters defined by the application.
> > >
> > > Signed-off-by: Xavier Roumegue <xavier.roumegue at oss.nxp.com>
> > > ---
> > >   include/libcamera/internal/converter.h | 109 ++++++++
> > >   include/libcamera/internal/meson.build |   1 +
> > >   src/libcamera/converter.cpp            | 331 +++++++++++++++++++++++++
> > >   src/libcamera/meson.build              |   1 +
> > >   4 files changed, 442 insertions(+)
> > >   create mode 100644 include/libcamera/internal/converter.h
> > >   create mode 100644 src/libcamera/converter.cpp
> > >
> > > diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h
> > > new file mode 100644
> > > index 00000000..19e8c7c5
> > > --- /dev/null
> > > +++ b/include/libcamera/internal/converter.h
> > > @@ -0,0 +1,109 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > +/*
> > > + * Copyright (C) 2020, Laurent Pinchart
> > > + * Copyright 2022 NXP
> > > + *
> > > + * converter.h - Generic stream converter interface
> >
> > As commented in v1, please align with this with the .cpp file
> My bad.. I missed the stream/converter mismatch. I will fix it on v3 :(
> Sorry for that

No need to resend just for this tiny bit :)

>
> Regards,
>  Xavier
> >
> > If not other comments, it can be fixed when applying
> > Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> >
> > Thanks
> >    j
> >
> >
> > > + */
> > > +
> > > +#pragma once
> > > +
> > > +#include <initializer_list>
> > > +#include <map>
> > > +#include <memory>
> > > +#include <string>
> > > +#include <tuple>
> > > +#include <vector>
> > > +
> > > +#include <libcamera/base/class.h>
> > > +#include <libcamera/base/signal.h>
> > > +
> > > +#include <libcamera/geometry.h>
> > > +#include <libcamera/pixel_format.h>
> > > +
> > > +namespace libcamera {
> > > +
> > > +class FrameBuffer;
> > > +class MediaDevice;
> > > +class Size;
> > > +class SizeRange;
> > > +struct StreamConfiguration;
> > > +
> > > +class Converter
> > > +{
> > > +public:
> > > +	Converter(MediaDevice *media);
> > > +	virtual ~Converter();
> > > +
> > > +	virtual int loadConfiguration(const std::string &filename) = 0;
> > > +
> > > +	virtual bool isValid() const = 0;
> > > +
> > > +	virtual std::vector<PixelFormat> formats(PixelFormat input) = 0;
> > > +	virtual SizeRange sizes(const Size &input) = 0;
> > > +
> > > +	virtual std::tuple<unsigned int, unsigned int>
> > > +	strideAndFrameSize(const PixelFormat &pixelFormat, const Size &size) = 0;
> > > +
> > > +	virtual int configure(const StreamConfiguration &inputCfg,
> > > +			      const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs) = 0;
> > > +	virtual int exportBuffers(unsigned int output, unsigned int count,
> > > +				  std::vector<std::unique_ptr<FrameBuffer>> *buffers) = 0;
> > > +
> > > +	virtual int start() = 0;
> > > +	virtual void stop() = 0;
> > > +
> > > +	virtual int queueBuffers(FrameBuffer *input,
> > > +				 const std::map<unsigned int, FrameBuffer *> &outputs) = 0;
> > > +
> > > +	Signal<FrameBuffer *> inputBufferReady;
> > > +	Signal<FrameBuffer *> outputBufferReady;
> > > +
> > > +	const std::string &deviceNode() const { return deviceNode_; };
> > > +
> > > +private:
> > > +	std::string deviceNode_;
> > > +};
> > > +
> > > +class ConverterFactoryBase
> > > +{
> > > +public:
> > > +	ConverterFactoryBase(const std::string name, std::initializer_list<std::string> aliases);
> > > +	virtual ~ConverterFactoryBase() = default;
> > > +
> > > +	const std::vector<std::string> &aliases() const { return aliases_; }
> > > +
> > > +	static std::unique_ptr<Converter> create(MediaDevice *media);
> > > +	static std::vector<ConverterFactoryBase *> &factories();
> > > +	static std::vector<std::string> names();
> > > +
> > > +private:
> > > +	LIBCAMERA_DISABLE_COPY_AND_MOVE(ConverterFactoryBase)
> > > +
> > > +	static void registerType(ConverterFactoryBase *factory);
> > > +
> > > +	virtual std::unique_ptr<Converter> createInstance(MediaDevice *media) const = 0;
> > > +
> > > +	std::string name_;
> > > +	std::vector<std::string> aliases_;
> > > +};
> > > +
> > > +template<typename _Converter>
> > > +class ConverterFactory : public ConverterFactoryBase
> > > +{
> > > +public:
> > > +	ConverterFactory(const char *name, std::initializer_list<std::string> aliases)
> > > +		: ConverterFactoryBase(name, aliases)
> > > +	{
> > > +	}
> > > +
> > > +	std::unique_ptr<Converter> createInstance(MediaDevice *media) const override
> > > +	{
> > > +		return std::make_unique<_Converter>(media);
> > > +	}
> > > +};
> > > +
> > > +#define REGISTER_CONVERTER(name, converter, ...) \
> > > +	static ConverterFactory<converter> global_##converter##Factory(name, { __VA_ARGS__ });
> > > +
> > > +} /* namespace libcamera */
> > > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> > > index 7a780d48..8f50d755 100644
> > > --- a/include/libcamera/internal/meson.build
> > > +++ b/include/libcamera/internal/meson.build
> > > @@ -19,6 +19,7 @@ libcamera_internal_headers = files([
> > >       'camera_sensor_properties.h',
> > >       'control_serializer.h',
> > >       'control_validator.h',
> > > +    'converter.h',
> > >       'delayed_controls.h',
> > >       'device_enumerator.h',
> > >       'device_enumerator_sysfs.h',
> > > diff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp
> > > new file mode 100644
> > > index 00000000..e370038f
> > > --- /dev/null
> > > +++ b/src/libcamera/converter.cpp
> > > @@ -0,0 +1,331 @@
> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > > +/*
> > > + * Copyright 2022 NXP
> > > + *
> > > + * converter.cpp - Generic Format converter interface
> > > + */
> > > +
> > > +#include "libcamera/internal/converter.h"
> > > +
> > > +#include <algorithm>
> > > +
> > > +#include <libcamera/base/log.h>
> > > +
> > > +#include "libcamera/internal/media_device.h"
> > > +
> > > +/**
> > > + * \file internal/converter.h
> > > + * \brief Abstract converter
> > > + */
> > > +
> > > +namespace libcamera {
> > > +
> > > +LOG_DEFINE_CATEGORY(Converter)
> > > +
> > > +/**
> > > + * \class Converter
> > > + * \brief Abstract Base Class for converter
> > > + *
> > > + * The Converter class is an Abstract Base Class defining the interfaces of
> > > + * converter implementations.
> > > + *
> > > + * Converters offer scaling and pixel format conversion services on a input
> > > + * stream. The converter can output multiple streams with individual conversion
> > > + * parameters from the same input stream.
> > > + */
> > > +
> > > +/**
> > > + * \brief Construct a Converter instance
> > > + * \param[in] media The media device implementing the converter
> > > + *
> > > + * This seeks for the entity implementing data streaming function in the media
> > > + * graph entities and use its device node as converter device node.
> > > + */
> > > +Converter::Converter(MediaDevice *media)
> > > +{
> > > +	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()) {
> > > +		LOG(Converter, Error)
> > > +			<< "No entity suitable for implementing a converter in "
> > > +			<< media->driver() << " entities list.";
> > > +		return;
> > > +	}
> > > +
> > > +	deviceNode_ = (*it)->deviceNode();
> > > +}
> > > +
> > > +Converter::~Converter()
> > > +{
> > > +}
> > > +
> > > +/**
> > > + * \fn Converter::loadConfiguration()
> > > + * \brief Load converter configuration from file
> > > + * \param[in] filename The file name path
> > > + *
> > > + * Load converter dependent configuration parameters to apply on the hardware.
> > > + *
> > > + * \return 0 on success or a negative error code otherwise
> > > + */
> > > +
> > > +/**
> > > + * \fn Converter::isValid()
> > > + * \brief Check if the converter configuration is valid
> > > + * \return True is the converter is valid, false otherwise
> > > + */
> > > +
> > > +/**
> > > + * \fn Converter::formats()
> > > + * \brief Retrieve the list of supported pixel formats for an input pixel format
> > > + * \param[in] input Input pixel format to retrieve output pixel format list for
> > > + * \return The list of supported output pixel formats
> > > + */
> > > +
> > > +/**
> > > + * \fn Converter::sizes()
> > > + * \brief Retrieve the range of minimum and maximum output sizes for an input size
> > > + * \param[in] input Input stream size to retrieve range for
> > > + * \return A range of output image sizes
> > > + */
> > > +
> > > +/**
> > > + * \fn Converter::strideAndFrameSize()
> > > + * \brief Retrieve the output stride and frame size for an input configutation
> > > + * \param[in] pixelFormat Input stream pixel format
> > > + * \param[in] size Input stream size
> > > + * \return A tuple indicating the stride and frame size or an empty tuple on error
> > > + */
> > > +
> > > +/**
> > > + * \fn Converter::configure()
> > > + * \brief Configure a set of output stream conversion from an input stream
> > > + * \param[in] inputCfg Input stream configuration
> > > + * \param[out] outputCfgs A list of output stream configurations
> > > + * \return 0 on success or a negative error code otherwise
> > > + */
> > > +
> > > +/**
> > > + * \fn Converter::exportBuffers()
> > > + * \brief Export buffers from the converter device
> > > + * \param[in] output Output stream index exporting the buffers
> > > + * \param[in] count Number of buffers to allocate
> > > + * \param[out] buffers Vector to store allocated buffers
> > > + *
> > > + * This function operates similarly as V4L2VideoDevice::exportBuffers() on the
> > > + * output stream indicated by the \a output index.
> > > + *
> > > + * \return The number of allocated buffers on success or a negative error code
> > > + * otherwise
> > > + */
> > > +
> > > +/**
> > > + * \fn Converter::start()
> > > + * \brief Start the converter streaming operation
> > > + * \return 0 on success or a negative error code otherwise
> > > + */
> > > +
> > > +/**
> > > + * \fn Converter::stop()
> > > + * \brief Stop the converter streaming operation
> > > + */
> > > +
> > > +/**
> > > + * \fn Converter::queueBuffers()
> > > + * \brief Queue buffers to converter device
> > > + * \param[in] input The frame buffer to apply the conversion
> > > + * \param[out] outputs The container holding the output stream indexes and
> > > + * their respective frame buffer outputs.
> > > + *
> > > + * This function queues the \a input frame buffer on the output streams of the
> > > + * \a outputs map key and retrieve the output frame buffer indicated by the
> > > + * buffer map value.
> > > + *
> > > + * \return 0 on success or a negative error code otherwise
> > > + */
> > > +
> > > +/**
> > > + * \var Converter::inputBufferReady
> > > + * \brief A signal emitted when the input frame buffer completes
> > > + */
> > > +
> > > +/**
> > > + * \var Converter::outputBufferReady
> > > + * \brief A signal emitted when the output frame buffer completes
> > > + */
> > > +
> > > +/**
> > > + * \fn Converter::deviceNode()
> > > + * \brief The converter device node attribute accessor
> > > + * \return The converter device node string
> > > + */
> > > +
> > > +/**
> > > + * \class ConverterFactoryBase
> > > + * \brief Base class for converter factories
> > > + *
> > > + * The ConverterFactoryBase class is the base of all specializations of the
> > > + * ConverterFactory class template. It implements the factory registration,
> > > + * maintains a registry of factories, and provides access to the registered
> > > + * factories.
> > > + */
> > > +
> > > +/**
> > > + * \brief Construct a converter factory base
> > > + * \param[in] name Name of the converter class
> > > + * \param[in] aliases Name aliases of the converter class
> > > + *
> > > + * Creating an instance of the factory base registers it with the global list of
> > > + * factories, accessible through the factories() function.
> > > + *
> > > + * The factory \a name is used as unique identifier. If the converter
> > > + * implementation fully relies on a generic framework, the name should be the
> > > + * same as the framework. Otherwise, if the implementation is specialized, the
> > > + * factory name should match the driver name implementing the function.
> > > + *
> > > + * The factory \a aliases holds a list of driver names implementing a generic
> > > + * subsystem without any personalizations.
> > > + */
> > > +ConverterFactoryBase::ConverterFactoryBase(const std::string name, std::initializer_list<std::string> aliases)
> > > +	: name_(name), aliases_(aliases)
> > > +{
> > > +	registerType(this);
> > > +}
> > > +
> > > +/**
> > > + * \fn ConverterFactoryBase::aliases()
> > > + * \return The names aliases
> > > + */
> > > +
> > > +/**
> > > + * \brief Create an instance of the converter corresponding to a named factory
> > > + * \param[in] media Name of the factory
> > > + *
> > > + * \return A unique pointer to a new instance of the converter subclass
> > > + * corresponding to the named factory or one of its alias. Otherwise a null
> > > + * pointer if no such factory exists
> > > + */
> > > +std::unique_ptr<Converter> ConverterFactoryBase::create(MediaDevice *media)
> > > +{
> > > +	const std::vector<ConverterFactoryBase *> &factories =
> > > +		ConverterFactoryBase::factories();
> > > +
> > > +	for (const ConverterFactoryBase *factory : factories) {
> > > +		const std::vector<std::string> &aliases = factory->aliases();
> > > +		auto it = std::find(aliases.begin(), aliases.end(), media->driver());
> > > +
> > > +		if (it == aliases.end() && media->driver() != factory->name_)
> > > +			continue;
> > > +
> > > +		LOG(Converter, Debug)
> > > +			<< "Creating converter from "
> > > +			<< factory->name_ << " factory with "
> > > +			<< (it == aliases.end() ? "no" : media->driver()) << " alias.";
> > > +
> > > +		return factory->createInstance(media);
> > > +	}
> > > +
> > > +	return nullptr;
> > > +}
> > > +
> > > +/**
> > > + * \brief Add a converter class to the registry
> > > + * \param[in] factory Factory to use to construct the converter class
> > > + *
> > > + * The caller is responsible to guarantee the uniqueness of the converter name.
> > > + */
> > > +void ConverterFactoryBase::registerType(ConverterFactoryBase *factory)
> > > +{
> > > +	std::vector<ConverterFactoryBase *> &factories =
> > > +		ConverterFactoryBase::factories();
> > > +
> > > +	factories.push_back(factory);
> > > +}
> > > +
> > > +/**
> > > + * \brief Retrieve the list of all converter factory names
> > > + * \return The list of all converter factory names
> > > + */
> > > +std::vector<std::string> ConverterFactoryBase::names()
> > > +{
> > > +	std::vector<std::string> list;
> > > +
> > > +	std::vector<ConverterFactoryBase *> &factories =
> > > +		ConverterFactoryBase::factories();
> > > +
> > > +	for (ConverterFactoryBase *factory : factories) {
> > > +		list.push_back(factory->name_);
> > > +		for (auto alias : factory->aliases())
> > > +			list.push_back(alias);
> > > +	}
> > > +
> > > +	return list;
> > > +}
> > > +
> > > +/**
> > > + * \brief Retrieve the list of all converter factories
> > > + * \return The list of converter factories
> > > + */
> > > +std::vector<ConverterFactoryBase *> &ConverterFactoryBase::factories()
> > > +{
> > > +	/*
> > > +	 * The static factories map is defined inside the function to ensure
> > > +	 * it gets initialized on first use, without any dependency on link
> > > +	 * order.
> > > +	 */
> > > +	static std::vector<ConverterFactoryBase *> factories;
> > > +	return factories;
> > > +}
> > > +
> > > +/**
> > > + * \var ConverterFactoryBase::name_
> > > + * \brief The name of the factory
> > > + */
> > > +
> > > +/**
> > > + * \var ConverterFactoryBase::aliases_
> > > + * \brief The list holding the factory aliases
> > > + */
> > > +
> > > +/**
> > > + * \class ConverterFactory
> > > + * \brief Registration of ConverterFactory classes and creation of instances
> > > + * \param _Converter The converter class type for this factory
> > > + *
> > > + * To facilitate discovery and instantiation of Converter classes, the
> > > + * ConverterFactory class implements auto-registration of converter helpers.
> > > + * Each Converter subclass shall register itself using the REGISTER_CONVERTER()
> > > + * macro, which will create a corresponding instance of a ConverterFactory
> > > + * subclass and register it with the static list of factories.
> > > + */
> > > +
> > > +/**
> > > + * \fn ConverterFactory::ConverterFactory(const char *name, std::initializer_list<std::string> aliases)
> > > + * \brief Construct a converter factory
> > > + * \details \copydetails ConverterFactoryBase::ConverterFactoryBase
> > > + *
> > > + */
> > > +
> > > +/**
> > > + * \fn ConverterFactory::createInstance() const
> > > + * \brief Create an instance of the Converter corresponding to the factory
> > > + * \param[in] media Media device pointer
> > > + * \return A unique pointer to a newly constructed instance of the Converter
> > > + * subclass corresponding to the factory
> > > + */
> > > +
> > > +/**
> > > + * \def REGISTER_CONVERTER
> > > + * \brief Register a converter with the Converter factory
> > > + * \param[in] name Converter name used to register the class
> > > + * \param[in] converter Class name of Converter derived class to register
> > > + * \param[in] ... Optional list of alias names
> > > + *
> > > + * Register a Converter subclass with the factory and make it available to try
> > > + * and match converters.
> > > + */
> > > +
> > > +} /* namespace libcamera */
> > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > > index 0494e808..e9d0324e 100644
> > > --- a/src/libcamera/meson.build
> > > +++ b/src/libcamera/meson.build
> > > @@ -13,6 +13,7 @@ libcamera_sources = files([
> > >       'controls.cpp',
> > >       'control_serializer.cpp',
> > >       'control_validator.cpp',
> > > +    'converter.cpp',
> > >       'delayed_controls.cpp',
> > >       'device_enumerator.cpp',
> > >       'device_enumerator_sysfs.cpp',
> > > --
> > > 2.38.1
> > >