[RFC] libcamera: converter: Replace usage of stream index by Stream pointer
diff mbox series

Message ID 20240524065419.94005-1-umang.jain@ideasonboard.com
State Superseded
Headers show
Series
  • [RFC] libcamera: converter: Replace usage of stream index by Stream pointer
Related show

Commit Message

Umang Jain May 24, 2024, 6:54 a.m. UTC 
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.

This patch (still an RFC) intends to drop stream index usage.
The index is replaced by Stream pointer. This is convenient since
Stream pointers are easily accessible at configuration time (from
StreamConfiguration) and from Request objects.

The v4l2_converter_m2m and simple pipeline handler are adapt 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_.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---

The patch is an RFC and needs more work to be finalised. I want to check
if this direction is decent enough to take since this is going to block
my DW100 converter rework.

- [PATCH v2 0/4] libcamera: rkisp1: Plumb i.MX8MP DW100 dewarper

- Some \todos still around validation of outputs framebuffers.
- compile tested with -Dpipelines=rkisp1,simple 

---
 include/libcamera/internal/converter.h        |  5 +-
 .../internal/converter/converter_v4l2_m2m.h   | 11 +++--
 .../internal/software_isp/software_isp.h      |  4 +-
 .../converter/converter_v4l2_m2m.cpp          | 47 +++++++++----------
 src/libcamera/pipeline/simple/simple.cpp      | 12 ++---
 src/libcamera/software_isp/software_isp.cpp   | 22 ++++-----
 6 files changed, 46 insertions(+), 55 deletions(-)

Comments

Kieran Bingham May 27, 2024, 10:25 a.m. UTC | #1 
Quoting Umang Jain (2024-05-24 07:54:19)
> 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.
> 
> This patch (still an RFC) intends to drop stream index usage.
> The index is replaced by Stream pointer. This is convenient since
> Stream pointers are easily accessible at configuration time (from
> StreamConfiguration) and from Request objects.

It's a little awkward that the term Stream is used in multiple places
(not your patch, the existing situation!)

But I think a convertor Stream does / should map to a libcamera::Stream.

> The v4l2_converter_m2m and simple pipeline handler are adapt 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_.
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
> 
> The patch is an RFC and needs more work to be finalised. I want to check
> if this direction is decent enough to take since this is going to block
> my DW100 converter rework.

I think it more or less still makes sense! Could do with a check from
the SoftISP team to consider it from that side too.


