<div dir="ltr"><div dir="ltr">Hi Laurent and Umang,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 2 Jun 2021 at 19:10, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@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">Hi Umang,<br>
<br>
On Wed, Jun 02, 2021 at 03:57:37PM +0530, Umang Jain wrote:<br>
> On 6/2/21 6:38 AM, Laurent Pinchart wrote:<br>
> > On Tue, Jun 01, 2021 at 04:54:56PM +0530, Umang Jain wrote:<br>
> >> We report a hard-coded frame duration value for CTS. However, when the<br>
> >> CTS is tested on nautilus, which has a 'imx258' sensor, the frame<br>
> >> duration was found to be out-of-range. Since different sensors have<br>
> >> different frame durations range-limits, we need to bump up our value<br>
> >> to match the lower end of the 'imx258' frame duration range-limit<br>
> >> (rounded up to the nearest micro-second).<br>
> >><br>
> >> This allows the following CTS test to pass on nautilus platform:<br>
> >>   - android.hardware.camera2.cts.CaptureRequestTest#testNoiseReductionModeControl<br>
> >><br>
> >> Signed-off-by: Umang Jain <<a href="mailto:umang.jain@ideasonboard.com" target="_blank">umang.jain@ideasonboard.com</a>><br>
> >> ---<br>
> >>   src/ipa/ipu3/ipu3.cpp | 2 +-<br>
> >>   1 file changed, 1 insertion(+), 1 deletion(-)<br>
> >><br>
> >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp<br>
> >> index 700a5660..e51b0401 100644<br>
> >> --- a/src/ipa/ipu3/ipu3.cpp<br>
> >> +++ b/src/ipa/ipu3/ipu3.cpp<br>
> >> @@ -271,7 +271,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,<br>
> >>   <br>
> >>    /* \todo Populate this with real values */<br>
> >>    ctrls.set(controls::FrameDuration,<br>
> >> -            static_cast<int64_t>(33334));<br>
> >> +            static_cast<int64_t>(34483));<br>
> ><br>
> > I think it's time to fix this properly by calculating the actual frame<br>
> > duration. It should hopefully not be more complicated than<br>
> ><br>
> >     duration = (width + hblank) * (height + vblank) / pixel_rate;<br>
><br>
> We need to get vblank, for each frame's exposure I think. We need to <br>
> plumb this into IPAIPU3 and JM seem to have the base helper class <br>
> in-progress. For now, I have slightly made an improvement by using the <br>
> minVBlank_ value instead. It doesn't makes CTS on nautilus pass, but <br>
> anyway is an improvement over hard-coded value. I'll check with Paul, to <br>
> see if it doesn't regress CTS on soraka. Let me know you thoughts!<br>
<br>
V4L2_CID_VBLANK is set by userspace. It can be updated by drivers when<br>
the format and/or selection rectangles are modified, but will not<br>
otherwise be changed automatically at runtime. The IPA should compute<br>
the desired vblank value and pass it to the pipeline handler to<br>
configure the sensor. I don't think that's related to the work that JM<br>
is doing.<br></blockquote><div><br></div><div>Sorry for jumping in on this topic, but for RPi IPA, it is possible for VBLANK to</div><div>change at runtime when we are running in variable framerate mode.  This</div><div>decision is down to the AGC and what exposure it decides to use.</div><div><br></div><div>This is also where we run into the problem with V4L2 control limits not allowing</div><div>us to update VBLANK and EXPOSURE at the same time without using</div><div>priority writes.</div><div><br></div><div>Naush</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
> >>   <br>
> >>    IPU3Action op;<br>
> >>    op.op = ActionMetadataReady;<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>