[libcamera-devel] ipa: ipu3: Rectify gain value reporting in request metadata
diff mbox series

Message ID 20211130052253.513278-1-umang.jain@ideasonboard.com
State Accepted
Commit 68e4f70a6937a69339c3f48502cd4e332c3a16ca
Headers show
Series
  • [libcamera-devel] ipa: ipu3: Rectify gain value reporting in request metadata
Related show

Commit Message

Umang Jain Nov. 30, 2021, 5:22 a.m. UTC
Report value of sensor's gain pertaining to the current request
completion, as that is the gain value the request completed with.
The Agc algorithm processes the gain value for incoming next request
hence it should not be reported in request's metadata.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
---
 src/ipa/ipu3/ipu3.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jean-Michel Hautbois Nov. 30, 2021, 9:47 a.m. UTC | #1
Hi Umang,

On 30/11/2021 06:22, Umang Jain wrote:
> Report value of sensor's gain pertaining to the current request
> completion, as that is the gain value the request completed with.
> The Agc algorithm processes the gain value for incoming next request
> hence it should not be reported in request's metadata.
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>

Thanks for the explanation in previous patch.
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@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 b0c75541..76182587 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -628,7 +628,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>   	int64_t frameDuration = (defVBlank_ + sensorInfo_.outputSize.height) * lineDuration_.get<std::micro>();
>   	ctrls.set(controls::FrameDuration, frameDuration);
>   
> -	ctrls.set(controls::AnalogueGain, context_.frameContext.agc.gain);
> +	ctrls.set(controls::AnalogueGain, context_.frameContext.sensor.gain);
>   
>   	ctrls.set(controls::ColourTemperature, context_.frameContext.awb.temperatureK);
>   
>
Kieran Bingham Nov. 30, 2021, 10:29 a.m. UTC | #2
Quoting Umang Jain (2021-11-30 05:22:53)
> Report value of sensor's gain pertaining to the current request
> completion, as that is the gain value the request completed with.
> The Agc algorithm processes the gain value for incoming next request
> hence it should not be reported in request's metadata.
> 
> Signed-off-by: Umang Jain <umang.jain@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 b0c75541..76182587 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -628,7 +628,7 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>         int64_t frameDuration = (defVBlank_ + sensorInfo_.outputSize.height) * lineDuration_.get<std::micro>();
>         ctrls.set(controls::FrameDuration, frameDuration);
>  
> -       ctrls.set(controls::AnalogueGain, context_.frameContext.agc.gain);
> +       ctrls.set(controls::AnalogueGain, context_.frameContext.sensor.gain);

Aha, yes that looks like a more reasonable way to align the Gain and
Exposure.


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


We could probably add some extra/better comments on the gain/exposure to
make it clear, but this is a valid fix on it's own, and if we add extra
documentation - we should align it with both the IPU3 and the RKISP now.

--
Kieran


>  
>         ctrls.set(controls::ColourTemperature, context_.frameContext.awb.temperatureK);
>  
> -- 
> 2.31.0
>

Patch
diff mbox series

diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index b0c75541..76182587 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -628,7 +628,7 @@  void IPAIPU3::parseStatistics(unsigned int frame,
 	int64_t frameDuration = (defVBlank_ + sensorInfo_.outputSize.height) * lineDuration_.get<std::micro>();
 	ctrls.set(controls::FrameDuration, frameDuration);
 
-	ctrls.set(controls::AnalogueGain, context_.frameContext.agc.gain);
+	ctrls.set(controls::AnalogueGain, context_.frameContext.sensor.gain);
 
 	ctrls.set(controls::ColourTemperature, context_.frameContext.awb.temperatureK);