[libcamera-devel] [PATCH v2 1/3] libcamera: controls: Add frame duration control

[Jacopo Mondi]

Hi Naush, Laurent,

On Mon, May 25, 2020 at 01:23:02AM +0300, Laurent Pinchart wrote:
> Hi Naush,
>
> Thank you for the patch.
>
> On Wed, May 13, 2020 at 10:11:18AM +0100, Naushir Patuck wrote:
> > Add a float array control (controls::FrameDurations) to specify the
> > minimum and maximum (in that order) frame duration to be used by the
> > camera sensor.
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> >  src/libcamera/control_ids.yaml | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > index 77ebc3f9..42694fc7 100644
> > --- a/src/libcamera/control_ids.yaml
> > +++ b/src/libcamera/control_ids.yaml
> > @@ -239,4 +239,18 @@ controls:
> >          pixel range (as if pixels ranged from 0 to 65535). The SensorBlackLevels
> >          control can only be returned in metadata.
> >        size: [4]
> > +
> > +  - FrameDurations:
> > +      type: float
>
> Do we need sub-microsecond precision, or would a uint32_t control work ?
> I'd rather have a fixed precision if possible.
>
> > +      description: |
> > +        Specifies the minimum and maximum (in that order) allowable frame
> > +        duration, in micro-seconds, for the sensor to use. This could also limit
> > +        the largest exposure times the sensor can use. For example, if a maximum
> > +        frame duration of 33ms is requested (corresponding to 30 frames per
> > +        second), the sensor will not be able raise the exposure time above 33ms.

Isn't it s/maximum frame duration/minimum frame duration/ ?

If at least 30fps are requested, the exposure time cannot be longer
than 33msec ? Am I reading this wrong ?

> > +        Note that the sensor may not always be able to provide the requested
> > +        frame duration limits depending on its mode configuration.

I've been really confused by this, until I realized I was having an
hard time trying to figure out how to use this for applications to
express a request for a precise duration and how should the library
have used it to report the actual frame durtion in metadata. Until I
realized this could probably be better interpreted not as a frame
duration request or result, but as an hint to the AE algorithm to
clamp its calculation into certain duration limtis.

And I agree a duration range might be a good hint to drive the AE
routine and if I got the rest of the series right, that's
what happens in the IPA with these patches.

If that's actually the intention, what about making of this an
"AeFrameDurationLimits" control, to make it clear it applies to AE and
use "FrameDuration" as an integer control to represent the requested
and precise frame duration in requests and metadata respectively.

On "FrameDuration" itself: would it make sense to only considered it
when running with AE off ? In that case it would be applications
responsibility to opportunely calculate the exposure time and the frame
duration, but I think it's expected if you run in manual mode. And for
reference, that's what Android does as well :)

It would also rule out the need to specify how the frameDurationLimits
would be used when running in manual mode and wanting to configure a
precise value there, as I guess you would have to give to min and max
the same value to express that.

this might open pandora's box, but: is the frame duration a property of
the camera, or of a single stream ?

>
> This looks good to me, but I'd like to discuss the corner cases I can
> already think about.
>
> - Are there use cases for an application to specify a minimum frame
>   duration only, or a maximum frame duration only (that is, any frame
>   rate lower than a limit, or any frame rate higher than a limit) ? If
>   so, how should that be specified ? We could set the minimum value to 0
>   to mean that the maximum frame rate is unbounded, and the maximum
>   value to UINT32_MAX to mean that the minimum frame rate is unbounded.
>   Is there anything I'm overlooking ?

If this becames an AE-related control and you specify that, I think
you should provide both values. If you don't care about one of the two
just use the values of the (forthcoming) property that reports the
min and max frame durations of a camera. If an applications does not
specify this control, the AE routine runs free, in the limits
of the camera sensor capabilities.

Does it make sense ?

Thanks
  j

>
> - Is 0 an acceptable value ? Or should 1 be the minimum value ?
>
> - What happens if the requested frame duration isn't achievable ? Should
>   we specify that the camera will use a frame duration as close to the
>   requested range as possible ? Or could there be cases where a
>   different behaviour would be needed ?
>
> - Not something we need to address now, but do you see any future
>   relation between this control and anti-banding (50 or 60Hz flicker
>   avoidance) ? Anti-banding will mostly restrict possible exposure
>   times, and should only indirectly interact with the frame duration if
>   I'm not mistaken, is that correct ?
>
> > +
> > +        \sa ExposureTime
> > +      size: [2]
> >  ...
>
> --
> Regards,
>
> Laurent Pinchart
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel