<div dir="ltr"><div><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div dir="ltr"><div style="font-size:12.8px"><div style="font-size:12.8px"><div style="color:rgb(136,136,136);font-size:small"><font face="arial, helvetica, sans-serif" color="#000000">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.</font></div><div style="color:rgb(136,136,136);font-size:small"><font face="arial, helvetica, sans-serif" color="#000000"><br></font></div><div style="color:rgb(136,136,136);font-size:small"><font face="arial, helvetica, sans-serif" color="#000000">It seems like we have at least the following options:</font></div><div style="color:rgb(136,136,136);font-size:small"><font face="arial, helvetica, sans-serif" color="#000000">1 Compute the durations the way IPU3 does it (probably wrong)</font></div><div style="color:rgb(136,136,136);font-size:small"><font face="arial, helvetica, sans-serif" color="#000000">2 Insert reasonable values (that sometimes might not work) and document this in a bug</font></div><div style="color:rgb(136,136,136);font-size:small"><font face="arial, helvetica, sans-serif" color="#000000">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.</font></div><div style="color:rgb(136,136,136);font-size:small"><font face="arial, helvetica, sans-serif" color="#000000"><br></font></div><div style="font-size:small"><font color="#000000" face="arial, helvetica, sans-serif">Personally, I advocate for (2). Let me know what you think.</font></div><div style="font-size:small"><font color="#000000" face="arial, helvetica, sans-serif"><br></font></div><div style="font-size:small"><font color="#000000" face="arial, helvetica, sans-serif">-Nicholas</font></div></div></div></div></div></div></div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Oct 26, 2022 at 4:02 AM Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Quoting Nicholas Roth (2022-10-26 05:40:11)<br>
> +Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank">kieran.bingham@ideasonboard.com</a>> since Laurent is out.<br>
> <br>
> > Could this be computed the same way we do it in the IPU3 IPA module ?<br>
> <br>
> It looks like each of the computed IPU3 FrameDurationLimit values<br>
> correspond to the minimum and maximum v4l2 blanking intervals. I suppose<br>
> that makes sense, though I suspect the minimum frame duration will be<br>
> constrained by i2c bandwidth and ISP limitations at the highest resolutions<br>
> rather than the sensor blanking intervals based on the datasheet.<br>
> <br>
> I'm not really sure what the best thing to do here is. I'm concerned that<br>
> if we follow the IPU3 computation, we could easily end up asking for the<br>
> ISP to push more pixels than it has throughput for. A very conservative<br>
> approach might be to cap this at a few FPS, which is the advertised rate at<br>
> the highest resolution, but I would not enjoy using the camera that way.<br>
> Curious to read others' thoughts on this.<br>
> <br>
> As a side-note, I'm really not sure why the IPU3 IPA treats<br>
> FrameDurationLimits as a 3-tuple, when control_ids.yaml explicitly states:<br>
>       description: |<br>
>         The minimum and maximum (in that order) frame duration,<br>
>         expressed in microseconds.<br>
> ...<br>
>       size: [2]<br>
<br>
Uhhh - yes, the code over at IPAIPU3::updateControls() seems quite ...<br>
odd. I'm not sure it's correct, it could be left over from some early<br>
development. I'll add it to a task to take a look at.<br>
<br>
> It looks like updateControls is only called once, during init(). We would<br>
> need a hook that changes this static configuration for each video mode.<br>
<br>
I believe it's supposed to be called after / during configure() too when<br>
modes are updated. If it's missing, there's likely a bug there.<br>
<br>
<br>
<br>
> On Tue, Oct 25, 2022 at 10:16 AM Nicholas Roth <<a href="mailto:nicholas@rothemail.net" target="_blank">nicholas@rothemail.net</a>><br>
> wrote:<br>
> <br>
> > > Careful, we're claiming full per-frame control capabilities with this<br>
> > set to 0 :)<br>
> ><br>
> > Good catch— I’ll revert this line. I couldn’t easily find this in the ISP<br>
> > manual and wanted to see what we’d need for FULL instead of LIMITED support<br>
> > while I was debugging a camera2 crash.<br>
> ><br>
> > > On Oct 25, 2022, at 8:20 AM, Jacopo Mondi <<a href="mailto:jacopo@jmondi.org" target="_blank">jacopo@jmondi.org</a>> wrote:<br>
> > ><br>
> > > Careful, we're claiming full per-frame control capabilities with this<br>
> > > set to 0 :)<br>
> ><br>
</blockquote></div>