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

[Jacopo Mondi]

Hi Laurent,

On Thu, May 28, 2020 at 03:55:41AM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Wed, May 27, 2020 at 11:47:34PM +0200, Jacopo Mondi wrote:
> > On Mon, May 25, 2020 at 01:23:02AM +0300, Laurent Pinchart wrote:
> > > 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 ?
>
> I may read it wrong, but I think Naush is correct here. If a *maximum*
> frame duration of 33ms is requested [that is, a *minimum* frame rate of
> 30fps], the sensor will not be able to *raise* the exposure time above
> 33ms.

Ouch, correct, sorry for the noise.

>
> > > > +        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.
>
> I think (Ae)FrameDurationLimits makes sense as a control (and as a
> property too, with the caveat that the minimum frame duration - highest
> frame rate - can be dependent on the stream resolution), and
> FrameDuration makes sense for metadata.
>
> More about FrameDuration as a control below.
>
> > 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 ?
>
> I've opened the same box in my previous e-mail :-) I think the frame
> duration is a property of the camera, as all streams share the same
> sensor, and are thus slave to the same frame rate. Some streams could
> drop frames, but I think I would then report that as a decimation factor
> per stream (not sure what the use cases would be though). This being

I was actually more concerned about processing that might stall the
pipeline, like JPEG encoding.

> said, the maximum frame rate (minimum frame duration) can depend on the
> configuration of the camera, due to bandwidth limitations. It shouldn't
> affect the (Ae)FrameDuration(Limits) control or metadata, but will
> affect the (Ae)FrameDurationLimits property. It's something we need to
> address eventually, not necessarily as part of this series, we could
> start with a limit that doesn't depend on the camera configuration and
> then build additional features on top.
>

I might actually need to fill that gap very soon as I need a way to
report the per-configuration frame duration interval. This is
something not part of this series, but it's good if we at least
stabilize on naming.

> > > 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 ?
>
> Yes that can work. Specifying anything below the minimum possible or
> above the maximum possible value supported by the camera will be clamped
> to the camera limits anyway.
>

True, we can clamp to those limits indeed

> > > - 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