[libcamera-devel] [RFC] libcamera: Use properties for Stream Configuration

Jacopo Mondi jacopo at jmondi.org
Fri Apr 19 19:15:38 CEST 2019


Use the stream's property to perform stream configuration.

---
Hello,
   this patch is clearly an RFC and its only intention is to get the
discussion started on how to use stream's properties to configure
streams.

This patch introduces a naive implementation of a StreamProperties
class, which gets associated to a stream partly at stream creation time
(i.e. stream name) and partly at stream configuration time, where
dependencies between streams should be evalutated and the list of
supported stream sizes and format created (currently, for IPU3, all
streams are created equal, with the viewfinder one supporting a smaller
size).

Application receives back a map of Stream * to StreamProperties and
cycle on them to find out which stream has been associated to which role
and use the stream properties to create a CameraConfiguration to be
then used for the 'configureStreams()' method.

I'm mostly interested in knowing your opinion on the pipeline handler
and application API, it this is the direction we should take and provide
a real storage class for StreamProperties.

Thanks
  j
----
---
 include/libcamera/camera.h               |  3 +-
 include/libcamera/stream.h               | 12 +++++
 src/cam/main.cpp                         | 62 +++++++++++++-----------
 src/libcamera/camera.cpp                 |  4 +-
 src/libcamera/include/pipeline_handler.h |  3 +-
 src/libcamera/pipeline/ipu3/ipu3.cpp     | 62 +++++++++---------------
 src/libcamera/pipeline/meson.build       | 10 ++--
 src/meson.build                          |  2 +-
 test/meson.build                         |  2 +-
 9 files changed, 84 insertions(+), 76 deletions(-)

diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
index b2dafda342fe..c01bf86da92e 100644
--- a/include/libcamera/camera.h
+++ b/include/libcamera/camera.h
@@ -22,6 +22,7 @@ class PipelineHandler;
 class Request;
 class Stream;
 class StreamConfiguration;
+class StreamProperties;
 class StreamUsage;
 
 class CameraConfiguration
@@ -73,7 +74,7 @@ public:
 	int release();
 
 	const std::set<Stream *> &streams() const;
-	CameraConfiguration
+	std::map<Stream *, StreamProperties>
 	streamConfiguration(const std::vector<StreamUsage> &usage);
 	int configureStreams(const CameraConfiguration &config);
 
diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
index 3caaefc9f8e9..d22289e07708 100644
--- a/include/libcamera/stream.h
+++ b/include/libcamera/stream.h
@@ -7,6 +7,7 @@
 #ifndef __LIBCAMERA_STREAM_H__
 #define __LIBCAMERA_STREAM_H__
 
+#include <map>
 #include <string>
 
 #include <libcamera/buffer.h>
@@ -47,6 +48,15 @@ private:
 	Size size_;
 };
 
+class StreamProperties
+{
+public:
+	std::string name_;
+	StreamUsage::Role role_;
+	std::vector<Size> sizes_;
+	std::vector<unsigned int> formats_;
+};
+
 class Stream
 {
 public:
@@ -72,6 +82,8 @@ public:
 	BufferPool &bufferPool() { return bufferPool_; }
 	const StreamConfiguration &configuration() const { return configuration_; }
 
+	StreamProperties props_;
+
 protected:
 	friend class Camera;
 
diff --git a/src/cam/main.cpp b/src/cam/main.cpp
index 46aba728393c..c57a72047b85 100644
--- a/src/cam/main.cpp
+++ b/src/cam/main.cpp
@@ -87,14 +87,12 @@ static int parseOptions(int argc, char *argv[])
 
 static int prepareCameraConfig(CameraConfiguration *config)
 {
+	std::map<Stream *, StreamProperties> props;
 	std::vector<StreamUsage> roles;
 
-	streamInfo.clear();
-
 	/* If no configuration is provided assume a single video stream. */
 	if (!options.isSet(OptStream)) {
-		*config = camera->streamConfiguration({ Stream::VideoRecording() });
-		streamInfo[config->front()] = "stream0";
+		props = camera->streamConfiguration({ Stream::VideoRecording() });
 		return 0;
 	}
 
@@ -121,36 +119,46 @@ static int prepareCameraConfig(CameraConfiguration *config)
 		}
 	}
 
