[libcamera-devel,v3,08/19] ipa: ipu3: tonemapping: Generate the LUT only on gamma change
diff mbox series

Message ID 20211022151218.111966-9-jeanmichel.hautbois@ideasonboard.com
State Changes Requested
Headers show
Series
  • Document IPU3 IPA
Related show

Commit Message

Jean-Michel Hautbois Oct. 22, 2021, 3:12 p.m. UTC 
The Tone mapping algorithm calculates the gamma curve for every frame,
regardless of whether the gamma value has changed or not. This issue is
exasperated as we currently hardcode the gamma to a single value.

Optimise the implementation to only recalculate the look up table when
the gamma setting is changed, and store the gamma setting of the LUT
curve as part of the IPA context.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/ipa/ipu3/algorithms/tone_mapping.cpp | 5 +++++
 src/ipa/ipu3/ipa_context.h               | 1 +
 src/ipa/ipu3/ipu3.cpp                    | 3 +++
 3 files changed, 9 insertions(+)

Comments

Laurent Pinchart Oct. 25, 2021, 9:23 p.m. UTC | #1 
Hi Jean-Michel,

Thank you for the patch.

On Fri, Oct 22, 2021 at 05:12:07PM +0200, Jean-Michel Hautbois wrote:
> The Tone mapping algorithm calculates the gamma curve for every frame,

s/Tone/tone/

> regardless of whether the gamma value has changed or not. This issue is
> exasperated as we currently hardcode the gamma to a single value.
> 
> Optimise the implementation to only recalculate the look up table when
> the gamma setting is changed, and store the gamma setting of the LUT
> curve as part of the IPA context.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

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

> ---
>  src/ipa/ipu3/algorithms/tone_mapping.cpp | 5 +++++
>  src/ipa/ipu3/ipa_context.h               | 1 +
>  src/ipa/ipu3/ipu3.cpp                    | 3 +++
>  3 files changed, 9 insertions(+)
> 
> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp
> index 3af96261..40337f9d 100644
> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp
> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp
> @@ -43,6 +43,9 @@ void ToneMapping::process([[maybe_unused]] IPAContext &context,
>  	 */
>  	gamma_ = 1.1;
>  
> +	if (context.frameContext.toneMapping.gamma == gamma_)
> +		return;
> +
>  	struct ipu3_uapi_gamma_corr_lut &lut =
>  		context.frameContext.toneMapping.gammaCorrection;
>  
> @@ -53,6 +56,8 @@ void ToneMapping::process([[maybe_unused]] IPAContext &context,
>  		/* The output value is expressed on 13 bits. */
>  		lut.lut[i] = gamma * 8191;
>  	}
> +
> +	context.frameContext.toneMapping.gamma = gamma_;
>  }
>  
>  } /* namespace ipa::ipu3::algorithms */
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index 847c03fe..e14bb561 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -48,6 +48,7 @@ struct IPAFrameContext {
>  	} awb;
>  
>  	struct {
> +		double gamma;
>  		struct ipu3_uapi_gamma_corr_lut gammaCorrection;
>  	} toneMapping;
>  };
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index cd27872b..f7632bc0 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -151,6 +151,9 @@
>   * \struct IPAFrameContext::toneMapping
>   * \brief Context for ToneMapping and Gamma control
>   *
> + * \var IPAFrameContext::toneMapping::gamma
> + * \brief Gamma value for the LUT
> + *
>   * \var IPAFrameContext::toneMapping::gammaCorrection
>   * \brief Per-pixel tone mapping implemented as a LUT
>   *
Kieran Bingham Oct. 25, 2021, 9:29 p.m. UTC | #2 
Quoting Jean-Michel Hautbois (2021-10-22 16:12:07)
> The Tone mapping algorithm calculates the gamma curve for every frame,
> regardless of whether the gamma value has changed or not. This issue is
> exasperated as we currently hardcode the gamma to a single value.
> 
> Optimise the implementation to only recalculate the look up table when
> the gamma setting is changed, and store the gamma setting of the LUT
> curve as part of the IPA context.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

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

