| Message ID | 20211110195901.85597-4-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@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. 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.... > + > parseStatistics(event.frame, event.frameTimestamp, stats); > break; > } > -- > 2.32.0 >
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@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. > >> + >> parseStatistics(event.frame, event.frameTimestamp, stats); >> break; >> } >> -- >> 2.32.0 >>
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@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 > >>
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>()); + parseStatistics(event.frame, event.frameTimestamp, stats); break; }
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@ideasonboard.com> --- src/ipa/ipu3/ipu3.cpp | 4 ++++ 1 file changed, 4 insertions(+)