[libcamera-devel] [PATCH v5 0/3] Python bindings

[David Plowman]

Hi again

On Thu, 17 Mar 2022 at 10:41, Tomi Valkeinen
<tomi.valkeinen at ideasonboard.com> wrote:
>
> On 17/03/2022 12:12, David Plowman wrote:
> > Hi again
> >
> > Just to answer the about about the "enum" type controls:
> >
> > On Thu, 17 Mar 2022 at 09:55, Tomi Valkeinen
> > <tomi.valkeinen at ideasonboard.com> wrote:
> >>
> >> On 17/03/2022 11:44, David Plowman wrote:
> >>> Hi everyone
> >>>
> >>> Thanks for the update!
> >>>
> >>> On Mon, 14 Mar 2022 at 15:46, Tomi Valkeinen
> >>> <tomi.valkeinen at ideasonboard.com> wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>> This is v5 of the series. The changes to v4:
> >>>>
> >>>> - Changed to pybind11 'smart_holders' branch which allows us to drop the
> >>>>     HACK for Camera public destructor
> >>>> - Changed Request.set_control() so that we can drop the HACK for
> >>>>     exposing Camera from Request
> >>>> - Moved Python enum defs to a separate file for clarity
> >>>> - "Forward" declare Python classes to fix docstring issues.
> >>>>
> >>>> The set_control change breaks the current users of that function.
> >>>> Previously you could do this:
> >>>>
> >>>> req.set_control("Brightness", 1)
> >>>>
> >>>> Now you need to do:
> >>>>
> >>>> cid = camera.find_control("Brightness")
> >>>> req.set_control(cid, 1)
> >>>>
> >>>
> >>> I see the need for this, and I guess it's fine - obviously we'll hide
> >>> that under our Picamera2 API as we wouldn't want our users to have to
> >>> do that!
> >>
> >> Yes, it's not very nice. Then again, having to do a string search every
> >> time we call set_control is not good either, at least if we can avoid it
> >> (and we can, as above). But I agree that in a higher level API things
> >> should be easy. Of course, it would be nice to have both versions even
> >> in this low level API, but... if Request doesn't expose camera, I don't
> >> think we can do it in a simple way.
> >>
> >>> There is one further thing I wanted to raise in relation to the Python
> >>> bindings for controls, now that a few folks are trying out Picamera2.
> >>>
> >>> I've had feedback that they don't like the fact that all the "enum"
> >>> control values (exposure modes, AWB modes, that kind of thing) are
> >>> simple integers up in the Python world. They complain that this makes
> >>> them quite unfriendly to use, and I'm inclined to agree! We could
> >>> always create Python-level definitions for them, and possibly add code
> >>> to convert stuff on the fly, but I wonder if Pybind11 can do anything
> >>> more automatic for us?
> >>
> >> Can you elaborate? Which enums are these?
> >
> > So users would prefer to write something like
> >
> > picamera2.set_controls({"AwbMode": AwbMode.Indoor})
> >
> > rather than
> >
> > picamera2.set_controls({"AwbMode": 4})
> >
> > The same applies to all the controls of this type, there including
> >
> > AeMeteringMode
> > AeConstraintMode
> > AeExposureMode
> > AwbMode
> > NoiseReductionMode
> > there will at some point be an AfMode and probably other AF ones
> > and so on.
> >
> > Basically it looks to be mostly anything with "Mode" on the end,
> > though I think we will encounter some others too. Also anything with
> > "State" on the end is probably similar, though in this case we're
> > talking about reported values, not ones you can set.
>
> Looks like these are defined in src/libcamera/control_ids.yaml, so we
> could have a script that generates pybind11 code to create the enums. Or
> we could just do it by hand.

Defining them by hand is fine with me, at least for the time being. I
just wonder whether it would get annoying in the longer run, so some
kind of script in the build system might be helpful.

>
> Is that your question, how and where to define the python enums? Or do
> you think there's some issue using those?

I don't think there's any problem, I guess I'm just flagging that
there's something we probably need to do. I could define them all in
Picamera2, but it seems like everyone using these Python bindings
would appreciate them, so they probably belong in libcamera somewhere.

Thanks!
David

>
>   Tomi