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

[Laurent Pinchart]

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.

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

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

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