| Message ID | 20260918080734.1228227-1-naush@raspberrypi.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi 2026. 09. 18. 9:59 keltezéssel, Naushir Patuck írta: > Hi, > > As promised way back in the May F2F (better late than never!), this series adds > a new two-phase camera enumeration API to the CameraManager. > > Today, CameraManager::start() creates and initialises every camera in the > system, including IPA initialisation, which can be quite a heavyweight > operation. Additionally, this stops application from possibly providing camera > specific IPA configurations during initialisation. > > The new API splits this operation in two. CameraManager::enumerate() returns a > CameraDescriptor for every camera in the system, carrying the camera id and the > properties known without touching the hardware. The application then picks the > ones it wants and calls CameraManager::initialize() on each descriptor to get a > fully initialised Camera. This API is optional, and applications can still use > start() and cameras() exactly as before. > > On the pipeline handler side, match() is split into survey() and createCamera(). An unfortunate choice of name because it conflicts with the already existing: In file included from ../src/libcamera/pipeline/rkisp1/rkisp1.cpp:45: ../include/libcamera/internal/pipeline_handler.h:46:21: error: ‘virtual int libcamera::PipelineHandler::createCamera(const libcamera::CameraDescriptor*)’ was hidden [-Werror=overloaded-virtual=] 46 | virtual int createCamera(const CameraDescriptor *descriptor); | ^~~~~~~~~~~~ ../src/libcamera/pipeline/rkisp1/rkisp1.cpp:216:13: note: by ‘int libcamera::PipelineHandlerRkISP1::createCamera(libcamera::MediaEntity*)’ 216 | int createCamera(MediaEntity *sensor); | ^~~~~~~~~~~~ so something needs to be adjusted there. (https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1750108) > survey() reports descriptors using only the DeviceEnumerator and must not > acquire or open anything, while createCamera() does the per-camera work that > match() used to do. Pipeline handlers may still keep working through match(), > where their cameras are created by start() but are not reported by enumerate(). > > The patches roughly grouped as follows: > > Patches 1-4: Helpers needed to identify a camera without opening it. Camera IDs > are generated from the media entity (sysfs/firmware node) rather than an opened > subdevice, the model name derivation is factored out, and the device enumerator > gains a non-acquiring searchAll(). > > Patches 5-7: The CameraDescriptor class and the survey()/createCamera() pipeline > handler API, plus the ability to acquire a specific media device. > > Patches 8-15: CameraManager changes. > > Patches 16-19: survey()/createCamera() implementations for the RPi (vc4/pisp) > and uvcvideo pipeline handlers, with their match() implementations removed once > they are dead code. > > Patch 20: Documentation updates. > > I've tested these changes on a branch of rpicam-apps [1] that implements the > enumeration API. Also done some basic testing with the uvcvideo pipeline > handler in cam. I'm unable to test any other pipeline handler, so their code > remains unchanged, using the existing match() API. > > There's a few fundamental changes to the CameraManager implemenation here so I'm > sure there is plenty to discuss :) As far as I recall the idea was to supply extra parameters to the camera, but I can't seem to find anything related to that. So the only advantage in its current form is that one can avoid initializing the unused cameras. Did I miss it, or do you have some more concrete ideas as to what one could control and how with this delayed initialization mechanism? --- I've been long wanting to remove the `CameraManager` singleton, and I have been reminded of that multiple times while reading the changes. Specifically I think the current way pipeline handlers are probed is suboptimal: creating a new instance, then calling match, then potentially destroying the whole thing, over and over. This also prevents pipeline handlers from having any kind of per-CameraManager data, which is exactly what the "virtual" pipeline handler needs and why the singleton is still here. But if a new thing, "PipelineHandler{Module,Driver,etc}", is added, then it could be instantiated *once* per CameraManager, and it could provide an entry point into pipeline handlers without having to instantiate them. (And it could provide the per-CameraManager data that is missing from the removal of the singleton.) At first glance it seems it would help with this changeset as well. For example, looking at `CameraManager::Private::pipes_`, or the "joining" in `CameraManager::Private::initializeThread()`. Thoughts? Regards, Barnabás Pőcze > > Regards, > Naush > > [1]: https://github.com/raspberrypi/rpicam-apps/tree/enumeration-api > > Naushir Patuck (20): > libcamera: sysfs: Add devicePath() helpers > libcamera: v4l2_subdevice: Refactor the model name derivation > libcamera: camera_sensor: Generate sensor IDs from a media entity > libcamera: device_enumerator: Add non-acquiring searchAll() > libcamera: Add the CameraDescriptor class > libcamera: pipeline_handler: Add survey() and createCamera() > libcamera: pipeline_handler: Allow acquiring a specific media device > libcamera: camera_manager: Defer IPAManager construction to first use > libcamera: camera_manager: Track active pipeline handler instances > libcamera: camera_manager: Extract the pipeline handler factory list > libcamera: camera_manager: Marshal work onto the camera manager thread > libcamera: camera_manager: Add CameraManager::enumerate() > libcamera: camera_manager: Add CameraManager::initialize() > libcamera: camera_manager: Create cameras through enumeration > libcamera: pipeline_handler: Make match() optional > pipeline: rpi: Add platform helpers for camera enumeration > pipeline: rpi: Implement survey() and createCamera() > pipeline: uvcvideo: Factor out camera ID generation > pipeline: uvcvideo: Implement survey() and createCamera() > Documentation: Describe the two-phase camera enumeration API > > .../guides/application-developer.rst | 41 ++ > Documentation/guides/pipeline-handler.rst | 101 +++++ > include/libcamera/camera_descriptor.h | 34 ++ > include/libcamera/camera_manager.h | 4 + > .../libcamera/internal/camera_descriptor.h | 37 ++ > include/libcamera/internal/camera_manager.h | 30 +- > include/libcamera/internal/camera_sensor.h | 1 + > .../libcamera/internal/device_enumerator.h | 1 + > include/libcamera/internal/meson.build | 1 + > include/libcamera/internal/pipeline_handler.h | 8 +- > include/libcamera/internal/sysfs.h | 3 + > include/libcamera/internal/v4l2_subdevice.h | 1 + > include/libcamera/meson.build | 1 + > src/libcamera/camera_descriptor.cpp | 141 +++++++ > src/libcamera/camera_manager.cpp | 372 ++++++++++++++++-- > src/libcamera/device_enumerator.cpp | 31 ++ > src/libcamera/meson.build | 1 + > .../pipeline/rpi/common/pipeline_base.cpp | 99 +++++ > .../pipeline/rpi/common/pipeline_base.h | 16 + > src/libcamera/pipeline/rpi/pisp/pisp.cpp | 121 +++--- > src/libcamera/pipeline/rpi/vc4/vc4.cpp | 79 ++-- > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 244 +++++++----- > src/libcamera/pipeline_handler.cpp | 107 +++++ > src/libcamera/sensor/camera_sensor.cpp | 27 ++ > src/libcamera/sensor/camera_sensor_legacy.cpp | 6 +- > src/libcamera/sensor/camera_sensor_raw.cpp | 3 +- > src/libcamera/sysfs.cpp | 61 +++ > src/libcamera/v4l2_device.cpp | 13 +- > src/libcamera/v4l2_subdevice.cpp | 60 +-- > 29 files changed, 1352 insertions(+), 292 deletions(-) > create mode 100644 include/libcamera/camera_descriptor.h > create mode 100644 include/libcamera/internal/camera_descriptor.h > create mode 100644 src/libcamera/camera_descriptor.cpp >
Hi Barnabás, On Tue, 22 Sept 2026 at 16:43, Barnabás Pőcze <barnabas.pocze@ideasonboard.com> wrote: > > Hi > > 2026. 09. 18. 9:59 keltezéssel, Naushir Patuck írta: > > Hi, > > > > As promised way back in the May F2F (better late than never!), this series adds > > a new two-phase camera enumeration API to the CameraManager. > > > > Today, CameraManager::start() creates and initialises every camera in the > > system, including IPA initialisation, which can be quite a heavyweight > > operation. Additionally, this stops application from possibly providing camera > > specific IPA configurations during initialisation. > > > > The new API splits this operation in two. CameraManager::enumerate() returns a > > CameraDescriptor for every camera in the system, carrying the camera id and the > > properties known without touching the hardware. The application then picks the > > ones it wants and calls CameraManager::initialize() on each descriptor to get a > > fully initialised Camera. This API is optional, and applications can still use > > start() and cameras() exactly as before. > > > > On the pipeline handler side, match() is split into survey() and createCamera(). > > An unfortunate choice of name because it conflicts with the already existing: > > In file included from ../src/libcamera/pipeline/rkisp1/rkisp1.cpp:45: > ../include/libcamera/internal/pipeline_handler.h:46:21: error: ‘virtual int libcamera::PipelineHandler::createCamera(const libcamera::CameraDescriptor*)’ was hidden [-Werror=overloaded-virtual=] > 46 | virtual int createCamera(const CameraDescriptor *descriptor); > | ^~~~~~~~~~~~ > ../src/libcamera/pipeline/rkisp1/rkisp1.cpp:216:13: note: by ‘int libcamera::PipelineHandlerRkISP1::createCamera(libcamera::MediaEntity*)’ > 216 | int createCamera(MediaEntity *sensor); > | ^~~~~~~~~~~~ > > so something needs to be adjusted there. > (https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1750108) > > > > survey() reports descriptors using only the DeviceEnumerator and must not > > acquire or open anything, while createCamera() does the per-camera work that > > match() used to do. Pipeline handlers may still keep working through match(), > > where their cameras are created by start() but are not reported by enumerate(). > > > > The patches roughly grouped as follows: > > > > Patches 1-4: Helpers needed to identify a camera without opening it. Camera IDs > > are generated from the media entity (sysfs/firmware node) rather than an opened > > subdevice, the model name derivation is factored out, and the device enumerator > > gains a non-acquiring searchAll(). > > > > Patches 5-7: The CameraDescriptor class and the survey()/createCamera() pipeline > > handler API, plus the ability to acquire a specific media device. > > > > Patches 8-15: CameraManager changes. > > > > Patches 16-19: survey()/createCamera() implementations for the RPi (vc4/pisp) > > and uvcvideo pipeline handlers, with their match() implementations removed once > > they are dead code. > > > > Patch 20: Documentation updates. > > > > I've tested these changes on a branch of rpicam-apps [1] that implements the > > enumeration API. Also done some basic testing with the uvcvideo pipeline > > handler in cam. I'm unable to test any other pipeline handler, so their code > > remains unchanged, using the existing match() API. > > > > There's a few fundamental changes to the CameraManager implemenation here so I'm > > sure there is plenty to discuss :) > > As far as I recall the idea was to supply extra parameters to the camera, but I > can't seem to find anything related to that. So the only advantage in its current > form is that one can avoid initializing the unused cameras. Did I miss it, or do > you have some more concrete ideas as to what one could control and how with this > delayed initialization mechanism? The subject of my F2F talk [1] was indeed about camera specific settings passed during initialiastion. However, while working on that I quickly ran into 2 issues: 1) Applications have no prior knowledge of which camera(s) are attached to the device so I cannot easily direct a set of parameters for a particular camera through the existing Camera Manager interface without costly init/re-init cycles. This is a bit of a chicken/egg situation, which can only be resolved with a 2-step enumeration like this patch provides. 2) We initialize Cameras and IPAs for cameras that we don't necessarily use in a given use case. This seems quite wasteful in some circumstances. So all of this is a precursor to the per-camera settings work that will follow after this lands. Hope that makes sense? > > --- > > I've been long wanting to remove the `CameraManager` singleton, and I have been reminded > of that multiple times while reading the changes. Specifically I think the current way > pipeline handlers are probed is suboptimal: creating a new instance, then calling match, > then potentially destroying the whole thing, over and over. > > This also prevents pipeline handlers from having any kind of per-CameraManager data, which > is exactly what the "virtual" pipeline handler needs and why the singleton is still here. But > if a new thing, "PipelineHandler{Module,Driver,etc}", is added, then it could be instantiated > *once* per CameraManager, and it could provide an entry point into pipeline handlers without > having to instantiate them. (And it could provide the per-CameraManager data that is missing > from the removal of the singleton.) > > At first glance it seems it would help with this changeset as well. For example, looking at > `CameraManager::Private::pipes_`, or the "joining" in `CameraManager::Private::initializeThread()`. > Thoughts? I think I agree with your concerns. In this series, survey() only needs to do enumeration, so instantiating a pipeline handler just to do this is also wasteful and unnecessary. A per-CameraManager::PipelineHandlerModule could be used to avoid this. Should I attempt prototyping something for the next revision of this series? Regards, Naush [1] https://docs.google.com/presentation/d/1m06G8NmeJ_4p7v2uIm0vK05vs2avi6hoPT4Z_0MFwyg/edit?usp=sharing > > > Regards, > Barnabás Pőcze > > > > > Regards, > > Naush > > > > [1]: https://github.com/raspberrypi/rpicam-apps/tree/enumeration-api > > > > Naushir Patuck (20): > > libcamera: sysfs: Add devicePath() helpers > > libcamera: v4l2_subdevice: Refactor the model name derivation > > libcamera: camera_sensor: Generate sensor IDs from a media entity > > libcamera: device_enumerator: Add non-acquiring searchAll() > > libcamera: Add the CameraDescriptor class > > libcamera: pipeline_handler: Add survey() and createCamera() > > libcamera: pipeline_handler: Allow acquiring a specific media device > > libcamera: camera_manager: Defer IPAManager construction to first use > > libcamera: camera_manager: Track active pipeline handler instances > > libcamera: camera_manager: Extract the pipeline handler factory list > > libcamera: camera_manager: Marshal work onto the camera manager thread > > libcamera: camera_manager: Add CameraManager::enumerate() > > libcamera: camera_manager: Add CameraManager::initialize() > > libcamera: camera_manager: Create cameras through enumeration > > libcamera: pipeline_handler: Make match() optional > > pipeline: rpi: Add platform helpers for camera enumeration > > pipeline: rpi: Implement survey() and createCamera() > > pipeline: uvcvideo: Factor out camera ID generation > > pipeline: uvcvideo: Implement survey() and createCamera() > > Documentation: Describe the two-phase camera enumeration API > > > > .../guides/application-developer.rst | 41 ++ > > Documentation/guides/pipeline-handler.rst | 101 +++++ > > include/libcamera/camera_descriptor.h | 34 ++ > > include/libcamera/camera_manager.h | 4 + > > .../libcamera/internal/camera_descriptor.h | 37 ++ > > include/libcamera/internal/camera_manager.h | 30 +- > > include/libcamera/internal/camera_sensor.h | 1 + > > .../libcamera/internal/device_enumerator.h | 1 + > > include/libcamera/internal/meson.build | 1 + > > include/libcamera/internal/pipeline_handler.h | 8 +- > > include/libcamera/internal/sysfs.h | 3 + > > include/libcamera/internal/v4l2_subdevice.h | 1 + > > include/libcamera/meson.build | 1 + > > src/libcamera/camera_descriptor.cpp | 141 +++++++ > > src/libcamera/camera_manager.cpp | 372 ++++++++++++++++-- > > src/libcamera/device_enumerator.cpp | 31 ++ > > src/libcamera/meson.build | 1 + > > .../pipeline/rpi/common/pipeline_base.cpp | 99 +++++ > > .../pipeline/rpi/common/pipeline_base.h | 16 + > > src/libcamera/pipeline/rpi/pisp/pisp.cpp | 121 +++--- > > src/libcamera/pipeline/rpi/vc4/vc4.cpp | 79 ++-- > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 244 +++++++----- > > src/libcamera/pipeline_handler.cpp | 107 +++++ > > src/libcamera/sensor/camera_sensor.cpp | 27 ++ > > src/libcamera/sensor/camera_sensor_legacy.cpp | 6 +- > > src/libcamera/sensor/camera_sensor_raw.cpp | 3 +- > > src/libcamera/sysfs.cpp | 61 +++ > > src/libcamera/v4l2_device.cpp | 13 +- > > src/libcamera/v4l2_subdevice.cpp | 60 +-- > > 29 files changed, 1352 insertions(+), 292 deletions(-) > > create mode 100644 include/libcamera/camera_descriptor.h > > create mode 100644 include/libcamera/internal/camera_descriptor.h > > create mode 100644 src/libcamera/camera_descriptor.cpp > > >