| Message ID | 20260904062840.46739-2-dmanresa@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hello, and thanks for the patch On 04/09/2026 07:28, D. Manresa wrote: > The AWB gains passed to the AGC traits are built as { red, blue, green }, > but the RGB vector expects { red, green, blue }. The luminance estimate > therefore weights the blue statistics with the green gain and the green > statistics with the blue gain, which biases the exposure whenever the two > gains differ (on an OV5670 they are 1.0 and ~1.8). > > Pass the gains in RGB order. > > Signed-off-by: D. Manresa <dmanresa@gmail.com> > --- Oh good spot. This needs re-basing now, as the AGC algorithm has undergone some changes recently, but the bug is still there. When you've rebased it you can add: Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> Thanks Dan > src/ipa/ipu3/algorithms/agc.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index 57cfb32..dd4f1ab 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -224,8 +224,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, > rgbTriples_, > {{ > context.activeState.awb.gains.red, > - context.activeState.awb.gains.blue, > context.activeState.awb.gains.green, > + context.activeState.awb.gains.blue, > }}, > bdsGrid_, > },
On 04/09/2026 07:35, Dan Scally wrote: > Hello, and thanks for the patch > > On 04/09/2026 07:28, D. Manresa wrote: >> The AWB gains passed to the AGC traits are built as { red, blue, green }, >> but the RGB vector expects { red, green, blue }. The luminance estimate >> therefore weights the blue statistics with the green gain and the green >> statistics with the blue gain, which biases the exposure whenever the two >> gains differ (on an OV5670 they are 1.0 and ~1.8). >> >> Pass the gains in RGB order. >> >> Signed-off-by: D. Manresa <dmanresa@gmail.com> >> --- > > Oh good spot. This needs re-basing now, as the AGC algorithm has undergone some changes recently, > but the bug is still there. When you've rebased it you can add: > > Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> Oh, actually I just realised that this bug will also be fixed by the libipa rework: https://patchwork.libcamera.org/patch/28166/ > > Thanks > Dan > >> src/ipa/ipu3/algorithms/agc.cpp | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp >> index 57cfb32..dd4f1ab 100644 >> --- a/src/ipa/ipu3/algorithms/agc.cpp >> +++ b/src/ipa/ipu3/algorithms/agc.cpp >> @@ -224,8 +224,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, >> rgbTriples_, >> {{ >> context.activeState.awb.gains.red, >> - context.activeState.awb.gains.blue, >> context.activeState.awb.gains.green, >> + context.activeState.awb.gains.blue, >> }}, >> bdsGrid_, >> }, >
diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index 57cfb32..dd4f1ab 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -224,8 +224,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, rgbTriples_, {{ context.activeState.awb.gains.red, - context.activeState.awb.gains.blue, context.activeState.awb.gains.green, + context.activeState.awb.gains.blue, }}, bdsGrid_, },
The AWB gains passed to the AGC traits are built as { red, blue, green }, but the RGB vector expects { red, green, blue }. The luminance estimate therefore weights the blue statistics with the green gain and the green statistics with the blue gain, which biases the exposure whenever the two gains differ (on an OV5670 they are 1.0 and ~1.8). Pass the gains in RGB order. Signed-off-by: D. Manresa <dmanresa@gmail.com> --- src/ipa/ipu3/algorithms/agc.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)