-	*config = camera->streamConfiguration(roles);
-
-	if (!config->isValid()) {
-		std::cerr << "Failed to get default stream configuration"
-			  << std::endl;
+	props = camera->streamConfiguration(roles);
+	if (props.empty()) {
+		std::cerr << "Failed to get stream properties" << std::endl;
 		return -EINVAL;
 	}
 
-	/* Apply configuration explicitly requested. */
-	CameraConfiguration::iterator it = config->begin();
-	for (auto const &value : streamOptions) {
-		KeyValueParser::Options conf = value.toKeyValues();
-		Stream *stream = *it;
-		it++;
+	/* Save the streams names and apply the requested configuration. */
+	for (auto it : props) {
+		Stream *stream = it.first;
+		StreamProperties &props = it.second;
 
-		if (conf.isSet("width"))
-			(*config)[stream].width = conf["width"];
+		streamInfo[stream] = props.name_;
 
-		if (conf.isSet("height"))
-			(*config)[stream].height = conf["height"];
+		for (auto const &value : streamOptions) {
+			KeyValueParser::Options conf = value.toKeyValues();
 
-		/* TODO: Translate 4CC string to ID. */
-		if (conf.isSet("pixelformat"))
-			(*config)[stream].pixelFormat = conf["pixelformat"];
-	}
+			if (!conf.isSet("role") ||
+			    conf["role"] != props.role_)
+				continue;
+
+			/* Get the stream max sizes */
+			Size maxSize = props.sizes_.back();
 
-	unsigned int index = 0;
-	for (Stream *stream : *config) {
-		streamInfo[stream] = "stream" + std::to_string(index);
-		index++;
+			if (conf.isSet("width"))
+				(*config)[stream].width = conf["width"];
+			else
+				(*config)[stream].width = maxSize.width;
+
+			if (conf.isSet("height"))
+				(*config)[stream].height = conf["height"];
+			else
+				(*config)[stream].height = maxSize.height;
+
+			/* TODO: Translate 4CC string to ID. */
+			unsigned int format = *props.formats_.begin();
+			if (conf.isSet("pixelformat"))
+				(*config)[stream].pixelFormat = conf["pixelformat"];
+			else
+				(*config)[stream].pixelFormat = format;
+		}
 	}
 
 	return 0;
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index 655996f26224..b750307e33bd 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -540,11 +540,11 @@ const std::set<Stream *> &Camera::streams() const
  * \return A valid CameraConfiguration if the requested usages can be satisfied,
  * or a invalid one otherwise
  */
-CameraConfiguration
+std::map<Stream *, StreamProperties>
 Camera::streamConfiguration(const std::vector<StreamUsage> &usages)
 {
 	if (disconnected_ || !usages.size() || usages.size() > streams_.size())
-		return CameraConfiguration();
+		return std::map<Stream *, StreamProperties>{};
 
 	return pipe_->streamConfiguration(this, usages);
 }
diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
index c3f7d4c29205..99d9a34efeb0 100644
--- a/src/libcamera/include/pipeline_handler.h
+++ b/src/libcamera/include/pipeline_handler.h
@@ -27,6 +27,7 @@ class PipelineHandler;
 class Request;
 class Stream;
 class StreamConfiguration;
+class StreamProperties;
 class StreamUsage;
 
 class CameraData
@@ -55,7 +56,7 @@ public:
 
 	virtual bool match(DeviceEnumerator *enumerator) = 0;
 
-	virtual CameraConfiguration
+	virtual std::map<Stream *, StreamProperties>
 	streamConfiguration(Camera *camera, const std::vector<StreamUsage> &usages) = 0;
 	virtual int configureStreams(Camera *camera, const CameraConfiguration &config) = 0;
 
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 2c2929207d35..553673a99953 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -141,7 +141,6 @@ public:
 	}
 
 	bool active_;
-	std::string name_;
 	ImgUDevice::ImgUOutput *device_;
 };
 
@@ -151,7 +150,7 @@ public:
 	PipelineHandlerIPU3(CameraManager *manager);
 	~PipelineHandlerIPU3();
 
-	CameraConfiguration
+	std::map<Stream *, StreamProperties>
 	streamConfiguration(Camera *camera,
 			    const std::vector<StreamUsage> &usages) override;
 	int configureStreams(Camera *camera,
@@ -219,19 +218,19 @@ PipelineHandlerIPU3::~PipelineHandlerIPU3()
 		imguMediaDev_->release();
 }
 
