| Message ID | 20230704225746.3838484-2-kieran.bingham@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Tue, Jul 04, 2023 at 11:57:45PM +0100, Kieran Bingham via libcamera-devel wrote: > The CameraManager->get(dev_t) helper was implemented only to support the > V4L2 Adaptation layer, and has been deprecated now that a new camera > property - SystemDevices has been introduced. > > Rework the implementation of getCameraIndex() to use the SystemDevices > property and remove reliance on the now deprecated call. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/v4l2/v4l2_compat_manager.cpp | 29 ++++++++++++++++++++++++----- > 1 file changed, 24 insertions(+), 5 deletions(-) > > diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp > index 0f7575c54b22..948dadad646d 100644 > --- a/src/v4l2/v4l2_compat_manager.cpp > +++ b/src/v4l2/v4l2_compat_manager.cpp > @@ -24,6 +24,7 @@ > > #include <libcamera/camera.h> > #include <libcamera/camera_manager.h> > +#include <libcamera/property_ids.h> > > #include "v4l2_camera_file.h" > > @@ -113,14 +114,32 @@ int V4L2CompatManager::getCameraIndex(int fd) > if (ret < 0) > return -1; > > - std::shared_ptr<Camera> target = cm_->get(statbuf.st_rdev); > - if (!target) > - return -1; > + const dev_t devnum = statbuf.st_rdev; > > + /* > + * Iterate each known camera and identify if it reports this nodes > + * device number in its list of SystemDevices. > + */ > auto cameras = cm_->cameras(); > for (auto [index, camera] : utils::enumerate(cameras)) { > - if (camera == target) > - return index; > + Span<const int64_t> devices = camera->properties() > + .get(properties::SystemDevices) > + .value_or(Span<int64_t>{}); > + > + /* > + * While there may be multiple Cameras that could reference the s/Cameras/Camera instances/ (or just cameras) > + * same device node, we take a first match as a best effort for > + * now. > + * > + * \todo Consider reworking the V4L2 adaptation layer to stop > + * trying to map video nodes directly to a camera and instead > + * hide all devices that may be used by libcamera and expose a > + * consistent 1:1 mapping with each Camera instance. Trying to word this in a way that would help me understand it better: * \todo Each camera can be accessed through any of the video * device nodes that it uses. This may confuse applications. * Consider reworking the V4L2 adaptation layer to instead * expose each Camera instance through a single video device * node (with a consistent and stable mapping). The other device * nodes could possibly be hidden from the application by * intercepting additional calls to the C library. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + */ > + for (const int64_t dev : devices) { > + if (dev == static_cast<int64_t>(devnum)) > + return index; > + } > } > > return -1;
diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp index 0f7575c54b22..948dadad646d 100644 --- a/src/v4l2/v4l2_compat_manager.cpp +++ b/src/v4l2/v4l2_compat_manager.cpp @@ -24,6 +24,7 @@ #include <libcamera/camera.h> #include <libcamera/camera_manager.h> +#include <libcamera/property_ids.h> #include "v4l2_camera_file.h" @@ -113,14 +114,32 @@ int V4L2CompatManager::getCameraIndex(int fd) if (ret < 0) return -1; - std::shared_ptr<Camera> target = cm_->get(statbuf.st_rdev); - if (!target) - return -1; + const dev_t devnum = statbuf.st_rdev; + /* + * Iterate each known camera and identify if it reports this nodes + * device number in its list of SystemDevices. + */ auto cameras = cm_->cameras(); for (auto [index, camera] : utils::enumerate(cameras)) { - if (camera == target) - return index; + Span<const int64_t> devices = camera->properties() + .get(properties::SystemDevices) + .value_or(Span<int64_t>{}); + + /* + * While there may be multiple Cameras that could reference the + * same device node, we take a first match as a best effort for + * now. + * + * \todo Consider reworking the V4L2 adaptation layer to stop + * trying to map video nodes directly to a camera and instead + * hide all devices that may be used by libcamera and expose a + * consistent 1:1 mapping with each Camera instance. + */ + for (const int64_t dev : devices) { + if (dev == static_cast<int64_t>(devnum)) + return index; + } } return -1;
The CameraManager->get(dev_t) helper was implemented only to support the V4L2 Adaptation layer, and has been deprecated now that a new camera property - SystemDevices has been introduced. Rework the implementation of getCameraIndex() to use the SystemDevices property and remove reliance on the now deprecated call. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- src/v4l2/v4l2_compat_manager.cpp | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-)