libcamera: Refactor the camera configuration storage and API

Refactor the CameraConfiguration structure to not rely on Stream
instances. This is a step towards making the camera configuration object
more powerful with configuration validation using "try" semantics.

The CameraConfiguration now exposes a simple vector-like API to access
the contained stream configurations. Both operator[]() and at() are
provided to access elements. The isEmpty() method is renamed to empty()
and the methods reordered to match the std::vector class.

As applications need access to the Stream instances associated with the
configuration entries in order to associate buffers with streams when
creating requests, expose the stream selected by the pipeline handler
through a new StreamConfiguration::stream().

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

8 files changed, 179 insertions, 165 deletions

diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index a3921f91..5848330f 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -46,72 +46,40 @@ LOG_DECLARE_CATEGORY(Camera)
  * \class CameraConfiguration
  * \brief Hold configuration for streams of the camera
 
- * The CameraConfiguration holds an ordered list of streams and their associated
- * StreamConfiguration. From a data storage point of view, the class operates as
- * a map of Stream pointers to StreamConfiguration, with entries accessed with
- * operator[](Stream *). Accessing an entry for a Stream pointer not yet stored
- * in the configuration inserts a new empty entry.
- *
- * The class also suppors iterators, and from that point of view operates as a
- * vector of Stream pointers. The streams are iterated in insertion order, and
- * the operator[](int) returns the Stream pointer based on its insertion index.
- * Accessing a stream with an invalid index returns a null pointer.
+ * The CameraConfiguration holds an ordered list of stream configurations. It
+ * supports iterators and operates as a vector of StreamConfiguration instances.
+ * The stream configurations are inserted by addConfiguration(), and the
+ * operator[](int) returns a reference to the StreamConfiguration based on its
+ * insertion index. Accessing a stream configuration with an invalid index
+ * results in undefined behaviour.
  */
 
 /**
  * \typedef CameraConfiguration::iterator
- * \brief Iterator for the streams in the configuration
+ * \brief Iterator for the stream configurations in the camera configuration
  */
 
 /**
  * \typedef CameraConfiguration::const_iterator
- * \brief Const iterator for the streams in the configuration
+ * \brief Const iterator for the stream configuration in the camera
+ * configuration
  */
 
 /**
  * \brief Create an empty camera configuration
  */
 CameraConfiguration::CameraConfiguration()
-	: order_({}), config_({})
-{
-}
-
-/**
- * \brief Retrieve an iterator to the first stream in the sequence
- * \return An iterator to the first stream
- */
-std::vector<Stream *>::iterator CameraConfiguration::begin()
+	: config_({})
 {
-	return order_.begin();
 }
 
 /**
- * \brief Retrieve an iterator pointing to the past-the-end stream in the
- * sequence
- * \return An iterator to the element following the last stream
- */
-std::vector<Stream *>::iterator CameraConfiguration::end()
-{
-	return order_.end();
-}
-
-/**
- * \brief Retrieve a const iterator to the first element of the streams
- * \return A const iterator to the first stream
- */
-std::vector<Stream *>::const_iterator CameraConfiguration::begin() const
-{
-	return order_.begin();
-}
-
-/**
- * \brief Retrieve a const iterator pointing to the past-the-end stream in the
- * sequence
- * \return A const iterator to the element following the last stream
+ * \brief Add a stream configuration to the camera configuration
+ * \param[in] cfg The stream configuration
  */
