[libcamera-devel] [PATCH 03/11] Adds rkisp1 metadata required for the Android HAL to use it.

[Nicholas Roth]

It looks like indeed we have a way to call updateControls() at the
appropriate places, now that I've looked at the ISP code more closely.

It seems like we have at least the following options:
1 Compute the durations the way IPU3 does it (probably wrong)
2 Insert reasonable values (that sometimes might not work) and document
this in a bug
3 Do nothing, document as a bug that RKISP1 does not work as an Android HAL
without a special patch, including my suggested defaults there in the bug
and the "how to get this working on Android" guide.

Personally, I advocate for (2). Let me know what you think.

-Nicholas


On Wed, Oct 26, 2022 at 4:02 AM Kieran Bingham <
kieran.bingham at ideasonboard.com> wrote:

> Quoting Nicholas Roth (2022-10-26 05:40:11)
> > +Kieran Bingham <kieran.bingham at ideasonboard.com> since Laurent is out.
> >
> > > Could this be computed the same way we do it in the IPU3 IPA module ?
> >
> > It looks like each of the computed IPU3 FrameDurationLimit values
> > correspond to the minimum and maximum v4l2 blanking intervals. I suppose
> > that makes sense, though I suspect the minimum frame duration will be
> > constrained by i2c bandwidth and ISP limitations at the highest
> resolutions
> > rather than the sensor blanking intervals based on the datasheet.
> >
> > I'm not really sure what the best thing to do here is. I'm concerned that
> > if we follow the IPU3 computation, we could easily end up asking for the
> > ISP to push more pixels than it has throughput for. A very conservative
> > approach might be to cap this at a few FPS, which is the advertised rate
> at
> > the highest resolution, but I would not enjoy using the camera that way.
> > Curious to read others' thoughts on this.
> >
> > As a side-note, I'm really not sure why the IPU3 IPA treats
> > FrameDurationLimits as a 3-tuple, when control_ids.yaml explicitly
> states:
> >       description: |
> >         The minimum and maximum (in that order) frame duration,
> >         expressed in microseconds.
> > ...
> >       size: [2]
>
> Uhhh - yes, the code over at IPAIPU3::updateControls() seems quite ...
> odd. I'm not sure it's correct, it could be left over from some early
> development. I'll add it to a task to take a look at.
>
> > It looks like updateControls is only called once, during init(). We would
> > need a hook that changes this static configuration for each video mode.
>
> I believe it's supposed to be called after / during configure() too when
> modes are updated. If it's missing, there's likely a bug there.
>
>
>
> > On Tue, Oct 25, 2022 at 10:16 AM Nicholas Roth <nicholas at rothemail.net>
> > wrote:
> >
> > > > Careful, we're claiming full per-frame control capabilities with this
> > > set to 0 :)
> > >
> > > Good catch— I’ll revert this line. I couldn’t easily find this in the
> ISP
> > > manual and wanted to see what we’d need for FULL instead of LIMITED
> support
> > > while I was debugging a camera2 crash.
> > >
> > > > On Oct 25, 2022, at 8:20 AM, Jacopo Mondi <jacopo at jmondi.org> wrote:
> > > >
> > > > Careful, we're claiming full per-frame control capabilities with this
> > > > set to 0 :)
> > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20221026/7610a10e/attachment.htm>