| Message ID | 20240301212121.9072-18-laurent.pinchart@ideasonboard.com |
|---|---|
| State | RFC |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent On Fri, Mar 01, 2024 at 11:21:06PM +0200, Laurent Pinchart wrote: > The CameraSensor class tests if the sensor HBLANK control is read-only > by comparing the minimum and maximum values, and documents this as being > a workaround for the lack of a read-only control flag in V4L2. This is > incorrect, as the V4L2 API provides such a flag. Use it to replace the > workaround. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/sensor/camera_sensor.cpp | 27 +++++++------------------- > 1 file changed, 7 insertions(+), 20 deletions(-) > > diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp > index 86ad9a85371c..402025566544 100644 > --- a/src/libcamera/sensor/camera_sensor.cpp > +++ b/src/libcamera/sensor/camera_sensor.cpp > @@ -188,28 +188,15 @@ 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. > */ Weird, I'm sure V4L2_CTRL_FLAG_READ_ONLY was there in 2022 when this has been introduced. Maybe the API we had to check flags was (or is) not the best one and we decided to compare values ? > - 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>(); > + const struct v4l2_query_ext_ctrl *hblankInfo = subdev_->controlInfo(V4L2_CID_HBLANK); > + if (hblankInfo && !(hblankInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) { > + ControlList ctrl(subdev_->controls()); > This could be fast-tracked too! Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > - if (hblankMin != hblankMax) { > - ControlList ctrl(subdev_->controls()); > - > - ctrl.set(V4L2_CID_HBLANK, hblankMin); > - ret = subdev_->setControls(&ctrl); > - if (ret) > - return ret; > - } > + ctrl.set(V4L2_CID_HBLANK, static_cast<int32_t>(hblankInfo->minimum)); > + ret = subdev_->setControls(&ctrl); > + if (ret) > + return ret; > } > > return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff); > -- > Regards, > > Laurent Pinchart >
On Mon, Mar 04, 2024 at 06:41:03PM +0100, Jacopo Mondi wrote: > Hi Laurent > > On Fri, Mar 01, 2024 at 11:21:06PM +0200, Laurent Pinchart wrote: > > The CameraSensor class tests if the sensor HBLANK control is read-only > > by comparing the minimum and maximum values, and documents this as being > > a workaround for the lack of a read-only control flag in V4L2. This is > > incorrect, as the V4L2 API provides such a flag. Use it to replace the > > workaround. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/libcamera/sensor/camera_sensor.cpp | 27 +++++++------------------- > > 1 file changed, 7 insertions(+), 20 deletions(-) > > > > diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp > > index 86ad9a85371c..402025566544 100644 > > --- a/src/libcamera/sensor/camera_sensor.cpp > > +++ b/src/libcamera/sensor/camera_sensor.cpp > > @@ -188,28 +188,15 @@ 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. > > */ > > Weird, I'm sure V4L2_CTRL_FLAG_READ_ONLY was there in 2022 when this > has been introduced. Maybe the API we had to check flags was (or is) > not the best one and we decided to compare values ? It puzzled me too, I really don't recall why we didn't use it. Naush, does it ring a bell ? > > - 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>(); > > + const struct v4l2_query_ext_ctrl *hblankInfo = subdev_->controlInfo(V4L2_CID_HBLANK); > > + if (hblankInfo && !(hblankInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) { > > + ControlList ctrl(subdev_->controls()); > > This could be fast-tracked too! > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > - if (hblankMin != hblankMax) { > > - ControlList ctrl(subdev_->controls()); > > - > > - ctrl.set(V4L2_CID_HBLANK, hblankMin); > > - ret = subdev_->setControls(&ctrl); > > - if (ret) > > - return ret; > > - } > > + ctrl.set(V4L2_CID_HBLANK, static_cast<int32_t>(hblankInfo->minimum)); > > + ret = subdev_->setControls(&ctrl); > > + if (ret) > > + return ret; > > } > > > > return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);
On Mon, 4 Mar 2024, 10:49 pm Laurent Pinchart, < laurent.pinchart@ideasonboard.com> wrote: > On Mon, Mar 04, 2024 at 06:41:03PM +0100, Jacopo Mondi wrote: > > Hi Laurent > > > > On Fri, Mar 01, 2024 at 11:21:06PM +0200, Laurent Pinchart wrote: > > > The CameraSensor class tests if the sensor HBLANK control is read-only > > > by comparing the minimum and maximum values, and documents this as > being > > > a workaround for the lack of a read-only control flag in V4L2. This is > > > incorrect, as the V4L2 API provides such a flag. Use it to replace the > > > workaround. > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > --- > > > src/libcamera/sensor/camera_sensor.cpp | 27 +++++++------------------- > > > 1 file changed, 7 insertions(+), 20 deletions(-) > > > > > > diff --git a/src/libcamera/sensor/camera_sensor.cpp > b/src/libcamera/sensor/camera_sensor.cpp > > > index 86ad9a85371c..402025566544 100644 > > > --- a/src/libcamera/sensor/camera_sensor.cpp > > > +++ b/src/libcamera/sensor/camera_sensor.cpp > > > @@ -188,28 +188,15 @@ 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. > > > */ > > > > Weird, I'm sure V4L2_CTRL_FLAG_READ_ONLY was there in 2022 when this > > has been introduced. Maybe the API we had to check flags was (or is) > > not the best one and we decided to compare values ? > > It puzzled me too, I really don't recall why we didn't use it. Naush, > does it ring a bell ? > My patch at https://patchwork.libcamera.org/patch/17936/ did make use of this flag. But it never got merged. > > > - 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>(); > > > + const struct v4l2_query_ext_ctrl *hblankInfo = > subdev_->controlInfo(V4L2_CID_HBLANK); > > > + if (hblankInfo && !(hblankInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) > { > > > + ControlList ctrl(subdev_->controls()); > > > > This could be fast-tracked too! > > > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > > - if (hblankMin != hblankMax) { > > > - ControlList ctrl(subdev_->controls()); > > > - > > > - ctrl.set(V4L2_CID_HBLANK, hblankMin); > > > - ret = subdev_->setControls(&ctrl); > > > - if (ret) > > > - return ret; > > > - } > > > + ctrl.set(V4L2_CID_HBLANK, > static_cast<int32_t>(hblankInfo->minimum)); > > > + ret = subdev_->setControls(&ctrl); > > > + if (ret) > > > + return ret; > > > } > > > > > > return > applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff); > > -- > Regards, > > Laurent Pinchart >
Quoting Naushir Patuck (2024-03-05 07:09:58) > On Mon, 4 Mar 2024, 10:49 pm Laurent Pinchart, < > laurent.pinchart@ideasonboard.com> wrote: > > > On Mon, Mar 04, 2024 at 06:41:03PM +0100, Jacopo Mondi wrote: > > > Hi Laurent > > > > > > On Fri, Mar 01, 2024 at 11:21:06PM +0200, Laurent Pinchart wrote: > > > > The CameraSensor class tests if the sensor HBLANK control is read-only > > > > by comparing the minimum and maximum values, and documents this as > > being > > > > a workaround for the lack of a read-only control flag in V4L2. This is > > > > incorrect, as the V4L2 API provides such a flag. Use it to replace the > > > > workaround. > > > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > --- > > > > src/libcamera/sensor/camera_sensor.cpp | 27 +++++++------------------- > > > > 1 file changed, 7 insertions(+), 20 deletions(-) > > > > > > > > diff --git a/src/libcamera/sensor/camera_sensor.cpp > > b/src/libcamera/sensor/camera_sensor.cpp > > > > index 86ad9a85371c..402025566544 100644 > > > > --- a/src/libcamera/sensor/camera_sensor.cpp > > > > +++ b/src/libcamera/sensor/camera_sensor.cpp > > > > @@ -188,28 +188,15 @@ 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. > > > > */ > > > > > > Weird, I'm sure V4L2_CTRL_FLAG_READ_ONLY was there in 2022 when this > > > has been introduced. Maybe the API we had to check flags was (or is) > > > not the best one and we decided to compare values ? > > > > It puzzled me too, I really don't recall why we didn't use it. Naush, > > does it ring a bell ? > > > > My patch at https://patchwork.libcamera.org/patch/17936/ did make use of > this flag. But it never got merged. Indeed, and I rebased Naush' patches to: - https://patchwork.libcamera.org/cover/19187/ Which seemed overall to be rejected, and Alain came along with: - https://patchwork.libcamera.org/patch/19214/ which I thought would get merged, but Alain hasn't been able to submit a new version with the comments handled. Does something in this series handle the equivalant change at src/ipa/rpi/common/ipa_base.cpp as well? -- Kieran > > > > > > > - 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>(); > > > > + const struct v4l2_query_ext_ctrl *hblankInfo = > > subdev_->controlInfo(V4L2_CID_HBLANK); > > > > + if (hblankInfo && !(hblankInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) > > { > > > > + ControlList ctrl(subdev_->controls()); > > > > > > This could be fast-tracked too! > > > > > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > > > > - if (hblankMin != hblankMax) { > > > > - ControlList ctrl(subdev_->controls()); > > > > - > > > > - ctrl.set(V4L2_CID_HBLANK, hblankMin); > > > > - ret = subdev_->setControls(&ctrl); > > > > - if (ret) > > > > - return ret; > > > > - } > > > > + ctrl.set(V4L2_CID_HBLANK, > > static_cast<int32_t>(hblankInfo->minimum)); > > > > + ret = subdev_->setControls(&ctrl); > > > > + if (ret) > > > > + return ret; > > > > } > > > > > > > > return > > applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff); > > > > -- > > Regards, > > > > Laurent Pinchart > >
On Tue, Mar 05, 2024 at 08:58:25AM +0000, Kieran Bingham wrote: > Quoting Naushir Patuck (2024-03-05 07:09:58) > > On Mon, 4 Mar 2024, 10:49 pm Laurent Pinchart wrote: > > > On Mon, Mar 04, 2024 at 06:41:03PM +0100, Jacopo Mondi wrote: > > > > On Fri, Mar 01, 2024 at 11:21:06PM +0200, Laurent Pinchart wrote: > > > > > The CameraSensor class tests if the sensor HBLANK control is read-only > > > > > by comparing the minimum and maximum values, and documents this as being > > > > > a workaround for the lack of a read-only control flag in V4L2. This is > > > > > incorrect, as the V4L2 API provides such a flag. Use it to replace the > > > > > workaround. > > > > > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > --- > > > > > src/libcamera/sensor/camera_sensor.cpp | 27 +++++++------------------- > > > > > 1 file changed, 7 insertions(+), 20 deletions(-) > > > > > > > > > > diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp > > > > > index 86ad9a85371c..402025566544 100644 > > > > > --- a/src/libcamera/sensor/camera_sensor.cpp > > > > > +++ b/src/libcamera/sensor/camera_sensor.cpp > > > > > @@ -188,28 +188,15 @@ 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. > > > > > */ > > > > > > > > Weird, I'm sure V4L2_CTRL_FLAG_READ_ONLY was there in 2022 when this > > > > has been introduced. Maybe the API we had to check flags was (or is) > > > > not the best one and we decided to compare values ? > > > > > > It puzzled me too, I really don't recall why we didn't use it. Naush, > > > does it ring a bell ? > > > > > > > My patch at https://patchwork.libcamera.org/patch/17936/ did make use of > > this flag. But it never got merged. > > Indeed, and I rebased Naush' patches to: > > - https://patchwork.libcamera.org/cover/19187/ > > Which seemed overall to be rejected, and Alain came along with: > > - https://patchwork.libcamera.org/patch/19214/ > > which I thought would get merged, but Alain hasn't been able to submit a > new version with the comments handled. > > > Does something in this series handle the equivalant change at > src/ipa/rpi/common/ipa_base.cpp as well? No it doesn't, I've only addressed the CameraSensor side. I've just replied to Alain's patch, which had fallen through the cracks (sorry about that). > > > > > - 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>(); > > > > > + const struct v4l2_query_ext_ctrl *hblankInfo = subdev_->controlInfo(V4L2_CID_HBLANK); > > > > > + if (hblankInfo && !(hblankInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) { > > > > > + ControlList ctrl(subdev_->controls()); > > > > > > > > This could be fast-tracked too! > > > > > > > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > > > > > > - if (hblankMin != hblankMax) { > > > > > - ControlList ctrl(subdev_->controls()); > > > > > - > > > > > - ctrl.set(V4L2_CID_HBLANK, hblankMin); > > > > > - ret = subdev_->setControls(&ctrl); > > > > > - if (ret) > > > > > - return ret; > > > > > - } > > > > > + ctrl.set(V4L2_CID_HBLANK, static_cast<int32_t>(hblankInfo->minimum)); > > > > > + ret = subdev_->setControls(&ctrl); > > > > > + if (ret) > > > > > + return ret; > > > > > } > > > > > > > > > > return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);
Quoting Laurent Pinchart (2024-03-05 11:15:17) > On Tue, Mar 05, 2024 at 08:58:25AM +0000, Kieran Bingham wrote: > > Quoting Naushir Patuck (2024-03-05 07:09:58) > > > On Mon, 4 Mar 2024, 10:49 pm Laurent Pinchart wrote: > > > > On Mon, Mar 04, 2024 at 06:41:03PM +0100, Jacopo Mondi wrote: > > > > > On Fri, Mar 01, 2024 at 11:21:06PM +0200, Laurent Pinchart wrote: > > > > > > The CameraSensor class tests if the sensor HBLANK control is read-only > > > > > > by comparing the minimum and maximum values, and documents this as being > > > > > > a workaround for the lack of a read-only control flag in V4L2. This is > > > > > > incorrect, as the V4L2 API provides such a flag. Use it to replace the > > > > > > workaround. > > > > > > > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > > --- > > > > > > src/libcamera/sensor/camera_sensor.cpp | 27 +++++++------------------- > > > > > > 1 file changed, 7 insertions(+), 20 deletions(-) > > > > > > > > > > > > diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp > > > > > > index 86ad9a85371c..402025566544 100644 > > > > > > --- a/src/libcamera/sensor/camera_sensor.cpp > > > > > > +++ b/src/libcamera/sensor/camera_sensor.cpp > > > > > > @@ -188,28 +188,15 @@ 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. > > > > > > */ > > > > > > > > > > Weird, I'm sure V4L2_CTRL_FLAG_READ_ONLY was there in 2022 when this > > > > > has been introduced. Maybe the API we had to check flags was (or is) > > > > > not the best one and we decided to compare values ? > > > > > > > > It puzzled me too, I really don't recall why we didn't use it. Naush, > > > > does it ring a bell ? > > > > > > > > > > My patch at https://patchwork.libcamera.org/patch/17936/ did make use of > > > this flag. But it never got merged. > > > > Indeed, and I rebased Naush' patches to: > > > > - https://patchwork.libcamera.org/cover/19187/ > > > > Which seemed overall to be rejected, and Alain came along with: > > > > - https://patchwork.libcamera.org/patch/19214/ > > > > which I thought would get merged, but Alain hasn't been able to submit a > > new version with the comments handled. > > > > > > Does something in this series handle the equivalant change at > > src/ipa/rpi/common/ipa_base.cpp as well? > > No it doesn't, I've only addressed the CameraSensor side. I've just > replied to Alain's patch, which had fallen through the cracks (sorry > about that). > > > > > > > - 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>(); > > > > > > + const struct v4l2_query_ext_ctrl *hblankInfo = subdev_->controlInfo(V4L2_CID_HBLANK); > > > > > > + if (hblankInfo && !(hblankInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) { > > > > > > + ControlList ctrl(subdev_->controls()); I certainly prefer this: const struct v4l2_query_ext_ctrl *hblankInfo = subdev_->controlInfo(V4L2_CID_HBLANK); if (hblankInfo ...) { } style over if (ctrls.infoMap()->find(V4L2_CID_HBLANK) != ctrls.infoMap()->end()) { const ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK); .. } as it only does the search once. But it would be good to not lose sight of the equivalent issue in the RPi IPA. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > > > > > > This could be fast-tracked too! > > > > > > > > > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > > > > > > > > > - if (hblankMin != hblankMax) { > > > > > > - ControlList ctrl(subdev_->controls()); > > > > > > - > > > > > > - ctrl.set(V4L2_CID_HBLANK, hblankMin); > > > > > > - ret = subdev_->setControls(&ctrl); > > > > > > - if (ret) > > > > > > - return ret; > > > > > > - } > > > > > > + ctrl.set(V4L2_CID_HBLANK, static_cast<int32_t>(hblankInfo->minimum)); > > > > > > + ret = subdev_->setControls(&ctrl); > > > > > > + if (ret) > > > > > > + return ret; > > > > > > } > > > > > > > > > > > > return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff); > > -- > Regards, > > Laurent Pinchart
diff --git a/src/libcamera/sensor/camera_sensor.cpp b/src/libcamera/sensor/camera_sensor.cpp index 86ad9a85371c..402025566544 100644 --- a/src/libcamera/sensor/camera_sensor.cpp +++ b/src/libcamera/sensor/camera_sensor.cpp @@ -188,28 +188,15 @@ 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. */ - 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>(); + const struct v4l2_query_ext_ctrl *hblankInfo = subdev_->controlInfo(V4L2_CID_HBLANK); + if (hblankInfo && !(hblankInfo->flags & V4L2_CTRL_FLAG_READ_ONLY)) { + ControlList ctrl(subdev_->controls()); - if (hblankMin != hblankMax) { - ControlList ctrl(subdev_->controls()); - - ctrl.set(V4L2_CID_HBLANK, hblankMin); - ret = subdev_->setControls(&ctrl); - if (ret) - return ret; - } + ctrl.set(V4L2_CID_HBLANK, static_cast<int32_t>(hblankInfo->minimum)); + ret = subdev_->setControls(&ctrl); + if (ret) + return ret; } return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff);
The CameraSensor class tests if the sensor HBLANK control is read-only by comparing the minimum and maximum values, and documents this as being a workaround for the lack of a read-only control flag in V4L2. This is incorrect, as the V4L2 API provides such a flag. Use it to replace the workaround. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/libcamera/sensor/camera_sensor.cpp | 27 +++++++------------------- 1 file changed, 7 insertions(+), 20 deletions(-)