[libcamera-devel] [PATCH 1/3] libcamera: stream: Add controlList attribute

[Jacopo Mondi]

Hi Marco,

On Wed, Apr 28, 2021 at 03:47:05PM +0200, Marco Felsch wrote:
> Hi Jacopo,
>
> On 21-03-22 18:43, Jacopo Mondi wrote:
> > Hi Marian,
> >
> > On Mon, Mar 22, 2021 at 11:42:40AM +0100, Marian Cichy wrote:
> > > By adding a controlList to the stream attributes, we have access to
> > > several more configuration parameters for a stream. In this patch
> > > series, it is for now only used to access the frame duration of a sensor.
> > >
> > > Signed-off-by: Marian Cichy <m.cichy at pengutronix.de>
> > > ---
> > >  include/libcamera/stream.h | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> > > index bb47c390..a6347e31 100644
> > > --- a/include/libcamera/stream.h
> > > +++ b/include/libcamera/stream.h
> > > @@ -13,6 +13,7 @@
> > >  #include <vector>
> > >
> > >  #include <libcamera/buffer.h>
> > > +#include <libcamera/controls.h>
> > >  #include <libcamera/geometry.h>
> > >  #include <libcamera/pixel_format.h>
> > >
> > > @@ -44,6 +45,7 @@ struct StreamConfiguration {
> > >  	Size size;
> > >  	unsigned int stride;
> > >  	unsigned int frameSize;
> > > +	ControlList controls;
> >
> > Documentation is missing.
> >
> > In general, as discussed on irc, I guess this is an acceptable first
> > step to be able to retrieve per-stream information.
>
> I picked up Marian's work to become familar with libcamera and now I
> have my first question on this topic.

Welcome on board :)

>
> Why is this necessary here? It think what we wanna do (retrieve the
> framerate) can be done by the pipeline_handler ControlInfoMap. So we
> only need to extended the simple-pipeline rather than adding it here,
> like all other pipeline handlers does.

Do you mean populating the Camera::contorls() info maps ?

I tried to explain my understanding in my last reply to patch [2/3]
https://patchwork.libcamera.org/patch/11639/

I'll try to summarize it here:

- The current frame duration (as well as other parameters) depend on
  the currently applied configuration as they depend on what sizes/formats
  are requested and how the sensor is configured.
- The ControlInfoMap instances access through Camera::controls() are
  initialized at pipeline creation time. We have to pick and euhristic
  to decide which sensor configuration to use, as the Camera's ControlInfoMap
  instances are populated using (also) the sensor's v4l2 controls, whose values
  and limits depend on the current configuration [*]
- Ideally, we would need a mechanism to update the Camera's
  ControlInfoMaps when a new configuration is applied. It's a bit of
  work, and would need to be a mchanism that all pipeline handler can
  use [**]
- As we don't have the above mechanism in place at the moment, the
  easiest way to do allow libcamera to expose to applications
  information that are configuration-dependent is to augment the
  CameraConfiguration (or StreamConfiguration as in this case) with a
  list of controls that report the current values
- The 'issue' with this series is that Marian wanted to do at
  Camera::generateConfiguration() time, which for the reasons
  I tried to outline in https://patchwork.libcamera.org/patch/11639/
  is not really possible.

So yes, we're missing a mechanism to
1) Update the Camera::controls() when a Configuration is applied to
the Camera
2) It could be worked around by adding controls to the
configuration-dependent components (CameraConfiguration and
StreamConfiguration)
3) However this can only be done after a configuration has been
applied and the sensor driver has been configured and its controls
values and limits updated.

Maybe there's an easier way out I might have missed, what do you think ?

Thanks
   j

[*] The most notable example is the VBLANK limits, that change
depending on the selected sensor's output sizes

[**] RPi in example has a series that would allow to update the V4L2
controls on format change https://patchwork.libcamera.org/patch/11797/
This is a first step. With an updated v4l2 control info, also the
Camera's controls info can be updated.
>
> Am I right or did I miss something?
>
> Regards,
>   Marco
>
>
> >
> > >
> > >  	unsigned int bufferCount;
> > >
> > > --
> > > 2.29.2
> > >
> > > _______________________________________________
> > > libcamera-devel mailing list
> > > libcamera-devel at lists.libcamera.org
> > > https://lists.libcamera.org/listinfo/libcamera-devel
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
> >
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |