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

[Jacopo Mondi]

Hi Naush,

On Thu, Jan 14, 2021 at 07:34:20AM +0000, Naushir Patuck wrote:
> Hi Laurent,
>
>
> On Thu, 14 Jan 2021 at 06:04, Laurent Pinchart <
> laurent.pinchart at ideasonboard.com> wrote:
>
> > Hi Naush,
> >

[snip]

> > > >
> > > > >            When reported in
> > > > > +          metadata, the control expresses the minimum and maximum
> > frame
> > > > > +          durations used after being clipped to these limits.
> > > > > +
> > > >
> > > > But this sounds weird. The previous part states that FrameDurations has
> > > > higher priority than all other parameters, but then this sentence says
> > > > it's clipped by "these limits".
> > >
> > > You are right, this does not read correct.  I wanted to express that the
> > > frame durations provided may be further limited by what the sensor mode
> > can
> > > actually achieve.  How about replacing the above paragraph of text with
> > the
> > > following:
> > >
> > > When reported in metadata, the control expresses the minimum and maximum
> > > frame durations used after being clipped to the sensor provided frame
> > > duration limits.
> >
> > Sounds good to me.
> >
> > > > > +          \sa AeExposureMode
> > > > > +          \sa ExposureTime
> > > > > +
> > > > > +          \todo Refer to the frame duration limits property to
> > describe how
> > > > > +          application-provided values gets clipped and reset.
> > > >
> > > > It hasn't been long, and the context is already starting to escape me.
> > > > Would it be possible to expand this just a little bit so that we'll
> > know
> > > > what it means in 3 months time ?
> > >
> > > Perhaps this makes more sense given the rewording above?  Or maybe a
> > reword
> > > as follows:
> > >
> > > \todo Refer to the frame duration limits property to describe how
> > > application-provided values get clipped to what is supported by the
> > sensor
> > > mode.
> > >
> > > Hopefully that makes things more readable?

I know where this last statment came from, as I had a frame durations
limit property on its way. But as it has not been finalized, and I'm
currently questioning if it is really required or not, can we drop
this last part so that the last obstacle for this series to be merged
is removed ?

Thanks
   j

> >
> > Not quite I'm afraid, but maybe it's just too early in the morning :-)
> >
> > Is this about documenting how other properties also get clipped by the
> > sensor mode ? Or something else ?
> >
>
> It's about how the frame durations are clipped by the sensor mode limits -
> as advertised by the sensor properties in the future.
> We can remove this statement entirely if you do not think it's appropriate,
> or a rewording as follows:
>
> \todo Refer to the frame duration limits property (when available) to
> obtain sensor
> mode limits used for clipping the application-provided values.
>
> Regards,
> Naush
>
>
>
> >
> > > > > +
> > > > > +          \todo Define how to calculate the capture frame rate by
> > > > > +          defining controls to report additional delays introduced
> > by
> > > > > +          the capture pipeline or post-processing stages (ie JPEG
> > > > > +          conversion, frame scaling).
> > > > > +
> > > > > +          \todo Provide an explicit definition of default control
> > values, for
> > > > > +          this and all other controls.
> > > > > +      size: [2]
> > > > > +
> > > > >    #
> > ----------------------------------------------------------------------------
> > > > >    # Draft controls section
> > > > >
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
> >

> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel