Message ID | 20181222230041.29999-4-niklas.soderlund@ragnatech.se |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Niklas, On Sun, Dec 23, 2018 at 12:00:32AM +0100, Niklas S??derlund wrote: > Provide a DeviceInfo class which holds all information from the initial > enumeration of a media device. Not all information available at a media > device is stored, only the information needed for a pipeline handler to > find a specific device. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/deviceenumerator.cpp | 78 ++++++++++++++++++++++++ > src/libcamera/include/deviceenumerator.h | 44 +++++++++++++ > src/libcamera/meson.build | 2 + > 3 files changed, 124 insertions(+) > create mode 100644 src/libcamera/deviceenumerator.cpp > create mode 100644 src/libcamera/include/deviceenumerator.h > > diff --git a/src/libcamera/deviceenumerator.cpp b/src/libcamera/deviceenumerator.cpp > new file mode 100644 > index 0000000000000000..7c44a65b45472ef3 > --- /dev/null > +++ b/src/libcamera/deviceenumerator.cpp > @@ -0,0 +1,78 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2018, Google Inc. > + * > + * deviceenumerator.cpp - Enumeration and matching > + */ > + > +#include "deviceenumerator.h" > +#include "log.h" > + > +namespace libcamera { > + > +/* ----------------------------------------------------------------------------- > + * DeviceInfo > + */ > + > +DeviceInfo::DeviceInfo(const std::string &devnode, const struct media_device_info &info, > + const std::map<std::string, std::string> &entities) > + : acquired_(false), devnode_(devnode), info_(info), entities_(entities) > +{ > + for (const auto &entity : entities_) > + LOG(Info) << "Device: " << devnode_ << " Entity: '" << entity.first << "' -> " << entity.second; > +} > + > +int DeviceInfo::acquire() > +{ > + if (acquired_) > + return -EBUSY; > + > + acquired_ = true; > + > + return 0; > +} > + > +void DeviceInfo::release() > +{ > + acquired_ = false; > +} > + > +bool DeviceInfo::busy() const > +{ > + return acquired_; > +} > + > +const std::string &DeviceInfo::devnode() const > +{ > + return devnode_; > +} > + > +const struct media_device_info &DeviceInfo::info() const > +{ > + return info_; > +} > + > +std::vector<std::string> DeviceInfo::entities() const > +{ > + std::vector<std::string> entities; > + > + for (const auto &entity : entities_) > + entities.push_back(entity.first); > + > + return entities; Are you returning (by copy) a variable with local scope? Why coldn't you just return (as const reference maybe) entities_ ? > +} > + > +bool DeviceInfo::lookup(const std::string &name, std::string &devnode) const > +{ > + auto it = entities_.find(name); > + > + if (it == entities_.end()) { > + LOG(Error) << "Trying to lookup entity '" << name << "' which do not exist"; nit: "does not exist" > + return false; > + } > + > + devnode = it->second; > + return true; > +} > + > +} /* namespace libcamera */ > diff --git a/src/libcamera/include/deviceenumerator.h b/src/libcamera/include/deviceenumerator.h > new file mode 100644 > index 0000000000000000..0136ed6ea23bf65e > --- /dev/null > +++ b/src/libcamera/include/deviceenumerator.h > @@ -0,0 +1,44 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2018, Google Inc. > + * > + * deviceenumerator.h - API to enumerate and find media devices > + */ > +#ifndef __LIBCAMERA_DEVICEENUMERATE_H__ > +#define __LIBCAMERA_DEVICEENUMERATE_H__ > + > +#include <map> > +#include <string> > +#include <vector> > + > +#include <linux/media.h> > + > +namespace libcamera { > + > +class DeviceInfo > +{ > +public: > + DeviceInfo(const std::string &devnode, const struct media_device_info &info, > + const std::map<std::string, std::string> &entities); > + > + int acquire(); > + void release(); > + bool busy() const; Trivial methods might be inlined here. > + > + const std::string &devnode() const; > + const struct media_device_info &info() const; > + std::vector<std::string> entities() const; > + > + bool lookup(const std::string &name, std::string &devnode) const; > + > +private: > + bool acquired_; > + > + std::string devnode_; > + struct media_device_info info_; > + std::map<std::string, std::string> entities_; > +}; > + > +} /* namespace libcamera */ > + > +#endif /* __LIBCAMERA_DEVICEENUMERATE_H__ */ > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > index 52b556a8ed4050cb..17cdf06dd2bedfb3 100644 > --- a/src/libcamera/meson.build > +++ b/src/libcamera/meson.build > @@ -1,10 +1,12 @@ > libcamera_sources = files([ > 'camera.cpp', > + 'deviceenumerator.cpp', > 'log.cpp', > 'main.cpp', > ]) > > libcamera_headers = files([ > + 'include/deviceenumerator.h', > 'include/log.h', > 'include/utils.h', > ]) > -- > 2.20.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, Thanks for your feedback. On 2018-12-24 11:42:16 +0100, Jacopo Mondi wrote: > Hi Niklas, > > On Sun, Dec 23, 2018 at 12:00:32AM +0100, Niklas S??derlund wrote: > > Provide a DeviceInfo class which holds all information from the initial > > enumeration of a media device. Not all information available at a media > > device is stored, only the information needed for a pipeline handler to > > find a specific device. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > > src/libcamera/deviceenumerator.cpp | 78 ++++++++++++++++++++++++ > > src/libcamera/include/deviceenumerator.h | 44 +++++++++++++ > > src/libcamera/meson.build | 2 + > > 3 files changed, 124 insertions(+) > > create mode 100644 src/libcamera/deviceenumerator.cpp > > create mode 100644 src/libcamera/include/deviceenumerator.h > > > > diff --git a/src/libcamera/deviceenumerator.cpp b/src/libcamera/deviceenumerator.cpp > > new file mode 100644 > > index 0000000000000000..7c44a65b45472ef3 > > --- /dev/null > > +++ b/src/libcamera/deviceenumerator.cpp > > @@ -0,0 +1,78 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2018, Google Inc. > > + * > > + * deviceenumerator.cpp - Enumeration and matching > > + */ > > + > > +#include "deviceenumerator.h" > > +#include "log.h" > > + > > +namespace libcamera { > > + > > +/* ----------------------------------------------------------------------------- > > + * DeviceInfo > > + */ > > + > > +DeviceInfo::DeviceInfo(const std::string &devnode, const struct media_device_info &info, > > + const std::map<std::string, std::string> &entities) > > + : acquired_(false), devnode_(devnode), info_(info), entities_(entities) > > +{ > > + for (const auto &entity : entities_) > > + LOG(Info) << "Device: " << devnode_ << " Entity: '" << entity.first << "' -> " << entity.second; > > +} > > + > > +int DeviceInfo::acquire() > > +{ > > + if (acquired_) > > + return -EBUSY; > > + > > + acquired_ = true; > > + > > + return 0; > > +} > > + > > +void DeviceInfo::release() > > +{ > > + acquired_ = false; > > +} > > + > > +bool DeviceInfo::busy() const > > +{ > > + return acquired_; > > +} > > + > > +const std::string &DeviceInfo::devnode() const > > +{ > > + return devnode_; > > +} > > + > > +const struct media_device_info &DeviceInfo::info() const > > +{ > > + return info_; > > +} > > + > > +std::vector<std::string> DeviceInfo::entities() const > > +{ > > + std::vector<std::string> entities; > > + > > + for (const auto &entity : entities_) > > + entities.push_back(entity.first); > > + > > + return entities; > > Are you returning (by copy) a variable with local scope? > Why coldn't you just return (as const reference maybe) entities_ ? This is by design. The idea is that a new copy of the entities in a vector instead of the class internal object of the model of entities as the use-case here is to do matching. Why would it be beneficial to return a internal data structure? > > > +} > > + > > +bool DeviceInfo::lookup(const std::string &name, std::string &devnode) const > > +{ > > + auto it = entities_.find(name); > > + > > + if (it == entities_.end()) { > > + LOG(Error) << "Trying to lookup entity '" << name << "' which do not exist"; > > nit: "does not exist" My comprehension of the Queens English is lacking so I trust you on this. For my education is not 'does' used when there is more then one while 'do' is used when there is exactly one? > > > + return false; > > + } > > + > > + devnode = it->second; > > + return true; > > +} > > + > > +} /* namespace libcamera */ > > diff --git a/src/libcamera/include/deviceenumerator.h b/src/libcamera/include/deviceenumerator.h > > new file mode 100644 > > index 0000000000000000..0136ed6ea23bf65e > > --- /dev/null > > +++ b/src/libcamera/include/deviceenumerator.h > > @@ -0,0 +1,44 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2018, Google Inc. > > + * > > + * deviceenumerator.h - API to enumerate and find media devices > > + */ > > +#ifndef __LIBCAMERA_DEVICEENUMERATE_H__ > > +#define __LIBCAMERA_DEVICEENUMERATE_H__ > > + > > +#include <map> > > +#include <string> > > +#include <vector> > > + > > +#include <linux/media.h> > > + > > +namespace libcamera { > > + > > +class DeviceInfo > > +{ > > +public: > > + DeviceInfo(const std::string &devnode, const struct media_device_info &info, > > + const std::map<std::string, std::string> &entities); > > + > > + int acquire(); > > + void release(); > > + bool busy() const; > > Trivial methods might be inlined here. There are other methods who can't be inlined for this class. Why spread the implementation around? > > > + > > + const std::string &devnode() const; > > + const struct media_device_info &info() const; > > + std::vector<std::string> entities() const; > > + > > + bool lookup(const std::string &name, std::string &devnode) const; > > + > > +private: > > + bool acquired_; > > + > > + std::string devnode_; > > + struct media_device_info info_; > > + std::map<std::string, std::string> entities_; > > +}; > > + > > +} /* namespace libcamera */ > > + > > +#endif /* __LIBCAMERA_DEVICEENUMERATE_H__ */ > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > index 52b556a8ed4050cb..17cdf06dd2bedfb3 100644 > > --- a/src/libcamera/meson.build > > +++ b/src/libcamera/meson.build > > @@ -1,10 +1,12 @@ > > libcamera_sources = files([ > > 'camera.cpp', > > + 'deviceenumerator.cpp', > > 'log.cpp', > > 'main.cpp', > > ]) > > > > libcamera_headers = files([ > > + 'include/deviceenumerator.h', > > 'include/log.h', > > 'include/utils.h', > > ]) > > -- > > 2.20.1 > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Niklas, just replying to a few things On Fri, Dec 28, 2018 at 04:06:57AM +0100, Niklas S??derlund wrote: > Hi Jacopo, > > Thanks for your feedback. > > On 2018-12-24 11:42:16 +0100, Jacopo Mondi wrote: > > Hi Niklas, > > > > On Sun, Dec 23, 2018 at 12:00:32AM +0100, Niklas S??derlund wrote: > > > Provide a DeviceInfo class which holds all information from the initial > > > enumeration of a media device. Not all information available at a media > > > device is stored, only the information needed for a pipeline handler to > > > find a specific device. > > > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > --- > > > src/libcamera/deviceenumerator.cpp | 78 ++++++++++++++++++++++++ > > > src/libcamera/include/deviceenumerator.h | 44 +++++++++++++ > > > src/libcamera/meson.build | 2 + > > > 3 files changed, 124 insertions(+) > > > create mode 100644 src/libcamera/deviceenumerator.cpp > > > create mode 100644 src/libcamera/include/deviceenumerator.h > > > > > > diff --git a/src/libcamera/deviceenumerator.cpp b/src/libcamera/deviceenumerator.cpp > > > new file mode 100644 > > > index 0000000000000000..7c44a65b45472ef3 > > > --- /dev/null > > > +++ b/src/libcamera/deviceenumerator.cpp > > > @@ -0,0 +1,78 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2018, Google Inc. > > > + * > > > + * deviceenumerator.cpp - Enumeration and matching > > > + */ > > > + > > > +#include "deviceenumerator.h" > > > +#include "log.h" > > > + > > > +namespace libcamera { > > > + > > > +/* ----------------------------------------------------------------------------- > > > + * DeviceInfo > > > + */ > > > + > > > +DeviceInfo::DeviceInfo(const std::string &devnode, const struct media_device_info &info, > > > + const std::map<std::string, std::string> &entities) > > > + : acquired_(false), devnode_(devnode), info_(info), entities_(entities) > > > +{ > > > + for (const auto &entity : entities_) > > > + LOG(Info) << "Device: " << devnode_ << " Entity: '" << entity.first << "' -> " << entity.second; > > > +} > > > + > > > +int DeviceInfo::acquire() > > > +{ > > > + if (acquired_) > > > + return -EBUSY; > > > + > > > + acquired_ = true; > > > + > > > + return 0; > > > +} > > > + > > > +void DeviceInfo::release() > > > +{ > > > + acquired_ = false; > > > +} > > > + > > > +bool DeviceInfo::busy() const > > > +{ > > > + return acquired_; > > > +} > > > + > > > +const std::string &DeviceInfo::devnode() const > > > +{ > > > + return devnode_; > > > +} > > > + > > > +const struct media_device_info &DeviceInfo::info() const > > > +{ > > > + return info_; > > > +} > > > + > > > +std::vector<std::string> DeviceInfo::entities() const > > > +{ > > > + std::vector<std::string> entities; > > > + > > > + for (const auto &entity : entities_) > > > + entities.push_back(entity.first); > > > + > > > + return entities; > > > > Are you returning (by copy) a variable with local scope? > > Why coldn't you just return (as const reference maybe) entities_ ? > > This is by design. The idea is that a new copy of the entities in a > vector instead of the class internal object of the model of entities as > the use-case here is to do matching. Why would it be beneficial to > return a internal data structure? > It would avoid moving around a vector by copy. You only use the returned vector to compare entities names, so you could just simply return a const reference to your internal data structure. I would actually ask why would it be beneficial to copy all entities names in a new vector and copy it back, if that's not going to be modified anyhow. Furthermore, you could have used operator= to copy the internal one: http://www.cplusplus.com/reference/vector/vector/operator=/ > > > > > +} > > > + > > > +bool DeviceInfo::lookup(const std::string &name, std::string &devnode) const > > > +{ > > > + auto it = entities_.find(name); > > > + > > > + if (it == entities_.end()) { > > > + LOG(Error) << "Trying to lookup entity '" << name << "' which do not exist"; > > > > nit: "does not exist" > > My comprehension of the Queens English is lacking so I trust you on Don't! > this. For my education is not 'does' used when there is more then one > while 'do' is used when there is exactly one? > You might be right, let's wait for some more educated (or native) speaker to school us. > > > > > + return false; > > > + } > > > + > > > + devnode = it->second; > > > + return true; > > > +} > > > + > > > +} /* namespace libcamera */ > > > diff --git a/src/libcamera/include/deviceenumerator.h b/src/libcamera/include/deviceenumerator.h > > > new file mode 100644 > > > index 0000000000000000..0136ed6ea23bf65e > > > --- /dev/null > > > +++ b/src/libcamera/include/deviceenumerator.h > > > @@ -0,0 +1,44 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2018, Google Inc. > > > + * > > > + * deviceenumerator.h - API to enumerate and find media devices > > > + */ > > > +#ifndef __LIBCAMERA_DEVICEENUMERATE_H__ > > > +#define __LIBCAMERA_DEVICEENUMERATE_H__ > > > + > > > +#include <map> > > > +#include <string> > > > +#include <vector> > > > + > > > +#include <linux/media.h> > > > + > > > +namespace libcamera { > > > + > > > +class DeviceInfo > > > +{ > > > +public: > > > + DeviceInfo(const std::string &devnode, const struct media_device_info &info, > > > + const std::map<std::string, std::string> &entities); > > > + > > > + int acquire(); > > > + void release(); > > > + bool busy() const; > > > > Trivial methods might be inlined here. > > There are other methods who can't be inlined for this class. Why spread > the implementation around? > I'm in favour of inlining single line and trivial methods/constructors as much as we could. It reduce the size of the cpp file, and makes clear from reading the header the method is trivial. I'm not much concerned about spreading per se, as we all navigate tags don't we? (let alone the discussion about the fact inlining in class declaration is a hint for the compiler to inline the method in the generated code. I think the compiler is smart enough to find that out by itself anyhow though) Thanks j > > > > > + > > > + const std::string &devnode() const; > > > + const struct media_device_info &info() const; > > > + std::vector<std::string> entities() const; > > > + > > > + bool lookup(const std::string &name, std::string &devnode) const; > > > + > > > +private: > > > + bool acquired_; > > > + > > > + std::string devnode_; > > > + struct media_device_info info_; > > > + std::map<std::string, std::string> entities_; > > > +}; > > > + > > > +} /* namespace libcamera */ > > > + > > > +#endif /* __LIBCAMERA_DEVICEENUMERATE_H__ */ > > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build > > > index 52b556a8ed4050cb..17cdf06dd2bedfb3 100644 > > > --- a/src/libcamera/meson.build > > > +++ b/src/libcamera/meson.build > > > @@ -1,10 +1,12 @@ > > > libcamera_sources = files([ > > > 'camera.cpp', > > > + 'deviceenumerator.cpp', > > > 'log.cpp', > > > 'main.cpp', > > > ]) > > > > > > libcamera_headers = files([ > > > + 'include/deviceenumerator.h', > > > 'include/log.h', > > > 'include/utils.h', > > > ]) > > > -- > > > 2.20.1 > > > > > > _______________________________________________ > > > libcamera-devel mailing list > > > libcamera-devel@lists.libcamera.org > > > https://lists.libcamera.org/listinfo/libcamera-devel > > > > -- > Regards, > Niklas Söderlund
Hi Jacopo, On Friday, 28 December 2018 10:21:48 EET Jacopo Mondi wrote: > On Fri, Dec 28, 2018 at 04:06:57AM +0100, Niklas S??derlund wrote: > > On 2018-12-24 11:42:16 +0100, Jacopo Mondi wrote: > > > On Sun, Dec 23, 2018 at 12:00:32AM +0100, Niklas S??derlund wrote: > > > > Provide a DeviceInfo class which holds all information from the > > > > initial enumeration of a media device. Not all information available > > > > at a media device is stored, only the information needed for a > > > > pipeline handler to find a specific device. > > > > > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > > --- > > > > > > > > src/libcamera/deviceenumerator.cpp | 78 +++++++++++++++++++++++ > > > > src/libcamera/include/deviceenumerator.h | 44 +++++++++++++ > > > > src/libcamera/meson.build | 2 + > > > > 3 files changed, 124 insertions(+) > > > > create mode 100644 src/libcamera/deviceenumerator.cpp > > > > create mode 100644 src/libcamera/include/deviceenumerator.h > > > > > > > > diff --git a/src/libcamera/deviceenumerator.cpp > > > > b/src/libcamera/deviceenumerator.cpp new file mode 100644 > > > > index 0000000000000000..7c44a65b45472ef3 > > > > --- /dev/null > > > > +++ b/src/libcamera/deviceenumerator.cpp > > > > @@ -0,0 +1,78 @@ > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > +/* > > > > + * Copyright (C) 2018, Google Inc. > > > > + * > > > > + * deviceenumerator.cpp - Enumeration and matching > > > > + */ > > > > + > > > > +#include "deviceenumerator.h" > > > > +#include "log.h" > > > > + > > > > +namespace libcamera { > > > > + > > > > +/* > > > > --------------------------------------------------------------------- > > > > -------- + * DeviceInfo > > > > + */ > > > > + > > > > +DeviceInfo::DeviceInfo(const std::string &devnode, const struct > > > > media_device_info &info, + const std::map<std::string, > > > > std::string> &entities) > > > > + : acquired_(false), devnode_(devnode), info_(info), > > > > entities_(entities) > > > > +{ > > > > + for (const auto &entity : entities_) > > > > + LOG(Info) << "Device: " << devnode_ << " Entity: '" << entity.first > > > > << "' -> " << entity.second; +} > > > > + > > > > +int DeviceInfo::acquire() > > > > +{ > > > > + if (acquired_) > > > > + return -EBUSY; > > > > + > > > > + acquired_ = true; > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +void DeviceInfo::release() > > > > +{ > > > > + acquired_ = false; > > > > +} > > > > + > > > > +bool DeviceInfo::busy() const > > > > +{ > > > > + return acquired_; > > > > +} > > > > + > > > > +const std::string &DeviceInfo::devnode() const > > > > +{ > > > > + return devnode_; > > > > +} > > > > + > > > > +const struct media_device_info &DeviceInfo::info() const > > > > +{ > > > > + return info_; > > > > +} > > > > + > > > > +std::vector<std::string> DeviceInfo::entities() const > > > > +{ > > > > + std::vector<std::string> entities; > > > > + > > > > + for (const auto &entity : entities_) > > > > + entities.push_back(entity.first); > > > > + > > > > + return entities; > > > > > > Are you returning (by copy) a variable with local scope? > > > Why coldn't you just return (as const reference maybe) entities_ ? > > > > This is by design. The idea is that a new copy of the entities in a > > vector instead of the class internal object of the model of entities as > > the use-case here is to do matching. Why would it be beneficial to > > return a internal data structure? > > It would avoid moving around a vector by copy. You only use > the returned vector to compare entities names, so you could just > simply return a const reference to your internal data structure. > > I would actually ask why would it be beneficial to copy all entities > names in a new vector and copy it back, if that's not going to be > modified anyhow. Furthermore, you could have used operator= to copy > the internal one: > http://www.cplusplus.com/reference/vector/vector/operator=/ What do you mean by "copying it back" ? Have you overlooked the fact that entities_ is a map, not a vector ? > > > > +} > > > > + > > > > +bool DeviceInfo::lookup(const std::string &name, std::string > > > > &devnode) const +{ > > > > + auto it = entities_.find(name); > > > > + > > > > + if (it == entities_.end()) { > > > > + LOG(Error) << "Trying to lookup entity '" << name << "' which do > > > > not exist"; > > > > > > > nit: "does not exist" > > > > My comprehension of the Queens English is lacking so I trust you on > > Don't! > > > this. For my education is not 'does' used when there is more then one > > while 'do' is used when there is exactly one? > > You might be right, let's wait for some more educated (or native) > speaker to school us. I'm certainly not native, and wouldn't claim to be more educated, but I think the subject of the verb refers to "entity", which is a third person singular, so "does" is the right form. > > > > + return false; > > > > + } > > > > + > > > > + devnode = it->second; > > > > + return true; > > > > +} > > > > + > > > > +} /* namespace libcamera */ > > > > diff --git a/src/libcamera/include/deviceenumerator.h > > > > b/src/libcamera/include/deviceenumerator.h new file mode 100644 > > > > index 0000000000000000..0136ed6ea23bf65e > > > > --- /dev/null > > > > +++ b/src/libcamera/include/deviceenumerator.h > > > > @@ -0,0 +1,44 @@ > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > +/* > > > > + * Copyright (C) 2018, Google Inc. > > > > + * > > > > + * deviceenumerator.h - API to enumerate and find media devices > > > > + */ > > > > +#ifndef __LIBCAMERA_DEVICEENUMERATE_H__ > > > > +#define __LIBCAMERA_DEVICEENUMERATE_H__ > > > > + > > > > +#include <map> > > > > +#include <string> > > > > +#include <vector> > > > > + > > > > +#include <linux/media.h> > > > > + > > > > +namespace libcamera { > > > > + > > > > +class DeviceInfo > > > > +{ > > > > +public: > > > > + DeviceInfo(const std::string &devnode, const struct > > > > media_device_info &info, + const std::map<std::string, > > > > std::string> &entities); > > > > + > > > > + int acquire(); > > > > + void release(); > > > > + bool busy() const; > > > > > > Trivial methods might be inlined here. > > > > There are other methods who can't be inlined for this class. Why spread > > the implementation around? > > I'm in favour of inlining single line and trivial methods/constructors as > much as we could. It reduce the size of the cpp file, and makes clear > from reading the header the method is trivial. I'm not much concerned about > spreading per se, as we all navigate tags don't we? Methods should only be inlined when the code size is roughly equivalent to a function call. Trivial getters are good candidates. Constructors may be, but only if they don't have a large list of member initializers. For destructors, even if the function is empty, it might call other destructors for members, and the destructor of the base class. Inlining destructors is thus usually a bad idea. Inlinline in C++ uses a different syntax than in C, but the result is similar, the code is inlined, and may thus increase the binary size. Please see the Google C++ style guide section about this. > (let alone the discussion about the fact inlining in class declaration > is a hint for the compiler to inline the method in the generated code. > I think the compiler is smart enough to find that out by itself > anyhow though) > > > > > + > > > > + const std::string &devnode() const; > > > > + const struct media_device_info &info() const; > > > > + std::vector<std::string> entities() const; > > > > + > > > > + bool lookup(const std::string &name, std::string &devnode) const; > > > > + > > > > +private: > > > > + bool acquired_; > > > > + > > > > + std::string devnode_; > > > > + struct media_device_info info_; > > > > + std::map<std::string, std::string> entities_; > > > > +}; > > > > + > > > > +} /* namespace libcamera */ > > > > + > > > > +#endif /* __LIBCAMERA_DEVICEENUMERATE_H__ */ [snip]
Hi Niklas, Thank you for the patch. On Sunday, 23 December 2018 01:00:32 EET Niklas Söderlund wrote: > Provide a DeviceInfo class which holds all information from the initial > enumeration of a media device. Not all information available at a media > device is stored, only the information needed for a pipeline handler to > find a specific device. Do I understand correctly that this will be replaced with MediaDevice ? If so I only have a small comment below, and for the rest, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/deviceenumerator.cpp | 78 ++++++++++++++++++++++++ > src/libcamera/include/deviceenumerator.h | 44 +++++++++++++ > src/libcamera/meson.build | 2 + > 3 files changed, 124 insertions(+) > create mode 100644 src/libcamera/deviceenumerator.cpp > create mode 100644 src/libcamera/include/deviceenumerator.h > > diff --git a/src/libcamera/deviceenumerator.cpp > b/src/libcamera/deviceenumerator.cpp new file mode 100644 > index 0000000000000000..7c44a65b45472ef3 > --- /dev/null > +++ b/src/libcamera/deviceenumerator.cpp > @@ -0,0 +1,78 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2018, Google Inc. > + * > + * deviceenumerator.cpp - Enumeration and matching > + */ > + > +#include "deviceenumerator.h" > +#include "log.h" > + > +namespace libcamera { > + > +/* ------------------------------------------------------------------------ > + * DeviceInfo > + */ > + > +DeviceInfo::DeviceInfo(const std::string &devnode, const struct > media_device_info &info, > + const std::map<std::string, std::string> > &entities) > + : acquired_(false), devnode_(devnode), info_(info), entities_(entities) > +{ > + for (const auto &entity : entities_) > + LOG(Info) << "Device: " << devnode_ << " Entity: '" << entity.first << "' > -> " << entity.second; > +} > + > +int DeviceInfo::acquire() > +{ > + if (acquired_) > + return -EBUSY; > + > + acquired_ = true; > + > + return 0; > +} > + > +void DeviceInfo::release() > +{ > + acquired_ = false; > +} > + > +bool DeviceInfo::busy() const > +{ > + return acquired_; > +} > + > +const std::string &DeviceInfo::devnode() const > +{ > + return devnode_; > +} > + > +const struct media_device_info &DeviceInfo::info() const > +{ > + return info_; > +} > + > +std::vector<std::string> DeviceInfo::entities() const > +{ > + std::vector<std::string> entities; > + > + for (const auto &entity : entities_) > + entities.push_back(entity.first); > + > + return entities; > +} > + > +bool DeviceInfo::lookup(const std::string &name, std::string &devnode) > const > +{ > + auto it = entities_.find(name); > + > + if (it == entities_.end()) { > + LOG(Error) << "Trying to lookup entity '" << name << "' which do not > exist"; > + return false; > + } > + > + devnode = it->second; > + return true; > +} > + > +} /* namespace libcamera */ > diff --git a/src/libcamera/include/deviceenumerator.h > b/src/libcamera/include/deviceenumerator.h new file mode 100644 > index 0000000000000000..0136ed6ea23bf65e > --- /dev/null > +++ b/src/libcamera/include/deviceenumerator.h > @@ -0,0 +1,44 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2018, Google Inc. > + * > + * deviceenumerator.h - API to enumerate and find media devices > + */ > +#ifndef __LIBCAMERA_DEVICEENUMERATE_H__ This doesn't match the file name. > +#define __LIBCAMERA_DEVICEENUMERATE_H__ > + > +#include <map> > +#include <string> > +#include <vector> > + > +#include <linux/media.h> > + > +namespace libcamera { > + > +class DeviceInfo > +{ > +public: > + DeviceInfo(const std::string &devnode, const struct media_device_info > &info, > + const std::map<std::string, std::string> &entities); > + > + int acquire(); > + void release(); > + bool busy() const; > + > + const std::string &devnode() const; > + const struct media_device_info &info() const; > + std::vector<std::string> entities() const; > + > + bool lookup(const std::string &name, std::string &devnode) const; > + > +private: > + bool acquired_; > + > + std::string devnode_; > + struct media_device_info info_; > + std::map<std::string, std::string> entities_; > +}; > + > +} /* namespace libcamera */ > + > +#endif /* __LIBCAMERA_DEVICEENUMERATE_H__ */ [snip]
Hi Laurent, Thanks for your comments. On 2018-12-28 19:20:59 +0200, Laurent Pinchart wrote: > Hi Niklas, > > Thank you for the patch. > > On Sunday, 23 December 2018 01:00:32 EET Niklas Söderlund wrote: > > Provide a DeviceInfo class which holds all information from the initial > > enumeration of a media device. Not all information available at a media > > device is stored, only the information needed for a pipeline handler to > > find a specific device. > > Do I understand correctly that this will be replaced with MediaDevice ? If so > I only have a small comment below, and for the rest, Yes the plan is to replace this with MediaDevice. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > > src/libcamera/deviceenumerator.cpp | 78 ++++++++++++++++++++++++ > > src/libcamera/include/deviceenumerator.h | 44 +++++++++++++ > > src/libcamera/meson.build | 2 + > > 3 files changed, 124 insertions(+) > > create mode 100644 src/libcamera/deviceenumerator.cpp > > create mode 100644 src/libcamera/include/deviceenumerator.h > > > > diff --git a/src/libcamera/deviceenumerator.cpp > > b/src/libcamera/deviceenumerator.cpp new file mode 100644 > > index 0000000000000000..7c44a65b45472ef3 > > --- /dev/null > > +++ b/src/libcamera/deviceenumerator.cpp > > @@ -0,0 +1,78 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2018, Google Inc. > > + * > > + * deviceenumerator.cpp - Enumeration and matching > > + */ > > + > > +#include "deviceenumerator.h" > > +#include "log.h" > > + > > +namespace libcamera { > > + > > +/* ------------------------------------------------------------------------ > > + * DeviceInfo > > + */ > > + > > +DeviceInfo::DeviceInfo(const std::string &devnode, const struct > > media_device_info &info, > > + const std::map<std::string, std::string> > > &entities) > > + : acquired_(false), devnode_(devnode), info_(info), entities_(entities) > > +{ > > + for (const auto &entity : entities_) > > + LOG(Info) << "Device: " << devnode_ << " Entity: '" << entity.first << > "' > > -> " << entity.second; > > +} > > + > > +int DeviceInfo::acquire() > > +{ > > + if (acquired_) > > + return -EBUSY; > > + > > + acquired_ = true; > > + > > + return 0; > > +} > > + > > +void DeviceInfo::release() > > +{ > > + acquired_ = false; > > +} > > + > > +bool DeviceInfo::busy() const > > +{ > > + return acquired_; > > +} > > + > > +const std::string &DeviceInfo::devnode() const > > +{ > > + return devnode_; > > +} > > + > > +const struct media_device_info &DeviceInfo::info() const > > +{ > > + return info_; > > +} > > + > > +std::vector<std::string> DeviceInfo::entities() const > > +{ > > + std::vector<std::string> entities; > > + > > + for (const auto &entity : entities_) > > + entities.push_back(entity.first); > > + > > + return entities; > > +} > > + > > +bool DeviceInfo::lookup(const std::string &name, std::string &devnode) > > const > > +{ > > + auto it = entities_.find(name); > > + > > + if (it == entities_.end()) { > > + LOG(Error) << "Trying to lookup entity '" << name << "' which do not > > exist"; > > + return false; > > + } > > + > > + devnode = it->second; > > + return true; > > +} > > + > > +} /* namespace libcamera */ > > diff --git a/src/libcamera/include/deviceenumerator.h > > b/src/libcamera/include/deviceenumerator.h new file mode 100644 > > index 0000000000000000..0136ed6ea23bf65e > > --- /dev/null > > +++ b/src/libcamera/include/deviceenumerator.h > > @@ -0,0 +1,44 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2018, Google Inc. > > + * > > + * deviceenumerator.h - API to enumerate and find media devices > > + */ > > +#ifndef __LIBCAMERA_DEVICEENUMERATE_H__ > > This doesn't match the file name. Thanks for noticing, fixed. > > > +#define __LIBCAMERA_DEVICEENUMERATE_H__ > > + > > +#include <map> > > +#include <string> > > +#include <vector> > > + > > +#include <linux/media.h> > > + > > +namespace libcamera { > > + > > +class DeviceInfo > > +{ > > +public: > > + DeviceInfo(const std::string &devnode, const struct media_device_info > > &info, > > + const std::map<std::string, std::string> &entities); > > + > > + int acquire(); > > + void release(); > > + bool busy() const; > > + > > + const std::string &devnode() const; > > + const struct media_device_info &info() const; > > + std::vector<std::string> entities() const; > > + > > + bool lookup(const std::string &name, std::string &devnode) const; > > + > > +private: > > + bool acquired_; > > + > > + std::string devnode_; > > + struct media_device_info info_; > > + std::map<std::string, std::string> entities_; > > +}; > > + > > +} /* namespace libcamera */ > > + > > +#endif /* __LIBCAMERA_DEVICEENUMERATE_H__ */ > > [snip] > > -- > Regards, > > Laurent Pinchart > > >
Hi Laurent, On Fri, Dec 28, 2018 at 07:15:20PM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > On Friday, 28 December 2018 10:21:48 EET Jacopo Mondi wrote: > > On Fri, Dec 28, 2018 at 04:06:57AM +0100, Niklas S??derlund wrote: > > > On 2018-12-24 11:42:16 +0100, Jacopo Mondi wrote: > > > > On Sun, Dec 23, 2018 at 12:00:32AM +0100, Niklas S??derlund wrote: > > > > > Provide a DeviceInfo class which holds all information from the > > > > > initial enumeration of a media device. Not all information available > > > > > at a media device is stored, only the information needed for a > > > > > pipeline handler to find a specific device. > > > > > > > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > > > --- > > > > > > > > > > src/libcamera/deviceenumerator.cpp | 78 +++++++++++++++++++++++ > > > > > src/libcamera/include/deviceenumerator.h | 44 +++++++++++++ > > > > > src/libcamera/meson.build | 2 + > > > > > 3 files changed, 124 insertions(+) > > > > > create mode 100644 src/libcamera/deviceenumerator.cpp > > > > > create mode 100644 src/libcamera/include/deviceenumerator.h > > > > > > > > > > diff --git a/src/libcamera/deviceenumerator.cpp > > > > > b/src/libcamera/deviceenumerator.cpp new file mode 100644 > > > > > index 0000000000000000..7c44a65b45472ef3 > > > > > --- /dev/null > > > > > +++ b/src/libcamera/deviceenumerator.cpp > > > > > @@ -0,0 +1,78 @@ > > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > > +/* > > > > > + * Copyright (C) 2018, Google Inc. > > > > > + * > > > > > + * deviceenumerator.cpp - Enumeration and matching > > > > > + */ > > > > > + > > > > > +#include "deviceenumerator.h" > > > > > +#include "log.h" > > > > > + > > > > > +namespace libcamera { > > > > > + > > > > > +/* > > > > > --------------------------------------------------------------------- > > > > > -------- + * DeviceInfo > > > > > + */ > > > > > + > > > > > +DeviceInfo::DeviceInfo(const std::string &devnode, const struct > > > > > media_device_info &info, + const std::map<std::string, > > > > > std::string> &entities) > > > > > + : acquired_(false), devnode_(devnode), info_(info), > > > > > entities_(entities) > > > > > +{ > > > > > + for (const auto &entity : entities_) > > > > > + LOG(Info) << "Device: " << devnode_ << " Entity: '" << > entity.first > > > > > << "' -> " << entity.second; +} > > > > > + > > > > > +int DeviceInfo::acquire() > > > > > +{ > > > > > + if (acquired_) > > > > > + return -EBUSY; > > > > > + > > > > > + acquired_ = true; > > > > > + > > > > > + return 0; > > > > > +} > > > > > + > > > > > +void DeviceInfo::release() > > > > > +{ > > > > > + acquired_ = false; > > > > > +} > > > > > + > > > > > +bool DeviceInfo::busy() const > > > > > +{ > > > > > + return acquired_; > > > > > +} > > > > > + > > > > > +const std::string &DeviceInfo::devnode() const > > > > > +{ > > > > > + return devnode_; > > > > > +} > > > > > + > > > > > +const struct media_device_info &DeviceInfo::info() const > > > > > +{ > > > > > + return info_; > > > > > +} > > > > > + > > > > > +std::vector<std::string> DeviceInfo::entities() const > > > > > +{ > > > > > + std::vector<std::string> entities; > > > > > + > > > > > + for (const auto &entity : entities_) > > > > > + entities.push_back(entity.first); > > > > > + > > > > > + return entities; > > > > > > > > Are you returning (by copy) a variable with local scope? > > > > Why coldn't you just return (as const reference maybe) entities_ ? > > > > > > This is by design. The idea is that a new copy of the entities in a > > > vector instead of the class internal object of the model of entities as > > > the use-case here is to do matching. Why would it be beneficial to > > > return a internal data structure? > > > > It would avoid moving around a vector by copy. You only use > > the returned vector to compare entities names, so you could just > > simply return a const reference to your internal data structure. > > > > I would actually ask why would it be beneficial to copy all entities > > names in a new vector and copy it back, if that's not going to be > > modified anyhow. Furthermore, you could have used operator= to copy > > the internal one: > > http://www.cplusplus.com/reference/vector/vector/operator=/ > > What do you mean by "copying it back" ? Have you overlooked the fact that > entities_ is a map, not a vector ? > Indeed I did. Sorry for the noise. Thanks j > > > > > +} > > > > > + > > > > > +bool DeviceInfo::lookup(const std::string &name, std::string > > > > > &devnode) const +{ > > > > > + auto it = entities_.find(name); > > > > > + > > > > > + if (it == entities_.end()) { > > > > > + LOG(Error) << "Trying to lookup entity '" << name << "' which do > > > > > not exist"; > > > > > > > > > nit: "does not exist" > > > > > > My comprehension of the Queens English is lacking so I trust you on > > > > Don't! > > > > > this. For my education is not 'does' used when there is more then one > > > while 'do' is used when there is exactly one? > > > > You might be right, let's wait for some more educated (or native) > > speaker to school us. > > I'm certainly not native, and wouldn't claim to be more educated, but I think > the subject of the verb refers to "entity", which is a third person singular, > so "does" is the right form. > > > > > > + return false; > > > > > + } > > > > > + > > > > > + devnode = it->second; > > > > > + return true; > > > > > +} > > > > > + > > > > > +} /* namespace libcamera */ > > > > > diff --git a/src/libcamera/include/deviceenumerator.h > > > > > b/src/libcamera/include/deviceenumerator.h new file mode 100644 > > > > > index 0000000000000000..0136ed6ea23bf65e > > > > > --- /dev/null > > > > > +++ b/src/libcamera/include/deviceenumerator.h > > > > > @@ -0,0 +1,44 @@ > > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > > +/* > > > > > + * Copyright (C) 2018, Google Inc. > > > > > + * > > > > > + * deviceenumerator.h - API to enumerate and find media devices > > > > > + */ > > > > > +#ifndef __LIBCAMERA_DEVICEENUMERATE_H__ > > > > > +#define __LIBCAMERA_DEVICEENUMERATE_H__ > > > > > + > > > > > +#include <map> > > > > > +#include <string> > > > > > +#include <vector> > > > > > + > > > > > +#include <linux/media.h> > > > > > + > > > > > +namespace libcamera { > > > > > + > > > > > +class DeviceInfo > > > > > +{ > > > > > +public: > > > > > + DeviceInfo(const std::string &devnode, const struct > > > > > media_device_info &info, + const std::map<std::string, > > > > > std::string> &entities); > > > > > + > > > > > + int acquire(); > > > > > + void release(); > > > > > + bool busy() const; > > > > > > > > Trivial methods might be inlined here. > > > > > > There are other methods who can't be inlined for this class. Why spread > > > the implementation around? > > > > I'm in favour of inlining single line and trivial methods/constructors as > > much as we could. It reduce the size of the cpp file, and makes clear > > from reading the header the method is trivial. I'm not much concerned about > > spreading per se, as we all navigate tags don't we? > > Methods should only be inlined when the code size is roughly equivalent to a > function call. Trivial getters are good candidates. Constructors may be, but > only if they don't have a large list of member initializers. For destructors, > even if the function is empty, it might call other destructors for members, > and the destructor of the base class. Inlining destructors is thus usually a > bad idea. Inlinline in C++ uses a different syntax than in C, but the result > is similar, the code is inlined, and may thus increase the binary size. Please > see the Google C++ style guide section about this. > > > (let alone the discussion about the fact inlining in class declaration > > is a hint for the compiler to inline the method in the generated code. > > I think the compiler is smart enough to find that out by itself > > anyhow though) > > > > > > > + > > > > > + const std::string &devnode() const; > > > > > + const struct media_device_info &info() const; > > > > > + std::vector<std::string> entities() const; > > > > > + > > > > > + bool lookup(const std::string &name, std::string &devnode) const; > > > > > + > > > > > +private: > > > > > + bool acquired_; > > > > > + > > > > > + std::string devnode_; > > > > > + struct media_device_info info_; > > > > > + std::map<std::string, std::string> entities_; > > > > > +}; > > > > > + > > > > > +} /* namespace libcamera */ > > > > > + > > > > > +#endif /* __LIBCAMERA_DEVICEENUMERATE_H__ */ > > [snip] > > -- > Regards, > > Laurent Pinchart > > >
diff --git a/src/libcamera/deviceenumerator.cpp b/src/libcamera/deviceenumerator.cpp new file mode 100644 index 0000000000000000..7c44a65b45472ef3 --- /dev/null +++ b/src/libcamera/deviceenumerator.cpp @@ -0,0 +1,78 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2018, Google Inc. + * + * deviceenumerator.cpp - Enumeration and matching + */ + +#include "deviceenumerator.h" +#include "log.h" + +namespace libcamera { + +/* ----------------------------------------------------------------------------- + * DeviceInfo + */ + +DeviceInfo::DeviceInfo(const std::string &devnode, const struct media_device_info &info, + const std::map<std::string, std::string> &entities) + : acquired_(false), devnode_(devnode), info_(info), entities_(entities) +{ + for (const auto &entity : entities_) + LOG(Info) << "Device: " << devnode_ << " Entity: '" << entity.first << "' -> " << entity.second; +} + +int DeviceInfo::acquire() +{ + if (acquired_) + return -EBUSY; + + acquired_ = true; + + return 0; +} + +void DeviceInfo::release() +{ + acquired_ = false; +} + +bool DeviceInfo::busy() const +{ + return acquired_; +} + +const std::string &DeviceInfo::devnode() const +{ + return devnode_; +} + +const struct media_device_info &DeviceInfo::info() const +{ + return info_; +} + +std::vector<std::string> DeviceInfo::entities() const +{ + std::vector<std::string> entities; + + for (const auto &entity : entities_) + entities.push_back(entity.first); + + return entities; +} + +bool DeviceInfo::lookup(const std::string &name, std::string &devnode) const +{ + auto it = entities_.find(name); + + if (it == entities_.end()) { + LOG(Error) << "Trying to lookup entity '" << name << "' which do not exist"; + return false; + } + + devnode = it->second; + return true; +} + +} /* namespace libcamera */ diff --git a/src/libcamera/include/deviceenumerator.h b/src/libcamera/include/deviceenumerator.h new file mode 100644 index 0000000000000000..0136ed6ea23bf65e --- /dev/null +++ b/src/libcamera/include/deviceenumerator.h @@ -0,0 +1,44 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2018, Google Inc. + * + * deviceenumerator.h - API to enumerate and find media devices + */ +#ifndef __LIBCAMERA_DEVICEENUMERATE_H__ +#define __LIBCAMERA_DEVICEENUMERATE_H__ + +#include <map> +#include <string> +#include <vector> + +#include <linux/media.h> + +namespace libcamera { + +class DeviceInfo +{ +public: + DeviceInfo(const std::string &devnode, const struct media_device_info &info, + const std::map<std::string, std::string> &entities); + + int acquire(); + void release(); + bool busy() const; + + const std::string &devnode() const; + const struct media_device_info &info() const; + std::vector<std::string> entities() const; + + bool lookup(const std::string &name, std::string &devnode) const; + +private: + bool acquired_; + + std::string devnode_; + struct media_device_info info_; + std::map<std::string, std::string> entities_; +}; + +} /* namespace libcamera */ + +#endif /* __LIBCAMERA_DEVICEENUMERATE_H__ */ diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index 52b556a8ed4050cb..17cdf06dd2bedfb3 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -1,10 +1,12 @@ libcamera_sources = files([ 'camera.cpp', + 'deviceenumerator.cpp', 'log.cpp', 'main.cpp', ]) libcamera_headers = files([ + 'include/deviceenumerator.h', 'include/log.h', 'include/utils.h', ])
Provide a DeviceInfo class which holds all information from the initial enumeration of a media device. Not all information available at a media device is stored, only the information needed for a pipeline handler to find a specific device. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/libcamera/deviceenumerator.cpp | 78 ++++++++++++++++++++++++ src/libcamera/include/deviceenumerator.h | 44 +++++++++++++ src/libcamera/meson.build | 2 + 3 files changed, 124 insertions(+) create mode 100644 src/libcamera/deviceenumerator.cpp create mode 100644 src/libcamera/include/deviceenumerator.h