[libcamera-devel] libcamera: Make libudev optional

1119 diff mbox series

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
able to enumerate devices. A sysfs-based device enumerator is planned as
a fallback but not implemented yet.

Signed-off-by: Laurent Pinchart <laurent.pinchart@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

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@ideasonboard.com>



> Signed-off-by: Laurent Pinchart <laurent.pinchart@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,
>

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@ideasonboard.com>

Thank you.

> > Signed-off-by: Laurent Pinchart <laurent.pinchart@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,