> ---
>  src/ipa/ipu3/algorithms/tone_mapping.cpp | 5 +++++
>  src/ipa/ipu3/ipa_context.h               | 1 +
>  src/ipa/ipu3/ipu3.cpp                    | 3 +++
>  3 files changed, 9 insertions(+)
> 
> diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp
> index 3af96261..40337f9d 100644
> --- a/src/ipa/ipu3/algorithms/tone_mapping.cpp
> +++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp
> @@ -43,6 +43,9 @@ void ToneMapping::process([[maybe_unused]] IPAContext &context,
>          */
>         gamma_ = 1.1;
>  
> +       if (context.frameContext.toneMapping.gamma == gamma_)
> +               return;
> +
>         struct ipu3_uapi_gamma_corr_lut &lut =
>                 context.frameContext.toneMapping.gammaCorrection;
>  
> @@ -53,6 +56,8 @@ void ToneMapping::process([[maybe_unused]] IPAContext &context,
>                 /* The output value is expressed on 13 bits. */
>                 lut.lut[i] = gamma * 8191;
>         }
> +
> +       context.frameContext.toneMapping.gamma = gamma_;
>  }
>  
>  } /* namespace ipa::ipu3::algorithms */
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index 847c03fe..e14bb561 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -48,6 +48,7 @@ struct IPAFrameContext {
>         } awb;
>  
>         struct {
> +               double gamma;
>                 struct ipu3_uapi_gamma_corr_lut gammaCorrection;
>         } toneMapping;
>  };
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index cd27872b..f7632bc0 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -151,6 +151,9 @@
>   * \struct IPAFrameContext::toneMapping
>   * \brief Context for ToneMapping and Gamma control
>   *
> + * \var IPAFrameContext::toneMapping::gamma
> + * \brief Gamma value for the LUT
> + *
>   * \var IPAFrameContext::toneMapping::gammaCorrection
>   * \brief Per-pixel tone mapping implemented as a LUT
>   *
> -- 
> 2.32.0
>

Patch
diff mbox series 

diff --git a/src/ipa/ipu3/algorithms/tone_mapping.cpp b/src/ipa/ipu3/algorithms/tone_mapping.cpp
index 3af96261..40337f9d 100644
--- a/src/ipa/ipu3/algorithms/tone_mapping.cpp
+++ b/src/ipa/ipu3/algorithms/tone_mapping.cpp
@@ -43,6 +43,9 @@  void ToneMapping::process([[maybe_unused]] IPAContext &context,
 	 */
 	gamma_ = 1.1;
 
+	if (context.frameContext.toneMapping.gamma == gamma_)
+		return;
+
 	struct ipu3_uapi_gamma_corr_lut &lut =
 		context.frameContext.toneMapping.gammaCorrection;
 
@@ -53,6 +56,8 @@  void ToneMapping::process([[maybe_unused]] IPAContext &context,
 		/* The output value is expressed on 13 bits. */
 		lut.lut[i] = gamma * 8191;
 	}
+
+	context.frameContext.toneMapping.gamma = gamma_;
 }
 
 } /* namespace ipa::ipu3::algorithms */
diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
index 847c03fe..e14bb561 100644
--- a/src/ipa/ipu3/ipa_context.h
+++ b/src/ipa/ipu3/ipa_context.h
@@ -48,6 +48,7 @@  struct IPAFrameContext {
 	} awb;
 
 	struct {
+		double gamma;
 		struct ipu3_uapi_gamma_corr_lut gammaCorrection;
 	} toneMapping;
 };
diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index cd27872b..f7632bc0 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -151,6 +151,9 @@ 
  * \struct IPAFrameContext::toneMapping
  * \brief Context for ToneMapping and Gamma control
  *
+ * \var IPAFrameContext::toneMapping::gamma
+ * \brief Gamma value for the LUT
+ *
  * \var IPAFrameContext::toneMapping::gammaCorrection
  * \brief Per-pixel tone mapping implemented as a LUT
  *