[libcamera-devel] [PATCH 2/5] libcamera: camera_sensor: Initialize controls
paul.elder at ideasonboard.com
paul.elder at ideasonboard.com
Mon Dec 28 11:13:50 CET 2020
On Mon, Dec 28, 2020 at 07:12:25PM +0900, paul.elder at ideasonboard.com wrote:
> Hi Jacopo,
>
> On Mon, Dec 28, 2020 at 10:39:25AM +0100, Jacopo Mondi wrote:
> > Hi Paul,
> >
> > On Mon, Dec 28, 2020 at 05:26:57PM +0900, paul.elder at ideasonboard.com wrote:
> > > Hi Jacopo,
> > >
> > > On Wed, Dec 23, 2020 at 07:45:13PM +0100, Jacopo Mondi wrote:
> > > > Initialize the ControlInfoMap of sensor available controls by
> > > > reporting the sensor exposure time range.
> > > >
> > > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > > > ---
> > > > include/libcamera/internal/camera_sensor.h | 2 ++
> > > > src/libcamera/camera_sensor.cpp | 42 +++++++++++++++++++++-
> > > > 2 files changed, 43 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> > > > index aee10aa6e3c7..0357b2a630f7 100644
> > > > --- a/include/libcamera/internal/camera_sensor.h
> > > > +++ b/include/libcamera/internal/camera_sensor.h
> > > > @@ -71,6 +71,7 @@ private:
> > > > int generateId();
> > > > int validateSensorDriver();
> > > > int initProperties();
> > > > + int initControls();
> > > >
> > > > const MediaEntity *entity_;
> > > > std::unique_ptr<V4L2Subdevice> subdev_;
> > > > @@ -85,6 +86,7 @@ private:
> > > > std::vector<Size> sizes_;
> > > >
> > > > ControlList properties_;
> > > > + ControlInfoMap controls_;
> > > > };
> > > >
> > > > } /* namespace libcamera */
> > > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> > > > index 3a65ac3de5bc..609f948c56a6 100644
> > > > --- a/src/libcamera/camera_sensor.cpp
> > > > +++ b/src/libcamera/camera_sensor.cpp
> > > > @@ -15,6 +15,7 @@
> > > > #include <regex>
> > > > #include <string.h>
> > > >
> > > > +#include <libcamera/control_ids.h>
> > > > #include <libcamera/property_ids.h>
> > > >
> > > > #include "libcamera/internal/bayer_format.h"
> > > > @@ -215,7 +216,7 @@ int CameraSensor::init()
> > > > if (ret)
> > > > return ret;
> > > >
> > > > - return 0;
> > > > + return initControls();
> > > > }
> > > >
> > > > int CameraSensor::validateSensorDriver()
> > > > @@ -229,6 +230,7 @@ int CameraSensor::validateSensorDriver()
> > > > const std::vector<uint32_t> mandatoryControls{
> > > > V4L2_CID_PIXEL_RATE,
> > > > V4L2_CID_HBLANK,
> > > > + V4L2_CID_EXPOSURE,
> > > > };
> > > >
> > > > ControlList ctrls = subdev_->getControls(mandatoryControls);
> > > > @@ -430,6 +432,44 @@ int CameraSensor::initProperties()
> > > > return 0;
> > > > }
> > > >
> > > > +int CameraSensor::initControls()
> > > > +{
> > > > + const ControlInfoMap &v4l2Controls = subdev_->controls();
> > > > +
> > > > + /* Exposure time limits expressed in micro-seconds. */
> > > > +
> > > > + /* Calculate the line length in pixels. */
> > > > + ControlList ctrls = subdev_->getControls({ V4L2_CID_HBLANK });
> > >
> > > I see that the presence of this control is declared as mandatory in
> > > validateSensorDriver() in 1/5, but it just returns -EINVAL and
> > > continues; would that cause a problem here?
> >
> > Do you mean 'validateSensorDriver()' returns -EINVAL ? In that case
>
> Ah yeah, I mean in the case that the mandatory control is not present,
> so validateSensorDriver() returns -EINVAL.
>
> > CameraSensor::init() bails out before calling this method. Or have I
>
> Wouldn't that mean that this method won't be called either if the
> optional controls aren't present in the driver?
Oh nvm, I misread the last patch.
Paul
> > mis-understood your question ?
> >
> > One thing I would add: reading a control might fail for other reason
> > than not being implemented in the driver. I should probably check for
> > if (ctrl.empty())
> > nonetheless
> >
> > >
> > > > + int32_t hblank = ctrls.get(V4L2_CID_HBLANK).get<int32_t>();
> > >
> > > Would it just make this zero and continue? And the user has been warned
> > > of unexpected behavior in the validation step already.
> > >
> >
> > We don't get here if validation fails if that's what you mean.
>
> I see.
>
>
> Paul
>
> > And in case reading the control fails for unrelated reasons, I would
> > fail instead of zeroing...
> >
> > > > + V4L2SubdeviceFormat format{};
> > > > + int ret = subdev_->getFormat(pad_, &format);
> > > > + if (ret)
> > > > + return ret;
> > > > + int32_t ppl = format.size.width + hblank;
> > > > +
> > > > + const ControlInfo &v4l2ExposureInfo = v4l2Controls.find(V4L2_CID_EXPOSURE)->second;
> > > > + int32_t minExposurePixels = v4l2ExposureInfo.min().get<int32_t>() * ppl;
> > > > + int32_t maxExposurePixels = v4l2ExposureInfo.max().get<int32_t>() * ppl;
> > > > + int32_t defExposurePixels = v4l2ExposureInfo.max().get<int32_t>() * ppl;
> > >
> > > s/max/def/ ?
> >
> > Good catch!
> >
> > Thanks
> > j
> >
> > >
> > >
> > > Paul
> > >
> > > > +
> > > > + /* Get the pixel rate (in useconds) and calculate the exposure timings. */
> > > > + const ControlInfo &pixelRateInfo = v4l2Controls.find(V4L2_CID_PIXEL_RATE)->second;
> > > > + float minPixelRate = pixelRateInfo.min().get<int64_t>() / 1e6f;
> > > > + float maxPixelRate = pixelRateInfo.max().get<int64_t>() / 1e6f;
> > > > + float defPixelRate = pixelRateInfo.def().get<int64_t>() / 1e6f;
> > > > +
> > > > + int32_t minExposure = static_cast<int32_t>(minExposurePixels / maxPixelRate);
> > > > + int32_t maxExposure = static_cast<int32_t>(maxExposurePixels / minPixelRate);
> > > > + int32_t defExposure = static_cast<int32_t>(defExposurePixels / defPixelRate);
> > > > +
> > > > + ControlInfoMap::Map map;
> > > > + map[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure,
> > > > + defExposure);
> > > > + controls_ = std::move(map);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > /**
> > > > * \fn CameraSensor::model()
> > > > * \brief Retrieve the sensor model name
> > > > --
> > > > 2.29.2
> > > >
> > > > _______________________________________________
> > > > libcamera-devel mailing list
> > > > libcamera-devel at lists.libcamera.org
> > > > https://lists.libcamera.org/listinfo/libcamera-devel
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
More information about the libcamera-devel
mailing list