-std::vector<Stream *>::const_iterator CameraConfiguration::end() const
+void CameraConfiguration::addConfiguration(const StreamConfiguration &cfg)
 {
-	return order_.end();
+	config_.push_back(cfg);
 }
 
 /**
@@ -125,12 +93,10 @@ std::vector<Stream *>::const_iterator CameraConfiguration::end() const
  */
 bool CameraConfiguration::isValid() const
 {
-	if (isEmpty())
+	if (empty())
 		return false;
 
-	for (auto const &it : config_) {
-		const StreamConfiguration &cfg = it.second;
-
+	for (const StreamConfiguration &cfg : config_) {
 		if (cfg.size.width == 0 || cfg.size.height == 0 ||
 		    cfg.pixelFormat == 0 || cfg.bufferCount == 0)
 			return false;
@@ -140,89 +106,116 @@ bool CameraConfiguration::isValid() const
 }
 
 /**
- * \brief Check if the camera configuration is empty
- * \return True if the configuration is empty
+ * \brief Retrieve a reference to a stream configuration
+ * \param[in] index Numerical index
+ *
+ * The \a index represents the zero based insertion order of stream
+ * configuration into the camera configuration with addConfiguration(). Calling
+ * this method with an invalid index results in undefined behaviour.
+ *
+ * \return The stream configuration
  */
-bool CameraConfiguration::isEmpty() const
+StreamConfiguration &CameraConfiguration::at(unsigned int index)
 {
-	return order_.empty();
+	return config_[index];
 }
 
 /**
- * \brief Retrieve the number of stream configurations
- * \return Number of stream configurations
+ * \brief Retrieve a const reference to a stream configuration
+ * \param[in] index Numerical index
+ *
+ * The \a index represents the zero based insertion order of stream
+ * configuration into the camera configuration with addConfiguration(). Calling
+ * this method with an invalid index results in undefined behaviour.
+ *
+ * \return The stream configuration
  */
-std::size_t CameraConfiguration::size() const
+const StreamConfiguration &CameraConfiguration::at(unsigned int index) const
 {
-	return order_.size();
+	return config_[index];
 }
 
 /**
- * \brief Access the first stream in the configuration
- * \return The first stream in the configuration
+ * \fn StreamConfiguration &CameraConfiguration::operator[](unsigned int)
+ * \brief Retrieve a reference to a stream configuration
+ * \param[in] index Numerical index
+ *
+ * The \a index represents the zero based insertion order of stream
+ * configuration into the camera configuration with addConfiguration(). Calling
+ * this method with an invalid index results in undefined behaviour.
+ *
+ * \return The stream configuration
+ */
+
+/**
+ * \fn const StreamConfiguration &CameraConfiguration::operator[](unsigned int) const
+ * \brief Retrieve a const reference to a stream configuration
+ * \param[in] index Numerical index
+ *
+ * The \a index represents the zero based insertion order of stream
+ * configuration into the camera configuration with addConfiguration(). Calling
+ * this method with an invalid index results in undefined behaviour.
+ *
+ * \return The stream configuration
+ */
+
+/**
+ * \brief Retrieve an iterator to the first stream configuration in the
+ * sequence
+ * \return An iterator to the first stream configuration
  */
-Stream *CameraConfiguration::front()
+CameraConfiguration::iterator CameraConfiguration::begin()
 {
-	return order_.front();
+	return config_.begin();
 }
 
 /**
- * \brief Access the first stream in the configuration
- * \return The first const stream pointer in the configuration
+ * \brief Retrieve a const iterator to the first element of the stream
+ * configurations
+ * \return A const iterator to the first stream configuration
  */
-const Stream *CameraConfiguration::front() const
+CameraConfiguration::const_iterator CameraConfiguration::begin() const
 {
-	return order_.front();
+	return config_.begin();
 }
 
 /**
- * \brief Retrieve a stream pointer from index
- * \param[in] index Numerical index
- *
- * The \a index represents the zero based insertion order of stream and stream
- * configuration into the camera configuration.
- *
- * \return The stream pointer at index, or a nullptr if the index is out of
- * bounds
+ * \brief Retrieve an iterator pointing to the past-the-end stream
+ * configuration in the sequence
+ * \return An iterator to the element following the last stream configuration
  */
-Stream *CameraConfiguration::operator[](unsigned int index) const
+CameraConfiguration::iterator CameraConfiguration::end()
 {
-	if (index >= order_.size())
-		return nullptr;
-
-	return order_.at(index);
+	return config_.end();
 }
 
 /**
- * \brief Retrieve a reference to a stream configuration
- * \param[in] stream Stream to retrieve configuration for
- *
- * If the camera configuration does not yet contain a configuration for
- * the requested stream, create and return an empty stream configuration.
- *
- * \return The configuration for the stream
+ * \brief Retrieve a const iterator pointing to the past-the-end stream
+ * configuration in the sequence
+ * \return A const iterator to the element following the last stream
+ * configuration
  */
-StreamConfiguration &CameraConfiguration::operator[](Stream *stream)
+CameraConfiguration::const_iterator CameraConfiguration::end() const
 {
-	if (config_.find(stream) == config_.end())
-		order_.push_back(stream);
+	return config_.end();
+}
 
-	return config_[stream];
+/**
+ * \brief Check if the camera configuration is empty
+ * \return True if the configuration is empty
+ */
+bool CameraConfiguration::empty() const
+{
+	return config_.empty();
 }
 
 /**
- * \brief Retrieve a const reference to a stream configuration
- * \param[in] stream Stream to retrieve configuration for
- *
- * No new stream configuration is created if called with \a stream that is not
- * already part of the camera configuration, doing so is an invalid operation
- * and results in undefined behaviour.
- *
- * \return The configuration for the stream
+ * \brief Retrieve the number of stream configurations
+ * \return Number of stream configurations
  */
-const StreamConfiguration &CameraConfiguration::operator[](Stream *stream) const
+std::size_t CameraConfiguration::size() const
 {
-	return config_.at(stream);
+	return config_.size();
 }
 
 /**
@@ -561,13 +554,9 @@ Camera::generateConfiguration(const StreamRoles &roles)
 	CameraConfiguration config = pipe_->generateConfiguration(this, roles);
 
 	std::ostringstream msg("streams configuration:", std::ios_base::ate);
-	unsigned int index = 0;
 
-	for (Stream *stream : config) {
-		const StreamConfiguration &cfg = config[stream];
-		msg << " (" << index << ") " << cfg.toString();
-		index++;
-	}
+	for (unsigned int index = 0; index < config.size(); ++index)
+		msg << " (" << index << ") " << config[index].toString();
 
 	LOG(Camera, Debug) << msg.str();
 
@@ -593,12 +582,15 @@ Camera::generateConfiguration(const StreamRoles &roles)
  *
  * This function affects the state of the camera, see \ref camera_operation.
  *
+ * Upon return the StreamConfiguration entries in \a config are associated with
+ * Stream instances which can be retrieved with StreamConfiguration::stream().
+ *
  * \return 0 on success or a negative error code otherwise
  * \retval -ENODEV The camera has been disconnected from the system
  * \retval -EACCES The camera is not in a state where it can be configured
  * \retval -EINVAL The configuration is not valid
  */
-int Camera::configure(const CameraConfiguration &config)
+int Camera::configure(CameraConfiguration &config)
 {
 	int ret;
 
@@ -615,16 +607,11 @@ int Camera::configure(const CameraConfiguration &config)
 	}
 
 	std::ostringstream msg("configuring streams:", std::ios_base::ate);
-	unsigned int index = 0;
 
-	for (Stream *stream : config) {
-		if (streams_.find(stream) == streams_.end())
-			return -EINVAL;
-
-		const StreamConfiguration &cfg = config[stream];
-		msg << std::dec << " (" << index << ") " << cfg.toString();
-
-		index++;
+	for (unsigned int index = 0; index < config.size(); ++index) {
+		StreamConfiguration &cfg = config[index];
+		cfg.setStream(nullptr);
+		msg << " (" << index << ") " << cfg.toString();
 	}
 
 	LOG(Camera, Info) << msg.str();
@@ -634,8 +621,11 @@ int Camera::configure(const CameraConfiguration &config)
 		return ret;
 
 	activeStreams_.clear();
-	for (Stream *stream : config) {
-		const StreamConfiguration &cfg = config[stream];
+	for (const StreamConfiguration &cfg : config) {
+		Stream *stream = cfg.stream();
+		if (!stream)
+			LOG(Camera, Fatal)
+				<< "Pipeline handler failed to update stream configuration";
 
 		stream->configuration_ = cfg;
 		activeStreams_.insert(stream);
diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
index 3352cb0e..a025905a 100644
--- a/src/libcamera/include/pipeline_handler.h
+++ b/src/libcamera/include/pipeline_handler.h
@@ -62,7 +62,7 @@ public:
 
 	virtual CameraConfiguration
 	generateConfiguration(Camera *camera, const StreamRoles &roles) = 0;
-	virtual int configure(Camera *camera, const CameraConfiguration &config) = 0;
+	virtual int configure(Camera *camera, CameraConfiguration &config) = 0;
 
 	virtual int allocateBuffers(Camera *camera,
 				    const std::set<Stream *> &streams) = 0;
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index d234a8ac..ed0ef69d 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -152,8 +152,7 @@ public:
 
 	CameraConfiguration
 	generateConfiguration(Camera *camera, const StreamRoles &roles) override;
-	int configure(Camera *camera,
-		      const CameraConfiguration &config) override;
+	int configure(Camera *camera, CameraConfiguration &config) override;
 
 	int allocateBuffers(Camera *camera,
 			    const std::set<Stream *> &streams) override;
@@ -299,14 +298,13 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera,
 		cfg.pixelFormat = V4L2_PIX_FMT_NV12;
 		cfg.bufferCount = IPU3_BUFFER_COUNT;
 
-		config[stream] = cfg;
+		config.addConfiguration(cfg);
 	}
 
 	return config;
 }
 
-int PipelineHandlerIPU3::configure(Camera *camera,
-				   const CameraConfiguration &config)
+int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config)
 {
 	IPU3CameraData *data = cameraData(camera);
 	IPU3Stream *outStream = &data->outStream_;
@@ -318,9 +316,13 @@ int PipelineHandlerIPU3::configure(Camera *camera,
 
 	outStream->active_ = false;
 	vfStream->active_ = false;
-	for (Stream *s : config) {
-		IPU3Stream *stream = static_cast<IPU3Stream *>(s);
-		const StreamConfiguration &cfg = config[stream];
+	for (StreamConfiguration &cfg : config) {
+		/*
+		 * Pick a stream for the configuration entry.
+		 * \todo: This is a naive temporary implementation that will be
+		 * reworked when implementing camera configuration validation.
+		 */
+		IPU3Stream *stream = vfStream->active_ ? outStream : vfStream;
 
 		/*
 		 * Verify that the requested size respects the IPU3 alignment
@@ -355,6 +357,7 @@ int PipelineHandlerIPU3::configure(Camera *camera,
 			sensorSize.height = cfg.size.height;
 
 		stream->active_ = true;
+		cfg.setStream(stream);
 	}
 
 	/*
@@ -379,10 +382,9 @@ int PipelineHandlerIPU3::configure(Camera *camera,
 		return ret;
 
 	/* Apply the format to the configured streams output devices. */
-	for (Stream *s : config) {
-		IPU3Stream *stream = static_cast<IPU3Stream *>(s);
-
-		ret = imgu->configureOutput(stream->device_, config[stream]);
+	for (StreamConfiguration &cfg : config) {
+		IPU3Stream *stream = static_cast<IPU3Stream *>(cfg.stream());
+		ret = imgu->configureOutput(stream->device_, cfg);
 		if (ret)
 			return ret;
 	}
@@ -393,15 +395,13 @@ int PipelineHandlerIPU3::configure(Camera *camera,
 	 * be at least one active stream in the configuration request).
 	 */
 	if (!outStream->active_) {
-		ret = imgu->configureOutput(outStream->device_,
-					    config[vfStream]);
+		ret = imgu->configureOutput(outStream->device_, config[0]);
 		if (ret)
 			return ret;
 	}
 
 	if (!vfStream->active_) {
-		ret = imgu->configureOutput(vfStream->device_,
-					    config[outStream]);
+		ret = imgu->configureOutput(vfStream->device_, config[0]);
 		if (ret)
 			return ret;
 	}
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 4bd8c510..ec6980b0 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -36,8 +36,7 @@ public:
 
 	CameraConfiguration generateConfiguration(Camera *camera,
 		const StreamRoles &roles) override;
-	int configure(Camera *camera,
-		const CameraConfiguration &config) override;
+	int configure(Camera *camera, CameraConfiguration &config) override;
 
 	int allocateBuffers(Camera *camera,
 		const std::set<Stream *> &streams) override;
@@ -117,16 +116,15 @@ CameraConfiguration PipelineHandlerRkISP1::generateConfiguration(Camera *camera,
 	cfg.size = data->sensor_->resolution();
 	cfg.bufferCount = RKISP1_BUFFER_COUNT;
 
-	config[&data->stream_] = cfg;
+	config.addConfiguration(cfg);
 
 	return config;
 }
 
-int PipelineHandlerRkISP1::configure(Camera *camera,
-				     const CameraConfiguration &config)
+int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration &config)
 {
 	RkISP1CameraData *data = cameraData(camera);
-	const StreamConfiguration &cfg = config[&data->stream_];
+	StreamConfiguration &cfg = config[0];
 	CameraSensor *sensor = data->sensor_;
 	int ret;
 
@@ -217,6 +215,8 @@ int PipelineHandlerRkISP1::configure(Camera *camera,
 		return -EINVAL;
 	}
 
+	cfg.setStream(&data->stream_);
+
 	return 0;
 }
 
diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
index d2e1f7d4..5dcc868b 100644
--- a/src/libcamera/pipeline/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo.cpp
@@ -27,8 +27,7 @@ public:
 
 	CameraConfiguration
 	generateConfiguration(Camera *camera, const StreamRoles &roles) override;
-	int configure(Camera *camera,
-		      const CameraConfiguration &config) override;
+	int configure(Camera *camera, CameraConfiguration &config) override;
 
 	int allocateBuffers(Camera *camera,
 			    const std::set<Stream *> &streams) override;
@@ -78,38 +77,38 @@ CameraConfiguration
 PipelineHandlerUVC::generateConfiguration(Camera *camera,
 					  const StreamRoles &roles)
 {
-	UVCCameraData *data = cameraData(camera);
 	CameraConfiguration config;
-	StreamConfiguration cfg{};
+	StreamConfiguration cfg;
 
 	cfg.pixelFormat = V4L2_PIX_FMT_YUYV;
 	cfg.size = { 640, 480 };
 	cfg.bufferCount = 4;
 
-	config[&data->stream_] = cfg;
+	config.addConfiguration(cfg);
 
 	return config;
 }
 
-int PipelineHandlerUVC::configure(Camera *camera,
-				  const CameraConfiguration &config)
+int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration &config)
 {
 	UVCCameraData *data = cameraData(camera);
-	const StreamConfiguration *cfg = &config[&data->stream_];
+	StreamConfiguration &cfg = config[0];
 	int ret;
 
 	V4L2DeviceFormat format = {};
-	format.fourcc = cfg->pixelFormat;
-	format.size = cfg->size;
+	format.fourcc = cfg.pixelFormat;
+	format.size = cfg.size;
 
 	ret = data->video_->setFormat(&format);
 	if (ret)
 		return ret;
 
-	if (format.size != cfg->size ||
-	    format.fourcc != cfg->pixelFormat)
+	if (format.size != cfg.size ||
+	    format.fourcc != cfg.pixelFormat)
 		return -EINVAL;
 
+	cfg.setStream(&data->stream_);
+
 	return 0;
 }
 
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index 17e2491e..af6b6f21 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -27,8 +27,7 @@ public:
 
 	CameraConfiguration
 	generateConfiguration(Camera *camera, const StreamRoles &roles) override;
-	int configure(Camera *camera,
-		      const CameraConfiguration &config) override;
+	int configure(Camera *camera, CameraConfiguration &config) override;
 
 	int allocateBuffers(Camera *camera,
 			    const std::set<Stream *> &streams) override;
@@ -78,38 +77,38 @@ CameraConfiguration
 PipelineHandlerVimc::generateConfiguration(Camera *camera,
 					   const StreamRoles &roles)
 {
-	VimcCameraData *data = cameraData(camera);
 	CameraConfiguration config;
-	StreamConfiguration cfg{};
+	StreamConfiguration cfg;
 
 	cfg.pixelFormat = V4L2_PIX_FMT_RGB24;
 	cfg.size = { 640, 480 };
 	cfg.bufferCount = 4;
 
-	config[&data->stream_] = cfg;
+	config.addConfiguration(cfg);
 
 	return config;
 }
 
-int PipelineHandlerVimc::configure(Camera *camera,
-				   const CameraConfiguration &config)
+int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration &config)
 {
 	VimcCameraData *data = cameraData(camera);
-	const StreamConfiguration *cfg = &config[&data->stream_];
+	StreamConfiguration &cfg = config[0];
 	int ret;
 
 	V4L2DeviceFormat format = {};
-	format.fourcc = cfg->pixelFormat;
-	format.size = cfg->size;
+	format.fourcc = cfg.pixelFormat;
+	format.size = cfg.size;
 
 	ret = data->video_->setFormat(&format);
 	if (ret)
 		return ret;
 
-	if (format.size != cfg->size ||
-	    format.fourcc != cfg->pixelFormat)
+	if (format.size != cfg.size ||
+	    format.fourcc != cfg.pixelFormat)
 		return -EINVAL;
 
+	cfg.setStream(&data->stream_);
+
 	return 0;
 }
 
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index 81c11149..4185e355 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -255,6 +255,10 @@ void PipelineHandler::unlock()
  * configuration of a subset of the streams can't be satisfied, the
  * whole configuration is considered invalid.
  *
+ * Once the configuration is validated and the camera configured, the pipeline
+ * handler shall associate a Stream instance to each StreamConfiguration entry
+ * in the CameraConfiguration with the StreamConfiguration::setStream() method.
+ *
  * \return 0 on success or a negative error code otherwise
  */
 
diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
index fe4c4ecf..0c59a31a 100644
--- a/src/libcamera/stream.cpp
+++ b/src/libcamera/stream.cpp
@@ -59,6 +59,28 @@ namespace libcamera {
  */
 
 /**
+ * \fn StreamConfiguration::stream()
+ * \brief Retrieve the stream associated with the configuration
+ *
+ * When a camera is configured with Camera::configure() Stream instances are
+ * associated with each stream configuration entry. This method retrieves the
+ * associated Stream, which remains valid until the next call to
+ * Camera::configure() or Camera::release().
+ *
+ * \return The stream associated with the configuration
+ */
+
+/**
+ * \fn StreamConfiguration::setStream()
+ * \brief Associate a stream with a configuration
+ *
+ * This method is meant for the PipelineHandler::configure() method and shall
+ * not be called by applications.
+ *
+ * \param[in] stream The stream
+ */
+
+/**
  * \brief Assemble and return a string describing the configuration
  *
  * \return A string describing the StreamConfiguration