[libcamera-devel] [PATCH] libcamera: Make libudev optional
Kieran Bingham
kieran.bingham at ideasonboard.com
Sat Apr 27 15:58:29 CEST 2019
Hi Laurent,
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.
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>
> 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
--
Kieran
More information about the libcamera-devel
mailing list