[libcamera-devel] [PATCH v2 3/4] libcamera: camera_sensor: Add function to apply a config

[Kieran Bingham]

In $SUBJECT : 
I would probably do:

-libcamera: camera_sensor: Add function to apply a config
+libcamera: camera_sensor: Support a SensorConfiguration


Quoting Jacopo Mondi via libcamera-devel (2023-07-31 12:31:14)
> Add to the CameraSensor class a function to apply to the sensor
> a full configuration.

Add a class function to the CameraSensor class to apply a full
configuration to the sensor.



> 
> The configuration shall be fully populated and shall apply without
> modifications to the sensor.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> Reviewed-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  include/libcamera/internal/camera_sensor.h |  5 ++
>  src/libcamera/camera_sensor.cpp            | 86 ++++++++++++++++++++++
>  2 files changed, 91 insertions(+)
> 
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index 02c77ab037da..06791c3c6425 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -29,6 +29,7 @@ namespace libcamera {
>  class BayerFormat;
>  class CameraLens;
>  class MediaEntity;
> +class SensorConfiguration;
>  
>  struct CameraSensorProperties;
>  
> @@ -58,6 +59,10 @@ public:
>                       Transform transform = Transform::Identity);
>         int tryFormat(V4L2SubdeviceFormat *format) const;
>  
> +       int applyConfiguration(const SensorConfiguration &config,
> +                              Transform transform = Transform::Identity,
> +                              V4L2SubdeviceFormat *sensorFormat = nullptr);
> +
>         const ControlInfoMap &controls() const;
>         ControlList getControls(const std::vector<uint32_t> &ids);
>         int setControls(ControlList *ctrls);
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index f3a5aa37149f..96b618572a3d 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -15,6 +15,7 @@
>  #include <math.h>
>  #include <string.h>
>  
> +#include <libcamera/camera.h>
>  #include <libcamera/property_ids.h>
>  
>  #include <libcamera/base/utils.h>
> @@ -822,6 +823,91 @@ int CameraSensor::tryFormat(V4L2SubdeviceFormat *format) const
>                                   V4L2Subdevice::Whence::TryFormat);
>  }
>  
> +/**
> + * \brief Apply a sensor configuration to the camera sensor
> + * \param[in] config The sensor configuration
> + * \param[in] transform The transform to be applied on the sensor.
> + * Defaults to Identity
> + * \param[out] sensorFormat Optional output parameter where the format
> + * actually applied to the sensor is returned to the caller
> + *
> + * Apply to the camera sensor the configuration \a config.
> + *
> + * \todo The configuration shall be fully populated and if any of the fields
> + * there specified cannot be applied as it is, an error code is returned.

and if any of the fields specified cannot be applied exactly, an error
code is returned.

> + *
> + * \return 0 if \a config is applied to the camera sensor, a negative error code
> + * otherwise
> + */
> +int CameraSensor::applyConfiguration(const SensorConfiguration &config,
> +                                    Transform transform,
> +                                    V4L2SubdeviceFormat *sensorFormat)
> +{
> +       if (!config) {
> +               LOG(CameraSensor, Error) << "Invalid sensor configuration";
> +               return -EINVAL;
> +       }
> +
> +       std::vector<unsigned int> filteredCodes;
> +       std::copy_if(mbusCodes_.begin(), mbusCodes_.end(),
> +                    std::back_inserter(filteredCodes),
> +                    [&config](unsigned int mbusCode) {
> +                            BayerFormat bayer = BayerFormat::fromMbusCode(mbusCode);
> +                            if (bayer.bitDepth == config.bitDepth)
> +                                    return true;
> +                            return false;
> +                    });
> +       if (filteredCodes.empty()) {
> +               LOG(CameraSensor, Error)
> +                       << "Cannot find any format with bit depth "
> +                       << config.bitDepth;
> +               return -EINVAL;
> +       }
> +
> +       /*
> +        * Compute the sensor's data frame size by applying the cropping
> +        * rectangle, subsampling and output crop to the sensor's pixel array
> +        * size.
> +        *
> +        * \todo The actual size computation is for now ignored and only the
> +        * output size is considered. This implies that resolutions obtained
> +        * with two different cropping/subsampling will look identical and
> +        * only the first found one will be considered.
> +        */
> +       V4L2SubdeviceFormat subdevFormat = {};
> +       for (unsigned int code : filteredCodes) {
> +               for (const Size &size : sizes(code)) {
> +                       if (size.width != config.outputSize.width ||
> +                           size.height != config.outputSize.height)
> +                               continue;
> +
> +                       subdevFormat.mbus_code = code;
> +                       subdevFormat.size = size;
> +                       break;
> +               }
> +       }
> +       if (!subdevFormat.mbus_code) {
> +               LOG(CameraSensor, Error) << "Invalid output size in sensor configuration";
> +               return -EINVAL;
> +       }
> +
> +       int ret = setFormat(&subdevFormat, transform);
> +       if (ret)
> +               return ret;
> +
> +       /*
> +        * Return to the caller the format actually applied to the sensor.
> +        * This is relevant if transform has changed the bayer pattern order.
> +        */
> +       if (sensorFormat)
> +               *sensorFormat = subdevFormat;
> +
> +       /* \todo Handle AnalogCrop. Most sensors do not support set_selection */
> +       /* \todo Handle scaling in the digital domain. */

Should these return an error if they are set as they are unhandled for
now? Or are we happy to just silently ignore here for now?

Except the known todos, nothing blocking here though so


Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


> +
> +       return 0;
> +}
> +
>  /**
>   * \brief Retrieve the supported V4L2 controls and their information
>   *
> -- 
> 2.40.1
>