| Message ID | 20260113000808.15395-8-laurent.pinchart@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta: > The IPAManager is currently instantiated when constructing the > CameraManager constructor, in the CameraManager::Private constructor. > This is the only sizeable object constructed with the CameraManager, all > other components are constructed when starting the manager. > > Early construction of the IPAManager isn't wrong per-se, but prevents > accessing the CameraManager from the IPAManager constructor (as the > former isn't fully constructed). It also results in internal objects > constructed in different threads, which isn't an issue at the moment as > none of the objects constructed by the IPAManager are thread-bound, but > could cause issues later. Finally, it prevents influencing the > IPAManager creation with parameters passed to the CameraManager::start() > function once we implement this, which would be useful for applications > to override configuration parameters related to IPA modules. > > Move the instantiation of the IPAManager to start time to fix all those > issues. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > src/libcamera/camera_manager.cpp | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp > index fc6e490bc476..5762c210ffc2 100644 > --- a/src/libcamera/camera_manager.cpp > +++ b/src/libcamera/camera_manager.cpp > @@ -41,7 +41,6 @@ LOG_DEFINE_CATEGORY(Camera) > CameraManager::Private::Private() > : Thread("CameraManager"), initialized_(false) > { > - ipaManager_ = std::make_unique<IPAManager>(configuration()); > } > > int CameraManager::Private::start() > @@ -94,6 +93,8 @@ void CameraManager::Private::run() > > int CameraManager::Private::init() > { > + ipaManager_ = std::make_unique<IPAManager>(configuration()); > + > enumerator_ = DeviceEnumerator::create(); > if (!enumerator_ || enumerator_->enumerate()) > return -ENODEV; > -- > Regards, > > Laurent Pinchart >
Quoting Laurent Pinchart (2026-01-13 01:07:39) > The IPAManager is currently instantiated when constructing the > CameraManager constructor, in the CameraManager::Private constructor. > This is the only sizeable object constructed with the CameraManager, all > other components are constructed when starting the manager. > > Early construction of the IPAManager isn't wrong per-se, but prevents > accessing the CameraManager from the IPAManager constructor (as the > former isn't fully constructed). It also results in internal objects > constructed in different threads, which isn't an issue at the moment as > none of the objects constructed by the IPAManager are thread-bound, but > could cause issues later. Finally, it prevents influencing the > IPAManager creation with parameters passed to the CameraManager::start() > function once we implement this, which would be useful for applications > to override configuration parameters related to IPA modules. > > Move the instantiation of the IPAManager to start time to fix all those > issues. Makes sense. Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> Cheers, Stefan > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/camera_manager.cpp | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp > index fc6e490bc476..5762c210ffc2 100644 > --- a/src/libcamera/camera_manager.cpp > +++ b/src/libcamera/camera_manager.cpp > @@ -41,7 +41,6 @@ LOG_DEFINE_CATEGORY(Camera) > CameraManager::Private::Private() > : Thread("CameraManager"), initialized_(false) > { > - ipaManager_ = std::make_unique<IPAManager>(configuration()); > } > > int CameraManager::Private::start() > @@ -94,6 +93,8 @@ void CameraManager::Private::run() > > int CameraManager::Private::init() > { > + ipaManager_ = std::make_unique<IPAManager>(configuration()); > + > enumerator_ = DeviceEnumerator::create(); > if (!enumerator_ || enumerator_->enumerate()) > return -ENODEV; > -- > Regards, > > Laurent Pinchart >
diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index fc6e490bc476..5762c210ffc2 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -41,7 +41,6 @@ LOG_DEFINE_CATEGORY(Camera) CameraManager::Private::Private() : Thread("CameraManager"), initialized_(false) { - ipaManager_ = std::make_unique<IPAManager>(configuration()); } int CameraManager::Private::start() @@ -94,6 +93,8 @@ void CameraManager::Private::run() int CameraManager::Private::init() { + ipaManager_ = std::make_unique<IPAManager>(configuration()); + enumerator_ = DeviceEnumerator::create(); if (!enumerator_ || enumerator_->enumerate()) return -ENODEV;
The IPAManager is currently instantiated when constructing the CameraManager constructor, in the CameraManager::Private constructor. This is the only sizeable object constructed with the CameraManager, all other components are constructed when starting the manager. Early construction of the IPAManager isn't wrong per-se, but prevents accessing the CameraManager from the IPAManager constructor (as the former isn't fully constructed). It also results in internal objects constructed in different threads, which isn't an issue at the moment as none of the objects constructed by the IPAManager are thread-bound, but could cause issues later. Finally, it prevents influencing the IPAManager creation with parameters passed to the CameraManager::start() function once we implement this, which would be useful for applications to override configuration parameters related to IPA modules. Move the instantiation of the IPAManager to start time to fix all those issues. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/libcamera/camera_manager.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)