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

[Naushir Patuck]

Hi Jacopo,


On Tue, 19 Jan 2021 at 15:15, Jacopo Mondi <jacopo at jmondi.org> wrote:

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

Yes, I am fine with removing this todo statement if this property is not
confirmed to be introduced.  I will post an update with it removed, as well
as the previous wording changes as discussed with Laurent.  Hopefully it
should then be good for merging.

Regards,
Naush



>
> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210119/cf2e7b99/attachment-0001.htm>