[v6,0/6] libcamera: process: Remove `ProcessManager` singleton
mbox series

Message ID 20250728113641.238256-1-barnabas.pocze@ideasonboard.com
Headers show
Series
  • libcamera: process: Remove `ProcessManager` singleton
Related show

Message

Barnabás Pőcze July 28, 2025, 11:36 a.m. UTC
The main goal is to remove the need for the ProcessManager singleton. That
is achieved by using pidfd + clone3(), which raises the minimum
kernel version to 5.4. There are also additional misc. changes.
---
changes in v6:
  * first couple patches merged
  * rebase

changes in v5:
  * send the actually intended version...

changes in v4:
  * address review comments

v5: https://patchwork.libcamera.org/cover/23252/
v4: https://patchwork.libcamera.org/cover/23242/
v3: https://patchwork.libcamera.org/cover/23013/
v2: https://patchwork.libcamera.org/patch/22609/
v1: https://patchwork.libcamera.org/patch/22522/


Barnabás Pőcze (6):
  libcamera: process: Use `pid_` member to decide if running
  libcamera: process: start(): Use span instead of vector
  libcamera: process: closeAllFdsExcept(): Take vector by value
  libcamera: process: Move `closeAllFdsExcept()`
  libcamera: process: Use `close_range()` when available
  libcamera: process: Remove `ProcessManager` singleton

 include/libcamera/internal/camera_manager.h |   1 -
 include/libcamera/internal/process.h        |  42 +--
 meson.build                                 |   6 +-
 src/libcamera/ipc_pipe_unixsocket.cpp       |   9 +-
 src/libcamera/process.cpp                   | 304 +++++++-------------
 test/ipc/unixsocket_ipc.cpp                 |   2 -
 test/log/log_process.cpp                    |   2 -
 test/process/process_test.cpp               |   7 +-
 8 files changed, 113 insertions(+), 260 deletions(-)

--
2.50.1

Comments

Christian Rauch Aug. 28, 2025, 6:16 p.m. UTC | #1
Hi Barnabás,

With the "ProcessManager" removed, there is still the "self_"-check in 
"CameraManager" that throws the fatal error "Multiple CameraManager 
objects are not allowed" when multiple "CameraManager" are instantiated 
in the same address space.

Is this still necessary? I did a quick check, remove the "static 
CameraManager *self_;" completely, and I am able to instantiate the 
CameraManager multiple times and read from two different cameras.

Is the "static CameraManager *self_;" and the check still required?

Best,
Christian


Am 28.07.25 um 13:36 schrieb Barnabás Pőcze:
> The main goal is to remove the need for the ProcessManager singleton. That
> is achieved by using pidfd + clone3(), which raises the minimum
> kernel version to 5.4. There are also additional misc. changes.
> ---
> changes in v6:
>    * first couple patches merged
>    * rebase
> 
> changes in v5:
>    * send the actually intended version...
> 
> changes in v4:
>    * address review comments
> 
> v5: https://patchwork.libcamera.org/cover/23252/
> v4: https://patchwork.libcamera.org/cover/23242/
> v3: https://patchwork.libcamera.org/cover/23013/
> v2: https://patchwork.libcamera.org/patch/22609/
> v1: https://patchwork.libcamera.org/patch/22522/
> 
> 
> Barnabás Pőcze (6):
>    libcamera: process: Use `pid_` member to decide if running
>    libcamera: process: start(): Use span instead of vector
>    libcamera: process: closeAllFdsExcept(): Take vector by value
>    libcamera: process: Move `closeAllFdsExcept()`
>    libcamera: process: Use `close_range()` when available
>    libcamera: process: Remove `ProcessManager` singleton
> 
>   include/libcamera/internal/camera_manager.h |   1 -
>   include/libcamera/internal/process.h        |  42 +--
>   meson.build                                 |   6 +-
>   src/libcamera/ipc_pipe_unixsocket.cpp       |   9 +-
>   src/libcamera/process.cpp                   | 304 +++++++-------------
>   test/ipc/unixsocket_ipc.cpp                 |   2 -
>   test/log/log_process.cpp                    |   2 -
>   test/process/process_test.cpp               |   7 +-
>   8 files changed, 113 insertions(+), 260 deletions(-)
> 
> --
> 2.50.1
Barnabás Pőcze Aug. 28, 2025, 6:20 p.m. UTC | #2
Hi

2025. 08. 28. 20:16 keltezéssel, Christian Rauch írta:
> Hi Barnabás,
> 
> With the "ProcessManager" removed, there is still the "self_"-check in "CameraManager" that throws the fatal error "Multiple CameraManager objects are not allowed" when multiple "CameraManager" are instantiated in the same address space.
> 
> Is this still necessary? I did a quick check, remove the "static CameraManager *self_;" completely, and I am able to instantiate the CameraManager multiple times and read from two different cameras.
> 
> Is the "static CameraManager *self_;" and the check still required?

Please see https://bugs.libcamera.org/show_bug.cgi?id=246


Regards,
Barnabás Pőcze


> 
> Best,
> Christian
> 
> 
> Am 28.07.25 um 13:36 schrieb Barnabás Pőcze:
>> The main goal is to remove the need for the ProcessManager singleton. That
>> is achieved by using pidfd + clone3(), which raises the minimum
>> kernel version to 5.4. There are also additional misc. changes.
>> ---
>> changes in v6:
>>    * first couple patches merged
>>    * rebase
>>
>> changes in v5:
>>    * send the actually intended version...
>>
>> changes in v4:
>>    * address review comments
>>
>> v5: https://patchwork.libcamera.org/cover/23252/
>> v4: https://patchwork.libcamera.org/cover/23242/
>> v3: https://patchwork.libcamera.org/cover/23013/
>> v2: https://patchwork.libcamera.org/patch/22609/
>> v1: https://patchwork.libcamera.org/patch/22522/
>>
>>
>> Barnabás Pőcze (6):
>>    libcamera: process: Use `pid_` member to decide if running
>>    libcamera: process: start(): Use span instead of vector
>>    libcamera: process: closeAllFdsExcept(): Take vector by value
>>    libcamera: process: Move `closeAllFdsExcept()`
>>    libcamera: process: Use `close_range()` when available
>>    libcamera: process: Remove `ProcessManager` singleton
>>
>>   include/libcamera/internal/camera_manager.h |   1 -
>>   include/libcamera/internal/process.h        |  42 +--
>>   meson.build                                 |   6 +-
>>   src/libcamera/ipc_pipe_unixsocket.cpp       |   9 +-
>>   src/libcamera/process.cpp                   | 304 +++++++-------------
>>   test/ipc/unixsocket_ipc.cpp                 |   2 -
>>   test/log/log_process.cpp                    |   2 -
>>   test/process/process_test.cpp               |   7 +-
>>   8 files changed, 113 insertions(+), 260 deletions(-)
>>
>> -- 
>> 2.50.1
>