[PATCH/RFC 16/32] libcamera: camera_sensor: Reorder functions
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Mon Mar 4 18:37:15 CET 2024
Hi Laurent
On Fri, Mar 01, 2024 at 11:21:05PM +0200, Laurent Pinchart wrote:
> The CameraSensor class has grown a lot since its creation, with many
> functions added for different types of purposes. They are not grouped by
> categories in the class definition, generating confusion when reading
> the header file. Improve readability by sorting functions by category:
>
> - Getters for static data (model, entity, focus lens, ...)
> - Format and sensor configuration accessors
> - Properties and controls (including test pattern mode)
>
> Update the .cpp file accordingly.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> include/libcamera/internal/camera_sensor.h | 29 +-
> src/libcamera/sensor/camera_sensor.cpp | 328 ++++++++++-----------
> 2 files changed, 179 insertions(+), 178 deletions(-)
>
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index b2f077b9cc75..750d6d729cac 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -46,15 +46,15 @@ public:
>
> const std::string &model() const { return model_; }
> const std::string &id() const { return id_; }
> +
> const MediaEntity *entity() const { return entity_; }
> + V4L2Subdevice *device() { return subdev_.get(); }
> +
> + CameraLens *focusLens() { return focusLens_.get(); }
> +
Should you move
const ControlList &properties() const { return properties_; }
const ControlInfoMap &controls() const;
here ?
(I understand if you want to keep them grouped though)
This can be fast-tracked too!
Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; }
> std::vector<Size> sizes(unsigned int mbusCode) const;
> Size resolution() const;
> - const std::vector<controls::draft::TestPatternModeEnum> &testPatternModes() const
> - {
> - return testPatternModes_;
> - }
> - int setTestPatternMode(controls::draft::TestPatternModeEnum mode);
>
> V4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes,
> const Size &size) const;
> @@ -66,18 +66,19 @@ public:
> Transform transform = Transform::Identity,
> V4L2SubdeviceFormat *sensorFormat = nullptr);
>
> + const ControlList &properties() const { return properties_; }
> + int sensorInfo(IPACameraSensorInfo *info) const;
> + Transform computeTransform(Orientation *orientation) const;
> +
> const ControlInfoMap &controls() const;
> ControlList getControls(const std::vector<uint32_t> &ids);
> int setControls(ControlList *ctrls);
>
> - V4L2Subdevice *device() { return subdev_.get(); }
> -
> - const ControlList &properties() const { return properties_; }
> - int sensorInfo(IPACameraSensorInfo *info) const;
> -
> - CameraLens *focusLens() { return focusLens_.get(); }
> -
> - Transform computeTransform(Orientation *orientation) const;
> + const std::vector<controls::draft::TestPatternModeEnum> &testPatternModes() const
> + {
> + return testPatternModes_;
> + }
> + int setTestPatternMode(controls::draft::TestPatternModeEnum mode);
>
> protected:
> std::string logPrefix() const override;
> @@ -91,8 +92,8 @@ private:
> void initStaticProperties();
> void initTestPatternModes();
> int initProperties();
> - int applyTestPatternMode(controls::draft::TestPatternModeEnum mode);
> int discoverAncillaryDevices();
> + int applyTestPatternMode(controls::draft::TestPatternModeEnum mode);
>
> const MediaEntity *entity_;
> std::unique_ptr<V4L2Subdevice> subdev_;
> diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp
> index 545f89d036df..86ad9a85371c 100644
> --- a/src/libcamera/sensor/camera_sensor.cpp
> +++ b/src/libcamera/sensor/camera_sensor.cpp
> @@ -215,6 +215,30 @@ int CameraSensor::init()
> return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);
> }
>
> +int CameraSensor::generateId()
> +{
> + const std::string devPath = subdev_->devicePath();
> +
> + /* Try to get ID from firmware description. */
> + id_ = sysfs::firmwareNodePath(devPath);
> + if (!id_.empty())
> + return 0;
> +
> + /*
> + * Virtual sensors not described in firmware
> + *
> + * Verify it's a platform device and construct ID from the device path
> + * and model of sensor.
> + */
> + if (devPath.find("/sys/devices/platform/", 0) == 0) {
> + id_ = devPath.substr(strlen("/sys/devices/")) + " " + model();
> + return 0;
> + }
> +
> + LOG(CameraSensor, Error) << "Can't generate sensor ID";
> + return -EINVAL;
> +}
> +
> int CameraSensor::validateSensorDriver()
> {
> int err = 0;
> @@ -580,6 +604,21 @@ int CameraSensor::discoverAncillaryDevices()
> * \return The sensor media entity
> */
>
> +/**
> + * \fn CameraSensor::device()
> + * \brief Retrieve the camera sensor device
> + * \todo Remove this function by integrating DelayedControl with CameraSensor
> + * \return The camera sensor device
> + */
> +
> +/**
> + * \fn CameraSensor::focusLens()
> + * \brief Retrieve the focus lens controller
> + *
> + * \return The focus lens controller. nullptr if no focus lens controller is
> + * connected to the sensor
> + */
> +
> /**
> * \fn CameraSensor::mbusCodes()
> * \brief Retrieve the media bus codes supported by the camera sensor
> @@ -632,64 +671,6 @@ Size CameraSensor::resolution() const
> return std::min(sizes_.back(), activeArea_.size());
> }
>
> -/**
> - * \fn CameraSensor::testPatternModes()
> - * \brief Retrieve all the supported test pattern modes of the camera sensor
> - * The test pattern mode values correspond to the controls::TestPattern control.
> - *
> - * \return The list of test pattern modes
> - */
> -
> -/**
> - * \brief Set the test pattern mode for the camera sensor
> - * \param[in] mode The test pattern mode
> - *
> - * The new \a mode is applied to the sensor if it differs from the active test
> - * pattern mode. Otherwise, this function is a no-op. Setting the same test
> - * pattern mode for every frame thus incurs no performance penalty.
> - */
> -int CameraSensor::setTestPatternMode(controls::draft::TestPatternModeEnum mode)
> -{
> - if (testPatternMode_ == mode)
> - return 0;
> -
> - if (testPatternModes_.empty()) {
> - LOG(CameraSensor, Error)
> - << "Camera sensor does not support test pattern modes.";
> - return -EINVAL;
> - }
> -
> - return applyTestPatternMode(mode);
> -}
> -
> -int CameraSensor::applyTestPatternMode(controls::draft::TestPatternModeEnum mode)
> -{
> - if (testPatternModes_.empty())
> - return 0;
> -
> - auto it = std::find(testPatternModes_.begin(), testPatternModes_.end(),
> - mode);
> - if (it == testPatternModes_.end()) {
> - LOG(CameraSensor, Error) << "Unsupported test pattern mode "
> - << mode;
> - return -EINVAL;
> - }
> -
> - LOG(CameraSensor, Debug) << "Apply test pattern mode " << mode;
> -
> - int32_t index = staticProps_->testPatternModes.at(mode);
> - ControlList ctrls{ controls() };
> - ctrls.set(V4L2_CID_TEST_PATTERN, index);
> -
> - int ret = setControls(&ctrls);
> - if (ret)
> - return ret;
> -
> - testPatternMode_ = mode;
> -
> - return 0;
> -}
> -
> /**
> * \brief Retrieve the best sensor format for a desired output
> * \param[in] mbusCodes The list of acceptable media bus codes
> @@ -919,80 +900,6 @@ int CameraSensor::applyConfiguration(const SensorConfiguration &config,
> return 0;
> }
>
> -/**
> - * \brief Retrieve the supported V4L2 controls and their information
> - *
> - * Control information is updated automatically to reflect the current sensor
> - * configuration when the setFormat() function is called, without invalidating
> - * any iterator on the ControlInfoMap.
> - *
> - * \return A map of the V4L2 controls supported by the sensor
> - */
> -const ControlInfoMap &CameraSensor::controls() const
> -{
> - return subdev_->controls();
> -}
> -
> -/**
> - * \brief Read V4L2 controls from the sensor
> - * \param[in] ids The list of controls to read, specified by their ID
> - *
> - * This function reads the value of all controls contained in \a ids, and
> - * returns their values as a ControlList. The control identifiers are defined by
> - * the V4L2 specification (V4L2_CID_*).
> - *
> - * If any control in \a ids is not supported by the device, is disabled (i.e.
> - * has the V4L2_CTRL_FLAG_DISABLED flag set), or if any other error occurs
> - * during validation of the requested controls, no control is read and this
> - * function returns an empty control list.
> - *
> - * \sa V4L2Device::getControls()
> - *
> - * \return The control values in a ControlList on success, or an empty list on
> - * error
> - */
> -ControlList CameraSensor::getControls(const std::vector<uint32_t> &ids)
> -{
> - return subdev_->getControls(ids);
> -}
> -
> -/**
> - * \brief Write V4L2 controls to the sensor
> - * \param[in] ctrls The list of controls to write
> - *
> - * This function writes the value of all controls contained in \a ctrls, and
> - * stores the values actually applied to the device in the corresponding \a
> - * ctrls entry. The control identifiers are defined by the V4L2 specification
> - * (V4L2_CID_*).
> - *
> - * If any control in \a ctrls is not supported by the device, is disabled (i.e.
> - * has the V4L2_CTRL_FLAG_DISABLED flag set), is read-only, or if any other
> - * error occurs during validation of the requested controls, no control is
> - * written and this function returns -EINVAL.
> - *
> - * If an error occurs while writing the controls, the index of the first
> - * control that couldn't be written is returned. All controls below that index
> - * are written and their values are updated in \a ctrls, while all other
> - * controls are not written and their values are not changed.
> - *
> - * \sa V4L2Device::setControls()
> - *
> - * \return 0 on success or an error code otherwise
> - * \retval -EINVAL One of the control is not supported or not accessible
> - * \retval i The index of the control that failed
> - */
> -int CameraSensor::setControls(ControlList *ctrls)
> -{
> - return subdev_->setControls(ctrls);
> -}
> -
> -/**
> - * \fn CameraSensor::device()
> - * \brief Retrieve the camera sensor device
> - * \todo Remove this function by integrating DelayedControl with CameraSensor
> - * \return The camera sensor device
> - */
> -
> /**
> * \fn CameraSensor::properties()
> * \brief Retrieve the camera sensor properties
> @@ -1088,14 +995,6 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const
> return 0;
> }
>
> -/**
> - * \fn CameraSensor::focusLens()
> - * \brief Retrieve the focus lens controller
> - *
> - * \return The focus lens controller. nullptr if no focus lens controller is
> - * connected to the sensor
> - */
> -
> /**
> * \brief Compute the Transform that gives the requested \a orientation
> * \param[inout] orientation The desired image orientation
> @@ -1155,33 +1054,134 @@ Transform CameraSensor::computeTransform(Orientation *orientation) const
> return transform;
> }
>
> +/**
> + * \brief Retrieve the supported V4L2 controls and their information
> + *
> + * Control information is updated automatically to reflect the current sensor
> + * configuration when the setFormat() function is called, without invalidating
> + * any iterator on the ControlInfoMap.
> + *
> + * \return A map of the V4L2 controls supported by the sensor
> + */
> +const ControlInfoMap &CameraSensor::controls() const
> +{
> + return subdev_->controls();
> +}
> +
> +/**
> + * \brief Read V4L2 controls from the sensor
> + * \param[in] ids The list of controls to read, specified by their ID
> + *
> + * This function reads the value of all controls contained in \a ids, and
> + * returns their values as a ControlList. The control identifiers are defined by
> + * the V4L2 specification (V4L2_CID_*).
> + *
> + * If any control in \a ids is not supported by the device, is disabled (i.e.
> + * has the V4L2_CTRL_FLAG_DISABLED flag set), or if any other error occurs
> + * during validation of the requested controls, no control is read and this
> + * function returns an empty control list.
> + *
> + * \sa V4L2Device::getControls()
> + *
> + * \return The control values in a ControlList on success, or an empty list on
> + * error
> + */
> +ControlList CameraSensor::getControls(const std::vector<uint32_t> &ids)
> +{
> + return subdev_->getControls(ids);
> +}
> +
> +/**
> + * \brief Write V4L2 controls to the sensor
> + * \param[in] ctrls The list of controls to write
> + *
> + * This function writes the value of all controls contained in \a ctrls, and
> + * stores the values actually applied to the device in the corresponding \a
> + * ctrls entry. The control identifiers are defined by the V4L2 specification
> + * (V4L2_CID_*).
> + *
> + * If any control in \a ctrls is not supported by the device, is disabled (i.e.
> + * has the V4L2_CTRL_FLAG_DISABLED flag set), is read-only, or if any other
> + * error occurs during validation of the requested controls, no control is
> + * written and this function returns -EINVAL.
> + *
> + * If an error occurs while writing the controls, the index of the first
> + * control that couldn't be written is returned. All controls below that index
> + * are written and their values are updated in \a ctrls, while all other
> + * controls are not written and their values are not changed.
> + *
> + * \sa V4L2Device::setControls()
> + *
> + * \return 0 on success or an error code otherwise
> + * \retval -EINVAL One of the control is not supported or not accessible
> + * \retval i The index of the control that failed
> + */
> +int CameraSensor::setControls(ControlList *ctrls)
> +{
> + return subdev_->setControls(ctrls);
> +}
> +
> +/**
> + * \fn CameraSensor::testPatternModes()
> + * \brief Retrieve all the supported test pattern modes of the camera sensor
> + * The test pattern mode values correspond to the controls::TestPattern control.
> + *
> + * \return The list of test pattern modes
> + */
> +
> +/**
> + * \brief Set the test pattern mode for the camera sensor
> + * \param[in] mode The test pattern mode
> + *
> + * The new \a mode is applied to the sensor if it differs from the active test
> + * pattern mode. Otherwise, this function is a no-op. Setting the same test
> + * pattern mode for every frame thus incurs no performance penalty.
> + */
> +int CameraSensor::setTestPatternMode(controls::draft::TestPatternModeEnum mode)
> +{
> + if (testPatternMode_ == mode)
> + return 0;
> +
> + if (testPatternModes_.empty()) {
> + LOG(CameraSensor, Error)
> + << "Camera sensor does not support test pattern modes.";
> + return -EINVAL;
> + }
> +
> + return applyTestPatternMode(mode);
> +}
> +
> +int CameraSensor::applyTestPatternMode(controls::draft::TestPatternModeEnum mode)
> +{
> + if (testPatternModes_.empty())
> + return 0;
> +
> + auto it = std::find(testPatternModes_.begin(), testPatternModes_.end(),
> + mode);
> + if (it == testPatternModes_.end()) {
> + LOG(CameraSensor, Error) << "Unsupported test pattern mode "
> + << mode;
> + return -EINVAL;
> + }
> +
> + LOG(CameraSensor, Debug) << "Apply test pattern mode " << mode;
> +
> + int32_t index = staticProps_->testPatternModes.at(mode);
> + ControlList ctrls{ controls() };
> + ctrls.set(V4L2_CID_TEST_PATTERN, index);
> +
> + int ret = setControls(&ctrls);
> + if (ret)
> + return ret;
> +
> + testPatternMode_ = mode;
> +
> + return 0;
> +}
> +
> std::string CameraSensor::logPrefix() const
> {
> return "'" + entity_->name() + "'";
> }
>
> -int CameraSensor::generateId()
> -{
> - const std::string devPath = subdev_->devicePath();
> -
> - /* Try to get ID from firmware description. */
> - id_ = sysfs::firmwareNodePath(devPath);
> - if (!id_.empty())
> - return 0;
> -
> - /*
> - * Virtual sensors not described in firmware
> - *
> - * Verify it's a platform device and construct ID from the device path
> - * and model of sensor.
> - */
> - if (devPath.find("/sys/devices/platform/", 0) == 0) {
> - id_ = devPath.substr(strlen("/sys/devices/")) + " " + model();
> - return 0;
> - }
> -
> - LOG(CameraSensor, Error) << "Can't generate sensor ID";
> - return -EINVAL;
> -}
> -
> } /* namespace libcamera */
> --
> Regards,
>
> Laurent Pinchart
>
More information about the libcamera-devel
mailing list