ProcessManager Singleton

[Laurent Pinchart]

I started replying and then realized that Barnabás beated me to it.

On Tue, Apr 22, 2025 at 11:46:13PM +0200, Barnabás Pőcze wrote:
> 2025. 04. 22. 23:31 keltezéssel, Christian Rauch írta:
> > Hi,
> > 
> > The "libcamera::ProcessManager" and hence the
> > "libcamera::CameraManager" (via "CameraManager::Private") can only
> > be instantiated once in a process. I have two questions regarding
> > the reason for this and about the way this is currently implemented.
> > Since this is a design decision and "not a bug", I am posting this
> > on the "devel" mailing list. Let me know if I should move this
> > somewhere else.
> > 
> > The "CameraManager" as well as the "ProcessManager" maintain a
> > static "self_" and check this when they are instantiated. However,
> > when instantiating a second "CameraManager" the
> > "CameraManager::self_"-check is actually never reached as the
> > process will quit already at the "ProcessManager::self_" check via a
> > "Fatal" log and its "std::abort()". At least the
> > "CameraManager::self_"-check appears redundant to me.
> > 
> > Q1: Is the constraint that either the "CameraManager" or the
> > "ProcessManager" must not be instantiated multiple times? Why is
> > this the case? If the constraint is on the "ProcessManager", what is
> > the purpose of the "CameraManager::self_"-check, which is never
> > reached?
> > 
> > Having the Fatal log / std::abort() triggered for additional
> > instances in the same process has some unexpected implications.
> > While a single "CameraManager" instance can easily be managed within
> > the same code base, this is not really possible when dealing with
> > dynamically loaded objects. Let's assume a plugin system, where
> > plugins are loaded from shared object files into the same process /
> > address space. Loading a "Raspberry Pi" plugin to handle
> > specifically Raspberry Pi camera modules would instantiate the first
> > "CameraManager". When a second "UVC" plugin is loaded for generic
> > USB cameras, this would also instantiate a "CameraManager", trigger
> > the "abort" and crash the whole process.
> > 
> > Q2: Is the "abort" really required? If there can really be only a
> > single "CameraManager" (and/or "ProcessManager") instance, why can't
> > different sections of the code, or different plugins in the example
> > above, share a common static "ProcessManager" singleton?
> 
> See https://bugs.libcamera.org/show_bug.cgi?id=246 ideally no
> singletons would exist. I believe this is the plan.
> 
> There are at least three singletons in libcamera at the moment:
>    * the virtual pipeline handler
>    * the camera manager
>    * the process manager
> 
> There are patches to replace the global ProcessManager with pidfd:
>    https://lists.libcamera.org/pipermail/libcamera-devel/2025-March/049339.html
> Then the virtual pipeline handler needs to be amended, and then the
> CameraManager's singleton requirement can finally be removed (hopefully).

I like the plan, and really like the ProcessManager removal series. It's
impeded by an issue in CI. While we figure out how to fix it, Barnabás,
could you submit a v4 that addresses the review comments ?

Regarding instantiating multiple camera managers, it would become
possible once we remove the other two singletons. There an issue though,
all the camera managers will compete to access the same kernel devices,
and the result will vary quite randomly depending on usage patterns from
applications.

-- 
Regards,

Laurent Pinchart