[libcamera-devel] [PATCH v2 1/2] libcamera: control: initialise control info to ControlTypeNone by default

[David Plowman]

On Fri, 30 Sept 2022 at 12:59, Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Quoting Tomi Valkeinen via libcamera-devel (2022-09-30 12:14:12)
> > Hi,
> >
> > On 30/09/2022 14:01, David Plowman wrote:
> > > Hi Christian
> > >
> > > I have a question about this because it's causing my Python code to
> > > break. Not in any major way, we just need to decide what we want to do
> > > about it.
> > >
> > > The problem is that you can no longer fetch the default value for a
> > > control, it causes a runtime error. The code in question is here:
> > > https://git.linuxtv.org/libcamera.git/tree/src/py/libcamera/py_helpers.cpp#n57
> > >
> > > There are a number of straightforward solutions, such as returning
> > > "py::none()", for example. Does that sound like a reasonable thing to
> > > do?
> >
> > Are we supposed to have uninitialized values? In that case py::none
> > sounds fine. Or is it a bug if a control has an uninitialized default?
> > Then throwing an exception, as we do now, sounds fine.
>
> Yes, I think the impact was we essentially introduced the abiltiy to
> 'not' specify a default value. There are controls, where it simply
> doesn't make sense to have a default.
>
> So I think returning py::none might be the most correct thing to do ?

Works for me. Patch incoming...

Thanks!
David

>
> --
> Kieran
>
>
> >
> >   Tomi
> >
> >
> > > Thanks!
> > > David
> > >
> > > On Tue, 30 Aug 2022 at 21:21, Christian Rauch via libcamera-devel
> > > <libcamera-devel at lists.libcamera.org> wrote:
> > >>
> > >> The default ControlInfo constructor allows to partially initialised the
> > >> min/max/def values. Uninitialised values are assigned to 0 by default. This
> > >> implicit initialisation makes it impossible to distinguish between and
> > >> uninitialised and an explicitly 0-initialised ControlValue.
> > >>
> > >> Default construct the ControlValue in the ControlInfo default contructor to
> > >> explicitly represent uninitialised values by the ControlTypeNone type.
> > >>
> > >> Signed-off-by: Christian Rauch <Rauch.Christian at gmx.de>
> > >> ---
> > >>   include/libcamera/controls.h | 6 +++---
> > >>   1 file changed, 3 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > >> index ebc168fc..38d0a3e8 100644
> > >> --- a/include/libcamera/controls.h
> > >> +++ b/include/libcamera/controls.h
> > >> @@ -268,9 +268,9 @@ private:
> > >>   class ControlInfo
> > >>   {
> > >>   public:
> > >> -       explicit ControlInfo(const ControlValue &min = 0,
> > >> -                            const ControlValue &max = 0,
> > >> -                            const ControlValue &def = 0);
> > >> +       explicit ControlInfo(const ControlValue &min = {},
> > >> +                            const ControlValue &max = {},
> > >> +                            const ControlValue &def = {});
> > >>          explicit ControlInfo(Span<const ControlValue> values,
> > >>                               const ControlValue &def = {});
> > >>          explicit ControlInfo(std::set<bool> values, bool def);
> > >> --
> > >> 2.34.1
> > >>
> >