[libcamera-devel] [PATCH 03/12] libcamera: deviceenumerator: add DeviceInfo class

[Niklas Söderlund]

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 at ideasonboard.com>
> 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at 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
> 
> 
> 

-- 
Regards,
Niklas Söderlund