[libcamera-devel,2/4] ipa: ipu3: Shorten exposure and gain lines
diff mbox series

Message ID 20211125102143.52556-3-jeanmichel.hautbois@ideasonboard.com
State Superseded
Headers show
Series
  • ipa: ipu3: Misc clean up
Related show

Commit Message

Jean-Michel Hautbois Nov. 25, 2021, 10:21 a.m. UTC 
When the effective sensor values are stored during the EventStatReady
event, the lines are long. Fix it.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
---
 src/ipa/ipu3/ipu3.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Kieran Bingham Nov. 25, 2021, 11:54 a.m. UTC | #1 
Quoting Jean-Michel Hautbois (2021-11-25 10:21:41)
> When the effective sensor values are stored during the EventStatReady
> event, the lines are long. Fix it.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> ---
>  src/ipa/ipu3/ipu3.cpp | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index b0c75541..7a010019 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -547,8 +547,10 @@ void IPAIPU3::processEvent(const IPU3Event &event)
>                 const ipu3_uapi_stats_3a *stats =
>                         reinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());
>  
> -               context_.frameContext.sensor.exposure = event.sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> -               context_.frameContext.sensor.gain = camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> +               context_.frameContext.sensor.exposure =
> +                       event.sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> +               context_.frameContext.sensor.gain =
> +                       camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());

This might also look /read better if we extract the control value first
?

		int32_t exposure = event.sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
		int32_t gain = event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>();

		context_.frameContext.sensor.exposure = exposure;
		context_.frameContext.sensor.gain = camHelper_->gain(gain);

The compiler will optimise away the extra local variable anyway I
expect, so it should generate identical code...

But either way:

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

>  
>                 parseStatistics(event.frame, event.frameTimestamp, stats);
>                 break;
> -- 
> 2.32.0
>
Laurent Pinchart Nov. 25, 2021, 12:05 p.m. UTC | #2 
On Thu, Nov 25, 2021 at 11:54:52AM +0000, Kieran Bingham wrote:
> Quoting Jean-Michel Hautbois (2021-11-25 10:21:41)
> > When the effective sensor values are stored during the EventStatReady
> > event, the lines are long. Fix it.
> > 
> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> > ---
> >  src/ipa/ipu3/ipu3.cpp | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> > index b0c75541..7a010019 100644
> > --- a/src/ipa/ipu3/ipu3.cpp
> > +++ b/src/ipa/ipu3/ipu3.cpp
> > @@ -547,8 +547,10 @@ void IPAIPU3::processEvent(const IPU3Event &event)
> >                 const ipu3_uapi_stats_3a *stats =
> >                         reinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());
> >  
> > -               context_.frameContext.sensor.exposure = event.sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> > -               context_.frameContext.sensor.gain = camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> > +               context_.frameContext.sensor.exposure =
> > +                       event.sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> > +               context_.frameContext.sensor.gain =
> > +                       camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> 
> This might also look /read better if we extract the control value first
> ?
> 
> 		int32_t exposure = event.sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> 		int32_t gain = event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>();
> 
> 		context_.frameContext.sensor.exposure = exposure;
> 		context_.frameContext.sensor.gain = camHelper_->gain(gain);
> 
> The compiler will optimise away the extra local variable anyway I
> expect, so it should generate identical code...
> 
> But either way:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> >  
> >                 parseStatistics(event.frame, event.frameTimestamp, stats);
> >                 break;

Patch
diff mbox series 

diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index b0c75541..7a010019 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -547,8 +547,10 @@  void IPAIPU3::processEvent(const IPU3Event &event)
 		const ipu3_uapi_stats_3a *stats =
 			reinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());
 
-		context_.frameContext.sensor.exposure = event.sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
-		context_.frameContext.sensor.gain = camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
+		context_.frameContext.sensor.exposure =
+			event.sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
+		context_.frameContext.sensor.gain =
+			camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
 
 		parseStatistics(event.frame, event.frameTimestamp, stats);
 		break;