> 
> - [PATCH v2 0/4] libcamera: rkisp1: Plumb i.MX8MP DW100 dewarper
> 
> - Some \todos still around validation of outputs framebuffers.
> - compile tested with -Dpipelines=rkisp1,simple 
> 
> ---
>  include/libcamera/internal/converter.h        |  5 +-
>  .../internal/converter/converter_v4l2_m2m.h   | 11 +++--
>  .../internal/software_isp/software_isp.h      |  4 +-
>  .../converter/converter_v4l2_m2m.cpp          | 47 +++++++++----------
>  src/libcamera/pipeline/simple/simple.cpp      | 12 ++---
>  src/libcamera/software_isp/software_isp.cpp   | 22 ++++-----
>  6 files changed, 46 insertions(+), 55 deletions(-)
> 
> diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h
> index 5d74db6b..b51563d7 100644
> --- a/include/libcamera/internal/converter.h
> +++ b/include/libcamera/internal/converter.h
> @@ -26,6 +26,7 @@ namespace libcamera {
>  class FrameBuffer;
>  class MediaDevice;
>  class PixelFormat;
> +class Stream;
>  struct StreamConfiguration;
>  
>  class Converter
> @@ -46,14 +47,14 @@ public:
>  
>         virtual int configure(const StreamConfiguration &inputCfg,
>                               const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs) = 0;
> -       virtual int exportBuffers(unsigned int output, unsigned int count,
> +       virtual int exportBuffers(const Stream *stream, 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;
> +                                const std::map<const Stream *, FrameBuffer *> &outputs) = 0;
>  
>         Signal<FrameBuffer *> inputBufferReady;
>         Signal<FrameBuffer *> outputBufferReady;
> diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h
> index 1126050c..711a1a5f 100644
> --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h
> +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h
> @@ -28,6 +28,7 @@ class FrameBuffer;
>  class MediaDevice;
>  class Size;
>  class SizeRange;
> +class Stream;
>  struct StreamConfiguration;
>  class V4L2M2MDevice;
>  
> @@ -47,20 +48,20 @@ public:
>  
>         int configure(const StreamConfiguration &inputCfg,
>                       const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfg);
> -       int exportBuffers(unsigned int output, unsigned int count,
> +       int exportBuffers(const Stream *stream, unsigned int count,
>                           std::vector<std::unique_ptr<FrameBuffer>> *buffers);
>  
>         int start();
>         void stop();
>  
>         int queueBuffers(FrameBuffer *input,
> -                        const std::map<unsigned int, FrameBuffer *> &outputs);
> +                        const std::map<const Stream *, FrameBuffer *> &outputs);
>  
>  private:
>         class Stream : protected Loggable
>         {
>         public:
> -               Stream(V4L2M2MConverter *converter, unsigned int index);
> +               Stream(V4L2M2MConverter *converter, const libcamera::Stream *stream);
>  
>                 bool isValid() const { return m2m_ != nullptr; }
>  
> @@ -82,7 +83,7 @@ private:
>                 void outputBufferReady(FrameBuffer *buffer);
>  
>                 V4L2M2MConverter *converter_;
> -               unsigned int index_;
> +               const libcamera::Stream *stream_;
>                 std::unique_ptr<V4L2M2MDevice> m2m_;
>  
>                 unsigned int inputBufferCount_;
> @@ -91,7 +92,7 @@ private:
>  
>         std::unique_ptr<V4L2M2MDevice> m2m_;
>  
> -       std::vector<Stream> streams_;
> +       std::map<const libcamera::Stream *, Stream> streams_;

This is where it gets 'funny'. We have to map a stream to a stream ;+)

I guess this is only used for indexing though.

>         std::map<FrameBuffer *, unsigned int> queue_;
>  };
>  
> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
> index 7e9fae6a..be3d1b2f 100644
> --- a/include/libcamera/internal/software_isp/software_isp.h
> +++ b/include/libcamera/internal/software_isp/software_isp.h
> @@ -62,7 +62,7 @@ public:
>                       const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs,
>                       const ControlInfoMap &sensorControls);
>  
> -       int exportBuffers(unsigned int output, unsigned int count,
> +       int exportBuffers(const Stream *stream, unsigned int count,
>                           std::vector<std::unique_ptr<FrameBuffer>> *buffers);
>  
>         void processStats(const ControlList &sensorControls);
> @@ -71,7 +71,7 @@ public:
>         void stop();
>  
>         int queueBuffers(FrameBuffer *input,
> -                        const std::map<unsigned int, FrameBuffer *> &outputs);
> +                        const std::map<const Stream *, FrameBuffer *> &outputs);
>  
>         void process(FrameBuffer *input, FrameBuffer *output);
>  
> diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp
> index d8929fc5..e0619689 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::Stream
>   */
>  
> -V4L2M2MConverter::Stream::Stream(V4L2M2MConverter *converter, unsigned int index)
> -       : converter_(converter), index_(index)
> +V4L2M2MConverter::Stream::Stream(V4L2M2MConverter *converter, const libcamera::Stream *stream)
> +       : converter_(converter), stream_(stream)
>  {
>         m2m_ = std::make_unique<V4L2M2MDevice>(converter->deviceNode());
>  
> @@ -157,7 +157,7 @@ int V4L2M2MConverter::Stream::queueBuffers(FrameBuffer *input, FrameBuffer *outp
>  
>  std::string V4L2M2MConverter::Stream::logPrefix() const
>  {
> -       return "stream" + std::to_string(index_);
> +       return stream_->configuration().toString();

What does this produce? I'm not sure if this will be correct. The
V4L2M2MConvertor might simply be only one 'component' of the
libcamera::Stream ?


>  }
>  
>  void V4L2M2MConverter::Stream::outputBufferReady(FrameBuffer *buffer)
> @@ -333,10 +333,13 @@ int V4L2M2MConverter::configure(const StreamConfiguration &inputCfg,
>         int ret = 0;
>  
>         streams_.clear();
> -       streams_.reserve(outputCfgs.size());
>  
>         for (unsigned int i = 0; i < outputCfgs.size(); ++i) {
> -               Stream &stream = streams_.emplace_back(this, i);
> +               const StreamConfiguration &cfg = outputCfgs[i];
> +               streams_.emplace(cfg.stream(),
> +                                Stream(this, cfg.stream()));
> +
> +               Stream &stream = streams_.at(cfg.stream());
>  

Aha, but this clears one bit up for me. The convertor does deal with
multiple libcamera::Streams and they do map to the Stream instance
above.

>                 if (!stream.isValid()) {
>                         LOG(Converter, Error)
> @@ -345,7 +348,7 @@ int V4L2M2MConverter::configure(const StreamConfiguration &inputCfg,
>                         break;
>                 }
>  
> -               ret = stream.configure(inputCfg, outputCfgs[i]);
> +               ret = stream.configure(inputCfg, cfg);
>                 if (ret < 0)
>                         break;
>         }
> @@ -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 libcamera::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 (Stream &stream : streams_) {
> -               ret = stream.start();
> +       for (auto &iter : streams_) {
> +               ret = iter.second.start();
>                 if (ret < 0) {
>                         stop();
>                         return ret;
> @@ -393,17 +397,16 @@ int V4L2M2MConverter::start()
>   */
>  void V4L2M2MConverter::stop()
>  {
> -       for (Stream &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 libcamera::Stream *, FrameBuffer *> &outputs)
>  {
> -       unsigned int mask = 0;
>         int ret;
>  
>         /*
> @@ -414,20 +417,12 @@ int V4L2M2MConverter::queueBuffers(FrameBuffer *input,
>         if (outputs.empty())
>                 return -EINVAL;
>  
> -       for (auto [index, buffer] : outputs) {
> -               if (!buffer)
> -                       return -EINVAL;
> -               if (index >= streams_.size())
> -                       return -EINVAL;
> -               if (mask & (1 << index))
> -                       return -EINVAL;
>  
> -               mask |= 1 << index;
> -       }
> +       /* \TODO: validate outputs against streams */
>  
>         /* 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 db3575c3..c22b2f89 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -278,7 +278,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_;
> @@ -837,7 +837,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;
> @@ -1304,9 +1304,9 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,
>          */
>         if (data->useConversion_)
>                 return data->converter_
> -                              ? data->converter_->exportBuffers(data->streamIndex(stream),
> +                              ? data->converter_->exportBuffers(stream,
>                                                                  count, buffers)
> -                              : data->swIsp_->exportBuffers(data->streamIndex(stream),
> +                              : data->swIsp_->exportBuffers(stream,
>                                                              count, buffers);
>         else
>                 return data->video_->exportBuffers(count, buffers);
> @@ -1399,7 +1399,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()) {
>                 /*
> @@ -1408,7 +1408,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 c9b6be56..b81b90bd 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -227,13 +227,13 @@ int SoftwareIsp::configure(const StreamConfiguration &inputCfg,
>   * \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;

Surely this should be
	if (stream == nullptr)

But can that ever even happen? We can't get here with a null stream can
we ? (We shouldn't!)

Perhaps there needs to be a check/mapping for each stream expected to be
supported - or maybe that will already be limited in
configure()/validate().

>         for (unsigned int i = 0; i < count; i++) {
> @@ -265,9 +265,8 @@ int SoftwareIsp::exportBuffers(unsigned int output, unsigned int count,
>   * \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)
>  {
> -       unsigned int mask = 0;
>  
>         /*
>          * Validate the outputs as a sanity check: at least one output is
> @@ -277,18 +276,13 @@ int SoftwareIsp::queueBuffers(FrameBuffer *input,
>         if (outputs.empty())
>                 return -EINVAL;
>  
> -       for (auto [index, buffer] : outputs) {
> -               if (!buffer)
> -                       return -EINVAL;
> -               if (index >= 1) /* only single stream atm */
> -                       return -EINVAL;
> -               if (mask & (1 << index))
> -                       return -EINVAL;
> +       /* \todo validate outputs against streams*/
>  
> -               mask |= 1 << index;
> -       }
> +       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;
>  }
> -- 
> 2.44.0
>
Umang Jain May 27, 2024, 12:05 p.m. UTC | #2 
Hi Kieran,

On 27/05/24 3:55 pm, Kieran Bingham wrote:
> Quoting Umang Jain (2024-05-24 07:54:19)
>> 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.
>>
>> This patch (still an RFC) intends to drop stream index usage.
>> The index is replaced by Stream pointer. This is convenient since
>> Stream pointers are easily accessible at configuration time (from
>> StreamConfiguration) and from Request objects.
> It's a little awkward that the term Stream is used in multiple places
> (not your patch, the existing situation!)
>
> But I think a convertor Stream does / should map to a libcamera::Stream.
>
>> The v4l2_converter_m2m and simple pipeline handler are adapt 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_.
>>
>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
>> ---
>>
>> The patch is an RFC and needs more work to be finalised. I want to check
>> if this direction is decent enough to take since this is going to block
>> my DW100 converter rework.
> I think it more or less still makes sense! Could do with a check from
> the SoftISP team to consider it from that side too.
>
>
>> - [PATCH v2 0/4] libcamera: rkisp1: Plumb i.MX8MP DW100 dewarper
>>
>> - Some \todos still around validation of outputs framebuffers.
>> - compile tested with -Dpipelines=rkisp1,simple
>>
>> ---
>>   include/libcamera/internal/converter.h        |  5 +-
>>   .../internal/converter/converter_v4l2_m2m.h   | 11 +++--
>>   .../internal/software_isp/software_isp.h      |  4 +-
>>   .../converter/converter_v4l2_m2m.cpp          | 47 +++++++++----------
>>   src/libcamera/pipeline/simple/simple.cpp      | 12 ++---
>>   src/libcamera/software_isp/software_isp.cpp   | 22 ++++-----
>>   6 files changed, 46 insertions(+), 55 deletions(-)
>>
>> diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h
>> index 5d74db6b..b51563d7 100644
>> --- a/include/libcamera/internal/converter.h
>> +++ b/include/libcamera/internal/converter.h
>> @@ -26,6 +26,7 @@ namespace libcamera {
>>   class FrameBuffer;
>>   class MediaDevice;
>>   class PixelFormat;
>> +class Stream;
>>   struct StreamConfiguration;
>>   
>>   class Converter
>> @@ -46,14 +47,14 @@ public:
>>   
>>          virtual int configure(const StreamConfiguration &inputCfg,
>>                                const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs) = 0;
>> -       virtual int exportBuffers(unsigned int output, unsigned int count,
>> +       virtual int exportBuffers(const Stream *stream, 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;
>> +                                const std::map<const Stream *, FrameBuffer *> &outputs) = 0;
>>   
>>          Signal<FrameBuffer *> inputBufferReady;
>>          Signal<FrameBuffer *> outputBufferReady;
>> diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h
>> index 1126050c..711a1a5f 100644
>> --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h
>> +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h
>> @@ -28,6 +28,7 @@ class FrameBuffer;
>>   class MediaDevice;
>>   class Size;
>>   class SizeRange;
>> +class Stream;
>>   struct StreamConfiguration;
>>   class V4L2M2MDevice;
>>   
>> @@ -47,20 +48,20 @@ public:
>>   
>>          int configure(const StreamConfiguration &inputCfg,
>>                        const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfg);
>> -       int exportBuffers(unsigned int output, unsigned int count,
>> +       int exportBuffers(const Stream *stream, unsigned int count,
>>                            std::vector<std::unique_ptr<FrameBuffer>> *buffers);
>>   
>>          int start();
>>          void stop();
>>   
>>          int queueBuffers(FrameBuffer *input,
>> -                        const std::map<unsigned int, FrameBuffer *> &outputs);
>> +                        const std::map<const Stream *, FrameBuffer *> &outputs);
>>   
>>   private:
>>          class Stream : protected Loggable
>>          {
>>          public:
>> -               Stream(V4L2M2MConverter *converter, unsigned int index);
>> +               Stream(V4L2M2MConverter *converter, const libcamera::Stream *stream);
>>   
>>                  bool isValid() const { return m2m_ != nullptr; }
>>   
>> @@ -82,7 +83,7 @@ private:
>>                  void outputBufferReady(FrameBuffer *buffer);
>>   
>>                  V4L2M2MConverter *converter_;
>> -               unsigned int index_;
>> +               const libcamera::Stream *stream_;
>>                  std::unique_ptr<V4L2M2MDevice> m2m_;
>>   
>>                  unsigned int inputBufferCount_;
>> @@ -91,7 +92,7 @@ private:
>>   
>>          std::unique_ptr<V4L2M2MDevice> m2m_;
>>   
>> -       std::vector<Stream> streams_;
>> +       std::map<const libcamera::Stream *, Stream> streams_;
> This is where it gets 'funny'. We have to map a stream to a stream ;+)

This is mapping a libcamera::Stream to a V4L2M2MConverter::Stream. The 
lack of namespace is confusing things, I am tempted to change it.
>
> I guess this is only used for indexing though.
>
>>          std::map<FrameBuffer *, unsigned int> queue_;
>>   };
>>   
>> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
>> index 7e9fae6a..be3d1b2f 100644
>> --- a/include/libcamera/internal/software_isp/software_isp.h
>> +++ b/include/libcamera/internal/software_isp/software_isp.h
>> @@ -62,7 +62,7 @@ public:
>>                        const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs,
>>                        const ControlInfoMap &sensorControls);
>>   
>> -       int exportBuffers(unsigned int output, unsigned int count,
>> +       int exportBuffers(const Stream *stream, unsigned int count,
>>                            std::vector<std::unique_ptr<FrameBuffer>> *buffers);
>>   
>>          void processStats(const ControlList &sensorControls);
>> @@ -71,7 +71,7 @@ public:
>>          void stop();
>>   
>>          int queueBuffers(FrameBuffer *input,
>> -                        const std::map<unsigned int, FrameBuffer *> &outputs);
>> +                        const std::map<const Stream *, FrameBuffer *> &outputs);
>>   
>>          void process(FrameBuffer *input, FrameBuffer *output);
>>   
>> diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp
>> index d8929fc5..e0619689 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::Stream
>>    */
>>   
>> -V4L2M2MConverter::Stream::Stream(V4L2M2MConverter *converter, unsigned int index)
>> -       : converter_(converter), index_(index)
>> +V4L2M2MConverter::Stream::Stream(V4L2M2MConverter *converter, const libcamera::Stream *stream)
>> +       : converter_(converter), stream_(stream)
>>   {
>>          m2m_ = std::make_unique<V4L2M2MDevice>(converter->deviceNode());
>>   
>> @@ -157,7 +157,7 @@ int V4L2M2MConverter::Stream::queueBuffers(FrameBuffer *input, FrameBuffer *outp
>>   
>>   std::string V4L2M2MConverter::Stream::logPrefix() const
>>   {
>> -       return "stream" + std::to_string(index_);
>> +       return stream_->configuration().toString();
> What does this produce? I'm not sure if this will be correct. The
> V4L2M2MConvertor might simply be only one 'component' of the
> libcamera::Stream ?

I don't understand here.
>
>
>>   }
>>   
>>   void V4L2M2MConverter::Stream::outputBufferReady(FrameBuffer *buffer)
>> @@ -333,10 +333,13 @@ int V4L2M2MConverter::configure(const StreamConfiguration &inputCfg,
>>          int ret = 0;
>>   
>>          streams_.clear();
>> -       streams_.reserve(outputCfgs.size());
>>   
>>          for (unsigned int i = 0; i < outputCfgs.size(); ++i) {
>> -               Stream &stream = streams_.emplace_back(this, i);
>> +               const StreamConfiguration &cfg = outputCfgs[i];
>> +               streams_.emplace(cfg.stream(),
>> +                                Stream(this, cfg.stream()));
>> +
>> +               Stream &stream = streams_.at(cfg.stream());
>>   
> Aha, but this clears one bit up for me. The convertor does deal with
> multiple libcamera::Streams and they do map to the Stream instance
> above.
>
>>                  if (!stream.isValid()) {
>>                          LOG(Converter, Error)
>> @@ -345,7 +348,7 @@ int V4L2M2MConverter::configure(const StreamConfiguration &inputCfg,
>>                          break;
>>                  }
>>   
>> -               ret = stream.configure(inputCfg, outputCfgs[i]);
>> +               ret = stream.configure(inputCfg, cfg);
>>                  if (ret < 0)
>>                          break;
>>          }
>> @@ -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 libcamera::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 (Stream &stream : streams_) {
>> -               ret = stream.start();
>> +       for (auto &iter : streams_) {
>> +               ret = iter.second.start();
>>                  if (ret < 0) {
>>                          stop();
>>                          return ret;
>> @@ -393,17 +397,16 @@ int V4L2M2MConverter::start()
>>    */
>>   void V4L2M2MConverter::stop()
>>   {
>> -       for (Stream &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 libcamera::Stream *, FrameBuffer *> &outputs)
>>   {
>> -       unsigned int mask = 0;
>>          int ret;
>>   
>>          /*
>> @@ -414,20 +417,12 @@ int V4L2M2MConverter::queueBuffers(FrameBuffer *input,
>>          if (outputs.empty())
>>                  return -EINVAL;
>>   
>> -       for (auto [index, buffer] : outputs) {
>> -               if (!buffer)
>> -                       return -EINVAL;
>> -               if (index >= streams_.size())
>> -                       return -EINVAL;
>> -               if (mask & (1 << index))
>> -                       return -EINVAL;
>>   
>> -               mask |= 1 << index;
>> -       }
>> +       /* \TODO: validate outputs against streams */
>>   
>>          /* 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 db3575c3..c22b2f89 100644
>> --- a/src/libcamera/pipeline/simple/simple.cpp
>> +++ b/src/libcamera/pipeline/simple/simple.cpp
>> @@ -278,7 +278,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_;
>> @@ -837,7 +837,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;
>> @@ -1304,9 +1304,9 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,
>>           */
>>          if (data->useConversion_)
>>                  return data->converter_
>> -                              ? data->converter_->exportBuffers(data->streamIndex(stream),
>> +                              ? data->converter_->exportBuffers(stream,
>>                                                                   count, buffers)
>> -                              : data->swIsp_->exportBuffers(data->streamIndex(stream),
>> +                              : data->swIsp_->exportBuffers(stream,
>>                                                               count, buffers);
>>          else
>>                  return data->video_->exportBuffers(count, buffers);
>> @@ -1399,7 +1399,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()) {
>>                  /*
>> @@ -1408,7 +1408,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 c9b6be56..b81b90bd 100644
>> --- a/src/libcamera/software_isp/software_isp.cpp
>> +++ b/src/libcamera/software_isp/software_isp.cpp
>> @@ -227,13 +227,13 @@ int SoftwareIsp::configure(const StreamConfiguration &inputCfg,
>>    * \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;
> Surely this should be
> 	if (stream == nullptr)

ah yes.
> But can that ever even happen? We can't get here with a null stream can
> we ? (We shouldn't!)

I am not sure, since the softISP is not using the converter interface, 
so things are left to will.
>
> Perhaps there needs to be a check/mapping for each stream expected to be
> supported - or maybe that will already be limited in
> configure()/validate().
>
>>          for (unsigned int i = 0; i < count; i++) {
>> @@ -265,9 +265,8 @@ int SoftwareIsp::exportBuffers(unsigned int output, unsigned int count,
>>    * \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)
>>   {
>> -       unsigned int mask = 0;
>>   
>>          /*
>>           * Validate the outputs as a sanity check: at least one output is
>> @@ -277,18 +276,13 @@ int SoftwareIsp::queueBuffers(FrameBuffer *input,
>>          if (outputs.empty())
>>                  return -EINVAL;
>>   
>> -       for (auto [index, buffer] : outputs) {
>> -               if (!buffer)
>> -                       return -EINVAL;
>> -               if (index >= 1) /* only single stream atm */
>> -                       return -EINVAL;
>> -               if (mask & (1 << index))
>> -                       return -EINVAL;
>> +       /* \todo validate outputs against streams*/
>>   
>> -               mask |= 1 << index;
>> -       }
>> +       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;
>>   }
>> -- 
>> 2.44.0
>>

Patch
diff mbox series 

diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h
index 5d74db6b..b51563d7 100644
--- a/include/libcamera/internal/converter.h
+++ b/include/libcamera/internal/converter.h
@@ -26,6 +26,7 @@  namespace libcamera {
 class FrameBuffer;
 class MediaDevice;
 class PixelFormat;
+class Stream;
 struct StreamConfiguration;
 
 class Converter
@@ -46,14 +47,14 @@  public:
 
 	virtual int configure(const StreamConfiguration &inputCfg,
 			      const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs) = 0;
-	virtual int exportBuffers(unsigned int output, unsigned int count,
+	virtual int exportBuffers(const Stream *stream, 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;
+				 const std::map<const Stream *, FrameBuffer *> &outputs) = 0;
 
 	Signal<FrameBuffer *> inputBufferReady;
 	Signal<FrameBuffer *> outputBufferReady;
diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h
index 1126050c..711a1a5f 100644
--- a/include/libcamera/internal/converter/converter_v4l2_m2m.h
+++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h
@@ -28,6 +28,7 @@  class FrameBuffer;
 class MediaDevice;
 class Size;
 class SizeRange;
+class Stream;
 struct StreamConfiguration;
 class V4L2M2MDevice;
 
@@ -47,20 +48,20 @@  public:
 
 	int configure(const StreamConfiguration &inputCfg,
 		      const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfg);
-	int exportBuffers(unsigned int output, unsigned int count,
+	int exportBuffers(const Stream *stream, unsigned int count,
 			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
 
 	int start();
 	void stop();
 
 	int queueBuffers(FrameBuffer *input,
-			 const std::map<unsigned int, FrameBuffer *> &outputs);
+			 const std::map<const Stream *, FrameBuffer *> &outputs);
 
 private:
 	class Stream : protected Loggable
 	{
 	public:
-		Stream(V4L2M2MConverter *converter, unsigned int index);
+		Stream(V4L2M2MConverter *converter, const libcamera::Stream *stream);
 
 		bool isValid() const { return m2m_ != nullptr; }
 
@@ -82,7 +83,7 @@  private:
 		void outputBufferReady(FrameBuffer *buffer);
 
 		V4L2M2MConverter *converter_;
-		unsigned int index_;
+		const libcamera::Stream *stream_;
 		std::unique_ptr<V4L2M2MDevice> m2m_;
 
 		unsigned int inputBufferCount_;
@@ -91,7 +92,7 @@  private:
 
 	std::unique_ptr<V4L2M2MDevice> m2m_;
 
-	std::vector<Stream> streams_;
+	std::map<const libcamera::Stream *, Stream> streams_;
 	std::map<FrameBuffer *, unsigned int> queue_;
 };
 
diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h
index 7e9fae6a..be3d1b2f 100644
--- a/include/libcamera/internal/software_isp/software_isp.h
+++ b/include/libcamera/internal/software_isp/software_isp.h
@@ -62,7 +62,7 @@  public:
 		      const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfgs,
 		      const ControlInfoMap &sensorControls);
 
-	int exportBuffers(unsigned int output, unsigned int count,
+	int exportBuffers(const Stream *stream, unsigned int count,
 			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
 
 	void processStats(const ControlList &sensorControls);
@@ -71,7 +71,7 @@  public:
 	void stop();
 
 	int queueBuffers(FrameBuffer *input,
-			 const std::map<unsigned int, FrameBuffer *> &outputs);
+			 const std::map<const Stream *, FrameBuffer *> &outputs);
 
 	void process(FrameBuffer *input, FrameBuffer *output);
 
diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp
index d8929fc5..e0619689 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::Stream
  */
 
-V4L2M2MConverter::Stream::Stream(V4L2M2MConverter *converter, unsigned int index)
-	: converter_(converter), index_(index)
+V4L2M2MConverter::Stream::Stream(V4L2M2MConverter *converter, const libcamera::Stream *stream)
+	: converter_(converter), stream_(stream)
 {
 	m2m_ = std::make_unique<V4L2M2MDevice>(converter->deviceNode());
 
@@ -157,7 +157,7 @@  int V4L2M2MConverter::Stream::queueBuffers(FrameBuffer *input, FrameBuffer *outp
 
 std::string V4L2M2MConverter::Stream::logPrefix() const
 {
-	return "stream" + std::to_string(index_);
+	return stream_->configuration().toString();
 }
 
 void V4L2M2MConverter::Stream::outputBufferReady(FrameBuffer *buffer)
@@ -333,10 +333,13 @@  int V4L2M2MConverter::configure(const StreamConfiguration &inputCfg,
 	int ret = 0;
 
 	streams_.clear();
-	streams_.reserve(outputCfgs.size());
 
 	for (unsigned int i = 0; i < outputCfgs.size(); ++i) {
-		Stream &stream = streams_.emplace_back(this, i);
+		const StreamConfiguration &cfg = outputCfgs[i];
+		streams_.emplace(cfg.stream(),
+				 Stream(this, cfg.stream()));
+
+		Stream &stream = streams_.at(cfg.stream());
 
 		if (!stream.isValid()) {
 			LOG(Converter, Error)
@@ -345,7 +348,7 @@  int V4L2M2MConverter::configure(const StreamConfiguration &inputCfg,
 			break;
 		}
 
-		ret = stream.configure(inputCfg, outputCfgs[i]);
+		ret = stream.configure(inputCfg, cfg);
 		if (ret < 0)
 			break;
 	}
@@ -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 libcamera::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 (Stream &stream : streams_) {
-		ret = stream.start();
+	for (auto &iter : streams_) {
+		ret = iter.second.start();
 		if (ret < 0) {
 			stop();
 			return ret;
@@ -393,17 +397,16 @@  int V4L2M2MConverter::start()
  */
 void V4L2M2MConverter::stop()
 {
-	for (Stream &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 libcamera::Stream *, FrameBuffer *> &outputs)
 {
-	unsigned int mask = 0;
 	int ret;
 
 	/*
@@ -414,20 +417,12 @@  int V4L2M2MConverter::queueBuffers(FrameBuffer *input,
 	if (outputs.empty())
 		return -EINVAL;
 
-	for (auto [index, buffer] : outputs) {
-		if (!buffer)
-			return -EINVAL;
-		if (index >= streams_.size())
-			return -EINVAL;
-		if (mask & (1 << index))
-			return -EINVAL;
 
-		mask |= 1 << index;
-	}
+	/* \TODO: validate outputs against streams */
 
 	/* 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 db3575c3..c22b2f89 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -278,7 +278,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_;
@@ -837,7 +837,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;
@@ -1304,9 +1304,9 @@  int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,
 	 */
 	if (data->useConversion_)
 		return data->converter_
-			       ? data->converter_->exportBuffers(data->streamIndex(stream),
+			       ? data->converter_->exportBuffers(stream,
 								 count, buffers)
-			       : data->swIsp_->exportBuffers(data->streamIndex(stream),
+			       : data->swIsp_->exportBuffers(stream,
 							     count, buffers);
 	else
 		return data->video_->exportBuffers(count, buffers);
@@ -1399,7 +1399,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()) {
 		/*
@@ -1408,7 +1408,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 c9b6be56..b81b90bd 100644
--- a/src/libcamera/software_isp/software_isp.cpp
+++ b/src/libcamera/software_isp/software_isp.cpp
@@ -227,13 +227,13 @@  int SoftwareIsp::configure(const StreamConfiguration &inputCfg,
  * \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++) {
@@ -265,9 +265,8 @@  int SoftwareIsp::exportBuffers(unsigned int output, unsigned int count,
  * \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)
 {
-	unsigned int mask = 0;
 
 	/*
 	 * Validate the outputs as a sanity check: at least one output is
@@ -277,18 +276,13 @@  int SoftwareIsp::queueBuffers(FrameBuffer *input,
 	if (outputs.empty())
 		return -EINVAL;
 
-	for (auto [index, buffer] : outputs) {
-		if (!buffer)
-			return -EINVAL;
-		if (index >= 1) /* only single stream atm */
-			return -EINVAL;
-		if (mask & (1 << index))
-			return -EINVAL;
+	/* \todo validate outputs against streams*/
 
-		mask |= 1 << index;
-	}
+	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;
 }