[libcamera-devel] [PATCH] libcamera: camera_sensor: only access V4L_CID_HBLANK if existing
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Nov 13 14:27:54 CET 2023
Hi Alain,
Quoting Alain Volmat via libcamera-devel (2023-11-13 10:08:51)
> Correct a crash in CameraSensor::init when trying to set the
> V4L2_CID_HBLANK control on sensor not implementing this control.
> The HBLANK sensor not being mandatory for non-RAW sensors, it can
> happen that the sensor does not expose this control.
> Perform check against availability of the control prior to usage in
> order to avoid the crash.
>
> Signed-off-by: Alain Volmat <alain.volmat at foss.st.com>
> ---
> src/libcamera/camera_sensor.cpp | 24 +++++++++++++-----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index d9261672..0ef78d9c 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -197,17 +197,19 @@ int CameraSensor::init()
> * \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;
> + if (ctrls.infoMap()->find(V4L2_CID_HBLANK) != ctrls.infoMap()->end()) {
> + 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;
> + }
Do we have a way to know if we're dealing with a raw or yuv/rgb sensor
in the CameraSensor class presently?
It might be beneficial to make sure we are explicit that this is not
required when !rawSensor here.
I see in validateSensorDriver() we already check V4L2_CID_HBLANK is a
mandatoryControl after a check on
if (!bayerFormat_)
so I guess that's our current switch point.
Is the HBLANK usable at all if there is no bayerFormat?
I'm also weary about the usage at CameraSensor::sensorInfo() where there
is a call to:
/*
* Retrieve the pixel rate, line length and minimum/maximum frame
* duration through V4L2 controls. Support for the V4L2_CID_PIXEL_RATE,
* V4L2_CID_HBLANK and V4L2_CID_VBLANK controls is mandatory.
*/
ControlList ctrls = subdev_->getControls({ V4L2_CID_PIXEL_RATE,
V4L2_CID_HBLANK,
V4L2_CID_VBLANK });
if (ctrls.empty()) {
LOG(CameraSensor, Error)
<< "Failed to retrieve camera info controls";
return -EINVAL;
}
Does supporting !HBLANK require a larger rework ? or is this the only
place that you have hit which requires a check?
--
Kieran
> }
>
> return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);
> --
> 2.25.1
>
More information about the libcamera-devel
mailing list