[libcamera-devel] [PATCH v2] Add enable_auto_focus option to the GStreamer plugin

[Nicolas Dufresne]

Le lundi 05 juin 2023 à 19:04 +0300, Laurent Pinchart a écrit
> > 
[...]

> > > Nicolas, is there any particular rule when it comes to naming properties
> > > for GStreamer elements ?
> > 
> > The GLib part of it is:
> > 
> >    A property name consists of one or more segments consisting of ASCII letters and
> >    digits, separated by either the - or _ character. The first character of a
> >    property name must be a letter. These are the same rules as for signal naming
> >    (see g_signal_new()).
> > 
> > I strongly prefer - instead of _. Generally speaking, I would like this
> > discussion to happen before we start adding controls. One of the thing I was
> > considering was to auto-generate the propertly from the yaml,
> 
> Seems like we have the same goal :-) I've just replied to your other
> e-mail suggesting this.
> 
> Another option would be to create the mappings dynamically, based on the
> information reported by the camera at runtime.

You can't add/remove/modify GObject properties at run-time. The properties are
considered part of the API and should be stable.

The alternative would be to generate per camera elements, as used for CODECs,
but this currently only works for hardwired hardware and would not work well for
hot-plug capable cameras. This would need lots more infrastructure into
GStreamer, and for this reason, I would refrain from going that direction.

> 
> > and clearly this
> > one is not a 1:1 mapping to the libcamera controls. But at the same time, there
> > is bunch of needed exception for semantic reasons. The yaml is not descriptive
> > enough to configure fully the properties unfortunately.
> 
> We can extend the yaml file with additional information if needed,
> and/or extend the information reported by the camera at runtime
> (assuming GStreamer wouldn't have unreasonable requirements of course
> :-)).

Currently known issues are arrays, which can possibly be exposed as lists
properties, but aren't as nice to use. Based on current usage of arrays, a
simple mapping like -red/-blue/-red would look nicer. To be discussed.

Ranges are not all well exposed. I think the idea is that range are per camera,
but this isn't valid for GObject properties.

There is probably more, but I cannot remember at the moment.

> 
> > As its easier to express what I'm hoping for with code, here's my own WIP, which
> > is much more C++ like then this one. I'm missing the AutoFocusMode.
> > 
> > https://gitlab.collabora.com/nicolas/libcamera/-/commit/83d67744ccbd60ad67d2afc70be7f2d95032e4d7
> 
> Nice ! Thank you for working on that. I'll try to give it a look ASAP.
> 
> > > 

Its a pleasure. Cedric, for your interest, this does not means in anyway we'll
reject your changes. I think the only changes I'd suggest is to use a enum, and
expose auto-focus-mode with the 2 modes you currently support. This way we won't
break our users application as soon as we have these generalized.

Nicolas