[v2,4/4] libcamera: converter: Replace usage of stream index by Stream pointer
diff mbox series

Message ID 20240529070248.12186-5-umang.jain@ideasonboard.com
State New
Headers show
Series
  • libcamera: converter: Replace usage of stream index by Stream pointer
Related show

Commit Message

Umang Jain May 29, 2024, 7:02 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.

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_.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 include/libcamera/internal/converter.h        |  5 ++-
 .../internal/converter/converter_v4l2_m2m.h   | 11 ++---
 .../internal/software_isp/software_isp.h      |  5 ++-
 .../converter/converter_v4l2_m2m.cpp          | 40 ++++++++++---------
 src/libcamera/pipeline/simple/simple.cpp      | 14 +++----
 src/libcamera/software_isp/software_isp.cpp   | 13 +++---
 6 files changed, 46 insertions(+), 42 deletions(-)

Comments

Kieran Bingham May 29, 2024, 10 a.m. UTC | #1 
Quoting Umang Jain (2024-05-29 08:02:48)
> 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_.

Unfortunately - this patch breaks the SoftISP on Lenovo-X13s presently.

I haven't looked into why yet, just that it breaks the camera.

--
Kieran


> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>  include/libcamera/internal/converter.h        |  5 ++-
>  .../internal/converter/converter_v4l2_m2m.h   | 11 ++---
>  .../internal/software_isp/software_isp.h      |  5 ++-
>  .../converter/converter_v4l2_m2m.cpp          | 40 ++++++++++---------
>  src/libcamera/pipeline/simple/simple.cpp      | 14 +++----
>  src/libcamera/software_isp/software_isp.cpp   | 13 +++---
>  6 files changed, 46 insertions(+), 42 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 0da62290..58fd19db 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 V4L2M2MStream : protected Loggable
>         {
>         public:
> -               V4L2M2MStream(V4L2M2MConverter *converter, unsigned int index);
> +               V4L2M2MStream(V4L2M2MConverter *converter, const Stream *stream);
>  
>                 bool isValid() const { return m2m_ != nullptr; }
>  
> @@ -82,7 +83,7 @@ private:
>                 void outputBufferReady(FrameBuffer *buffer);
>  
>                 V4L2M2MConverter *converter_;
> -               unsigned int index_;
> +               const Stream *stream_;
>                 std::unique_ptr<V4L2M2MDevice> m2m_;
>  
>                 unsigned int inputBufferCount_;
> @@ -91,7 +92,7 @@ private:
>  
>         std::unique_ptr<V4L2M2MDevice> m2m_;
>  
> -       std::vector<V4L2M2MStream> streams_;
> +       std::map<const Stream *, V4L2M2MStream> 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..09cc0f6f 100644
> --- a/include/libcamera/internal/software_isp/software_isp.h
> +++ b/include/libcamera/internal/software_isp/software_isp.h
> @@ -37,6 +37,7 @@ namespace libcamera {
>  class DebayerCpu;
>  class FrameBuffer;
>  class PixelFormat;
> +class Stream;
>  struct StreamConfiguration;
>  
>  LOG_DECLARE_CATEGORY(SoftwareIsp)
> @@ -62,7 +63,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 +72,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 6309a0c0..a48b2a87 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,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) {
> -               V4L2M2MStream &stream = streams_.emplace_back(this, i);
> +               const StreamConfiguration &cfg = outputCfgs[i];
> +               streams_.emplace(cfg.stream(),
> +                                V4L2M2MStream(this, cfg.stream()));
> +
> +               V4L2M2MStream &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 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 db3575c3..01ad91a7 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,10 +1304,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);
>  }
> @@ -1399,7 +1397,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 +1406,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 ac10d82d..7cadd585 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,7 +265,7 @@ 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)
>  {
>         /*
>          * Validate the outputs as a sanity check: at least one output is
> @@ -274,14 +274,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;
>  }
> -- 
> 2.44.0
>
Kieran Bingham May 29, 2024, 11:09 a.m. UTC | #2 
Quoting Kieran Bingham (2024-05-29 11:00:15)
> Quoting Umang Jain (2024-05-29 08:02:48)
> > 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_.
> 
> Unfortunately - this patch breaks the SoftISP on Lenovo-X13s presently.
> 
> I haven't looked into why yet, just that it breaks the camera.
> 

The CI also picked up that the Doxygen names need cleanup in here:

 - https://gitlab.freedesktop.org/camera/libcamera/-/jobs/59287644

FAILED: Documentation/api-html
/usr/bin/doxygen Documentation/Doxyfile
/builds/camera/libcamera/src/libcamera/converter.cpp:113: error: argument 'output' of command @param is not found in the argument list of libcamera::Converter::exportBuffers(const Stream *stream, unsigned int count, std::vector< std::unique_ptr< FrameBuffer > > *buffers)=0 (warning treated as error, aborting now)

--
Regards

Kieran

> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> > ---
> >  include/libcamera/internal/converter.h        |  5 ++-
> >  .../internal/converter/converter_v4l2_m2m.h   | 11 ++---
> >  .../internal/software_isp/software_isp.h      |  5 ++-
> >  .../converter/converter_v4l2_m2m.cpp          | 40 ++++++++++---------
> >  src/libcamera/pipeline/simple/simple.cpp      | 14 +++----
> >  src/libcamera/software_isp/software_isp.cpp   | 13 +++---
> >  6 files changed, 46 insertions(+), 42 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 0da62290..58fd19db 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 V4L2M2MStream : protected Loggable
> >         {
> >         public:
> > -               V4L2M2MStream(V4L2M2MConverter *converter, unsigned int index);
> > +               V4L2M2MStream(V4L2M2MConverter *converter, const Stream *stream);
> >  
> >                 bool isValid() const { return m2m_ != nullptr; }
> >  
> > @@ -82,7 +83,7 @@ private:
> >                 void outputBufferReady(FrameBuffer *buffer);
> >  
> >                 V4L2M2MConverter *converter_;
> > -               unsigned int index_;
> > +               const Stream *stream_;
> >                 std::unique_ptr<V4L2M2MDevice> m2m_;
> >  
> >                 unsigned int inputBufferCount_;
> > @@ -91,7 +92,7 @@ private:
> >  
> >         std::unique_ptr<V4L2M2MDevice> m2m_;
> >  
> > -       std::vector<V4L2M2MStream> streams_;
> > +       std::map<const Stream *, V4L2M2MStream> 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..09cc0f6f 100644
> > --- a/include/libcamera/internal/software_isp/software_isp.h
> > +++ b/include/libcamera/internal/software_isp/software_isp.h
> > @@ -37,6 +37,7 @@ namespace libcamera {
> >  class DebayerCpu;
> >  class FrameBuffer;
> >  class PixelFormat;
> > +class Stream;
> >  struct StreamConfiguration;
> >  
> >  LOG_DECLARE_CATEGORY(SoftwareIsp)
> > @@ -62,7 +63,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 +72,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 6309a0c0..a48b2a87 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,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) {
> > -               V4L2M2MStream &stream = streams_.emplace_back(this, i);
> > +               const StreamConfiguration &cfg = outputCfgs[i];
> > +               streams_.emplace(cfg.stream(),
> > +                                V4L2M2MStream(this, cfg.stream()));
> > +
> > +               V4L2M2MStream &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 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 db3575c3..01ad91a7 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,10 +1304,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);
> >  }
> > @@ -1399,7 +1397,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 +1406,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 ac10d82d..7cadd585 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,7 +265,7 @@ 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)
> >  {
> >         /*
> >          * Validate the outputs as a sanity check: at least one output is
> > @@ -274,14 +274,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;
> >  }
> > -- 
> > 2.44.0
> >
Umang Jain May 29, 2024, 11:10 a.m. UTC | #3 
Hi Kieran,

On 29/05/24 3:30 pm, Kieran Bingham wrote:
> Quoting Umang Jain (2024-05-29 08:02:48)
>> 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_.
> Unfortunately - this patch breaks the SoftISP on Lenovo-X13s presently.

ah darn...

Thanks for testing! I am setting up software ISP pipeline on my side. I 
will investigate.
>
> I haven't looked into why yet, just that it breaks the camera.
>
> --
> Kieran
>
>
>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
>> ---
>>   include/libcamera/internal/converter.h        |  5 ++-
>>   .../internal/converter/converter_v4l2_m2m.h   | 11 ++---
>>   .../internal/software_isp/software_isp.h      |  5 ++-
>>   .../converter/converter_v4l2_m2m.cpp          | 40 ++++++++++---------
>>   src/libcamera/pipeline/simple/simple.cpp      | 14 +++----
>>   src/libcamera/software_isp/software_isp.cpp   | 13 +++---
>>   6 files changed, 46 insertions(+), 42 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 0da62290..58fd19db 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 V4L2M2MStream : protected Loggable
>>          {
>>          public:
>> -               V4L2M2MStream(V4L2M2MConverter *converter, unsigned int index);
>> +               V4L2M2MStream(V4L2M2MConverter *converter, const Stream *stream);
>>   
>>                  bool isValid() const { return m2m_ != nullptr; }
>>   
>> @@ -82,7 +83,7 @@ private:
>>                  void outputBufferReady(FrameBuffer *buffer);
>>   
>>                  V4L2M2MConverter *converter_;
>> -               unsigned int index_;
>> +               const Stream *stream_;
>>                  std::unique_ptr<V4L2M2MDevice> m2m_;
>>   
>>                  unsigned int inputBufferCount_;
>> @@ -91,7 +92,7 @@ private:
>>   
>>          std::unique_ptr<V4L2M2MDevice> m2m_;
>>   
>> -       std::vector<V4L2M2MStream> streams_;
>> +       std::map<const Stream *, V4L2M2MStream> 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..09cc0f6f 100644
>> --- a/include/libcamera/internal/software_isp/software_isp.h
>> +++ b/include/libcamera/internal/software_isp/software_isp.h
>> @@ -37,6 +37,7 @@ namespace libcamera {
>>   class DebayerCpu;
>>   class FrameBuffer;
>>   class PixelFormat;
>> +class Stream;
>>   struct StreamConfiguration;
>>   
>>   LOG_DECLARE_CATEGORY(SoftwareIsp)
>> @@ -62,7 +63,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 +72,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 6309a0c0..a48b2a87 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,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) {
>> -               V4L2M2MStream &stream = streams_.emplace_back(this, i);
>> +               const StreamConfiguration &cfg = outputCfgs[i];
>> +               streams_.emplace(cfg.stream(),
>> +                                V4L2M2MStream(this, cfg.stream()));
>> +
>> +               V4L2M2MStream &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 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 db3575c3..01ad91a7 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,10 +1304,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);
>>   }
>> @@ -1399,7 +1397,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 +1406,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 ac10d82d..7cadd585 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,7 +265,7 @@ 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)
>>   {
>>          /*
>>           * Validate the outputs as a sanity check: at least one output is
>> @@ -274,14 +274,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;
>>   }
>> -- 
>> 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 0da62290..58fd19db 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 V4L2M2MStream : protected Loggable
 	{
 	public:
-		V4L2M2MStream(V4L2M2MConverter *converter, unsigned int index);
+		V4L2M2MStream(V4L2M2MConverter *converter, const Stream *stream);
 
 		bool isValid() const { return m2m_ != nullptr; }
 
@@ -82,7 +83,7 @@  private:
 		void outputBufferReady(FrameBuffer *buffer);
 
 		V4L2M2MConverter *converter_;
-		unsigned int index_;
+		const Stream *stream_;
 		std::unique_ptr<V4L2M2MDevice> m2m_;
 
 		unsigned int inputBufferCount_;
@@ -91,7 +92,7 @@  private:
 
 	std::unique_ptr<V4L2M2MDevice> m2m_;
 
-	std::vector<V4L2M2MStream> streams_;
+	std::map<const Stream *, V4L2M2MStream> 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..09cc0f6f 100644
--- a/include/libcamera/internal/software_isp/software_isp.h
+++ b/include/libcamera/internal/software_isp/software_isp.h
@@ -37,6 +37,7 @@  namespace libcamera {
 class DebayerCpu;
 class FrameBuffer;
 class PixelFormat;
+class Stream;
 struct StreamConfiguration;
 
 LOG_DECLARE_CATEGORY(SoftwareIsp)
@@ -62,7 +63,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 +72,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 6309a0c0..a48b2a87 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,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) {
-		V4L2M2MStream &stream = streams_.emplace_back(this, i);
+		const StreamConfiguration &cfg = outputCfgs[i];
+		streams_.emplace(cfg.stream(),
+				 V4L2M2MStream(this, cfg.stream()));
+
+		V4L2M2MStream &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 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 db3575c3..01ad91a7 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,10 +1304,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);
 }
@@ -1399,7 +1397,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 +1406,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 ac10d82d..7cadd585 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,7 +265,7 @@  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)
 {
 	/*
 	 * Validate the outputs as a sanity check: at least one output is
@@ -274,14 +274,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;
 }