[libcamera-devel] [PATCH] libcamera: camera_sensor: only access V4L_CID_HBLANK if existing
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Nov 21 10:44:41 CET 2023
On Wed, Nov 15, 2023 at 03:57:23PM +0100, Jacopo Mondi via libcamera-devel wrote:
> Hi Alain
>
> On Mon, Nov 13, 2023 at 11:08:51AM +0100, Alain Volmat via libcamera-devel wrote:
> > 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.
> > */
>
> Actually the above comment suggest that there's no way to know if a
> control is RO, while I see
>
> if (hflipInfo && !(hflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY) &&
> vflipInfo && !(vflipInfo->flags & V4L2_CTRL_FLAG_READ_ONLY))
>
> in other parts of the class.
>
> This is unrelated to this change though
>
> > - 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()) {
>
> Most YUV/RGB sensor drivers I know of do not support HBLANK, so I
> would have expected this issue to be hit earlier than this. Possibly
> libcamera is not very populare with YUV/RGB sensor :)
>
> > + 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;
> > + }
>
> The patch looks good to me
> Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
Likewise;
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > }
> >
> > return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list