[libcamera-devel] [PATCH 09/11] libcamera: camera_sensor: Add a new class to model a camera sensor

Niklas Söderlund niklas.soderlund at ragnatech.se
Mon Apr 15 23:39:29 CEST 2019


Hi Laurent,

Thanks for your work.

On 2019-04-15 19:56:58 +0300, Laurent Pinchart wrote:
> The CameraSensor class abstracts camera sensors and provides helper
> functions to ease interactions with them. It is currently limited to
> sensors that expose a single subdev, and offer the same frame sizes for
> all media bus codes, but will be extended to support more complex
> sensors as the needs arise.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/libcamera/camera_sensor.cpp       | 245 ++++++++++++++++++++++++++
>  src/libcamera/include/camera_sensor.h |  56 ++++++
>  src/libcamera/meson.build             |   2 +
>  3 files changed, 303 insertions(+)
>  create mode 100644 src/libcamera/camera_sensor.cpp
>  create mode 100644 src/libcamera/include/camera_sensor.h
> 
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> new file mode 100644
> index 000000000000..aca9e77fd986
> --- /dev/null
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -0,0 +1,245 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * camera_sensor.cpp - A camera sensor
> + */
> +
> +#include <algorithm>
> +#include <float.h>
> +#include <limits.h>
> +#include <math.h>
> +
> +#include "camera_sensor.h"
> +#include "formats.h"
> +#include "v4l2_subdevice.h"
> +
> +/**
> + * \file camera_sensor.h
> + * \brief A camera sensor
> + */
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(CameraSensor);
> +
> +/**
> + * \class CameraSensor
> + * \brief A camera sensor
> + *
> + * The CameraSensor class eases handling of sensors for pipeline handlers by
> + * hiding the details of the V4L2 subdevice kernel API and caching sensor
> + * information.
> + *
> + * The implementation is currently limited to sensors that expose a single V4L2
> + * subdevice with a single pad, and support the same frame sizes for all
> + * supported media bus codes. It will be extended to support more complex
> + * devices as the needs arise.
> + */
> +
> +/**
> + * \brief Construct a CameraSensor
> + * \param[in] entity The media entity for the camera sensor
> + *
> + * Once constructed the instance must be initialized with init().
> + */
> +CameraSensor::CameraSensor(const MediaEntity *entity)
> +	: entity_(entity)
> +{
> +	subdev_ = new V4L2Subdevice(entity);
> +}
> +
> +/**
> + * \brief Destroy a CameraSensor
> + */
> +CameraSensor::~CameraSensor()
> +{
> +	delete subdev_;
> +}
> +
> +/**
> + * \brief Initialize the camera sensor instance
> + *
> + * This methods perform the initialisation steps of the CameraSensor that may
> + * fail. It shall be called once and only once after constructing the instance.
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int CameraSensor::init()
> +{
> +	int ret;
> +
> +	if (entity_->pads().size() != 1) {
> +		LOG(CameraSensor, Error)
> +			<< "Sensors with more than one pad are not supported"
> +			<< std::endl;
> +		return -EINVAL;
> +	}
> +
> +	ret = subdev_->open();
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Enumerate and cache media bus codes and sizes. */
> +	const FormatEnum formats = subdev_->formats(0);
> +	if (formats.empty()) {
> +		LOG(CameraSensor, Error)
> +			<< "No image format found" << std::endl;
> +		return -EINVAL;
> +	}
> +
> +	std::transform(formats.begin(), formats.end(),
> +		       std::back_inserter(mbusCodes_),
> +		       [](decltype(*formats.begin()) f) { return f.first; });
> +
> +	const std::vector<SizeRange> &sizes = formats.begin()->second;

I might add a comment somewhere around here mentioning the limitation 
and why only sizes from formats.begin() are considered. I had to reread 
the commit message to figure it out. Not a big issue as it's documented 
elsewhere so it won't be forgotten. Maybe me pointing it out here for 
other reviewers benefit is enough.

> +	std::transform(sizes.begin(), sizes.end(), std::back_inserter(sizes_),
> +		       [](const SizeRange &range) { return range.max; });
> +
> +	/*
> +	 * Verify the assumption that all available media bus codes support the
> +	 * same frame sizes.
> +	 */
> +	for (auto it = ++formats.begin(); it != formats.end(); ++it) {
> +		if (it->second != sizes) {
> +			LOG(CameraSensor, Error)
> +				<< "Frame sizes differ between media bus codes"
> +				<< std::endl;
> +			return -EINVAL;
> +		}
> +	}
> +
> +	/* Sort the sizes. */
> +	std::sort(sizes_.begin(), sizes_.end());
> +
> +	return 0;
> +}
> +
> +/**
> + * \fn CameraSensor::entity()
> + * \brief Retrieve the sensor media entity
> + * \return The sensor media entity
> + */
> +
> +/**
> + * \fn CameraSensor::mbusCodes()
> + * \brief Retrieve the media bus codes supported by the camera sensor
> + * \return The supported media bus codes
> + */
> +
> +/**
> + * \fn CameraSensor::sizes()
> + * \brief Retrieve the frame sizes supported by the camera sensor
> + * \return The supported frame sizes
> + */
> +
> +/**
> + * \fn CameraSensor::resolution()
> + * \brief Retrieve the camera sensor resolution
> + * \return The camera sensor resolution in pixels
> + */

Maybe move the function implementation here adding a comment that the 
sizes_ vector is sorted so that the resolution will always be the 
largest one supported.

