libcamera: converter: Replace usage of stream index by Stream pointer

The converter interface uses the unsigned int output stream index to map
to the output frame buffers. This is cumbersome to implement new
converters because one has to keep around additional book keeping
to track the streams with their correct indexes.

The v4l2_converter_m2m and simple pipeline handler are adapted to
use the new interface. This work roped in software ISP as well,
which also seems to use indexes (although it doesn't implement converter
interface) because of a common conversionQueue_ queue used for
converter_ and swIsp_.

The logPrefix is no longer able to generate an index from a stream, and
is updated to be more expressive by reporting the stream configuration
instead, for example, reporting "1920x1080-MJPEG" in place of
"stream0".

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Tested-by: Andrei Konovalov <andrey.konovalov.ynk@gmail.com> # sm8250 RB5

4 files changed, 40 insertions, 39 deletions

diff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp
index d3d38c1b..2ab46133 100644
--- a/src/libcamera/converter.cpp
+++ b/src/libcamera/converter.cpp
@@ -111,12 +111,12 @@ Converter::~Converter()
 /**
  * \fn Converter::exportBuffers()
  * \brief Export buffers from the converter device
- * \param[in] output Output stream index exporting the buffers
+ * \param[in] stream Output stream pointer exporting the buffers
  * \param[in] count Number of buffers to allocate
  * \param[out] buffers Vector to store allocated buffers
  *
  * This function operates similarly to V4L2VideoDevice::exportBuffers() on the
- * output stream indicated by the \a output index.
+ * output stream indicated by the \a output.
  *
  * \return The number of allocated buffers on success or a negative error code
  * otherwise
@@ -137,7 +137,7 @@ Converter::~Converter()
  * \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
+ * \param[out] outputs The container holding the output stream pointers and
  * their respective frame buffer outputs.
  *
  * This function queues the \a input frame buffer on the output streams of the
diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp
index 6309a0c0..2e77872e 100644
--- a/src/libcamera/converter/converter_v4l2_m2m.cpp
+++ b/src/libcamera/converter/converter_v4l2_m2m.cpp
@@ -35,8 +35,8 @@ LOG_DECLARE_CATEGORY(Converter)
  * V4L2M2MConverter::V4L2M2MStream
  */
 
