| Message ID | 20251023144841.403689-5-stefan.klug@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Quoting Stefan Klug (2025-10-23 23:48:05) > With the upcoming addition of V4L2 requests support, the converters need > to keep a handle to the corresponding media device. > > Prepare for that by changing the constructor parameter from a raw > pointer to a shared pointer. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > include/libcamera/internal/converter.h | 9 +++++---- > .../libcamera/internal/converter/converter_v4l2_m2m.h | 2 +- > src/libcamera/converter.cpp | 5 +++-- > src/libcamera/converter/converter_v4l2_m2m.cpp | 4 ++-- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- > src/libcamera/pipeline/simple/simple.cpp | 4 ++-- > 6 files changed, 14 insertions(+), 12 deletions(-) > > diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h > index 644ec429bb25..4915af7ac5de 100644 > --- a/include/libcamera/internal/converter.h > +++ b/include/libcamera/internal/converter.h > @@ -46,7 +46,7 @@ public: > Up, > }; > > - Converter(MediaDevice *media, Features features = Feature::None); > + Converter(std::shared_ptr<MediaDevice> media, Features features = Feature::None); > virtual ~Converter(); > > virtual int loadConfiguration(const std::string &filename) = 0; > @@ -107,7 +107,7 @@ public: > > const std::vector<std::string> &compatibles() const { return compatibles_; } > > - static std::unique_ptr<Converter> create(MediaDevice *media); > + static std::unique_ptr<Converter> create(std::shared_ptr<MediaDevice> media); > static std::vector<ConverterFactoryBase *> &factories(); > static std::vector<std::string> names(); > > @@ -116,7 +116,8 @@ private: > > static void registerType(ConverterFactoryBase *factory); > > - virtual std::unique_ptr<Converter> createInstance(MediaDevice *media) const = 0; > + virtual std::unique_ptr<Converter> > + createInstance(std::shared_ptr<MediaDevice> media) const = 0; > > std::string name_; > std::vector<std::string> compatibles_; > @@ -131,7 +132,7 @@ public: > { > } > > - std::unique_ptr<Converter> createInstance(MediaDevice *media) const override > + std::unique_ptr<Converter> createInstance(std::shared_ptr<MediaDevice> media) const override > { > return std::make_unique<_Converter>(media); > } > diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h > index 0ad7bf7fdbe2..d316754040dd 100644 > --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h > +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h > @@ -36,7 +36,7 @@ class V4L2M2MDevice; > class V4L2M2MConverter : public Converter > { > public: > - V4L2M2MConverter(MediaDevice *media); > + V4L2M2MConverter(std::shared_ptr<MediaDevice> media); > > int loadConfiguration([[maybe_unused]] const std::string &filename) override { return 0; } > bool isValid() const override { return m2m_ != nullptr; } > diff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp > index d551b908d523..142fb29a1272 100644 > --- a/src/libcamera/converter.cpp > +++ b/src/libcamera/converter.cpp > @@ -8,6 +8,7 @@ > #include "libcamera/internal/converter.h" > > #include <algorithm> > +#include <memory> > > #include <libcamera/base/log.h> > > @@ -68,7 +69,7 @@ LOG_DEFINE_CATEGORY(Converter) > * This searches for the entity implementing the data streaming function in the > * media graph entities and use its device node as the converter device node. > */ > -Converter::Converter(MediaDevice *media, Features features) > +Converter::Converter(std::shared_ptr<MediaDevice> media, Features features) > { > const std::vector<MediaEntity *> &entities = media->entities(); > auto it = std::find_if(entities.begin(), entities.end(), > @@ -332,7 +333,7 @@ ConverterFactoryBase::ConverterFactoryBase(const std::string name, std::initiali > * \return A new instance of the converter subclass corresponding to the media > * device, or null if the media device driver name doesn't match anything > */ > -std::unique_ptr<Converter> ConverterFactoryBase::create(MediaDevice *media) > +std::unique_ptr<Converter> ConverterFactoryBase::create(std::shared_ptr<MediaDevice> media) > { > const std::vector<ConverterFactoryBase *> &factories = > ConverterFactoryBase::factories(); > diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp > index 0ce7a7a67f11..b2bd54f368d8 100644 > --- a/src/libcamera/converter/converter_v4l2_m2m.cpp > +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp > @@ -10,6 +10,7 @@ > > #include <algorithm> > #include <limits.h> > +#include <memory> > #include <set> > > #include <libcamera/base/log.h> > @@ -261,8 +262,7 @@ void V4L2M2MConverter::V4L2M2MStream::captureBufferReady(FrameBuffer *buffer) > * \brief Construct a V4L2M2MConverter instance > * \param[in] media The media device implementing the converter > */ > - > -V4L2M2MConverter::V4L2M2MConverter(MediaDevice *media) > +V4L2M2MConverter::V4L2M2MConverter(std::shared_ptr<MediaDevice> media) > : Converter(media) > { > if (deviceNode().empty()) > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 959cdb41fe7e..0e2a210e1e80 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -1445,7 +1445,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) > > std::shared_ptr<MediaDevice> dwpMediaDevice = enumerator->search(dwp); > if (dwpMediaDevice) { > - dewarper_ = std::make_unique<V4L2M2MConverter>(dwpMediaDevice.get()); > + dewarper_ = std::make_unique<V4L2M2MConverter>(dwpMediaDevice); > if (dewarper_->isValid()) { > dewarper_->outputBufferReady.connect( > this, &PipelineHandlerRkISP1::dewarpBufferReady); > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 1183be9a6b07..204b29f92907 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -413,7 +413,7 @@ public: > > V4L2VideoDevice *video(const MediaEntity *entity); > V4L2Subdevice *subdev(const MediaEntity *entity); > - MediaDevice *converter() { return converter_.get(); } > + std::shared_ptr<MediaDevice> converter() { return converter_; } > bool swIspEnabled() const { return swIspEnabled_; } > > protected: > @@ -586,7 +586,7 @@ int SimpleCameraData::init() > int ret; > > /* Open the converter, if any. */ > - MediaDevice *converter = pipe->converter(); > + std::shared_ptr<MediaDevice> converter = pipe->converter(); > if (converter) { > converter_ = ConverterFactoryBase::create(converter); > if (!converter_) { > -- > 2.48.1 >
Hi Stefan, Thank you for the patch! Quoting Stefan Klug (2025-10-23 15:48:05) > With the upcoming addition of V4L2 requests support, the converters need > to keep a handle to the corresponding media device. > > Prepare for that by changing the constructor parameter from a raw > pointer to a shared pointer. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com> > --- > include/libcamera/internal/converter.h | 9 +++++---- > .../libcamera/internal/converter/converter_v4l2_m2m.h | 2 +- > src/libcamera/converter.cpp | 5 +++-- > src/libcamera/converter/converter_v4l2_m2m.cpp | 4 ++-- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- > src/libcamera/pipeline/simple/simple.cpp | 4 ++-- > 6 files changed, 14 insertions(+), 12 deletions(-) > > diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h > index 644ec429bb25..4915af7ac5de 100644 > --- a/include/libcamera/internal/converter.h > +++ b/include/libcamera/internal/converter.h > @@ -46,7 +46,7 @@ public: > Up, > }; > > - Converter(MediaDevice *media, Features features = Feature::None); > + Converter(std::shared_ptr<MediaDevice> media, Features features = Feature::None); > virtual ~Converter(); > > virtual int loadConfiguration(const std::string &filename) = 0; > @@ -107,7 +107,7 @@ public: > > const std::vector<std::string> &compatibles() const { return compatibles_; } > > - static std::unique_ptr<Converter> create(MediaDevice *media); > + static std::unique_ptr<Converter> create(std::shared_ptr<MediaDevice> media); > static std::vector<ConverterFactoryBase *> &factories(); > static std::vector<std::string> names(); > > @@ -116,7 +116,8 @@ private: > > static void registerType(ConverterFactoryBase *factory); > > - virtual std::unique_ptr<Converter> createInstance(MediaDevice *media) const = 0; > + virtual std::unique_ptr<Converter> > + createInstance(std::shared_ptr<MediaDevice> media) const = 0; > > std::string name_; > std::vector<std::string> compatibles_; > @@ -131,7 +132,7 @@ public: > { > } > > - std::unique_ptr<Converter> createInstance(MediaDevice *media) const override > + std::unique_ptr<Converter> createInstance(std::shared_ptr<MediaDevice> media) const override > { > return std::make_unique<_Converter>(media); > } > diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h > index 0ad7bf7fdbe2..d316754040dd 100644 > --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h > +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h > @@ -36,7 +36,7 @@ class V4L2M2MDevice; > class V4L2M2MConverter : public Converter > { > public: > - V4L2M2MConverter(MediaDevice *media); > + V4L2M2MConverter(std::shared_ptr<MediaDevice> media); > > int loadConfiguration([[maybe_unused]] const std::string &filename) override { return 0; } > bool isValid() const override { return m2m_ != nullptr; } > diff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp > index d551b908d523..142fb29a1272 100644 > --- a/src/libcamera/converter.cpp > +++ b/src/libcamera/converter.cpp > @@ -8,6 +8,7 @@ > #include "libcamera/internal/converter.h" > > #include <algorithm> > +#include <memory> > > #include <libcamera/base/log.h> > > @@ -68,7 +69,7 @@ LOG_DEFINE_CATEGORY(Converter) > * This searches for the entity implementing the data streaming function in the > * media graph entities and use its device node as the converter device node. > */ > -Converter::Converter(MediaDevice *media, Features features) > +Converter::Converter(std::shared_ptr<MediaDevice> media, Features features) > { > const std::vector<MediaEntity *> &entities = media->entities(); > auto it = std::find_if(entities.begin(), entities.end(), > @@ -332,7 +333,7 @@ ConverterFactoryBase::ConverterFactoryBase(const std::string name, std::initiali > * \return A new instance of the converter subclass corresponding to the media > * device, or null if the media device driver name doesn't match anything > */ > -std::unique_ptr<Converter> ConverterFactoryBase::create(MediaDevice *media) > +std::unique_ptr<Converter> ConverterFactoryBase::create(std::shared_ptr<MediaDevice> media) > { > const std::vector<ConverterFactoryBase *> &factories = > ConverterFactoryBase::factories(); > diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp > index 0ce7a7a67f11..b2bd54f368d8 100644 > --- a/src/libcamera/converter/converter_v4l2_m2m.cpp > +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp > @@ -10,6 +10,7 @@ > > #include <algorithm> > #include <limits.h> > +#include <memory> > #include <set> > > #include <libcamera/base/log.h> > @@ -261,8 +262,7 @@ void V4L2M2MConverter::V4L2M2MStream::captureBufferReady(FrameBuffer *buffer) > * \brief Construct a V4L2M2MConverter instance > * \param[in] media The media device implementing the converter > */ > - > -V4L2M2MConverter::V4L2M2MConverter(MediaDevice *media) > +V4L2M2MConverter::V4L2M2MConverter(std::shared_ptr<MediaDevice> media) > : Converter(media) > { > if (deviceNode().empty()) > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 959cdb41fe7e..0e2a210e1e80 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -1445,7 +1445,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) > > std::shared_ptr<MediaDevice> dwpMediaDevice = enumerator->search(dwp); > if (dwpMediaDevice) { > - dewarper_ = std::make_unique<V4L2M2MConverter>(dwpMediaDevice.get()); > + dewarper_ = std::make_unique<V4L2M2MConverter>(dwpMediaDevice); > if (dewarper_->isValid()) { > dewarper_->outputBufferReady.connect( > this, &PipelineHandlerRkISP1::dewarpBufferReady); > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 1183be9a6b07..204b29f92907 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -413,7 +413,7 @@ public: > > V4L2VideoDevice *video(const MediaEntity *entity); > V4L2Subdevice *subdev(const MediaEntity *entity); > - MediaDevice *converter() { return converter_.get(); } > + std::shared_ptr<MediaDevice> converter() { return converter_; } > bool swIspEnabled() const { return swIspEnabled_; } > > protected: > @@ -586,7 +586,7 @@ int SimpleCameraData::init() > int ret; > > /* Open the converter, if any. */ > - MediaDevice *converter = pipe->converter(); > + std::shared_ptr<MediaDevice> converter = pipe->converter(); > if (converter) { > converter_ = ConverterFactoryBase::create(converter); > if (!converter_) { > -- > 2.48.1 >
diff --git a/include/libcamera/internal/converter.h b/include/libcamera/internal/converter.h index 644ec429bb25..4915af7ac5de 100644 --- a/include/libcamera/internal/converter.h +++ b/include/libcamera/internal/converter.h @@ -46,7 +46,7 @@ public: Up, }; - Converter(MediaDevice *media, Features features = Feature::None); + Converter(std::shared_ptr<MediaDevice> media, Features features = Feature::None); virtual ~Converter(); virtual int loadConfiguration(const std::string &filename) = 0; @@ -107,7 +107,7 @@ public: const std::vector<std::string> &compatibles() const { return compatibles_; } - static std::unique_ptr<Converter> create(MediaDevice *media); + static std::unique_ptr<Converter> create(std::shared_ptr<MediaDevice> media); static std::vector<ConverterFactoryBase *> &factories(); static std::vector<std::string> names(); @@ -116,7 +116,8 @@ private: static void registerType(ConverterFactoryBase *factory); - virtual std::unique_ptr<Converter> createInstance(MediaDevice *media) const = 0; + virtual std::unique_ptr<Converter> + createInstance(std::shared_ptr<MediaDevice> media) const = 0; std::string name_; std::vector<std::string> compatibles_; @@ -131,7 +132,7 @@ public: { } - std::unique_ptr<Converter> createInstance(MediaDevice *media) const override + std::unique_ptr<Converter> createInstance(std::shared_ptr<MediaDevice> media) const override { return std::make_unique<_Converter>(media); } diff --git a/include/libcamera/internal/converter/converter_v4l2_m2m.h b/include/libcamera/internal/converter/converter_v4l2_m2m.h index 0ad7bf7fdbe2..d316754040dd 100644 --- a/include/libcamera/internal/converter/converter_v4l2_m2m.h +++ b/include/libcamera/internal/converter/converter_v4l2_m2m.h @@ -36,7 +36,7 @@ class V4L2M2MDevice; class V4L2M2MConverter : public Converter { public: - V4L2M2MConverter(MediaDevice *media); + V4L2M2MConverter(std::shared_ptr<MediaDevice> media); int loadConfiguration([[maybe_unused]] const std::string &filename) override { return 0; } bool isValid() const override { return m2m_ != nullptr; } diff --git a/src/libcamera/converter.cpp b/src/libcamera/converter.cpp index d551b908d523..142fb29a1272 100644 --- a/src/libcamera/converter.cpp +++ b/src/libcamera/converter.cpp @@ -8,6 +8,7 @@ #include "libcamera/internal/converter.h" #include <algorithm> +#include <memory> #include <libcamera/base/log.h> @@ -68,7 +69,7 @@ LOG_DEFINE_CATEGORY(Converter) * This searches for the entity implementing the data streaming function in the * media graph entities and use its device node as the converter device node. */ -Converter::Converter(MediaDevice *media, Features features) +Converter::Converter(std::shared_ptr<MediaDevice> media, Features features) { const std::vector<MediaEntity *> &entities = media->entities(); auto it = std::find_if(entities.begin(), entities.end(), @@ -332,7 +333,7 @@ ConverterFactoryBase::ConverterFactoryBase(const std::string name, std::initiali * \return A new instance of the converter subclass corresponding to the media * device, or null if the media device driver name doesn't match anything */ -std::unique_ptr<Converter> ConverterFactoryBase::create(MediaDevice *media) +std::unique_ptr<Converter> ConverterFactoryBase::create(std::shared_ptr<MediaDevice> media) { const std::vector<ConverterFactoryBase *> &factories = ConverterFactoryBase::factories(); diff --git a/src/libcamera/converter/converter_v4l2_m2m.cpp b/src/libcamera/converter/converter_v4l2_m2m.cpp index 0ce7a7a67f11..b2bd54f368d8 100644 --- a/src/libcamera/converter/converter_v4l2_m2m.cpp +++ b/src/libcamera/converter/converter_v4l2_m2m.cpp @@ -10,6 +10,7 @@ #include <algorithm> #include <limits.h> +#include <memory> #include <set> #include <libcamera/base/log.h> @@ -261,8 +262,7 @@ void V4L2M2MConverter::V4L2M2MStream::captureBufferReady(FrameBuffer *buffer) * \brief Construct a V4L2M2MConverter instance * \param[in] media The media device implementing the converter */ - -V4L2M2MConverter::V4L2M2MConverter(MediaDevice *media) +V4L2M2MConverter::V4L2M2MConverter(std::shared_ptr<MediaDevice> media) : Converter(media) { if (deviceNode().empty()) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 959cdb41fe7e..0e2a210e1e80 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -1445,7 +1445,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) std::shared_ptr<MediaDevice> dwpMediaDevice = enumerator->search(dwp); if (dwpMediaDevice) { - dewarper_ = std::make_unique<V4L2M2MConverter>(dwpMediaDevice.get()); + dewarper_ = std::make_unique<V4L2M2MConverter>(dwpMediaDevice); if (dewarper_->isValid()) { dewarper_->outputBufferReady.connect( this, &PipelineHandlerRkISP1::dewarpBufferReady); diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 1183be9a6b07..204b29f92907 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -413,7 +413,7 @@ public: V4L2VideoDevice *video(const MediaEntity *entity); V4L2Subdevice *subdev(const MediaEntity *entity); - MediaDevice *converter() { return converter_.get(); } + std::shared_ptr<MediaDevice> converter() { return converter_; } bool swIspEnabled() const { return swIspEnabled_; } protected: @@ -586,7 +586,7 @@ int SimpleCameraData::init() int ret; /* Open the converter, if any. */ - MediaDevice *converter = pipe->converter(); + std::shared_ptr<MediaDevice> converter = pipe->converter(); if (converter) { converter_ = ConverterFactoryBase::create(converter); if (!converter_) {
With the upcoming addition of V4L2 requests support, the converters need to keep a handle to the corresponding media device. Prepare for that by changing the constructor parameter from a raw pointer to a shared pointer. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- include/libcamera/internal/converter.h | 9 +++++---- .../libcamera/internal/converter/converter_v4l2_m2m.h | 2 +- src/libcamera/converter.cpp | 5 +++-- src/libcamera/converter/converter_v4l2_m2m.cpp | 4 ++-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- src/libcamera/pipeline/simple/simple.cpp | 4 ++-- 6 files changed, 14 insertions(+), 12 deletions(-)