[libcamera-devel] [PATCH] ipa: ipu3: Don't call SetControls for each frame

[Laurent Pinchart]

On Fri, Feb 12, 2021 at 03:36:36PM +0000, Kieran Bingham wrote:
> On 09/02/2021 15:03, Jean-Michel Hautbois wrote:
> > Setting a default EXPOSURE and ANALOGUE_GAIN makes sense for the first
> > frame but there is no need to force it for all frames.
> > It will be called later by the 3A algorithms.
> > 
> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> > ---
> >  src/ipa/ipu3/ipu3.cpp | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> > index b11b03ef..ae7e8c0b 100644
> > --- a/src/ipa/ipu3/ipu3.cpp
> > +++ b/src/ipa/ipu3/ipu3.cpp
> > @@ -188,9 +188,6 @@ void IPAIPU3::fillParams(unsigned int frame, ipu3_uapi_params *params)
> >  	op.operation = IPU3_IPA_ACTION_PARAM_FILLED;
> >  
> >  	queueFrameAction.emit(frame, op);
> > -
> > -	/* \todo Calculate new values for exposure_ and gain_. */
> > -	setControls(frame);
> 
> Does this leave the setControls() function unused? I wonder if it should
> be removed in that case.
> 
> I hear that this patch is anticipated to be able to stop the exposure
> being set to max, without any means of setting it so I'm not opposed to
> this going in as a fix anyway.
> 
> Especially as this will all change soon enough with your other work on top.

Depending on how "soon" that would be, we could also implement support
for controls::Exposure right away, so that it could be set from
applications. This shouldn't take long, but we would however need to
also implement support for controls in qcam and cam, and that part is a
bigger piece of work. Probably not the priority right now, so this patch
should be fine.

> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> >  }
> >  
> >  void IPAIPU3::parseStatistics(unsigned int frame,
> > 

-- 
Regards,

Laurent Pinchart