[libcamera-devel] [PATCH 1/2] android: camera_device: Fix sensor frame duration

[Naushir Patuck]

Hi Jacopo and Paul,

On Fri, 21 May 2021 at 12:53, Jacopo Mondi <jacopo at jmondi.org> wrote:

> Hi Paul,
>    the Fix in subject is probably not required, as the
> ANDROID_SENSOR_FRAME_DURATION property was not registered at all.
>
> On Fri, May 21, 2021 at 07:55:33PM +0900, Paul Elder wrote:
> > The sensor frame duration should be set by IPA. Get the information for
> > the result metadata from libcamera.
>
> how about:
>
> "Libcamera reports the frame duration through the
> controls::FrameDuration controls. Populate the
> android.sensor.frameDuration result metadata using the libcamera
> control value"
>
> >
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > ---
> >  src/android/camera_device.cpp | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/src/android/camera_device.cpp
> b/src/android/camera_device.cpp
> > index b32e8be5..779ce554 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -2252,6 +2252,16 @@ CameraDevice::getResultMetadata(const
> Camera3RequestDescriptor &descriptor) cons
> >               resultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME,
> exposure);
> >       }
> >
> > +     if (metadata.contains(controls::FrameDurations)) {
> > +             Span<const int64_t> durations =
> > +                     metadata.get(controls::FrameDurations);
> > +             if (durations[0] == durations[1]) {
>
> Yes, we have a bit of an unspecified behaviour here.
>
> What is the use case for reporting two different frame durations ?
>
> Indeed I see this one:
> src/ipa/raspberrypi/raspberrypi.cpp:
> libcameraMetadata_.set(controls::FrameDurations,
> src/ipa/raspberrypi/raspberrypi.cpp:                           {
> static_cast<int64_t>(minFrameDuration_),
> src/ipa/raspberrypi/raspberrypi.cpp:
>  static_cast<int64_t>(maxFrameDuration_) });
>
> Where the RPi IPA reports the min/max frame durations received as
> controls part of a Request before using them to clamp the AE computed
> exposure time and blankings.
>
> +Naush, +David
>
> Wouldn't it be better to report the actual duration by knowing the
> actual exposure+blanking values once AGC has run ?
>

This was discussed some time back, and I think we loosely agreed that the
values
returned here in the Request metadata are the min/max frame duration values
that
are actually going to be used, as the may have been clipped based on sensor
limitations.  Note that this gets put into the metadata only when a user
request comes
through, and not on every frame.  Here is the description I worded in the
yaml file:

> When reported in metadata, the control expresses the minimum and maximum
frame
> durations used after being clipped to the sensor provided frame duration
limits.

Our IPA does not return controls::FrameDurations per-frame, instead we
return
controls::SensorTimestamp that (almost) gives you the inter-frame duration
after
exposure+blanking.

Regards,
Naush



>
> Thanks
>    j
>
> > +                     int64_t duration = durations[0] * 1000;
> > +
>  resultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,
> > +                                              duration);
> > +             }
> > +     }
> > +
> >       if (metadata.contains(controls::ScalerCrop)) {
> >               Rectangle crop = metadata.get(controls::ScalerCrop);
> >               int32_t cropRect[] = {
> > --
> > 2.27.0
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210521/653ccbce/attachment.htm>