[libcamera-devel] [PATCH v2 1/1] libcamera: camera_sensor: Reset HBLANK when setting the format

[Naushir Patuck]

Hi David,

Thanks for fixing this.

On Wed, 23 Nov 2022 at 11:36, David Plowman via libcamera-devel <
libcamera-devel at lists.libcamera.org> wrote:

> We no longer reset the HBLANK in init because we may not own the
> camera and the operation may fail. Instead, we reset it when we set
> the camera format so that all pipeline handlers, even ones that do not
> control HBLANK themselves, will have a well-defined value.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
>

Seems reasonable!

Reviewed-By: Naushir Patuck <naush at raspberrypi.com>


> ---
>  src/libcamera/camera_sensor.cpp | 53 ++++++++++++++++-----------------
>  1 file changed, 26 insertions(+), 27 deletions(-)
>
> diff --git a/src/libcamera/camera_sensor.cpp
> b/src/libcamera/camera_sensor.cpp
> index 572a313a..ec7bb0e7 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -176,32 +176,6 @@ int CameraSensor::init()
>         if (ret)
>                 return ret;
>
> -       /*
> -        * 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.
> -        */
> -       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) {
> -               ControlList ctrl(subdev_->controls());
> -
> -               ctrl.set(V4L2_CID_HBLANK, hblankMin);
> -               ret = subdev_->setControls(&ctrl);
> -               if (ret)
> -                       return ret;
> -       }
> -
>         return
> applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);
>  }
>
> @@ -748,7 +722,32 @@ int CameraSensor::setFormat(V4L2SubdeviceFormat
> *format)
>                 return ret;
>
>         updateControlInfo();
> -       return 0;
> +
> +       /*
> +        * 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.
> +        */
> +       const ControlInfo hblank = controls().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) {
> +               ControlList ctrl(subdev_->controls());
> +
> +               ctrl.set(V4L2_CID_HBLANK, hblankMin);
> +               ret = subdev_->setControls(&ctrl);
> +       }
> +
> +       return ret;
>  }
>
>  /**
> --
> 2.30.2
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20221123/551c5490/attachment.htm>