[libcamera-devel] [PATCH v2 1/2] libcamera: controls: Add read-only flag to ControlInfo
Naushir Patuck
naush at raspberrypi.com
Mon Dec 12 11:16:35 CET 2022
Hi Jacopo,
Thank you for your feedback!
On Mon, 12 Dec 2022 at 09:50, Jacopo Mondi <jacopo at jmondi.org> wrote:
> Hi Naush
>
> On Fri, Dec 02, 2022 at 12:40:04PM +0000, Naushir Patuck via
> libcamera-devel wrote:
> > Add a read-only flag to the ControlInfo flag to indicate whether a V4L2
> control
> > is read-only or not. This flag only makes sense for V2L2 controls at
> preset, so
> > only update the ControlInfo constructor signature used by the V4L2Device
> class
> > to set the value of the flag.
> >
>
> It feels a bit the three constructors are mis-aligned now :/
>
> I would have gone for a default parameter for all three of them to be
> honest... What do others think ?
>
I do kind of agree with this, the constructor signatures were the one bit
where I was unsure what to do.
Happy to change all the constructors to have a readOnly param if others
agree!
Regards,
Naush
>
> > Add a ControlInfo::readOnly() member function to retrive this flag.
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> > include/libcamera/controls.h | 5 ++++-
> > src/libcamera/controls.cpp | 17 +++++++++++++----
> > src/libcamera/v4l2_device.cpp | 12 ++++++++----
> > 3 files changed, 25 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > index cf94205577a5..488663a7ba04 100644
> > --- a/include/libcamera/controls.h
> > +++ b/include/libcamera/controls.h
> > @@ -270,7 +270,8 @@ class ControlInfo
> > public:
> > explicit ControlInfo(const ControlValue &min = {},
> > const ControlValue &max = {},
> > - const ControlValue &def = {});
> > + const ControlValue &def = {},
> > + bool readOnly = false);
> > explicit ControlInfo(Span<const ControlValue> values,
> > const ControlValue &def = {});
> > explicit ControlInfo(std::set<bool> values, bool def);
> > @@ -279,6 +280,7 @@ public:
> > const ControlValue &min() const { return min_; }
> > const ControlValue &max() const { return max_; }
> > const ControlValue &def() const { return def_; }
> > + bool readOnly() const { return readOnly_; }
> > const std::vector<ControlValue> &values() const { return values_; }
> >
> > std::string toString() const;
> > @@ -297,6 +299,7 @@ private:
> > ControlValue min_;
> > ControlValue max_;
> > ControlValue def_;
> > + bool readOnly_;
> > std::vector<ControlValue> values_;
> > };
> >
> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> > index 6dbf9b348709..fc66abad600d 100644
> > --- a/src/libcamera/controls.cpp
> > +++ b/src/libcamera/controls.cpp
> > @@ -484,11 +484,13 @@ void ControlValue::reserve(ControlType type, bool
> isArray, std::size_t numElemen
> > * \param[in] min The control minimum value
> > * \param[in] max The control maximum value
> > * \param[in] def The control default value
> > + * \param[in] readOnly Read-only status of a V4L2 control
> > */
> > ControlInfo::ControlInfo(const ControlValue &min,
> > const ControlValue &max,
> > - const ControlValue &def)
> > - : min_(min), max_(max), def_(def)
> > + const ControlValue &def,
> > + bool readOnly)
> > + : min_(min), max_(max), def_(def), readOnly_(readOnly)
> > {
> > }
> >
> > @@ -525,7 +527,8 @@ ControlInfo::ControlInfo(Span<const ControlValue>
> values,
> > * default value is \a def.
> > */
> > ControlInfo::ControlInfo(std::set<bool> values, bool def)
> > - : min_(false), max_(true), def_(def), values_({ false, true })
> > + : min_(false), max_(true), def_(def), readOnly_(false),
> > + values_({ false, true })
> > {
> > ASSERT(values.count(def) && values.size() == 2);
> > }
> > @@ -538,7 +541,7 @@ ControlInfo::ControlInfo(std::set<bool> values, bool
> def)
> > * value. The minimum, maximum, and default values will all be \a value.
> > */
> > ControlInfo::ControlInfo(bool value)
> > - : min_(value), max_(value), def_(value)
> > + : min_(value), max_(value), def_(value), readOnly_(false)
> > {
> > values_ = { value };
> > }
> > @@ -571,6 +574,12 @@ ControlInfo::ControlInfo(bool value)
> > * \return A ControlValue with the default value for the control
> > */
> >
> > +/**
> > + * \fn ControlInfo::readOnly()
> > + * \brief Identifies if a V4L2 control is flagged as read-only
> > + * \return True if the V4L2 control is read-only, false otherwise
>
> I would not mention V4L2 here and keep the documentation generic
>
> > + */
> > +
> > /**
> > * \fn ControlInfo::values()
> > * \brief Retrieve the list of valid values
> > diff --git a/src/libcamera/v4l2_device.cpp
> b/src/libcamera/v4l2_device.cpp
> > index 57a88d96b12c..9018f1b0b9e1 100644
> > --- a/src/libcamera/v4l2_device.cpp
> > +++ b/src/libcamera/v4l2_device.cpp
> > @@ -535,17 +535,20 @@ std::optional<ControlInfo>
> V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl
> > case V4L2_CTRL_TYPE_U8:
> > return ControlInfo(static_cast<uint8_t>(ctrl.minimum),
> > static_cast<uint8_t>(ctrl.maximum),
> > -
> static_cast<uint8_t>(ctrl.default_value));
> > +
> static_cast<uint8_t>(ctrl.default_value),
> > + !!(ctrl.flags &
> V4L2_CTRL_FLAG_READ_ONLY));
> >
> > case V4L2_CTRL_TYPE_BOOLEAN:
> > return ControlInfo(static_cast<bool>(ctrl.minimum),
> > static_cast<bool>(ctrl.maximum),
> > - static_cast<bool>(ctrl.default_value));
> > + static_cast<bool>(ctrl.default_value),
> > + !!(ctrl.flags &
> V4L2_CTRL_FLAG_READ_ONLY));
> >
> > case V4L2_CTRL_TYPE_INTEGER64:
> > return ControlInfo(static_cast<int64_t>(ctrl.minimum),
> > static_cast<int64_t>(ctrl.maximum),
> > -
> static_cast<int64_t>(ctrl.default_value));
> > +
> static_cast<int64_t>(ctrl.default_value),
> > + !!(ctrl.flags &
> V4L2_CTRL_FLAG_READ_ONLY));
> >
> > case V4L2_CTRL_TYPE_INTEGER_MENU:
> > case V4L2_CTRL_TYPE_MENU:
> > @@ -554,7 +557,8 @@ std::optional<ControlInfo>
> V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl
> > default:
> > return ControlInfo(static_cast<int32_t>(ctrl.minimum),
> > static_cast<int32_t>(ctrl.maximum),
> > -
> static_cast<int32_t>(ctrl.default_value));
> > +
> static_cast<int32_t>(ctrl.default_value),
> > + !!(ctrl.flags &
> V4L2_CTRL_FLAG_READ_ONLY));
> > }
> > }
> >
> > --
> > 2.25.1
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20221212/ac65c975/attachment.htm>
More information about the libcamera-devel
mailing list