[libcamera-devel] [PATCH 09/11] libcamera: camera_sensor: Add a new class to model a camera sensor
Jacopo Mondi
jacopo at jmondi.org
Tue Apr 16 18:25:48 CEST 2019
Hi Laurent,
On Mon, Apr 15, 2019 at 07:56:58PM +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
a very brief \brief this one
> + *
> + * The CameraSensor class eases handling of sensors for pipeline handlers by
> + * hiding the details of the V4L2 subdevice kernel API and caching sensor
> + * information.
"information such as sizes and supported image formats" ?
> + *
> + * 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
s/for/backing ?
> + *
> + * 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
s/methods/method
s/perform/performs
Or have I missed why the plural here?
> + * 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;
Here and in the rest of the series, LOG() does not need an std::endl
unless you want an additional empty line after the message.
> + return -EINVAL;
> + }
> +
> + ret = subdev_->open();
Does this need to be closed before deleting it?
> + 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;
> + std::transform(sizes.begin(), sizes.end(), std::back_inserter(sizes_),
> + [](const SizeRange &range) { return range.max; });
neat! I hope we could keep something similar when supporting
per-format sizes...
> +
> + /*
> + * 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
I was sure we enforced a blank line here, but some classes do and some
other does not :(
> + * \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
> + */
mbusCode is V4L2-specific, isn't it? I can't suggest a better name
though, "formats" is confusing, just "codes" is not nice... Up to
you...
> +
> +/**
> + * \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
> + */
This is the maximum resolution the sensor supports, right?
> +
> +/**
> + * \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;
will we need a Size::area() ?
> + 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)
Couldn't you just compare "sz < size" ?
* - A size with smaller width and smaller height is smaller.
* - A size with smaller area is smaller.
* - A size with smaller width is smaller.
doesn't it apply here?
> + 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) {
This is puzzling, but most probably what you want. If ratio is <= than
the best one, then if ratio or area is < than the current best one
then select the format. So there is no priority between ratio and
area differences, right?
> + 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;
As you include "v4l2_subdevice.h" you don't need this
> +
> +class CameraSensor : public Loggable
nit: other classes that inherits Loggable inherits its protected
fields only. Is this desirable?
Thanks
j
> +{
> +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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190416/2ba5d3a3/attachment.sig>
More information about the libcamera-devel
mailing list