[19/35] libcamera: software_isp: ccm: Populate CCM table to Debayer params structure
diff mbox series

Message ID 20250611013245.133785-20-bryan.odonoghue@linaro.org
State New
Headers show
Series
  • Add GLES 2.0 GPUISP to libcamera
Related show

Commit Message

Bryan O'Donoghue June 11, 2025, 1:32 a.m. UTC 
Populate the DebayerParams CCM table during ccm::prepare(). A copy is made
of the CCM into the DebayerParams structure.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 src/ipa/simple/algorithms/ccm.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Bryan O'Donoghue June 16, 2025, 7:06 p.m. UTC | #1 
On 11/06/2025 02:32, Bryan O'Donoghue wrote:
> Populate the DebayerParams CCM table during ccm::prepare(). A copy is made
> of the CCM into the DebayerParams structure.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>   src/ipa/simple/algorithms/ccm.cpp | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/simple/algorithms/ccm.cpp b/src/ipa/simple/algorithms/ccm.cpp
> index 0a98406c..cb023878 100644
> --- a/src/ipa/simple/algorithms/ccm.cpp
> +++ b/src/ipa/simple/algorithms/ccm.cpp
> @@ -14,6 +14,7 @@
>   #include <libcamera/control_ids.h>
> 
>   #include "libcamera/internal/matrix.h"
> +#include "libcamera/internal/software_isp/debayer_params.h"
> 
>   namespace {
> 
> @@ -84,7 +85,7 @@ void Ccm::applySaturation(Matrix<float, 3, 3> &ccm, float saturation)
>   }
> 
>   void Ccm::prepare(IPAContext &context, const uint32_t frame,
> -		  IPAFrameContext &frameContext, [[maybe_unused]] DebayerParams *params)
> +		  IPAFrameContext &frameContext, DebayerParams *params)
>   {
>   	auto &saturation = context.activeState.knobs.saturation;
> 
> @@ -108,6 +109,7 @@ void Ccm::prepare(IPAContext &context, const uint32_t frame,
>   	context.activeState.ccm.ccm = ccm;
>   	frameContext.ccm.ccm = ccm;
>   	frameContext.saturation = saturation;
> +	params->ccm = ccm;

Ping @Milan - I take the copy here and again in the next patch in lut.cpp.

Is it enough to latch the CCM matrix here ?

---
bod
Bryan O'Donoghue June 16, 2025, 7:11 p.m. UTC | #2 
On 11/06/2025 02:32, Bryan O'Donoghue wrote:
> Populate the DebayerParams CCM table during ccm::prepare(). A copy is made
> of the CCM into the DebayerParams structure.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>   src/ipa/simple/algorithms/ccm.cpp | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/simple/algorithms/ccm.cpp b/src/ipa/simple/algorithms/ccm.cpp
> index 0a98406c..cb023878 100644
> --- a/src/ipa/simple/algorithms/ccm.cpp
> +++ b/src/ipa/simple/algorithms/ccm.cpp
> @@ -14,6 +14,7 @@
>   #include <libcamera/control_ids.h>
> 
>   #include "libcamera/internal/matrix.h"
> +#include "libcamera/internal/software_isp/debayer_params.h"
> 
>   namespace {
> 
> @@ -84,7 +85,7 @@ void Ccm::applySaturation(Matrix<float, 3, 3> &ccm, float saturation)
>   }
> 
>   void Ccm::prepare(IPAContext &context, const uint32_t frame,
> -		  IPAFrameContext &frameContext, [[maybe_unused]] DebayerParams *params)
> +		  IPAFrameContext &frameContext, DebayerParams *params)
>   {
>   	auto &saturation = context.activeState.knobs.saturation;
> 
> @@ -108,6 +109,7 @@ void Ccm::prepare(IPAContext &context, const uint32_t frame,
>   	context.activeState.ccm.ccm = ccm;
>   	frameContext.ccm.ccm = ccm;
>   	frameContext.saturation = saturation;
> +	params->ccm = ccm;
>   	context.activeState.ccm.changed = true;
>   }
> 
> --
> 2.49.0
> 

Ping Milan

Or is this additional latch in lut relevant ?

Its not 100% clear to me - I could trace the data inside of IPA or I 
could just ask someone like you who knows the IPA code better.

---
bod
Milan Zamazal June 17, 2025, 1:48 p.m. UTC | #3 
Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:

> Ping @Milan - I take the copy here and again in the next patch in lut.cpp.
>
> Is it enough to latch the CCM matrix here ?
>
> Or is this additional latch in lut relevant ?
>
> Its not 100% clear to me - I could trace the data inside of IPA or I could just ask someone like you who knows the IPA code better.

ccm.cpp selects the right CCM and applies saturation to it.  Then
lut.cpp applies white balance gains.  So there is no need for this
patch, lut.cpp has the final word and it's enough to set the param
there.

Patch
diff mbox series 

diff --git a/src/ipa/simple/algorithms/ccm.cpp b/src/ipa/simple/algorithms/ccm.cpp
index 0a98406c..cb023878 100644
--- a/src/ipa/simple/algorithms/ccm.cpp
+++ b/src/ipa/simple/algorithms/ccm.cpp
@@ -14,6 +14,7 @@ 
 #include <libcamera/control_ids.h>
 
 #include "libcamera/internal/matrix.h"
+#include "libcamera/internal/software_isp/debayer_params.h"
 
 namespace {
 
@@ -84,7 +85,7 @@  void Ccm::applySaturation(Matrix<float, 3, 3> &ccm, float saturation)
 }
 
 void Ccm::prepare(IPAContext &context, const uint32_t frame,
-		  IPAFrameContext &frameContext, [[maybe_unused]] DebayerParams *params)
+		  IPAFrameContext &frameContext, DebayerParams *params)
 {
 	auto &saturation = context.activeState.knobs.saturation;
 
@@ -108,6 +109,7 @@  void Ccm::prepare(IPAContext &context, const uint32_t frame,
 	context.activeState.ccm.ccm = ccm;
 	frameContext.ccm.ccm = ccm;
 	frameContext.saturation = saturation;
+	params->ccm = ccm;
 	context.activeState.ccm.changed = true;
 }