| Message ID | 20221202124005.3643-3-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@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@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 >
Quoting Naushir Patuck via libcamera-devel (2022-12-02 12:40:05) > 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@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)); Well, certainly once there's a .readOnly()... Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > } > > 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. > */ > 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 >
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. */ 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);
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@raspberrypi.com> --- src/ipa/raspberrypi/raspberrypi.cpp | 12 +----------- src/libcamera/camera_sensor.cpp | 14 ++------------ 2 files changed, 3 insertions(+), 23 deletions(-)