[libcamera-devel] [PATCH 2/2] ipa: ipu3: Bump frame duration value

[Laurent Pinchart]

Hi Umang,

On Wed, Jun 02, 2021 at 03:57:37PM +0530, Umang Jain wrote:
> On 6/2/21 6:38 AM, Laurent Pinchart wrote:
> > On Tue, Jun 01, 2021 at 04:54:56PM +0530, Umang Jain wrote:
> >> We report a hard-coded frame duration value for CTS. However, when the
> >> CTS is tested on nautilus, which has a 'imx258' sensor, the frame
> >> duration was found to be out-of-range. Since different sensors have
> >> different frame durations range-limits, we need to bump up our value
> >> to match the lower end of the 'imx258' frame duration range-limit
> >> (rounded up to the nearest micro-second).
> >>
> >> This allows the following CTS test to pass on nautilus platform:
> >>   - android.hardware.camera2.cts.CaptureRequestTest#testNoiseReductionModeControl
> >>
> >> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> >> ---
> >>   src/ipa/ipu3/ipu3.cpp | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> >> index 700a5660..e51b0401 100644
> >> --- a/src/ipa/ipu3/ipu3.cpp
> >> +++ b/src/ipa/ipu3/ipu3.cpp
> >> @@ -271,7 +271,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,
> >>   
> >>   	/* \todo Populate this with real values */
> >>   	ctrls.set(controls::FrameDuration,
> >> -		  static_cast<int64_t>(33334));
> >> +		  static_cast<int64_t>(34483));
> >
> > I think it's time to fix this properly by calculating the actual frame
> > duration. It should hopefully not be more complicated than
> >
> > 	duration = (width + hblank) * (height + vblank) / pixel_rate;
>
> We need to get vblank, for each frame's exposure I think. We need to 
> plumb this into IPAIPU3 and JM seem to have the base helper class 
> in-progress. For now, I have slightly made an improvement by using the 
> minVBlank_ value instead. It doesn't makes CTS on nautilus pass, but 
> anyway is an improvement over hard-coded value. I'll check with Paul, to 
> see if it doesn't regress CTS on soraka. Let me know you thoughts!

V4L2_CID_VBLANK is set by userspace. It can be updated by drivers when
the format and/or selection rectangles are modified, but will not
otherwise be changed automatically at runtime. The IPA should compute
the desired vblank value and pass it to the pipeline handler to
configure the sensor. I don't think that's related to the work that JM
is doing.

> >>   
> >>   	IPU3Action op;
> >>   	op.op = ActionMetadataReady;

-- 
Regards,

Laurent Pinchart