With or without the above nit-pick comments addressed,

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> +
> +/**
> + * \brief Retrieve the best sensor format for a desired output
> + * \param[in] mbusCodes The list of acceptable media bus codes
> + * \param[in] size The desired size
> + *
> + * Media bus codes are selected from \a mbusCodes, which lists all acceptable
> + * codes in decreasing order of preference. This method selects the first code
> + * from the list that is supported by the sensor. If none of the desired codes
> + * is supported, it returns an error.
> + *
> + * \a size indicates the desired size at the output of the sensor. This method
> + * selects the best size supported by the sensor according to the following
> + * criteria.
> + *
> + * - The desired \a size shall fit in the sensor output size to avoid the need
> + *   to up-scale.
> + * - The sensor output size shall match the desired aspect ratio to avoid the
> + *   need to crop the field of view.
> + * - The sensor output size shall be as small as possible to lower the required
> + *   bandwidth.
> + *
> + * The use of this method is optional, as the above criteria may not match the
> + * needs of all pipeline handlers. Pipeline handlers may implement custom
> + * sensor format selection when needed.
> + *
> + * The returned sensor output format is guaranteed to be acceptable by the
> + * setFormat() method without any modification.
> + *
> + * \return The best sensor output format matching the desired media bus codes
> + * and size on success, or an empty format otherwise.
> + */
> +V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbusCodes,
> +					    const Size &size) const
> +{
> +	V4L2SubdeviceFormat format{};
> +
> +	for (unsigned int code : mbusCodes_) {
> +		if (std::any_of(mbusCodes.begin(), mbusCodes.end(),
> +				[code](unsigned int c) { return c == code; })) {
> +			format.mbus_code = code;
> +			break;
> +		}
> +	}
> +
> +	if (!format.mbus_code) {
> +		LOG(CameraSensor, Debug)
> +			<< "No supported format found" << std::endl;
> +		return format;
> +	}
> +
> +	unsigned int desiredArea = size.width * size.height;
> +	unsigned int bestArea = UINT_MAX;
> +	float desiredRatio = static_cast<float>(size.width) / size.height;
> +	float bestRatio = FLT_MAX;
> +	const Size *bestSize = nullptr;
> +
> +	for (const Size &sz : sizes_) {
> +		if (sz.width < size.width || sz.height < size.height)
> +			continue;
> +
> +		float ratio = static_cast<float>(sz.width) / sz.height;
> +		float ratioDiff = fabsf(ratio - desiredRatio);
> +		unsigned int area = sz.width * sz.height;
> +		unsigned int areaDiff = area - desiredArea;
> +
> +		if (ratioDiff > bestRatio)
> +			continue;
> +
> +		if (ratioDiff < bestRatio || areaDiff < bestArea) {
> +			bestRatio = ratioDiff;
> +			bestArea = areaDiff;
> +			bestSize = &sz;
> +		}
> +	}
> +
> +	if (!bestSize) {
> +		LOG(CameraSensor, Debug)
> +			<< "No supported size found" << std::endl;
> +		return format;
> +	}
> +
> +	format.width = bestSize->width;
> +	format.height = bestSize->height;
> +
> +	return format;
> +}
> +
> +/**
> + * \brief Set the sensor output format
> + * \param[in] format The desired sensor output format
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int CameraSensor::setFormat(V4L2SubdeviceFormat *format)
> +{
> +	return subdev_->setFormat(0, format);
> +}
> +
> +std::string CameraSensor::logPrefix() const
> +{
> +	return "'" + subdev_->entity()->name() + "'";
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h
> new file mode 100644
> index 000000000000..7d92af05248c
> --- /dev/null
> +++ b/src/libcamera/include/camera_sensor.h
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * camera_sensor.h - A camera sensor
> + */
> +#ifndef __LIBCAMERA_CAMERA_SENSOR_H__
> +#define __LIBCAMERA_CAMERA_SENSOR_H__
> +
> +#include <string>
> +#include <vector>
> +
> +#include <libcamera/geometry.h>
> +
> +#include "log.h"
> +#include "v4l2_subdevice.h"
> +
> +namespace libcamera {
> +
> +class MediaEntity;
> +class V4L2SubdeviceFormat;
> +
> +class CameraSensor : public Loggable
> +{
> +public:
> +	explicit CameraSensor(const MediaEntity *entity);
> +	~CameraSensor();
> +
> +	CameraSensor(const CameraSensor &) = delete;
> +	CameraSensor &operator=(const CameraSensor &) = delete;
> +
> +	int init();
> +
> +	const MediaEntity *entity() const { return entity_; }
> +	const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; }
> +	const std::vector<Size> &sizes() const { return sizes_; }
> +	const Size &resolution() const { return sizes_.back(); }
> +
> +	V4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes,
> +				      const Size &size) const;
> +	int setFormat(V4L2SubdeviceFormat *format);
> +
> +protected:
> +	std::string logPrefix() const;
> +
> +private:
> +	const MediaEntity *entity_;
> +	V4L2Subdevice *subdev_;
> +
> +	std::vector<unsigned int> mbusCodes_;
> +	std::vector<Size> sizes_;
> +};
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_CAMERA_SENSOR_H__ */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index cd36ac307518..cf4edec05755 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -2,6 +2,7 @@ libcamera_sources = files([
>      'buffer.cpp',
>      'camera.cpp',
>      'camera_manager.cpp',
> +    'camera_sensor.cpp',
>      'device_enumerator.cpp',
>      'event_dispatcher.cpp',
>      'event_dispatcher_poll.cpp',
> @@ -23,6 +24,7 @@ libcamera_sources = files([
>  ])
>  
>  libcamera_headers = files([
> +    'include/camera_sensor.h',
>      'include/device_enumerator.h',
>      'include/event_dispatcher_poll.h',
>      'include/formats.h',
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund


More information about the libcamera-devel mailing list