-CameraConfiguration
+std::map<Stream *, StreamProperties>
 PipelineHandlerIPU3::streamConfiguration(Camera *camera,
 					 const std::vector<StreamUsage> &usages)
 {
 	IPU3CameraData *data = cameraData(camera);
-	CameraConfiguration cameraConfig = {};
+	std::map<Stream *, StreamProperties> propertiesMap;
 	std::set<IPU3Stream *> streams = {
 		&data->outStream_,
 		&data->vfStream_,
 	};
 
 	for (const StreamUsage &usage : usages) {
-		StreamConfiguration streamConfig = {};
+		StreamProperties streamProperties = {};
 		StreamUsage::Role role = usage.role();
 		IPU3Stream *stream = nullptr;
 
@@ -253,17 +252,13 @@ PipelineHandlerIPU3::streamConfiguration(Camera *camera,
 				break;
 			}
 
-			/*
-			 * FIXME: Soraka: the maximum resolution reported by
-			 * both sensors (2592x1944 for ov5670 and 4224x3136 for
-			 * ov13858) are returned as default configurations but
-			 * they're not correctly processed by the ImgU.
-			 * Resolutions up tp 2560x1920 have been validated.
-			 *
-			 * \todo Clarify ImgU alignment requirements.
-			 */
-			streamConfig.width = 2560;
-			streamConfig.height = 1920;
+			streamProperties = stream->props_;
+			streamProperties.role_ = role;
+			streamProperties.formats_ = { V4L2_PIX_FMT_NV12 };
+			streamProperties.sizes_ = {
+				{ 320, 240 }, { 640, 480 }, { 1024, 768 },
+				{ 1920, 1080 },	{ 2560, 1920 },
+			};
 
 			break;
 
@@ -285,18 +280,13 @@ PipelineHandlerIPU3::streamConfiguration(Camera *camera,
 
 			stream = &data->vfStream_;
 
-			/*
-			 * Align the requested viewfinder size to the
-			 * maximum available sensor resolution and to the
-			 * IPU3 alignment constraints.
-			 */
-			const Size &res = data->cio2_.sensor_->resolution();
-			unsigned int width = std::min(usage.size().width,
-						      res.width);
-			unsigned int height = std::min(usage.size().height,
-						       res.height);
-			streamConfig.width = width & ~7;
-			streamConfig.height = height & ~3;
+			streamProperties.role_ = role;
+			streamProperties = stream->props_;
+			streamProperties.formats_ = { V4L2_PIX_FMT_NV12 };
+			streamProperties.sizes_ = {
+				{ 320, 240 }, { 640, 480 }, { 1024, 768 },
+				{ 1920, 1080 },	{ 2560, 1920 },
+			};
 
 			break;
 		}
@@ -308,21 +298,17 @@ PipelineHandlerIPU3::streamConfiguration(Camera *camera,
 		}
 
 		if (!stream)
-			return CameraConfiguration{};
+			return std::map<Stream *, StreamProperties>{};
 
 		streams.erase(stream);
 
-		streamConfig.pixelFormat = V4L2_PIX_FMT_NV12;
-		streamConfig.bufferCount = IPU3_BUFFER_COUNT;
-
-		cameraConfig[stream] = streamConfig;
+		propertiesMap[stream] = streamProperties;
 
 		LOG(IPU3, Debug)
-			<< "Stream '" << stream->name_ << "' format set to "
-			<< streamConfig.toString();
+			<< "Stream '" << stream->props_.name_ << "'";
 	}
 
-	return cameraConfig;
+	return propertiesMap;
 }
 
 int PipelineHandlerIPU3::configureStreams(Camera *camera,
@@ -743,9 +729,9 @@ int PipelineHandlerIPU3::registerCameras()
 		 */
 		data->imgu_ = numCameras ? &imgu1_ : &imgu0_;
 		data->outStream_.device_ = &data->imgu_->output_;
-		data->outStream_.name_ = "output";
+		data->outStream_.props_.name_ = "output";
 		data->vfStream_.device_ = &data->imgu_->viewfinder_;
-		data->vfStream_.name_ = "viewfinder";
+		data->outStream_.props_.name_ = "viewfinder";
 
 		/*
 		 * Connect video devices' 'bufferReady' signals to their
diff --git a/src/libcamera/pipeline/meson.build b/src/libcamera/pipeline/meson.build
index 0d466225a72e..d7766edf96d8 100644
--- a/src/libcamera/pipeline/meson.build
+++ b/src/libcamera/pipeline/meson.build
@@ -1,7 +1,7 @@
-libcamera_sources += files([
-    'uvcvideo.cpp',
-    'vimc.cpp',
-])
+#libcamera_sources += files([
+#    'uvcvideo.cpp',
+#    'vimc.cpp',
+#])
 
 subdir('ipu3')
-subdir('rkisp1')
+#subdir('rkisp1')
diff --git a/src/meson.build b/src/meson.build
index 4e41fd3e7887..018e9d75c0ac 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -1,3 +1,3 @@
 subdir('libcamera')
 subdir('cam')
-subdir('qcam')
+#subdir('qcam')
diff --git a/test/meson.build b/test/meson.build
index d501f2beaf96..edbcc090f524 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -1,6 +1,6 @@
 subdir('libtest')
 
-subdir('camera')
+#subdir('camera')
 subdir('media_device')
 subdir('pipeline')
 subdir('v4l2_device')
-- 
2.21.0



More information about the libcamera-devel mailing list