[libcamera-devel] [PATCH v5 03/10] ipa: add rkisp1 metadata to fix Android HAL

[Naushir Patuck]

Hi all,


On Fri, 28 Oct 2022 at 10:09, Jacopo Mondi via libcamera-devel <
libcamera-devel at lists.libcamera.org> wrote:

> Hi Nicholas
>
> subject as
>
> ipa: rkisp1: Add FrameDurationLimits control
>
> This is not just for Android
>
> On Thu, Oct 27, 2022 at 10:17:19PM -0500, Nicholas Roth via
> libcamera-devel wrote:
> > From: Nicholas Roth <nicholas at rothemail.net>
> >
> > Currently, the Android HAL does not work on rkisp1-based devices because
> > required FrameDurationLimits metadata is missing from the ISP
>
> IPA = Image Processing Algorithm
> it's the software component part of libcamera that drives the image
> enhancement algorithms. IPA is libcamera lingo, in other context it
> might be simply called "3A library" even if it does way more than just
> Auto[exposure, white-balance, focus].
>
> ISP = Image Signal Processor
> it's the HW component that performs image enhancement computations and
> produces statistics on the processed image
>
> The two terms are not interchangeable
>
> > implementation.
> >
> > This change introduces sensible defaults for FrameDurationLimits that
> > should generally work most of the time.
> >
> > In theory, it should be possible to implement more sophisticated logic
> > to detect frame durations. The appropriate API hooks for doing so exist
> > and are used in IPAIPU3::updateControls(), although the implementation
> > may be suspect. This is left as future work.
> >
> > Signed-off-by: Nicholas Roth <nicholas at rothemail.net>
> > ---
> >  src/ipa/rkisp1/rkisp1.cpp | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> > index ba3c547e..dd12c341 100644
> > --- a/src/ipa/rkisp1/rkisp1.cpp
> > +++ b/src/ipa/rkisp1/rkisp1.cpp
> > @@ -100,6 +100,12 @@ const ControlInfoMap::Map rkisp1Controls{
> >       { &controls::Contrast, ControlInfo(0.0f, 1.993f) },
> >       { &controls::Saturation, ControlInfo(0.0f, 1.993f) },
> >       { &controls::Sharpness, ControlInfo(0.0f, 10.0f, 1.0f) },
> > +     /* libcamera requires a fixed value for minimum frame duration,
> > +      * but this depends on the frame size and the rkisp1 device
> datasheets
> > +      * measure this in pixels per second. Neither the datasheets nor
> the driver
>
> What are you referring to as "the rkisp1 device datasheet measures
> this in pixels per second" ? Is it the ISP bandwidth ? Where is this
> information available from ?
>
> I'm still of opinion that we should report the sensor's duration
> limits, but maybe I'm missing a point here.
>
>
> > +      * specify a maximum. The minimum below is for 1920x1920. The
> maximum
>
> Is 1920x1920 is the RkISP1 self-path maximum output resolution that
> you have used ? If the self-path has a scaler (something I'm not 100%
> sure) images in such resolution might be obtained by scaling the frame
> as produced by the sensor, or by simply cropping them.
>
> If you connect to the ISP a sensor whose minimum resolution and
> max framerate is 2592x1944 at 15FPS you would get a min frame duration of:
>
>                 (2592 + hblank) * (1944 + vblank) / pixel_rate  = 66666
> usec
>
> which is larger than what you report here, as the sensor would be
> clocking out frames at such rate, regardless of the final output
> resolution. Moreover I'm not sure how you got to 48505 from 1920x1920,
> have you taken into account the pixel rate of the sensor you're using ?
> This can't be hardcoded but should come from the CameraSensor,
> different platforms use different sensors and we can't assume anything
> about it in the IPA module.
>
> I would say that the ISP output size limits should not be taken into
> account. It would be different if we had a clear documentation about
> how bandwidth limitations constraints the durations, but I'm afraid
> the relationship between the two parameters is hard to measure, if
> ever documented.
>

I agree with Jacopo on this - FrameDurationLimits ought to specify the
sensor
limits and not the ISP.  I can't speak for other platforms, but on a
Raspberry Pi the ISP throughput depends on many many things, such as
pipeline configuration, hardware clock rate, memory bandwidth availability
to
name a few.  It would be wholly impractical to provide such a number in the
FrameDurationLimits control.

Naush



>
> > +      * corresponds to two seconds. */
>
> Multi-line comments should be
>
>         /*
>          * comment on multiple
>          * lines
>          */
>
> > +     { &controls::FrameDurationLimits, ControlInfo(48505, 2000000) },
> >       { &controls::draft::NoiseReductionMode,
> ControlInfo(controls::draft::NoiseReductionModeValues) },
> >  };
> >
> > --
> > 2.34.1
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20221028/8623b785/attachment.htm>