[libcamera-devel] [PATCH 1/3] libcamera: converter: split ConverterMD (media device) out of Converter

Andrey Konovalov andrey.konovalov at linaro.org
Sun Sep 10 19:50:25 CEST 2023


Make Converter a bit more generic by not requiring it to use a media device.
For this split out ConverterMD from Converter class, and rename
ConverterFactoryBase / ConverterFactory to ConverterMDFactoryBase /
ConverterMDFactory for consistency.

Signed-off-by: Andrey Konovalov <andrey.konovalov at linaro.org>
---
 include/libcamera/internal/converter.h        |  49 +---
 .../internal/converter/converter_v4l2_m2m.h   |   4 +-
 include/libcamera/internal/converter_media.h  |  86 +++++++
 include/libcamera/internal/meson.build        |   1 +
 src/libcamera/converter.cpp                   | 191 +-------------
 .../converter/converter_v4l2_m2m.cpp          |   4 +-
 src/libcamera/converter_media.cpp             | 241 ++++++++++++++++++
 src/libcamera/meson.build                     |   1 +
 src/libcamera/pipeline/simple/simple.cpp      |   4 +-
 9 files changed, 337 insertions(+), 244 deletions(-)
 create mode 100644 include/libcamera/internal/converter_media.h
 create mode 100644 src/libcamera/converter_media.cpp

diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h
index 834ec5ab..da1f5d73 100644
--- a/include/libcamera/internal/converter.h
+++ b/include/libcamera/internal/converter.h
@@ -24,14 +24,13 @@
 namespace libcamera {
 
 class FrameBuffer;
-class MediaDevice;
 class PixelFormat;
 struct StreamConfiguration;
 
 class Converter
 {
 public:
-	Converter(MediaDevice *media);
+	Converter();
 	virtual ~Converter();
 
 	virtual int loadConfiguration(const std::string &filename) = 0;
@@ -57,52 +56,6 @@ public:
 
 	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> compatibles);
-	virtual ~ConverterFactoryBase() = default;
-
-	const std::vector<std::string> &compatibles() const { return compatibles_; }
-
-	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> compatibles_;
-};
-
-template<typename _Converter>
-class ConverterFactory : public ConverterFactoryBase
-{
-public:
-	ConverterFactory(const char *name, std::initializer_list<std::string> compatibles)
-		: ConverterFactoryBase(name, compatibles)
-	{
-	}
-
-	std::unique_ptr<Converter> createInstance(MediaDevice *media) const override
-	{
-		return std::make_unique<_Converter>(media);
-	}
-};
-
-#define REGISTER_CONVERTER(name, converter, compatibles) \
-	static ConverterFactory<converter> global_##converter##Factory(name, compatibles);
-
 } /* namespace libcamera */
diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h
index 815916d0..aeb8c0e9 100644
--- a/include/libcamera/internal/converter/converter_v4l2_m2m.h
+++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h
@@ -20,7 +20,7 @@
 
 #include <libcamera/pixel_format.h>
 
