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

[David Plowman]

Hi Tomi

Thanks for those py::enum definitions - they work great, for example:
"picamera2.set_controls({"AwbMode": libcamera.AwbMode.Indoor})"

David

On Thu, 17 Mar 2022 at 11:50, Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Quoting Tomi Valkeinen (2022-03-17 10:03:25)
> > On 17/03/2022 11:55, Tomi Valkeinen 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!
> >
> > I think the core question is whether a Request is tied to a Camera. I
> > thought it is, as it is created from the Camera. And if that's the case,
> > I don't see why the Camera cannot be exposed from the Request.
>
> This is how I understand it, and the patch I have/had locally to add and
> expose this is followed by an addition of an error check to make sure
> that requests queued to a camera that were not created by that camera get
> rejected.
>
> I could probably still do that using the private class infrastructure
> now though. (re-sketching up that patch now).
>
> > Maybe Laurent can explain the reasons why not to expose it.
> >
> > > 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.
> >
> > Note that there are multiple ways to approach this. E.g.:
> >
> > req.set_control(camera.controls["Brightness"], 1)
> >
> > or:
> >
> > req.set_control(camera.controls.Brightness, 1)
> >
> > or even (although this could perhaps conflict too easily):
> >
> > req.set_control(camera.Brightness, 1)
> >
> > Or we could go the other way:
> >
> > camera.set_control(req, "Brightness", 1)
> >
> > or:
> >
> > camera.controls.Brightness.set(req, 1)
> >
> >   Tomi