| Message ID | 20250411130423.2164577-6-stefan.klug@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Stefan, Thanks for the patch. Quoting Stefan Klug (2025-04-11 15:04:12) > The Y mode of the histogram get's captured at the ISP output, before the s/get's/gets/ > output formatter. This has the side effect, that the first and the last s/,// > bins of are empty in case of limited YUV range. Another side effect is s/bins of/bins/ > that gamma and GWDR processing is included in the histogram which makes > algorithm development very difficult. In RGB mode the histogram is taken > after xtalk (CCM) and is therefore independent of gamma and WDR. The > limited range issue also does not apply. In the ISP reference it is > however stated that "it is not possible to calculate a luminance or > grayscale histogram from an RGB histogram since the position information > is lost during its generation". Hm, I see, so we need to use RGB mode so that we can tap before gamma and wdr. Those are both probably nasty to reverse. It's probably better to use RGB mode then. I suppose in the future we should use hist64, though the rk3399 will be stuck with this. > During testing the RGB histogram provided good data and better > algorithmic stability at a possible (but not measured) inaccuracy. Good, we get better data :) > Another option would be to pass the color space information into the IPA > and strip the histogram accordingly. For ease of implementation switch to > the RGB mode. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/ipa/rkisp1/algorithms/agc.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp > index 0e4fd3663167..a2c8d5403ba2 100644 > --- a/src/ipa/rkisp1/algorithms/agc.cpp > +++ b/src/ipa/rkisp1/algorithms/agc.cpp > @@ -379,7 +379,7 @@ void Agc::prepare(IPAContext &context, const uint32_t frame, > hstConfig.setEnabled(true); > > hstConfig->meas_window = context.configuration.agc.measureWindow; > - hstConfig->mode = RKISP1_CIF_ISP_HISTOGRAM_MODE_Y_HISTOGRAM; > + hstConfig->mode = RKISP1_CIF_ISP_HISTOGRAM_MODE_RGB_COMBINED; > > Span<uint8_t> weights{ > hstConfig->hist_weight, > -- > 2.43.0 >
diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp index 0e4fd3663167..a2c8d5403ba2 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -379,7 +379,7 @@ void Agc::prepare(IPAContext &context, const uint32_t frame, hstConfig.setEnabled(true); hstConfig->meas_window = context.configuration.agc.measureWindow; - hstConfig->mode = RKISP1_CIF_ISP_HISTOGRAM_MODE_Y_HISTOGRAM; + hstConfig->mode = RKISP1_CIF_ISP_HISTOGRAM_MODE_RGB_COMBINED; Span<uint8_t> weights{ hstConfig->hist_weight,
The Y mode of the histogram get's captured at the ISP output, before the output formatter. This has the side effect, that the first and the last bins of are empty in case of limited YUV range. Another side effect is that gamma and GWDR processing is included in the histogram which makes algorithm development very difficult. In RGB mode the histogram is taken after xtalk (CCM) and is therefore independent of gamma and WDR. The limited range issue also does not apply. In the ISP reference it is however stated that "it is not possible to calculate a luminance or grayscale histogram from an RGB histogram since the position information is lost during its generation". During testing the RGB histogram provided good data and better algorithmic stability at a possible (but not measured) inaccuracy. Another option would be to pass the color space information into the IPA and strip the histogram accordingly. For ease of implementation switch to the RGB mode. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- src/ipa/rkisp1/algorithms/agc.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)