[libcamera-devel] [PATCH v2 5/8] libcamera: camera_sensor: Expose a DelayedControls interface

[Jacopo Mondi]

Hi Niklas,

On Tue, Nov 10, 2020 at 01:27:07AM +0100, Niklas Söderlund wrote:
> Expose the optional DelayedControls interface to pipeline handlers. If
> used by a pipeline generic delays are used. In the future the delay
> values should be fetched to match the specific sensor module, either
> from a new kernel interface or worst case a sensors database.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  include/libcamera/internal/camera_sensor.h |  5 ++++
>  src/libcamera/camera_sensor.cpp            | 31 ++++++++++++++++++++++
>  2 files changed, 36 insertions(+)
>
> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> index b9eba89f00fba882..6be1cfd49134c534 100644
> --- a/include/libcamera/internal/camera_sensor.h
> +++ b/include/libcamera/internal/camera_sensor.h
> @@ -14,6 +14,7 @@
>  #include <libcamera/controls.h>
>  #include <libcamera/geometry.h>
>
> +#include "libcamera/internal/delayed_controls.h"
>  #include "libcamera/internal/formats.h"
>  #include "libcamera/internal/log.h"
>  #include "libcamera/internal/v4l2_subdevice.h"
> @@ -61,6 +62,8 @@ public:
>  	ControlList getControls(const std::vector<uint32_t> &ids);
>  	int setControls(ControlList *ctrls);
>
> +	DelayedControls *delayedContols();
> +
>  	const ControlList &properties() const { return properties_; }
>  	int sensorInfo(CameraSensorInfo *info) const;
>
> @@ -83,6 +86,8 @@ private:
>  	std::vector<Size> sizes_;
>
>  	ControlList properties_;
> +
> +	std::unique_ptr<DelayedControls> delayedControls_;
>  };
>
>  } /* namespace libcamera */
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 935de528c4963453..6c92c97f4cc2547a 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -493,6 +493,37 @@ int CameraSensor::setControls(ControlList *ctrls)
>  	return subdev_->setControls(ctrls);
>  }
>
> +/**
> + * \brief Get the sensors delayed controls interface
> + *
> + * Access the sensors delayed controls interface. This interface aids pipelines
> + * keeping tack of controls that needs time to take effect. The interface is
> + * optional to use and does not interact with setControls() and getControls()
> + * that operates directly on the sensor device.
> + *
> + * \sa DelayedControls
> + * \return The delayed controls interface
> + */
> +DelayedControls *CameraSensor::delayedContols()
> +{
> +	if (!delayedControls_) {
> +		/*
> +		 * \todo Read dealy values from the sensor itself or from a
> +		 * a sensor database. For now use generic values taken from
> +		 * the Raspberry Pi and listed as generic values.
> +		 */
> +		std::unordered_map<uint32_t, unsigned int> delays = {
> +			{ V4L2_CID_ANALOGUE_GAIN, 1 },
> +			{ V4L2_CID_EXPOSURE, 2 },
> +		};

I would really not hardcode them here.

As long as we don't have a sensor database or a place where these
values can be fetched from isn't it better to provide something like

        using DelayMap = std::unordered_map<const ControlId *, unsigned int>;

        CameraSensor::initControlDelays(DelayMap map)
        {
                ....
                /*
                 * CameraSensor gets the numerical ID from the
                 * DelayMap and construct a new map making sure the
                 * controls are supported by the subdev
                 */
		delayedControls_ =
			std::make_unique<DelayedControls>(subdev_.get(), delays);

        }

So that pipeline handlers receive delays from the IPA (as RPi
currently does if I'm not mistaken) and intialize Delayed controls on
the CameraSensor passing controls as standard libcamera ControlId ?

> +
> +		delayedControls_ =
> +			std::make_unique<DelayedControls>(subdev_.get(), delays);
> +	}
> +
> +	return delayedControls_.get();
> +}
> +
>  /**
>   * \brief Assemble and return the camera sensor info
>   * \param[out] info The camera sensor info
> --
> 2.29.2
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel