[libcamera-devel] [PATCH v3 11/13] libcamera: camera_sensor: Add a new class to model a camera sensor
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Apr 18 17:27:02 CEST 2019
Hi Jacopo,
On Thu, Apr 18, 2019 at 05:21:56PM +0200, Jacopo Mondi wrote:
> On Thu, Apr 18, 2019 at 06:07:15PM +0300, Laurent Pinchart wrote:
> > On Thu, Apr 18, 2019 at 04:44:42PM +0200, Jacopo Mondi wrote:
> >> On Thu, Apr 18, 2019 at 05:14:35PM +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>
> >>> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> >>> ---
> >>> Changes since v1:
> >>>
> >>> - Add sensor entity function check
> >>> - Sort the media bus codes
> >>> - Remove the unneeded std::endl for LOG()
> >>> - Improve documentation
> >>> - Inherit from Loggable with the protected qualifier
> >>> ---
> >>> src/libcamera/camera_sensor.cpp | 258 ++++++++++++++++++++++++++
> >>> src/libcamera/include/camera_sensor.h | 56 ++++++
> >>> src/libcamera/meson.build | 2 +
> >>> 3 files changed, 316 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..ca4dd0c92efa
> >>> --- /dev/null
> >>> +++ b/src/libcamera/camera_sensor.cpp
> >>> @@ -0,0 +1,258 @@
> >>> +/* 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 <iomanip>
> >>> +#include <limits.h>
> >>> +#include <math.h>
> >>
> >> Don't we separate C++ and C includes?
> >
> > I've double-checked and we only do so in media_device.cpp and
> > media_object.cpp, nowhere else.
>
> Ah, so it was just me!
> It comes from here though:
> https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes
>
> but let's fix those for consistency then
I don't care too much about mixing the C and C++ headers or keeping them
separate. If you prefer to separate them, feel free to send a patch
(note that there's no blank line between C and C++ headers in those C++
guidelines). I however think it makes sense to include the .h
corresponding to the .cpp first, for the rationale explained in the
guidelines. When it comes to headers from other libraries and from the
project (items 6 and 7) I would prefer a blank line between them.
> >>> +
> >>> +#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 based on V4L2 subdevices
> >>> + *
> >>> + * 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 backing 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 method performs 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";
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + if (entity_->function() != MEDIA_ENT_F_CAM_SENSOR) {
> >>> + LOG(CameraSensor, Error)
> >>> + << "Invalid sensor function 0x"
> >>> + << std::hex << std::setfill('0') << std::setw(8)
> >>> + << entity_->function();
> >>> + 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";
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + std::transform(formats.begin(), formats.end(),
> >>> + std::back_inserter(mbusCodes_),
> >>> + [](decltype(*formats.begin()) f) { return f.first; });
> >>> +
> >>> + /*
> >>> + * Extract the supported sizes from the first format as we only support
> >>> + * sensors that offer the same frame sizes for all media bus codes.
> >>> + * Verify this assumption and reject the sensor if it isn't true.
> >>> + */
> >>> + const std::vector<SizeRange> &sizes = formats.begin()->second;
> >>> + std::transform(sizes.begin(), sizes.end(), std::back_inserter(sizes_),
> >>> + [](const SizeRange &range) { return range.max; });
> >>> +
> >>> + for (auto it = ++formats.begin(); it != formats.end(); ++it) {
> >>> + if (it->second != sizes) {
> >>> + LOG(CameraSensor, Error)
> >>> + << "Frame sizes differ between media bus codes";
> >>> + return -EINVAL;
> >>> + }
> >>> + }
> >>> +
> >>> + /* Sort the media bus codes and sizes. */
> >>> + std::sort(mbusCodes_.begin(), mbusCodes_.end());
> >>> + 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 sorted in increasing order
> >>> + */
> >>> +
> >>> +/**
> >>> + * \fn CameraSensor::sizes()
> >>> + * \brief Retrieve the frame sizes supported by the camera sensor
> >>> + * \return The supported frame sizes sorted in increasing order
> >>> + */
> >>> +
> >>> +/**
> >>> + * \brief Retrieve the camera sensor resolution
> >>> + * \return The camera sensor resolution in pixels
> >>> + */
> >>> +const Size &CameraSensor::resolution() const
> >>> +{
> >>> + /*
> >>> + * The sizes_ vector is sorted in ascending order, the resolution is
> >>> + * thus the last element of the vector.
> >>> + */
> >>> + return sizes_.back();
> >>> +}
> >>> +
> >>> +/**
> >>> + * \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";
> >>
> >> Debug or Error?
> >>
> >>> + 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";
> >>
> >> Same. Were these intentional?
> >
> > They are, to let users of the CameraSensor class probe for supported
> > formats without generating errors.
> >
> >> Minors apart:
> >> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> >>
> >>> + 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..7f2f906be8df
> >>> --- /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"
> >>> +
> >>> +namespace libcamera {
> >>> +
> >>> +class MediaEntity;
> >>> +class V4L2Subdevice;
> >>> +class V4L2SubdeviceFormat;
> >>> +
> >>> +class CameraSensor : protected 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;
> >>> +
> >>> + 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
More information about the libcamera-devel
mailing list