-V4L2M2MConverter::V4L2M2MStream::V4L2M2MStream(V4L2M2MConverter *converter, unsigned int index)
-	: converter_(converter), index_(index)
+V4L2M2MConverter::V4L2M2MStream::V4L2M2MStream(V4L2M2MConverter *converter, const Stream *stream)
+	: converter_(converter), stream_(stream)
 {
 	m2m_ = std::make_unique<V4L2M2MDevice>(converter->deviceNode());
 
@@ -157,7 +157,7 @@ int V4L2M2MConverter::V4L2M2MStream::queueBuffers(FrameBuffer *input, FrameBuffe
 
 std::string V4L2M2MConverter::V4L2M2MStream::logPrefix() const
 {
-	return "stream" + std::to_string(index_);
+	return stream_->configuration().toString();
 }
 
 void V4L2M2MConverter::V4L2M2MStream::outputBufferReady(FrameBuffer *buffer)
@@ -333,21 +333,24 @@ int V4L2M2MConverter::configure(const StreamConfiguration &inputCfg,
 	int ret = 0;
 
 	streams_.clear();
-	streams_.reserve(outputCfgs.size());
 
 	for (unsigned int i = 0; i < outputCfgs.size(); ++i) {
-		V4L2M2MStream &stream = streams_.emplace_back(this, i);
+		const StreamConfiguration &cfg = outputCfgs[i];
+		std::unique_ptr<V4L2M2MStream> stream =
+			std::make_unique<V4L2M2MStream>(this, cfg.stream());
 
-		if (!stream.isValid()) {
+		if (!stream->isValid()) {
 			LOG(Converter, Error)
 				<< "Failed to create stream " << i;
 			ret = -EINVAL;
 			break;
 		}
 
-		ret = stream.configure(inputCfg, outputCfgs[i]);
+		ret = stream->configure(inputCfg, cfg);
 		if (ret < 0)
 			break;
+
+		streams_.emplace(cfg.stream(), std::move(stream));
 	}
 
 	if (ret < 0) {
@@ -361,13 +364,14 @@ int V4L2M2MConverter::configure(const StreamConfiguration &inputCfg,
 /**
  * \copydoc libcamera::Converter::exportBuffers
  */
-int V4L2M2MConverter::exportBuffers(unsigned int output, unsigned int count,
+int V4L2M2MConverter::exportBuffers(const Stream *stream, unsigned int count,
 				    std::vector<std::unique_ptr<FrameBuffer>> *buffers)
 {
-	if (output >= streams_.size())
+	auto iter = streams_.find(stream);
+	if (iter == streams_.end())
 		return -EINVAL;
 
-	return streams_[output].exportBuffers(count, buffers);
+	return iter->second->exportBuffers(count, buffers);
 }
 
 /**
@@ -377,8 +381,8 @@ int V4L2M2MConverter::start()
 {
 	int ret;
 
-	for (V4L2M2MStream &stream : streams_) {
-		ret = stream.start();
+	for (auto &iter : streams_) {
+		ret = iter.second->start();
 		if (ret < 0) {
 			stop();
 			return ret;
@@ -393,15 +397,15 @@ int V4L2M2MConverter::start()
  */
 void V4L2M2MConverter::stop()
 {
-	for (V4L2M2MStream &stream : utils::reverse(streams_))
-		stream.stop();
+	for (auto &iter : streams_)
+		iter.second->stop();
 }
 
 /**
  * \copydoc libcamera::Converter::queueBuffers
  */
 int V4L2M2MConverter::queueBuffers(FrameBuffer *input,
-				   const std::map<unsigned int, FrameBuffer *> &outputs)
+				   const std::map<const Stream *, FrameBuffer *> &outputs)
 {
 	std::set<FrameBuffer *> outputBufs;
 	int ret;
@@ -414,11 +418,9 @@ int V4L2M2MConverter::queueBuffers(FrameBuffer *input,
 	if (outputs.empty())
 		return -EINVAL;
 
-	for (auto [index, buffer] : outputs) {
+	for (auto [stream, buffer] : outputs) {
 		if (!buffer)
 			return -EINVAL;
-		if (index >= streams_.size())
-			return -EINVAL;
 
 		outputBufs.insert(buffer);
 	}
@@ -427,8 +429,8 @@ int V4L2M2MConverter::queueBuffers(FrameBuffer *input,
 		return -EINVAL;
 
 	/* Queue the input and output buffers to all the streams. */
-	for (auto [index, buffer] : outputs) {
-		ret = streams_[index].queueBuffers(input, buffer);
+	for (auto [stream, buffer] : outputs) {
+		ret = streams_.at(stream)->queueBuffers(input, buffer);
 		if (ret < 0)
 			return ret;
 	}
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index eb36578e..5eb1dd21 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -277,7 +277,7 @@ public:
 	std::map<PixelFormat, std::vector<const Configuration *>> formats_;
 
 	std::vector<std::unique_ptr<FrameBuffer>> conversionBuffers_;
-	std::queue<std::map<unsigned int, FrameBuffer *>> conversionQueue_;
+	std::queue<std::map<const Stream *, FrameBuffer *>> conversionQueue_;
 	bool useConversion_;
 
 	std::unique_ptr<Converter> converter_;
@@ -836,7 +836,7 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)
 	Request *request = buffer->request();
 
 	if (useConversion_ && !conversionQueue_.empty()) {
-		const std::map<unsigned int, FrameBuffer *> &outputs =
+		const std::map<const Stream *, FrameBuffer *> &outputs =
 			conversionQueue_.front();
 		if (!outputs.empty()) {
 			FrameBuffer *outputBuffer = outputs.begin()->second;
@@ -1303,10 +1303,8 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,
 	 */
 	if (data->useConversion_)
 		return data->converter_
-			       ? data->converter_->exportBuffers(data->streamIndex(stream),
-								 count, buffers)
-			       : data->swIsp_->exportBuffers(data->streamIndex(stream),
-							     count, buffers);
+			       ? data->converter_->exportBuffers(stream, count, buffers)
+			       : data->swIsp_->exportBuffers(stream, count, buffers);
 	else
 		return data->video_->exportBuffers(count, buffers);
 }
@@ -1398,7 +1396,7 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)
 	SimpleCameraData *data = cameraData(camera);
 	int ret;
 
-	std::map<unsigned int, FrameBuffer *> buffers;
+	std::map<const Stream *, FrameBuffer *> buffers;
 
 	for (auto &[stream, buffer] : request->buffers()) {
 		/*
@@ -1407,7 +1405,7 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)
 		 * completion handler.
 		 */
 		if (data->useConversion_) {
-			buffers.emplace(data->streamIndex(stream), buffer);
+			buffers.emplace(stream, buffer);
 		} else {
 			ret = data->video_->queueBuffer(buffer);
 			if (ret < 0)
diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
index 3fb7ec8c..c8748d88 100644
--- a/src/libcamera/software_isp/software_isp.cpp
+++ b/src/libcamera/software_isp/software_isp.cpp
@@ -241,19 +241,19 @@ int SoftwareIsp::configure(const StreamConfiguration &inputCfg,
 
 /**
  * \brief Export the buffers from the Software ISP
- * \param[in] output Output stream index exporting the buffers
+ * \param[in] stream Output stream exporting the buffers
  * \param[in] count Number of buffers to allocate
  * \param[out] buffers Vector to store the allocated buffers
  * \return The number of allocated buffers on success or a negative error code
  * otherwise
  */
-int SoftwareIsp::exportBuffers(unsigned int output, unsigned int count,
+int SoftwareIsp::exportBuffers(const Stream *stream, unsigned int count,
 			       std::vector<std::unique_ptr<FrameBuffer>> *buffers)
 {
 	ASSERT(debayer_ != nullptr);
 
 	/* single output for now */
-	if (output >= 1)
+	if (stream == nullptr)
 		return -EINVAL;
 
 	for (unsigned int i = 0; i < count; i++) {
@@ -280,12 +280,12 @@ int SoftwareIsp::exportBuffers(unsigned int output, unsigned int count,
 /**
  * \brief Queue buffers to Software ISP
  * \param[in] input The input framebuffer
- * \param[in] outputs The container holding the output stream indexes and
+ * \param[in] outputs The container holding the output stream pointers and
  * their respective frame buffer outputs
  * \return 0 on success, a negative errno on failure
  */
 int SoftwareIsp::queueBuffers(FrameBuffer *input,
-			      const std::map<unsigned int, FrameBuffer *> &outputs)
+			      const std::map<const Stream *, FrameBuffer *> &outputs)
 {
 	/*
 	 * Validate the outputs as a sanity check: at least one output is
@@ -294,14 +294,15 @@ int SoftwareIsp::queueBuffers(FrameBuffer *input,
 	if (outputs.empty())
 		return -EINVAL;
 
-	for (auto [index, buffer] : outputs) {
+	for (auto [stream, buffer] : outputs) {
 		if (!buffer)
 			return -EINVAL;
-		if (index >= 1) /* only single stream atm */
+		if (outputs.size() != 1) /* only single stream atm */
 			return -EINVAL;
 	}
 
-	process(input, outputs.at(0));
+	for (auto iter = outputs.begin(); iter != outputs.end(); iter++)
+		process(input, iter->second);
 
 	return 0;
 }