[v1,08/33] libcamera: converter_v4l2_m2m: Add suport for V4L2 requests

24507 diff mbox series

Add support for V4L2 reuests.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>

---

ToDo:
- Fix the place where the MediaObject is acquired/released
---
 .../internal/converter/converter_v4l2_m2m.h   |  9 ++++-
 .../converter/converter_v4l2_m2m.cpp          | 34 ++++++++++++++++++-
 2 files changed, 41 insertions(+), 2 deletions(-)

Quoting Stefan Klug (2025-09-30 13:26:29)
> Add support for V4L2 reuests.

requests.

Perhaps we could be a bit more expansive on the commit message too.

> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 
> ---
> 
> ToDo:
> - Fix the place where the MediaObject is acquired/released
> ---
>  .../internal/converter/converter_v4l2_m2m.h   |  9 ++++-
>  .../converter/converter_v4l2_m2m.cpp          | 34 ++++++++++++++++++-
>  2 files changed, 41 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h
> index 1b2a88c4a608..3b8fc5392b7d 100644
> --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h
> +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h
> @@ -73,7 +73,12 @@ public:
>         std::pair<Rectangle, Rectangle> inputCropBounds() override { return inputCropBounds_; }
>         std::pair<Rectangle, Rectangle> inputCropBounds(const Stream *stream) override;
>  
> -private:
> +       int allocateRequests(unsigned int count,
> +                            std::vector<std::unique_ptr<V4L2Request>> *requests);
> +
> +       bool supportsRequests();
> +
> +protected:
>         class V4L2M2MStream : protected Loggable
>         {
>         public:
> @@ -122,6 +127,8 @@ private:
>         std::map<const Stream *, std::unique_ptr<V4L2M2MStream>> streams_;
>         std::map<FrameBuffer *, unsigned int> queue_;
>         std::pair<Rectangle, Rectangle> inputCropBounds_;
> +
> +       std::shared_ptr<MediaDevice> media_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp
> index ff11a9735db7..c6153d728c9a 100644
> --- a/src/libcamera/converter/converter_v4l2_m2m.cpp
> +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp
> @@ -9,6 +9,7 @@
>  #include "libcamera/internal/converter/converter_v4l2_m2m.h"
>  
>  #include <algorithm>
> +#include <errno.h>
>  #include <limits.h>
>  #include <memory>
>  #include <set>
> @@ -266,7 +267,7 @@ void V4L2M2MConverter::V4L2M2MStream::captureBufferReady(FrameBuffer *buffer)
>   * \param[in] media The media device implementing the converter
>   */
>  V4L2M2MConverter::V4L2M2MConverter(std::shared_ptr<MediaDevice> media)
> -       : Converter(media)
> +       : Converter(media), media_(media)
>  {
>         if (deviceNode().empty())
>                 return;
> @@ -742,6 +743,37 @@ int V4L2M2MConverter::queueBuffers(FrameBuffer *input,
>         return 0;
>  }
>  
> +/**
> + * \copydoc libcamera::MediaDevice::allocateRequests
> + */
> +int V4L2M2MConverter::allocateRequests(unsigned int count,
> +                                      std::vector<std::unique_ptr<V4L2Request>> *requests)
> +{
> +       /* \todo The acquire() must be moved to the right place. */
> +       media_->acquire();
> +       if (!media_->busy())
> +               LOG(Converter, Error)
> +                       << "MediaDevice must be valid.";
> +       int ret = media_->allocateRequests(count, requests);
> +       media_->release();
> +       return ret;
> +}
> +
> +/**
> + * \copydoc libcamera::MediaDevice::supportsRequests
> + */
> +bool V4L2M2MConverter::supportsRequests()
> +{
> +       /* \todo The acquire() must be moved to the right place. */
> +       media_->acquire();
> +       if (!media_->busy())
> +               LOG(Converter, Error)
> +                       << "MediaDevice must be valid.";
> +       bool ret = media_->supportsRequests();

I think it shouldn't matter in practice - as if we're not able to
acquire the camera we won't stream - but I expect even if a camera is
streaming - another process might try to enumerate it or detect the
capabilities (without acquiring) - so I think determining if the device
'supports' requests should be possible without locking/acquireing the
media device...

But it probably doesn't make much material difference?


> +       media_->release();
> +       return ret;
> +}
> +
>  /*
>   * \todo This should be extended to include Feature::Flag to denote
>   * what each converter supports feature-wise.
> -- 
> 2.48.1
>

Hi Kieran,

Thank you for the review.

Quoting Kieran Bingham (2025-10-03 12:29:44)
> Quoting Stefan Klug (2025-09-30 13:26:29)
> > Add support for V4L2 reuests.
> 
> requests.
> 
> Perhaps we could be a bit more expansive on the commit message too.
> 
> > 
> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > 
> > ---
> > 
> > ToDo:
> > - Fix the place where the MediaObject is acquired/released
> > ---
> >  .../internal/converter/converter_v4l2_m2m.h   |  9 ++++-
> >  .../converter/converter_v4l2_m2m.cpp          | 34 ++++++++++++++++++-
> >  2 files changed, 41 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h
> > index 1b2a88c4a608..3b8fc5392b7d 100644
> > --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h
> > +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h
> > @@ -73,7 +73,12 @@ public:
> >         std::pair<Rectangle, Rectangle> inputCropBounds() override { return inputCropBounds_; }
> >         std::pair<Rectangle, Rectangle> inputCropBounds(const Stream *stream) override;
> >  
> > -private:
> > +       int allocateRequests(unsigned int count,
> > +                            std::vector<std::unique_ptr<V4L2Request>> *requests);
> > +
> > +       bool supportsRequests();
> > +
> > +protected:
> >         class V4L2M2MStream : protected Loggable
> >         {
> >         public:
> > @@ -122,6 +127,8 @@ private:
> >         std::map<const Stream *, std::unique_ptr<V4L2M2MStream>> streams_;
> >         std::map<FrameBuffer *, unsigned int> queue_;
> >         std::pair<Rectangle, Rectangle> inputCropBounds_;
> > +
> > +       std::shared_ptr<MediaDevice> media_;
> >  };
> >  
> >  } /* namespace libcamera */
> > diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp
> > index ff11a9735db7..c6153d728c9a 100644
> > --- a/src/libcamera/converter/converter_v4l2_m2m.cpp
> > +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp
> > @@ -9,6 +9,7 @@
> >  #include "libcamera/internal/converter/converter_v4l2_m2m.h"
> >  
> >  #include <algorithm>
> > +#include <errno.h>
> >  #include <limits.h>
> >  #include <memory>
> >  #include <set>
> > @@ -266,7 +267,7 @@ void V4L2M2MConverter::V4L2M2MStream::captureBufferReady(FrameBuffer *buffer)
> >   * \param[in] media The media device implementing the converter
> >   */
> >  V4L2M2MConverter::V4L2M2MConverter(std::shared_ptr<MediaDevice> media)
> > -       : Converter(media)
> > +       : Converter(media), media_(media)
> >  {
> >         if (deviceNode().empty())
> >                 return;
> > @@ -742,6 +743,37 @@ int V4L2M2MConverter::queueBuffers(FrameBuffer *input,
> >         return 0;
> >  }
> >  
> > +/**
> > + * \copydoc libcamera::MediaDevice::allocateRequests
> > + */
> > +int V4L2M2MConverter::allocateRequests(unsigned int count,
> > +                                      std::vector<std::unique_ptr<V4L2Request>> *requests)
> > +{
> > +       /* \todo The acquire() must be moved to the right place. */
> > +       media_->acquire();
> > +       if (!media_->busy())
> > +               LOG(Converter, Error)
> > +                       << "MediaDevice must be valid.";
> > +       int ret = media_->allocateRequests(count, requests);
> > +       media_->release();
> > +       return ret;
> > +}
> > +
> > +/**
> > + * \copydoc libcamera::MediaDevice::supportsRequests
> > + */
> > +bool V4L2M2MConverter::supportsRequests()
> > +{
> > +       /* \todo The acquire() must be moved to the right place. */
> > +       media_->acquire();
> > +       if (!media_->busy())
> > +               LOG(Converter, Error)
> > +                       << "MediaDevice must be valid.";
> > +       bool ret = media_->supportsRequests();
> 
> I think it shouldn't matter in practice - as if we're not able to
> acquire the camera we won't stream - but I expect even if a camera is
> streaming - another process might try to enumerate it or detect the
> capabilities (without acquiring) - so I think determining if the device
> 'supports' requests should be possible without locking/acquireing the
> media device...
> 
> But it probably doesn't make much material difference?

I was unsure about the contracts here. MediaDevice::open() is private,
so the only way to ensure it is opened is by acquiring it. Do you have a
suggestion for a better solution?

Best regards,
Stefan

> 
> 
> > +       media_->release();
> > +       return ret;
> > +}
> > +
> >  /*
> >   * \todo This should be extended to include Feature::Flag to denote
> >   * what each converter supports feature-wise.
> > -- 
> > 2.48.1
> >