[libcamera-devel] [PATCH v2 2/2] libcamera:camera_sensor, ipa: raspberrypi: Test readOnly() for HBLANK control

[Jacopo Mondi]

On Fri, Dec 02, 2022 at 12:40:05PM +0000, Naushir Patuck via libcamera-devel wrote:
> Use ControlInfo::readOnly() to test if the camera sensor accepts V4L2_CID_HBLANK
> control changes. This replaces the current workaround where we test if the
> V4L2_CID_HBLANK min and max values are the same to determine if a control is
> read-only.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  src/ipa/raspberrypi/raspberrypi.cpp | 12 +-----------
>  src/libcamera/camera_sensor.cpp     | 14 ++------------
>  2 files changed, 3 insertions(+), 23 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index bead436def3c..57f01496fc44 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -1239,17 +1239,7 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)
>  	ctrls.set(V4L2_CID_EXPOSURE, exposureLines);
>  	ctrls.set(V4L2_CID_ANALOGUE_GAIN, gainCode);
>
> -	/*
> -	 * At present, there is no way of knowing if a control is read-only.
> -	 * As a workaround, assume that if the minimum and maximum values of
> -	 * the V4L2_CID_HBLANK control are the same, it implies the control
> -	 * is read-only. This seems to be the case for all the cameras our IPA
> -	 * works with.
> -	 *
> -	 * \todo The control API ought to have a flag to specify if a control
> -	 * is read-only which could be used below.
> -	 */
> -	if (mode_.minLineLength != mode_.maxLineLength)
> +	if (!sensorCtrls_.at(V4L2_CID_HBLANK).readOnly())
>  		ctrls.set(V4L2_CID_HBLANK, static_cast<int32_t>(hblank));
>  }
>
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index ae3127d6b693..3785b6441b90 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -185,20 +185,10 @@ int CameraSensor::init()
>  	 * Set HBLANK to the minimum to start with a well-defined line length,
>  	 * allowing IPA modules that do not modify HBLANK to use the sensor
>  	 * minimum line length in their calculations.
> -	 *
> -	 * At present, there is no way of knowing if a control is read-only.
> -	 * As a workaround, assume that if the minimum and maximum values of
> -	 * the V4L2_CID_HBLANK control are the same, it implies the control
> -	 * is read-only.
> -	 *
> -	 * \todo The control API ought to have a flag to specify if a control
> -	 * is read-only which could be used below.

I guess this fixes the todo

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
   j

>  	 */
>  	const ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK);
> -	const int32_t hblankMin = hblank.min().get<int32_t>();
> -	const int32_t hblankMax = hblank.max().get<int32_t>();
> -
> -	if (hblankMin != hblankMax) {
> +	if (!hblank.readOnly()) {
> +		const int32_t hblankMin = hblank.min().get<int32_t>();
>  		ControlList ctrl(subdev_->controls());
>
>  		ctrl.set(V4L2_CID_HBLANK, hblankMin);
> --
> 2.25.1
>