-#include "libcamera/internal/converter.h"
+#include "libcamera/internal/converter_media.h"
 
 namespace libcamera {
 
@@ -31,7 +31,7 @@ class SizeRange;
 struct StreamConfiguration;
 class V4L2M2MDevice;
 
-class V4L2M2MConverter : public Converter
+class V4L2M2MConverter : public ConverterMD
 {
 public:
 	V4L2M2MConverter(MediaDevice *media);
diff --git a/include/libcamera/internal/converter_media.h b/include/libcamera/internal/converter_media.h
new file mode 100644
index 00000000..2b56ebdb
--- /dev/null
+++ b/include/libcamera/internal/converter_media.h
@@ -0,0 +1,86 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2020, Laurent Pinchart
+ * Copyright 2022 NXP
+ *
+ * converter_media.h - Generic media device based format converter interface
+ */
+
+#pragma once
+
+#include <functional>
+#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/internal/converter.h"
+
+namespace libcamera {
+
+class FrameBuffer;
+class MediaDevice;
+class PixelFormat;
+struct StreamConfiguration;
+
+class ConverterMD : public Converter
+{
+public:
+	ConverterMD(MediaDevice *media);
+	~ConverterMD();
+
+	const std::string &deviceNode() const { return deviceNode_; }
+
+private:
+	std::string deviceNode_;
+};
+
+class ConverterMDFactoryBase
+{
+public:
+	ConverterMDFactoryBase(const std::string name, std::initializer_list<std::string> compatibles);
+	virtual ~ConverterMDFactoryBase() = default;
+
+	const std::vector<std::string> &compatibles() const { return compatibles_; }
+
+	static std::unique_ptr<ConverterMD> create(MediaDevice *media);
+	static std::vector<ConverterMDFactoryBase *> &factories();
+	static std::vector<std::string> names();
+
+private:
+	LIBCAMERA_DISABLE_COPY_AND_MOVE(ConverterMDFactoryBase)
+
+	static void registerType(ConverterMDFactoryBase *factory);
+
+	virtual std::unique_ptr<ConverterMD> createInstance(MediaDevice *media) const = 0;
+
+	std::string name_;
+	std::vector<std::string> compatibles_;
+};
+
+template<typename _ConverterMD>
+class ConverterMDFactory : public ConverterMDFactoryBase
+{
+public:
+	ConverterMDFactory(const char *name, std::initializer_list<std::string> compatibles)
+		: ConverterMDFactoryBase(name, compatibles)
+	{
+	}
+
+	std::unique_ptr<ConverterMD> createInstance(MediaDevice *media) const override
+	{
+		return std::make_unique<_ConverterMD>(media);
+	}
+};
+
+#define REGISTER_CONVERTER_MD(name, converter, compatibles) \
+	static ConverterMDFactory<converter> global_##converter##MDFactory(name, compatibles);
+
+} /* namespace libcamera */
diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
index 7f1f3440..e9c41346 100644
--- a/include/libcamera/internal/meson.build
+++ b/include/libcamera/internal/meson.build
@@ -21,6 +21,7 @@ libcamera_internal_headers = files([
     'control_serializer.h',
     'control_validator.h',
     'converter.h',
+    'converter_media.h',
     'delayed_controls.h',
     'device_enumerator.h',
     'device_enumerator_sysfs.h',
diff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp
index fa0f1ec8..92dcdc03 100644
--- a/src/libcamera/converter.cpp
+++ b/src/libcamera/converter.cpp
@@ -38,26 +38,9 @@ LOG_DEFINE_CATEGORY(Converter)
 
 /**
  * \brief Construct a Converter instance
- * \param[in] media The media device implementing the converter
- *
- * This searches for the entity implementing the data streaming function in the
- * media graph entities and use its device node as the converter device node.
  */
-Converter::Converter(MediaDevice *media)
+Converter::Converter()
 {
-	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()
@@ -159,176 +142,4 @@ Converter::~Converter()
  * \brief A signal emitted on each frame buffer completion of the output queue
  */
 
-/**
- * \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] compatibles 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 compatibles holds a list of driver names implementing a generic
- * subsystem without any personalizations.
- */
-ConverterFactoryBase::ConverterFactoryBase(const std::string name, std::initializer_list<std::string> compatibles)
-	: name_(name), compatibles_(compatibles)
-{
-	registerType(this);
-}
-
-/**
- * \fn ConverterFactoryBase::compatibles()
- * \return The names compatibles
- */
-
-/**
- * \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> &compatibles = factory->compatibles();
-		auto it = std::find(compatibles.begin(), compatibles.end(), media->driver());
-
-		if (it == compatibles.end() && media->driver() != factory->name_)
-			continue;
-
-		LOG(Converter, Debug)
-			<< "Creating converter from "
-			<< factory->name_ << " factory with "
-			<< (it == compatibles.end() ? "no" : media->driver()) << " alias.";
-
-		std::unique_ptr<Converter> converter = factory->createInstance(media);
-		if (converter->isValid())
-			return converter;
-	}
-
-	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->compatibles())
-			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::compatibles_
- * \brief The list holding the factory compatibles
- */
-
-/**
- * \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> compatibles)
- * \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] compatibles List of compatible names
- *
- * Register a Converter subclass with the factory and make it available to try
- * and match converters.
- */
-
 } /* namespace libcamera */
diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp
index 2a4d1d99..d0a5e3bf 100644
--- a/src/libcamera/converter/converter_v4l2_m2m.cpp
+++ b/src/libcamera/converter/converter_v4l2_m2m.cpp
@@ -194,7 +194,7 @@ void V4L2M2MConverter::Stream::captureBufferReady(FrameBuffer *buffer)
  */
 
 V4L2M2MConverter::V4L2M2MConverter(MediaDevice *media)
-	: Converter(media)
+	: ConverterMD(media)
 {
 	if (deviceNode().empty())
 		return;
@@ -448,6 +448,6 @@ static std::initializer_list<std::string> compatibles = {
 	"pxp",
 };
 
-REGISTER_CONVERTER("v4l2_m2m", V4L2M2MConverter, compatibles)
+REGISTER_CONVERTER_MD("v4l2_m2m", V4L2M2MConverter, compatibles)
 
 } /* namespace libcamera */
diff --git a/src/libcamera/converter_media.cpp b/src/libcamera/converter_media.cpp
new file mode 100644
index 00000000..f5024764
--- /dev/null
+++ b/src/libcamera/converter_media.cpp
@@ -0,0 +1,241 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright 2022 NXP
+ *
+ * converter.cpp - Generic format converter interface
+ */
+
+#include "libcamera/internal/converter_media.h"
+
+#include <algorithm>
+
+#include <libcamera/base/log.h>
+
+#include "libcamera/internal/media_device.h"
+
+#include "linux/media.h"
+
+/**
+ * \file internal/converter_media.h
+ * \brief Abstract media device based converter
+ */
+
+namespace libcamera {
+
+LOG_DECLARE_CATEGORY(Converter)
+
+/**
+ * \class ConverterMD
+ * \brief Abstract Base Class for media device based converter
+ *
+ * The ConverterMD class is an Abstract Base Class defining the interfaces of
+ * media device based converter implementations.
+ *
+ * Converters offer scaling and pixel format conversion services on an input
+ * stream. The converter can output multiple streams with individual conversion
+ * parameters from the same input stream.
+ */
+
+/**
+ * \brief Construct a ConverterMD instance
+ * \param[in] media The media device implementing the converter
+ *
+ * This searches for the entity implementing the data streaming function in the
+ * media graph entities and use its device node as the converter device node.
+ */
+ConverterMD::ConverterMD(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();
+}
+
+ConverterMD::~ConverterMD()
+{
+}
+
+/**
+ * \fn ConverterMD::deviceNode()
+ * \brief The converter device node attribute accessor
+ * \return The converter device node string
+ */
+
+/**
+ * \class ConverterMDFactoryBase
+ * \brief Base class for media device based converter factories
+ *
+ * The ConverterMDFactoryBase class is the base of all specializations of the
+ * ConverterMDFactory class template. It implements the factory registration,
+ * maintains a registry of factories, and provides access to the registered
+ * factories.
+ */
+
+/**
+ * \brief Construct a media device based converter factory base
+ * \param[in] name Name of the converter class
+ * \param[in] compatibles 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 compatibles holds a list of driver names implementing a generic
+ * subsystem without any personalizations.
+ */
+ConverterMDFactoryBase::ConverterMDFactoryBase(const std::string name, std::initializer_list<std::string> compatibles)
+	: name_(name), compatibles_(compatibles)
+{
+	registerType(this);
+}
+
+/**
+ * \fn ConverterMDFactoryBase::compatibles()
+ * \return The names compatibles
+ */
+
+/**
+ * \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 media device based
+ * converter subclass corresponding to the named factory or one of its alias.
+ * Otherwise a null pointer if no such factory exists.
+ */
+std::unique_ptr<ConverterMD> ConverterMDFactoryBase::create(MediaDevice *media)
+{
+	const std::vector<ConverterMDFactoryBase *> &factories =
+		ConverterMDFactoryBase::factories();
+
+	for (const ConverterMDFactoryBase *factory : factories) {
+		const std::vector<std::string> &compatibles = factory->compatibles();
+		auto it = std::find(compatibles.begin(), compatibles.end(), media->driver());
+
+		if (it == compatibles.end() && media->driver() != factory->name_)
+			continue;
+
+		LOG(Converter, Debug)
+			<< "Creating converter from "
+			<< factory->name_ << " factory with "
+			<< (it == compatibles.end() ? "no" : media->driver()) << " alias.";
+
+		std::unique_ptr<ConverterMD> converter = factory->createInstance(media);
+		if (converter->isValid())
+			return converter;
+	}
+
+	return nullptr;
+}
+
+/**
+ * \brief Add a media device based converter class to the registry
+ * \param[in] factory Factory to use to construct the media device based
+ * converter class
+ *
+ * The caller is responsible to guarantee the uniqueness of the converter name.
+ */
+void ConverterMDFactoryBase::registerType(ConverterMDFactoryBase *factory)
+{
+	std::vector<ConverterMDFactoryBase *> &factories =
+		ConverterMDFactoryBase::factories();
+
+	factories.push_back(factory);
+}
+
+/**
+ * \brief Retrieve the list of all media device based converter factory names
+ * \return The list of all media device based converter factory names
+ */
+std::vector<std::string> ConverterMDFactoryBase::names()
+{
+	std::vector<std::string> list;
+
+	std::vector<ConverterMDFactoryBase *> &factories =
+		ConverterMDFactoryBase::factories();
+
+	for (ConverterMDFactoryBase *factory : factories) {
+		list.push_back(factory->name_);
+		for (auto alias : factory->compatibles())
+			list.push_back(alias);
+	}
+
+	return list;
+}
+
+/**
+ * \brief Retrieve the list of all media device based converter factories
+ * \return The list of media device based converter factories
+ */
+std::vector<ConverterMDFactoryBase *> &ConverterMDFactoryBase::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<ConverterMDFactoryBase *> factories;
+	return factories;
+}
+
+/**
+ * \var ConverterMDFactoryBase::name_
+ * \brief The name of the factory
+ */
+
+/**
+ * \var ConverterMDFactoryBase::compatibles_
+ * \brief The list holding the factory compatibles
+ */
+
+/**
+ * \class ConverterMDFactory
+ * \brief Registration of ConverterMDFactory classes and creation of instances
+ * \param _Converter The converter class type for this factory
+ *
+ * To facilitate discovery and instantiation of ConverterMD classes, the
+ * ConverterMDFactory class implements auto-registration of converter helpers.
+ * Each ConverterMD subclass shall register itself using the
+ * REGISTER_CONVERTER() macro, which will create a corresponding instance of a
+ * ConverterMDFactory subclass and register it with the static list of
+ * factories.
+ */
+
+/**
+ * \fn ConverterMDFactory::ConverterMDFactory(const char *name, std::initializer_list<std::string> compatibles)
+ * \brief Construct a media device converter factory
+ * \details \copydetails ConverterMDFactoryBase::ConverterMDFactoryBase
+ */
+
+/**
+ * \fn ConverterMDFactory::createInstance() const
+ * \brief Create an instance of the ConverterMD corresponding to the factory
+ * \param[in] media Media device pointer
+ * \return A unique pointer to a newly constructed instance of the ConverterMD
+ * subclass corresponding to the factory
+ */
+
+/**
+ * \def REGISTER_CONVERTER_MD
+ * \brief Register a media device based converter with the ConverterMD factory
+ * \param[in] name ConverterMD name used to register the class
+ * \param[in] converter Class name of ConverterMD derived class to register
+ * \param[in] compatibles List of compatible names
+ *
+ * Register a ConverterMD 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 b24f8296..af8b1812 100644
--- a/src/libcamera/meson.build
+++ b/src/libcamera/meson.build
@@ -14,6 +14,7 @@ libcamera_sources = files([
     'control_serializer.cpp',
     'control_validator.cpp',
     'converter.cpp',
+    'converter_media.cpp',
     'delayed_controls.cpp',
     'device_enumerator.cpp',
     'device_enumerator_sysfs.cpp',
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 05ba76bc..f0622a74 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -30,7 +30,7 @@
 
 #include "libcamera/internal/camera.h"
 #include "libcamera/internal/camera_sensor.h"
-#include "libcamera/internal/converter.h"
+#include "libcamera/internal/converter_media.h"
 #include "libcamera/internal/device_enumerator.h"
 #include "libcamera/internal/media_device.h"
 #include "libcamera/internal/pipeline_handler.h"
@@ -497,7 +497,7 @@ int SimpleCameraData::init()
 	/* Open the converter, if any. */
 	MediaDevice *converter = pipe->converter();
 	if (converter) {
-		converter_ = ConverterFactoryBase::create(converter);
+		converter_ = ConverterMDFactoryBase::create(converter);
 		if (!converter_) {
 			LOG(SimplePipeline, Warning)
 				<< "Failed to create converter, disabling format conversion";
-- 
2.34.1



More information about the libcamera-devel mailing list