[libcamera-devel] [PATCH v2 1/3] controls: Split FrameDurations into FrameDuration and FrameDurationLimits

[Jacopo Mondi]

Hi Paul, Laurent,

On Mon, May 24, 2021 at 03:13:15PM +0300, Laurent Pinchart wrote:
> Hi Paul,
>
> Thank you for the patch.
>
> On Mon, May 24, 2021 at 06:41:21PM +0900, Paul Elder wrote:
> > We need a separate control to report the nominal frame duration, but
> > it's also useful to report the min/max frame duration values that will
> > be used. Split the FrameDurations control into FrameDuration and
> > FrameDurationLimits respectively to support both of these.
>
> I'm not 100% sure yet it will be useful to report the adjusted
> FrameDurationLimits in metadata, but a separate control for the nominal
> frame duration is certainly less confusing.
>
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> > ---

[snip]

> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > index 88d81ac4..338fbdc9 100644
> > --- a/src/libcamera/control_ids.yaml
> > +++ b/src/libcamera/control_ids.yaml
> > @@ -323,7 +323,13 @@ controls:
> >          step to respect the received gain factor and shall report
> >          their total value in the request metadata.
> >
> > -  - FrameDurations:
> > +  - FrameDuration:
> > +      type: int64_t
> > +      description: |
> > +        The nominal frame duration from start of frame exposure to start of
> > +        next exposure, expressed in microseconds.

Isn't this a bit poor ? Should we at least specify this is meant to be
returned in metadata only ?


> > +
> > +  - FrameDurationLimits:
> >        type: int64_t
> >        description: |
> >          The minimum and maximum (in that order) frame duration,
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 8ae47c6d..6e17753f 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -980,9 +980,9 @@ int PipelineHandlerIPU3::initControls(IPU3CameraData *data)
> >  		frameDurations[i] = frameSize / (sensorInfo.pixelRate / 1000000U);
> >  	}
> >
> > -	controls[&controls::FrameDurations] = ControlInfo(frameDurations[0],
> > -							  frameDurations[1],
> > -							  frameDurations[2]);
> > +	controls[&controls::FrameDuration] = ControlInfo(frameDurations[0],
> > +							 frameDurations[1],
> > +							 frameDurations[2]);
> >
> >  	/*
> >  	 * Compute the scaler crop limits.
>
> --
> Regards,
>
> Laurent Pinchart