[libcamera-devel] [PATCH] libcamera: Make libudev optional
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Apr 27 16:10:43 CEST 2019
Hi Kieran,
On Sat, Apr 27, 2019 at 03:58:29PM +0200, Kieran Bingham wrote:
> On 27/04/2019 04:16, Laurent Pinchart wrote:
> > libcamera depends on libudev for device enumeration. It is however
> > useful to allow building documentation without requiring the dependency
> > to be installed. Make the libudev dependency optional and compile the
> > udev-based device enumerator out when libudev is not present.
> >
> > Note that will libcamera will compile without libudev, it will not be
>
> s/Note that will/Note that while/
>
> > able to enumerate devices. A sysfs-based device enumerator is planned as
> > a fallback but not implemented yet.
>
> I think being able to build the documentation standalone is certainly a
> good thing.
>
> I'd possibly have looked at the route of providing a meson configuration
> option to enable only the documentation, as we might find that a
> platform without libudev now doesn't work correctly - but as that will
> be fixed separately I'm not going to block this on that point.
I think it would be useful too, but given that we plan to have a
sysfs-based enumerator for platforms without libudev, I think this patch
goes in the right direction anyway.
> Ignoring the documentation requirement, this does step a long way to
> getting configurable device enumeration methods (with the benefit of
> letting us build our documentation without requiring libudev) so:
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
Thank you.
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > Documentation/Doxyfile.in | 4 +-
> > meson.build | 3 +-
> > src/libcamera/camera_manager.cpp | 2 +-
> > src/libcamera/device_enumerator.cpp | 157 +----------------
> > src/libcamera/device_enumerator_udev.cpp | 163 ++++++++++++++++++
> > src/libcamera/include/device_enumerator.h | 24 ---
> > .../include/device_enumerator_udev.h | 42 +++++
> > src/libcamera/meson.build | 10 +-
> > 8 files changed, 223 insertions(+), 182 deletions(-)
> > create mode 100644 src/libcamera/device_enumerator_udev.cpp
> > create mode 100644 src/libcamera/include/device_enumerator_udev.h
> >
> > diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
> > index 3e2b7fd9da0e..950ad4fef40e 100644
> > --- a/Documentation/Doxyfile.in
> > +++ b/Documentation/Doxyfile.in
> > @@ -833,7 +833,9 @@ RECURSIVE = YES
> > # Note that relative paths are relative to the directory from which doxygen is
> > # run.
> >
> > -EXCLUDE = ../src/libcamera/pipeline/
> > +EXCLUDE = ../src/libcamera/device_enumerator_udev.cpp \
> > + ../src/libcamera/include/device_enumerator_udev.h \
> > + ../src/libcamera/pipeline/
> >
> > # The EXCLUDE_SYMLINKS tag can be used to select whether or not files or
> > # directories that are symbolic links (a Unix file system feature) are excluded
> > diff --git a/meson.build b/meson.build
> > index 3434dd7c1b66..d272ff33b100 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -19,7 +19,6 @@ config_h = configuration_data()
> > if cc.has_header_symbol('stdlib.h', 'secure_getenv', prefix: '#define _GNU_SOURCE')
> > config_h.set('HAVE_SECURE_GETENV', 1)
> > endif
> > -configure_file(output: 'config.h', configuration: config_h)
> >
> > common_arguments = [
> > '-Wno-unused-parameter',
> > @@ -49,6 +48,8 @@ if get_option('tests')
> > subdir('test')
> > endif
> >
> > +configure_file(output: 'config.h', configuration: config_h)
> > +
> > pkg_mod = import('pkgconfig')
> > pkg_mod.generate(libraries : libcamera,
> > version : '1.0',
> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> > index 40a39bd2a6d9..cf881ce2e641 100644
> > --- a/src/libcamera/camera_manager.cpp
> > +++ b/src/libcamera/camera_manager.cpp
> > @@ -80,7 +80,7 @@ int CameraManager::start()
> > return -EBUSY;
> >
> > enumerator_ = DeviceEnumerator::create();
> > - if (enumerator_->enumerate())
> > + if (!enumerator_ || enumerator_->enumerate())
> > return -ENODEV;
> >
> > /*
> > diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
> > index 49467546bab8..f6878b3d58b3 100644
> > --- a/src/libcamera/device_enumerator.cpp
> > +++ b/src/libcamera/device_enumerator.cpp
> > @@ -6,14 +6,9 @@
> > */
> >
> > #include "device_enumerator.h"
> > +#include "device_enumerator_udev.h"
> >
> > -#include <fcntl.h>
> > -#include <libudev.h>
> > #include <string.h>
> > -#include <sys/ioctl.h>
> > -#include <unistd.h>
> > -
> > -#include <libcamera/event_notifier.h>
> >
> > #include "log.h"
> > #include "media_device.h"
> > @@ -148,13 +143,11 @@ std::unique_ptr<DeviceEnumerator> DeviceEnumerator::create()
> > {
> > std::unique_ptr<DeviceEnumerator> enumerator;
> >
> > - /**
> > - * \todo Add compile time checks to only try udev enumerator if libudev
> > - * is available.
> > - */
> > +#ifdef HAVE_LIBUDEV
> > enumerator = utils::make_unique<DeviceEnumeratorUdev>();
> > if (!enumerator->init())
> > return enumerator;
> > +#endif
> >
> > /*
> > * Either udev is not available or udev initialization failed. Fall back
> > @@ -327,148 +320,4 @@ std::shared_ptr<MediaDevice> DeviceEnumerator::search(const DeviceMatch &dm)
> > * fails
> > */
> >
> > -/**
> > - * \class DeviceEnumeratorUdev
> > - * \brief Device enumerator based on libudev
> > - */
> > -
> > -DeviceEnumeratorUdev::DeviceEnumeratorUdev()
> > - : udev_(nullptr)
> > -{
> > -}
> > -
> > -DeviceEnumeratorUdev::~DeviceEnumeratorUdev()
> > -{
> > - delete notifier_;
> > -
> > - if (monitor_)
> > - udev_monitor_unref(monitor_);
> > - if (udev_)
> > - udev_unref(udev_);
> > -}
> > -
> > -int DeviceEnumeratorUdev::init()
> > -{
> > - int ret;
> > -
> > - if (udev_)
> > - return -EBUSY;
> > -
> > - udev_ = udev_new();
> > - if (!udev_)
> > - return -ENODEV;
> > -
> > - monitor_ = udev_monitor_new_from_netlink(udev_, "udev");
> > - if (!monitor_)
> > - return -ENODEV;
> > -
> > - ret = udev_monitor_filter_add_match_subsystem_devtype(monitor_, "media",
> > - nullptr);
> > - if (ret < 0)
> > - return ret;
> > -
> > - return 0;
> > -}
> > -
> > -int DeviceEnumeratorUdev::enumerate()
> > -{
> > - struct udev_enumerate *udev_enum = nullptr;
> > - struct udev_list_entry *ents, *ent;
> > - int ret;
> > -
> > - udev_enum = udev_enumerate_new(udev_);
> > - if (!udev_enum)
> > - return -ENOMEM;
> > -
> > - ret = udev_enumerate_add_match_subsystem(udev_enum, "media");
> > - if (ret < 0)
> > - goto done;
> > -
> > - ret = udev_enumerate_scan_devices(udev_enum);
> > - if (ret < 0)
> > - goto done;
> > -
> > - ents = udev_enumerate_get_list_entry(udev_enum);
> > - if (!ents)
> > - goto done;
> > -
> > - udev_list_entry_foreach(ent, ents) {
> > - struct udev_device *dev;
> > - const char *devnode;
> > - const char *syspath = udev_list_entry_get_name(ent);
> > -
> > - dev = udev_device_new_from_syspath(udev_, syspath);
> > - if (!dev) {
> > - LOG(DeviceEnumerator, Warning)
> > - << "Failed to get device for '"
> > - << syspath << "', skipping";
> > - continue;
> > - }
> > -
> > - devnode = udev_device_get_devnode(dev);
> > - if (!devnode) {
> > - udev_device_unref(dev);
> > - ret = -ENODEV;
> > - goto done;
> > - }
> > -
> > - addDevice(devnode);
> > -
> > - udev_device_unref(dev);
> > - }
> > -done:
> > - udev_enumerate_unref(udev_enum);
> > - if (ret < 0)
> > - return ret;
> > -
> > - ret = udev_monitor_enable_receiving(monitor_);
> > - if (ret < 0)
> > - return ret;
> > -
> > - int fd = udev_monitor_get_fd(monitor_);
> > - notifier_ = new EventNotifier(fd, EventNotifier::Read);
> > - notifier_->activated.connect(this, &DeviceEnumeratorUdev::udevNotify);
> > -
> > - return 0;
> > -}
> > -
> > -std::string DeviceEnumeratorUdev::lookupDeviceNode(int major, int minor)
> > -{
> > - struct udev_device *device;
> > - const char *name;
> > - dev_t devnum;
> > - std::string deviceNode = std::string();
> > -
> > - devnum = makedev(major, minor);
> > - device = udev_device_new_from_devnum(udev_, 'c', devnum);
> > - if (!device)
> > - return std::string();
> > -
> > - name = udev_device_get_devnode(device);
> > - if (name)
> > - deviceNode = name;
> > -
> > - udev_device_unref(device);
> > -
> > - return deviceNode;
> > -}
> > -
> > -void DeviceEnumeratorUdev::udevNotify(EventNotifier *notifier)
> > -{
> > - struct udev_device *dev = udev_monitor_receive_device(monitor_);
> > - std::string action(udev_device_get_action(dev));
> > - std::string deviceNode(udev_device_get_devnode(dev));
> > -
> > - LOG(DeviceEnumerator, Debug)
> > - << action << " device " << udev_device_get_devnode(dev);
> > -
> > - if (action == "add") {
> > - addDevice(deviceNode);
> > - } else if (action == "remove") {
> > - removeDevice(deviceNode);
> > - }
> > -
> > - udev_device_unref(dev);
> > -}
> > -
> > } /* namespace libcamera */
> > diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
>
> I like that the udev specific code is moved out to it's own unit.
>
>
>
> > new file mode 100644
> > index 000000000000..cb2d21b90506
> > --- /dev/null
> > +++ b/src/libcamera/device_enumerator_udev.cpp
> > @@ -0,0 +1,163 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2018-2019, Google Inc.
> > + *
> > + * device_enumerator_udev.cpp - udev-based device enumerator
> > + */
> > +
> > +#include "device_enumerator_udev.h"
> > +
> > +#include <fcntl.h>
> > +#include <libudev.h>
> > +#include <string.h>
> > +#include <sys/ioctl.h>
> > +#include <unistd.h>
> > +
> > +#include <libcamera/event_notifier.h>
> > +
> > +#include "log.h"
> > +
> > +namespace libcamera {
> > +
> > +LOG_DECLARE_CATEGORY(DeviceEnumerator)
> > +
> > +DeviceEnumeratorUdev::DeviceEnumeratorUdev()
> > + : udev_(nullptr)
> > +{
> > +}
> > +
> > +DeviceEnumeratorUdev::~DeviceEnumeratorUdev()
> > +{
> > + delete notifier_;
> > +
> > + if (monitor_)
> > + udev_monitor_unref(monitor_);
> > + if (udev_)
> > + udev_unref(udev_);
> > +}
> > +
> > +int DeviceEnumeratorUdev::init()
> > +{
> > + int ret;
> > +
> > + if (udev_)
> > + return -EBUSY;
> > +
> > + udev_ = udev_new();
> > + if (!udev_)
> > + return -ENODEV;
> > +
> > + monitor_ = udev_monitor_new_from_netlink(udev_, "udev");
> > + if (!monitor_)
> > + return -ENODEV;
> > +
> > + ret = udev_monitor_filter_add_match_subsystem_devtype(monitor_, "media",
> > + nullptr);
> > + if (ret < 0)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > +int DeviceEnumeratorUdev::enumerate()
> > +{
> > + struct udev_enumerate *udev_enum = nullptr;
> > + struct udev_list_entry *ents, *ent;
> > + int ret;
> > +
> > + udev_enum = udev_enumerate_new(udev_);
> > + if (!udev_enum)
> > + return -ENOMEM;
> > +
> > + ret = udev_enumerate_add_match_subsystem(udev_enum, "media");
> > + if (ret < 0)
> > + goto done;
> > +
> > + ret = udev_enumerate_scan_devices(udev_enum);
> > + if (ret < 0)
> > + goto done;
> > +
> > + ents = udev_enumerate_get_list_entry(udev_enum);
> > + if (!ents)
> > + goto done;
> > +
> > + udev_list_entry_foreach(ent, ents) {
> > + struct udev_device *dev;
> > + const char *devnode;
> > + const char *syspath = udev_list_entry_get_name(ent);
> > +
> > + dev = udev_device_new_from_syspath(udev_, syspath);
> > + if (!dev) {
> > + LOG(DeviceEnumerator, Warning)
> > + << "Failed to get device for '"
> > + << syspath << "', skipping";
> > + continue;
> > + }
> > +
> > + devnode = udev_device_get_devnode(dev);
> > + if (!devnode) {
> > + udev_device_unref(dev);
> > + ret = -ENODEV;
> > + goto done;
> > + }
> > +
> > + addDevice(devnode);
> > +
> > + udev_device_unref(dev);
> > + }
> > +done:
> > + udev_enumerate_unref(udev_enum);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = udev_monitor_enable_receiving(monitor_);
> > + if (ret < 0)
> > + return ret;
> > +
> > + int fd = udev_monitor_get_fd(monitor_);
> > + notifier_ = new EventNotifier(fd, EventNotifier::Read);
> > + notifier_->activated.connect(this, &DeviceEnumeratorUdev::udevNotify);
> > +
> > + return 0;
> > +}
> > +
> > +std::string DeviceEnumeratorUdev::lookupDeviceNode(int major, int minor)
> > +{
> > + struct udev_device *device;
> > + const char *name;
> > + dev_t devnum;
> > + std::string deviceNode = std::string();
> > +
> > + devnum = makedev(major, minor);
> > + device = udev_device_new_from_devnum(udev_, 'c', devnum);
> > + if (!device)
> > + return std::string();
> > +
> > + name = udev_device_get_devnode(device);
> > + if (name)
> > + deviceNode = name;
> > +
> > + udev_device_unref(device);
> > +
> > + return deviceNode;
> > +}
> > +
> > +void DeviceEnumeratorUdev::udevNotify(EventNotifier *notifier)
> > +{
> > + struct udev_device *dev = udev_monitor_receive_device(monitor_);
> > + std::string action(udev_device_get_action(dev));
> > + std::string deviceNode(udev_device_get_devnode(dev));
> > +
> > + LOG(DeviceEnumerator, Debug)
> > + << action << " device " << udev_device_get_devnode(dev);
> > +
> > + if (action == "add") {
> > + addDevice(deviceNode);
> > + } else if (action == "remove") {
> > + removeDevice(deviceNode);
> > + }
> > +
> > + udev_device_unref(dev);
> > +}
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h
> > index 2801861864b8..02aec3bc50c6 100644
> > --- a/src/libcamera/include/device_enumerator.h
> > +++ b/src/libcamera/include/device_enumerator.h
> > @@ -7,19 +7,14 @@
> > #ifndef __LIBCAMERA_DEVICE_ENUMERATOR_H__
> > #define __LIBCAMERA_DEVICE_ENUMERATOR_H__
> >
> > -#include <map>
> > #include <memory>
> > #include <string>
> > #include <vector>
> >
> > #include <linux/media.h>
> >
> > -struct udev;
> > -struct udev_monitor;
> > -
> > namespace libcamera {
> >
> > -class EventNotifier;
> > class MediaDevice;
> >
> > class DeviceMatch
> > @@ -58,25 +53,6 @@ private:
> > virtual std::string lookupDeviceNode(int major, int minor) = 0;
> > };
> >
> > -class DeviceEnumeratorUdev: public DeviceEnumerator
> > -{
> > -public:
> > - DeviceEnumeratorUdev();
> > - ~DeviceEnumeratorUdev();
> > -
> > - int init() final;
> > - int enumerate() final;
> > -
> > -private:
> > - struct udev *udev_;
> > - struct udev_monitor *monitor_;
> > - EventNotifier *notifier_;
> > -
> > - std::string lookupDeviceNode(int major, int minor) final;
> > -
> > - void udevNotify(EventNotifier *notifier);
> > -};
> > -
> > } /* namespace libcamera */
> >
> > #endif /* __LIBCAMERA_DEVICE_ENUMERATOR_H__ */
> > diff --git a/src/libcamera/include/device_enumerator_udev.h b/src/libcamera/include/device_enumerator_udev.h
> > new file mode 100644
> > index 000000000000..80f9372bca36
> > --- /dev/null
> > +++ b/src/libcamera/include/device_enumerator_udev.h
> > @@ -0,0 +1,42 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2018-2019, Google Inc.
> > + *
> > + * device_enumerator_udev.h - udev-based device enumerator
> > + */
> > +#ifndef __LIBCAMERA_DEVICE_ENUMERATOR_UDEV_H__
> > +#define __LIBCAMERA_DEVICE_ENUMERATOR_UDEV_H__
> > +
> > +#include <string>
> > +
> > +#include "device_enumerator.h"
> > +
> > +struct udev;
> > +struct udev_monitor;
> > +
> > +namespace libcamera {
> > +
> > +class EventNotifier;
> > +
> > +class DeviceEnumeratorUdev : public DeviceEnumerator
> > +{
> > +public:
> > + DeviceEnumeratorUdev();
> > + ~DeviceEnumeratorUdev();
> > +
> > + int init() final;
> > + int enumerate() final;
> > +
> > +private:
> > + struct udev *udev_;
> > + struct udev_monitor *monitor_;
> > + EventNotifier *notifier_;
> > +
> > + std::string lookupDeviceNode(int major, int minor) final;
> > +
> > + void udevNotify(EventNotifier *notifier);
> > +};
> > +
> > +} /* namespace libcamera */
> > +
> > +#endif /* __LIBCAMERA_DEVICE_ENUMERATOR_UDEV_H__ */
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > index cf4edec05755..2b678237b64f 100644
> > --- a/src/libcamera/meson.build
> > +++ b/src/libcamera/meson.build
> > @@ -26,6 +26,7 @@ libcamera_sources = files([
> > libcamera_headers = files([
> > 'include/camera_sensor.h',
> > 'include/device_enumerator.h',
> > + 'include/device_enumerator_udev.h',
> > 'include/event_dispatcher_poll.h',
> > 'include/formats.h',
> > 'include/log.h',
> > @@ -46,7 +47,14 @@ includes = [
> >
> > subdir('pipeline')
> >
> > -libudev = dependency('libudev')
> > +libudev = dependency('libudev', required: false)
> > +
> > +if libudev.found()
> > + config_h.set('HAVE_LIBUDEV', 1)
> > + libcamera_sources += files([
> > + 'device_enumerator_udev.cpp',
> > + ])
> > +endif
> >
> > libcamera = shared_library('camera',
> > libcamera_sources,
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list