[libcamera-devel] [PATCH v2 03/14] ipa: ipu3: Use sensor controls to update frameContext

[Kieran Bingham]

Quoting Jean-Michel Hautbois (2021-11-10 22:10:54)
> Hi Kieran,
> 
> On 10/11/2021 22:50, Kieran Bingham wrote:
> > Hi JM,
> > 
> > Quoting Jean-Michel Hautbois (2021-11-10 19:58:50)
> >> The pipeline handler populates the new sensorControls ControlList, to
> >> have the effective exposure and gain values for the current frame. This
> >> is done when a statistics buffer is received.
> >>
> >> Make those values the frameContext::agc values for the frame when the
> >> EventStatReady event is received.
> >>
> >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> >> ---
> >>   src/ipa/ipu3/ipu3.cpp | 4 ++++
> >>   1 file changed, 4 insertions(+)
> >>
> >> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> >> index bcc3863b..1111f200 100644
> >> --- a/src/ipa/ipu3/ipu3.cpp
> >> +++ b/src/ipa/ipu3/ipu3.cpp
> >> @@ -549,6 +549,10 @@ void IPAIPU3::processEvent(const IPU3Event &event)
> >>                  const ipu3_uapi_stats_3a *stats =
> >>                          reinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());
> >>   
> >> +               /* \todo move those into processControls ? */
> >> +               context_.frameContext.agc.exposure = event.sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> >> +               context_.frameContext.agc.gain = camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> > 
> > My only worry with this reading it now, is that we are storing the
> > exposure and gain of the frame that that is associated with the
> > statistics in what I had thought was a structure that maps the 'output'
> > of the algorithms.
> 
> I am not sure it is only the output, it is a frame context, input and 
> output.
> 
> > 
> > You've explored this a bit more now - so you probably have a much better
> > idea than me. This is ultimately an 'input' to the algorithms. Do we
> > need to distinguish between the 'input' and 'output' ? (I.e. do we need
> > to have both values kept so that the algorithm can determine how much it
> > is changing? Or is that not a problem because it's all handled
> > internally anyway....
> 
> We update the value for the input of the algorithms, and they will 
> update those values. It is debatable though: should we have two 
> frameContexts, one before and one after the algorithm for instance ? I 
> can't see a real value to add this though.

I don't think it would be 'two' frameContexts... it could just be:
	struct sensor {
		int32_t exposure;
		int32_t gain;
	}

added to the current frameContext structure perhaps and stored so it can
be identified by the algorithms when they run as an input/metadata from
the sensor?

> 
> > 
> >> +
> >>                  parseStatistics(event.frame, event.frameTimestamp, stats);
> >>                  break;
> >>          }
> >> -- 
> >> 2.32.0
> >>