Skip to content

use global shared camera manager - #150

Open
AntoBrandi wants to merge 4 commits into
christianrauch:mainfrom
AntoBrandi:fix/multi-camera
Open

use global shared camera manager#150
AntoBrandi wants to merge 4 commits into
christianrauch:mainfrom
AntoBrandi:fix/multi-camera

Conversation

@AntoBrandi

@AntoBrandi AntoBrandi commented Dec 13, 2025

Copy link
Copy Markdown

I was having the same problem described here: #113 while trying to load multiple camera drivers (as Composable Nodes) into the same ROS 2 Container.

This is especially useful to optimize resources and guarantee zero-copy transmission from the nodes in the same container.
ROS2 container

This PR fixes the error

[0:15:23.264404928] [1855] FATAL Camera camera_manager.cpp:310 Multiple CameraManager objects are not allowed

by having a global, shared CameraManager object.

To reproduce the issue:

  1. Start a new ROS 2 Container
    ros2 run rclcpp_components component_container
  2. Load the first CameraNode inside the container to connect to the right camera
    ros2 component load /ComponentManager camera_ros camera::CameraNode --parameter camera:=1 --parameter frame_id:=right_camera --node-namespace /right
  3. Load the second CameraNode in the same container to connect to the left camera
    ros2 component load /ComponentManager camera_ros camera::CameraNode --parameter camera:=0 --parameter frame_id:=left_camera --node-namespace /left

@christianrauch christianrauch left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your scenario with the two composable nodes makes sense.

Note that the long-term goal is to drop the singleton requirement for the CameraManager. See https://gitlab.freedesktop.org/camera/libcamera/-/issues/246. I am fine with a static camera manager in the node class, but would prefer if this is supported by libcamera as other camera nodes or libraries using libcamera could potentially also be loaded into the container.

Comment thread src/CameraNode.cpp Outdated
Comment thread src/CameraNode.cpp Outdated
Comment thread src/CameraNode.cpp
Comment on lines -81 to +87
libcamera::CameraManager camera_manager;
std::shared_ptr<libcamera::CameraManager> camera_manager;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need the static g_camera_manager as well as the camera_manager?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we need both.
The static std::weak_ptr<...> g_camera_manager just checks whether a CameraManager already exists. This is the case when another CameraNode was started.
The std::shared_ptr<...> camera_manager is the actual owner of the CameraManager and keeps it alive.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The static std::weak_ptr<...> g_camera_manager just checks whether a CameraManager already exists. This is the case when another CameraNode was started.

But can this not be determined by the value of the pointer? If camera_manager is nullptr, this is the first time the node runs, and we have to initialise a libcamera::CameraManager. If it is not nullptr, we can just reuse it, using the mutex of course. You can also encapsulate this in something like a get_camera_manager_instance method, that always creates an instance, either by constructing a new CameraManager, or returning the previously constructed.

@christianrauch

Copy link
Copy Markdown
Owner

Can you test, if this works with a single static libcamera::CameraManager with a mutex in place of managing these two pointers?

@christianrauch

Copy link
Copy Markdown
Owner

@AntoBrandi Are you still in this? I did a quick test with static libcamera::CameraManager camera_manager; and a static std::mutex camera_manager_mutex; and this seems to work too.

@christianrauch

Copy link
Copy Markdown
Owner

@AntoBrandi The static_camera_manager branch implements the proposed solution using a static libcamera::CameraManager camera_manager; for all instances and a static std::mutex camera_manager_mutex; to manage the concurrent access. This makes it possible to load the camera::CameraNode composable node twice into the same component_container. Does this work for you too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants