| Message ID | 20210923081625.60276-12-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
On Thu, Sep 23, 2021 at 10:16:24AM +0200, Jean-Michel Hautbois wrote: > Now that we know how exactly the AWB statistics are formatted, use a > simplified loop in processBrightness() to parse the green values and get the > histogram. > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/agc.cpp | 54 +++++++++++++-------------------- > src/ipa/ipu3/algorithms/agc.h | 2 ++ > 2 files changed, 23 insertions(+), 33 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > index 5ff50f4a..ebbc5676 100644 > --- a/src/ipa/ipu3/algorithms/agc.cpp > +++ b/src/ipa/ipu3/algorithms/agc.cpp > @@ -6,6 +6,7 @@ > */ > > #include "agc.h" > +#include "awb.h" > > #include <algorithm> > #include <chrono> > @@ -47,9 +48,6 @@ static constexpr uint32_t kMaxExposure = 1976; > static constexpr uint32_t knumHistogramBins = 256; > static constexpr double kEvGainTarget = 0.5; > > -/* A cell is 8 bytes and contains averages for RGB values and saturation ratio */ > -static constexpr uint8_t kCellSize = 8; > - > Agc::Agc() > : frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s), > maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s), > @@ -60,6 +58,10 @@ Agc::Agc() > int Agc::configure([[maybe_unused]] IPAContext &context, looks like context is no longer [[maybe_unused]]. > const IPAConfigInfo &configInfo) > { > + const ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid; > + /* The grid is aligned to the next multiple of 4 */ > + stride_ = (grid.width + 3) / 4 * 4; This is familiar... How about putting a stride into context.configuration.grid.stride and calculating it once during calculateBdsGrid()? > + > lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s > / configInfo.sensorInfo.pixelRate; > maxExposureTime_ = kMaxExposure * lineDuration_; > @@ -70,37 +72,23 @@ int Agc::configure([[maybe_unused]] IPAContext &context, > void Agc::processBrightness(const ipu3_uapi_stats_3a *stats, > const ipu3_uapi_grid_config &grid) > { > - const struct ipu3_uapi_grid_config statsAeGrid = stats->stats_4a_config.awb_config.grid; > - Rectangle aeRegion = { statsAeGrid.x_start, > - statsAeGrid.y_start, > - static_cast<unsigned int>(statsAeGrid.x_end - statsAeGrid.x_start) + 1, > - static_cast<unsigned int>(statsAeGrid.y_end - statsAeGrid.y_start) + 1 }; > - Point topleft = aeRegion.topLeft(); > - int topleftX = topleft.x >> grid.block_width_log2; > - int topleftY = topleft.y >> grid.block_height_log2; > - > - /* Align to the grid cell width and height */ > - uint32_t startX = topleftX << grid.block_width_log2; > - uint32_t startY = topleftY * grid.width << grid.block_width_log2; > - uint32_t endX = (startX + (aeRegion.size().width >> grid.block_width_log2)) << grid.block_width_log2; > - uint32_t i, j; > - uint32_t count = 0; > - > uint32_t hist[knumHistogramBins] = { 0 }; > - for (j = topleftY; > - j < topleftY + (aeRegion.size().height >> grid.block_height_log2); > - j++) { > - for (i = startX + startY; i < endX + startY; i += kCellSize) { > - /* > - * The grid width (and maybe height) is not reliable. > - * We observed a bit shift which makes the value 160 to be 32 in the stats grid. > - * Use the one passed at init time. > - */ > - if (stats->awb_raw_buffer.meta_data[i + 4 + j * grid.width] == 0) { > - uint8_t Gr = stats->awb_raw_buffer.meta_data[i + 0 + j * grid.width]; > - uint8_t Gb = stats->awb_raw_buffer.meta_data[i + 3 + j * grid.width]; > - hist[(Gr + Gb) / 2]++; > - count++; > + > + for (unsigned int cellY = 0; cellY < grid.height; cellY++) { > + for (unsigned int cellX = 0; cellX < stride_; cellX++) { > + uint32_t cellPosition = cellY * stride_ + cellX > + * sizeof(Ipu3AwbCell); > + > + /* Cast the initial IPU3 structure to simplify the reading */ > + const Ipu3AwbCell *cell = > + reinterpret_cast<const Ipu3AwbCell *>( > + &stats->awb_raw_buffer.meta_data[cellPosition] > + ); > + > + if (cell->satRatio == 0) { Is the satRatio usage clear yet? Is it always 0? or do you see other values here? Should it be less than a saturation threshold? (perhaps not unless we have a way to configure that anyway) > + uint8_t gr = cell->greenRedAvg; > + uint8_t gb = cell->greenBlueAvg; > + hist[(gr + gb) / 2]++; > } > } > } > diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h > index e36797d3..64b71c65 100644 > --- a/src/ipa/ipu3/algorithms/agc.h > +++ b/src/ipa/ipu3/algorithms/agc.h > @@ -50,6 +50,8 @@ private: > Duration prevExposureNoDg_; > Duration currentExposure_; > Duration currentExposureNoDg_; > + > + uint32_t stride_; > }; > > } /* namespace ipa::ipu3::algorithms */ > -- > 2.30.2 > -- Kieran
On 28/09/2021 18:26, Kieran Bingham wrote: > On Thu, Sep 23, 2021 at 10:16:24AM +0200, Jean-Michel Hautbois wrote: >> Now that we know how exactly the AWB statistics are formatted, use a >> simplified loop in processBrightness() to parse the green values and get the >> histogram. >> >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> >> --- >> src/ipa/ipu3/algorithms/agc.cpp | 54 +++++++++++++-------------------- >> src/ipa/ipu3/algorithms/agc.h | 2 ++ >> 2 files changed, 23 insertions(+), 33 deletions(-) >> >> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp >> index 5ff50f4a..ebbc5676 100644 >> --- a/src/ipa/ipu3/algorithms/agc.cpp >> +++ b/src/ipa/ipu3/algorithms/agc.cpp >> @@ -6,6 +6,7 @@ >> */ >> >> #include "agc.h" >> +#include "awb.h" >> >> #include <algorithm> >> #include <chrono> >> @@ -47,9 +48,6 @@ static constexpr uint32_t kMaxExposure = 1976; >> static constexpr uint32_t knumHistogramBins = 256; >> static constexpr double kEvGainTarget = 0.5; >> >> -/* A cell is 8 bytes and contains averages for RGB values and saturation ratio */ >> -static constexpr uint8_t kCellSize = 8; >> - >> Agc::Agc() >> : frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s), >> maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s), >> @@ -60,6 +58,10 @@ Agc::Agc() >> int Agc::configure([[maybe_unused]] IPAContext &context, > > looks like context is no longer [[maybe_unused]]. > > >> const IPAConfigInfo &configInfo) >> { >> + const ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid; >> + /* The grid is aligned to the next multiple of 4 */ >> + stride_ = (grid.width + 3) / 4 * 4; > > This is familiar... > > How about putting a stride into context.configuration.grid.stride and > calculating it once during calculateBdsGrid()? > Then I need to pass the grid to the processBrightness(), again (and in AWB too) while we could get rid of it. Or cache the value in configure() ? >> + >> lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s >> / configInfo.sensorInfo.pixelRate; >> maxExposureTime_ = kMaxExposure * lineDuration_; >> @@ -70,37 +72,23 @@ int Agc::configure([[maybe_unused]] IPAContext &context, >> void Agc::processBrightness(const ipu3_uapi_stats_3a *stats, >> const ipu3_uapi_grid_config &grid) >> { >> - const struct ipu3_uapi_grid_config statsAeGrid = stats->stats_4a_config.awb_config.grid; >> - Rectangle aeRegion = { statsAeGrid.x_start, >> - statsAeGrid.y_start, >> - static_cast<unsigned int>(statsAeGrid.x_end - statsAeGrid.x_start) + 1, >> - static_cast<unsigned int>(statsAeGrid.y_end - statsAeGrid.y_start) + 1 }; >> - Point topleft = aeRegion.topLeft(); >> - int topleftX = topleft.x >> grid.block_width_log2; >> - int topleftY = topleft.y >> grid.block_height_log2; >> - >> - /* Align to the grid cell width and height */ >> - uint32_t startX = topleftX << grid.block_width_log2; >> - uint32_t startY = topleftY * grid.width << grid.block_width_log2; >> - uint32_t endX = (startX + (aeRegion.size().width >> grid.block_width_log2)) << grid.block_width_log2; >> - uint32_t i, j; >> - uint32_t count = 0; >> - >> uint32_t hist[knumHistogramBins] = { 0 }; >> - for (j = topleftY; >> - j < topleftY + (aeRegion.size().height >> grid.block_height_log2); >> - j++) { >> - for (i = startX + startY; i < endX + startY; i += kCellSize) { >> - /* >> - * The grid width (and maybe height) is not reliable. >> - * We observed a bit shift which makes the value 160 to be 32 in the stats grid. >> - * Use the one passed at init time. >> - */ >> - if (stats->awb_raw_buffer.meta_data[i + 4 + j * grid.width] == 0) { >> - uint8_t Gr = stats->awb_raw_buffer.meta_data[i + 0 + j * grid.width]; >> - uint8_t Gb = stats->awb_raw_buffer.meta_data[i + 3 + j * grid.width]; >> - hist[(Gr + Gb) / 2]++; >> - count++; >> + >> + for (unsigned int cellY = 0; cellY < grid.height; cellY++) { >> + for (unsigned int cellX = 0; cellX < stride_; cellX++) { >> + uint32_t cellPosition = cellY * stride_ + cellX >> + * sizeof(Ipu3AwbCell); >> + >> + /* Cast the initial IPU3 structure to simplify the reading */ >> + const Ipu3AwbCell *cell = >> + reinterpret_cast<const Ipu3AwbCell *>( >> + &stats->awb_raw_buffer.meta_data[cellPosition] >> + ); >> + >> + if (cell->satRatio == 0) { > > Is the satRatio usage clear yet? Is it always 0? or do you see other > values here? > > Should it be less than a saturation threshold? (perhaps not unless we > have a way to configure that anyway) > > >> + uint8_t gr = cell->greenRedAvg; >> + uint8_t gb = cell->greenBlueAvg; >> + hist[(gr + gb) / 2]++; >> } >> } >> } >> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h >> index e36797d3..64b71c65 100644 >> --- a/src/ipa/ipu3/algorithms/agc.h >> +++ b/src/ipa/ipu3/algorithms/agc.h >> @@ -50,6 +50,8 @@ private: >> Duration prevExposureNoDg_; >> Duration currentExposure_; >> Duration currentExposureNoDg_; >> + >> + uint32_t stride_; >> }; >> >> } /* namespace ipa::ipu3::algorithms */ >> -- >> 2.30.2 >> > > -- > Kieran >
On 29/09/2021 10:02, Jean-Michel Hautbois wrote: > On 28/09/2021 18:26, Kieran Bingham wrote: >> On Thu, Sep 23, 2021 at 10:16:24AM +0200, Jean-Michel Hautbois wrote: >>> Now that we know how exactly the AWB statistics are formatted, use a >>> simplified loop in processBrightness() to parse the green values and get the >>> histogram. >>> >>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> >>> --- >>> src/ipa/ipu3/algorithms/agc.cpp | 54 +++++++++++++-------------------- >>> src/ipa/ipu3/algorithms/agc.h | 2 ++ >>> 2 files changed, 23 insertions(+), 33 deletions(-) >>> >>> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp >>> index 5ff50f4a..ebbc5676 100644 >>> --- a/src/ipa/ipu3/algorithms/agc.cpp >>> +++ b/src/ipa/ipu3/algorithms/agc.cpp >>> @@ -6,6 +6,7 @@ >>> */ >>> >>> #include "agc.h" >>> +#include "awb.h" >>> >>> #include <algorithm> >>> #include <chrono> >>> @@ -47,9 +48,6 @@ static constexpr uint32_t kMaxExposure = 1976; >>> static constexpr uint32_t knumHistogramBins = 256; >>> static constexpr double kEvGainTarget = 0.5; >>> >>> -/* A cell is 8 bytes and contains averages for RGB values and saturation ratio */ >>> -static constexpr uint8_t kCellSize = 8; >>> - >>> Agc::Agc() >>> : frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s), >>> maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s), >>> @@ -60,6 +58,10 @@ Agc::Agc() >>> int Agc::configure([[maybe_unused]] IPAContext &context, >> >> looks like context is no longer [[maybe_unused]]. >> >> >>> const IPAConfigInfo &configInfo) >>> { >>> + const ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid; >>> + /* The grid is aligned to the next multiple of 4 */ >>> + stride_ = (grid.width + 3) / 4 * 4; >> >> This is familiar... >> >> How about putting a stride into context.configuration.grid.stride and >> calculating it once during calculateBdsGrid()? >> > > Then I need to pass the grid to the processBrightness(), again (and in > AWB too) while we could get rid of it. Or cache the value in configure() ? I think it's fine, and better to cache it in configure. -- Kieran
Hello, On Tue, Sep 28, 2021 at 05:26:01PM +0100, Kieran Bingham wrote: > On Thu, Sep 23, 2021 at 10:16:24AM +0200, Jean-Michel Hautbois wrote: > > Now that we know how exactly the AWB statistics are formatted, use a > > simplified loop in processBrightness() to parse the green values and get the > > histogram. > > > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > --- > > src/ipa/ipu3/algorithms/agc.cpp | 54 +++++++++++++-------------------- > > src/ipa/ipu3/algorithms/agc.h | 2 ++ > > 2 files changed, 23 insertions(+), 33 deletions(-) > > > > diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp > > index 5ff50f4a..ebbc5676 100644 > > --- a/src/ipa/ipu3/algorithms/agc.cpp > > +++ b/src/ipa/ipu3/algorithms/agc.cpp > > @@ -6,6 +6,7 @@ > > */ > > > > #include "agc.h" > > +#include "awb.h" > > > > #include <algorithm> > > #include <chrono> > > @@ -47,9 +48,6 @@ static constexpr uint32_t kMaxExposure = 1976; > > static constexpr uint32_t knumHistogramBins = 256; > > static constexpr double kEvGainTarget = 0.5; > > > > -/* A cell is 8 bytes and contains averages for RGB values and saturation ratio */ > > -static constexpr uint8_t kCellSize = 8; > > - > > Agc::Agc() > > : frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s), > > maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s), > > @@ -60,6 +58,10 @@ Agc::Agc() > > int Agc::configure([[maybe_unused]] IPAContext &context, > > looks like context is no longer [[maybe_unused]]. > > > const IPAConfigInfo &configInfo) > > { > > + const ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid; > > + /* The grid is aligned to the next multiple of 4 */ > > + stride_ = (grid.width + 3) / 4 * 4; > > This is familiar... utils::alignUp() to the rescue again :-) > How about putting a stride into context.configuration.grid.stride and > calculating it once during calculateBdsGrid()? > > > + > > lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s > > / configInfo.sensorInfo.pixelRate; > > maxExposureTime_ = kMaxExposure * lineDuration_; > > @@ -70,37 +72,23 @@ int Agc::configure([[maybe_unused]] IPAContext &context, > > void Agc::processBrightness(const ipu3_uapi_stats_3a *stats, > > const ipu3_uapi_grid_config &grid) > > { > > - const struct ipu3_uapi_grid_config statsAeGrid = stats->stats_4a_config.awb_config.grid; > > - Rectangle aeRegion = { statsAeGrid.x_start, > > - statsAeGrid.y_start, > > - static_cast<unsigned int>(statsAeGrid.x_end - statsAeGrid.x_start) + 1, > > - static_cast<unsigned int>(statsAeGrid.y_end - statsAeGrid.y_start) + 1 }; > > - Point topleft = aeRegion.topLeft(); > > - int topleftX = topleft.x >> grid.block_width_log2; > > - int topleftY = topleft.y >> grid.block_height_log2; > > - > > - /* Align to the grid cell width and height */ > > - uint32_t startX = topleftX << grid.block_width_log2; > > - uint32_t startY = topleftY * grid.width << grid.block_width_log2; > > - uint32_t endX = (startX + (aeRegion.size().width >> grid.block_width_log2)) << grid.block_width_log2; > > - uint32_t i, j; > > - uint32_t count = 0; > > - > > uint32_t hist[knumHistogramBins] = { 0 }; > > - for (j = topleftY; > > - j < topleftY + (aeRegion.size().height >> grid.block_height_log2); > > - j++) { > > - for (i = startX + startY; i < endX + startY; i += kCellSize) { > > - /* > > - * The grid width (and maybe height) is not reliable. > > - * We observed a bit shift which makes the value 160 to be 32 in the stats grid. > > - * Use the one passed at init time. > > - */ > > - if (stats->awb_raw_buffer.meta_data[i + 4 + j * grid.width] == 0) { > > - uint8_t Gr = stats->awb_raw_buffer.meta_data[i + 0 + j * grid.width]; > > - uint8_t Gb = stats->awb_raw_buffer.meta_data[i + 3 + j * grid.width]; > > - hist[(Gr + Gb) / 2]++; > > - count++; > > + > > + for (unsigned int cellY = 0; cellY < grid.height; cellY++) { > > + for (unsigned int cellX = 0; cellX < stride_; cellX++) { Same issue as in AWB, you'll use the padding cells in your calculations, while they should be ignored. > > + uint32_t cellPosition = cellY * stride_ + cellX > > + * sizeof(Ipu3AwbCell); > > + > > + /* Cast the initial IPU3 structure to simplify the reading */ > > + const Ipu3AwbCell *cell = > > + reinterpret_cast<const Ipu3AwbCell *>( > > + &stats->awb_raw_buffer.meta_data[cellPosition] > > + ); > > + > > + if (cell->satRatio == 0) { > > Is the satRatio usage clear yet? Is it always 0? or do you see other > values here? > > Should it be less than a saturation threshold? (perhaps not unless we > have a way to configure that anyway) > > > + uint8_t gr = cell->greenRedAvg; > > + uint8_t gb = cell->greenBlueAvg; > > + hist[(gr + gb) / 2]++; > > } > > } > > } > > diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h > > index e36797d3..64b71c65 100644 > > --- a/src/ipa/ipu3/algorithms/agc.h > > +++ b/src/ipa/ipu3/algorithms/agc.h > > @@ -50,6 +50,8 @@ private: > > Duration prevExposureNoDg_; > > Duration currentExposure_; > > Duration currentExposureNoDg_; > > + > > + uint32_t stride_; > > }; > > > > } /* namespace ipa::ipu3::algorithms */
Hi Kieran, On 28/09/2021 18:26, Kieran Bingham wrote: > On Thu, Sep 23, 2021 at 10:16:24AM +0200, Jean-Michel Hautbois wrote: >> Now that we know how exactly the AWB statistics are formatted, use a >> simplified loop in processBrightness() to parse the green values and get the >> histogram. >> >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> >> --- >> src/ipa/ipu3/algorithms/agc.cpp | 54 +++++++++++++-------------------- >> src/ipa/ipu3/algorithms/agc.h | 2 ++ >> 2 files changed, 23 insertions(+), 33 deletions(-) >> >> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp >> index 5ff50f4a..ebbc5676 100644 >> --- a/src/ipa/ipu3/algorithms/agc.cpp >> +++ b/src/ipa/ipu3/algorithms/agc.cpp >> @@ -6,6 +6,7 @@ >> */ >> >> #include "agc.h" >> +#include "awb.h" >> >> #include <algorithm> >> #include <chrono> >> @@ -47,9 +48,6 @@ static constexpr uint32_t kMaxExposure = 1976; >> static constexpr uint32_t knumHistogramBins = 256; >> static constexpr double kEvGainTarget = 0.5; >> >> -/* A cell is 8 bytes and contains averages for RGB values and saturation ratio */ >> -static constexpr uint8_t kCellSize = 8; >> - >> Agc::Agc() >> : frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s), >> maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s), >> @@ -60,6 +58,10 @@ Agc::Agc() >> int Agc::configure([[maybe_unused]] IPAContext &context, > > looks like context is no longer [[maybe_unused]]. > > >> const IPAConfigInfo &configInfo) >> { >> + const ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid; >> + /* The grid is aligned to the next multiple of 4 */ >> + stride_ = (grid.width + 3) / 4 * 4; > > This is familiar... > > How about putting a stride into context.configuration.grid.stride and > calculating it once during calculateBdsGrid()? > >> + >> lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s >> / configInfo.sensorInfo.pixelRate; >> maxExposureTime_ = kMaxExposure * lineDuration_; >> @@ -70,37 +72,23 @@ int Agc::configure([[maybe_unused]] IPAContext &context, >> void Agc::processBrightness(const ipu3_uapi_stats_3a *stats, >> const ipu3_uapi_grid_config &grid) >> { >> - const struct ipu3_uapi_grid_config statsAeGrid = stats->stats_4a_config.awb_config.grid; >> - Rectangle aeRegion = { statsAeGrid.x_start, >> - statsAeGrid.y_start, >> - static_cast<unsigned int>(statsAeGrid.x_end - statsAeGrid.x_start) + 1, >> - static_cast<unsigned int>(statsAeGrid.y_end - statsAeGrid.y_start) + 1 }; >> - Point topleft = aeRegion.topLeft(); >> - int topleftX = topleft.x >> grid.block_width_log2; >> - int topleftY = topleft.y >> grid.block_height_log2; >> - >> - /* Align to the grid cell width and height */ >> - uint32_t startX = topleftX << grid.block_width_log2; >> - uint32_t startY = topleftY * grid.width << grid.block_width_log2; >> - uint32_t endX = (startX + (aeRegion.size().width >> grid.block_width_log2)) << grid.block_width_log2; >> - uint32_t i, j; >> - uint32_t count = 0; >> - >> uint32_t hist[knumHistogramBins] = { 0 }; >> - for (j = topleftY; >> - j < topleftY + (aeRegion.size().height >> grid.block_height_log2); >> - j++) { >> - for (i = startX + startY; i < endX + startY; i += kCellSize) { >> - /* >> - * The grid width (and maybe height) is not reliable. >> - * We observed a bit shift which makes the value 160 to be 32 in the stats grid. >> - * Use the one passed at init time. >> - */ >> - if (stats->awb_raw_buffer.meta_data[i + 4 + j * grid.width] == 0) { >> - uint8_t Gr = stats->awb_raw_buffer.meta_data[i + 0 + j * grid.width]; >> - uint8_t Gb = stats->awb_raw_buffer.meta_data[i + 3 + j * grid.width]; >> - hist[(Gr + Gb) / 2]++; >> - count++; >> + >> + for (unsigned int cellY = 0; cellY < grid.height; cellY++) { >> + for (unsigned int cellX = 0; cellX < stride_; cellX++) { >> + uint32_t cellPosition = cellY * stride_ + cellX >> + * sizeof(Ipu3AwbCell); >> + >> + /* Cast the initial IPU3 structure to simplify the reading */ >> + const Ipu3AwbCell *cell = >> + reinterpret_cast<const Ipu3AwbCell *>( >> + &stats->awb_raw_buffer.meta_data[cellPosition] >> + ); >> + >> + if (cell->satRatio == 0) { > > Is the satRatio usage clear yet? Is it always 0? or do you see other > values here? > > Should it be less than a saturation threshold? (perhaps not unless we > have a way to configure that anyway) I now have a clearer idea on how it works, but it needs to be configurable indeed. > > >> + uint8_t gr = cell->greenRedAvg; >> + uint8_t gb = cell->greenBlueAvg; >> + hist[(gr + gb) / 2]++; >> } >> } >> } >> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h >> index e36797d3..64b71c65 100644 >> --- a/src/ipa/ipu3/algorithms/agc.h >> +++ b/src/ipa/ipu3/algorithms/agc.h >> @@ -50,6 +50,8 @@ private: >> Duration prevExposureNoDg_; >> Duration currentExposure_; >> Duration currentExposureNoDg_; >> + >> + uint32_t stride_; >> }; >> >> } /* namespace ipa::ipu3::algorithms */ >> -- >> 2.30.2 >> > > -- > Kieran >
diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index 5ff50f4a..ebbc5676 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -6,6 +6,7 @@ */ #include "agc.h" +#include "awb.h" #include <algorithm> #include <chrono> @@ -47,9 +48,6 @@ static constexpr uint32_t kMaxExposure = 1976; static constexpr uint32_t knumHistogramBins = 256; static constexpr double kEvGainTarget = 0.5; -/* A cell is 8 bytes and contains averages for RGB values and saturation ratio */ -static constexpr uint8_t kCellSize = 8; - Agc::Agc() : frameCount_(0), lastFrame_(0), iqMean_(0.0), lineDuration_(0s), maxExposureTime_(0s), prevExposure_(0s), prevExposureNoDg_(0s), @@ -60,6 +58,10 @@ Agc::Agc() int Agc::configure([[maybe_unused]] IPAContext &context, const IPAConfigInfo &configInfo) { + const ipu3_uapi_grid_config &grid = context.configuration.grid.bdsGrid; + /* The grid is aligned to the next multiple of 4 */ + stride_ = (grid.width + 3) / 4 * 4; + lineDuration_ = configInfo.sensorInfo.lineLength * 1.0s / configInfo.sensorInfo.pixelRate; maxExposureTime_ = kMaxExposure * lineDuration_; @@ -70,37 +72,23 @@ int Agc::configure([[maybe_unused]] IPAContext &context, void Agc::processBrightness(const ipu3_uapi_stats_3a *stats, const ipu3_uapi_grid_config &grid) { - const struct ipu3_uapi_grid_config statsAeGrid = stats->stats_4a_config.awb_config.grid; - Rectangle aeRegion = { statsAeGrid.x_start, - statsAeGrid.y_start, - static_cast<unsigned int>(statsAeGrid.x_end - statsAeGrid.x_start) + 1, - static_cast<unsigned int>(statsAeGrid.y_end - statsAeGrid.y_start) + 1 }; - Point topleft = aeRegion.topLeft(); - int topleftX = topleft.x >> grid.block_width_log2; - int topleftY = topleft.y >> grid.block_height_log2; - - /* Align to the grid cell width and height */ - uint32_t startX = topleftX << grid.block_width_log2; - uint32_t startY = topleftY * grid.width << grid.block_width_log2; - uint32_t endX = (startX + (aeRegion.size().width >> grid.block_width_log2)) << grid.block_width_log2; - uint32_t i, j; - uint32_t count = 0; - uint32_t hist[knumHistogramBins] = { 0 }; - for (j = topleftY; - j < topleftY + (aeRegion.size().height >> grid.block_height_log2); - j++) { - for (i = startX + startY; i < endX + startY; i += kCellSize) { - /* - * The grid width (and maybe height) is not reliable. - * We observed a bit shift which makes the value 160 to be 32 in the stats grid. - * Use the one passed at init time. - */ - if (stats->awb_raw_buffer.meta_data[i + 4 + j * grid.width] == 0) { - uint8_t Gr = stats->awb_raw_buffer.meta_data[i + 0 + j * grid.width]; - uint8_t Gb = stats->awb_raw_buffer.meta_data[i + 3 + j * grid.width]; - hist[(Gr + Gb) / 2]++; - count++; + + for (unsigned int cellY = 0; cellY < grid.height; cellY++) { + for (unsigned int cellX = 0; cellX < stride_; cellX++) { + uint32_t cellPosition = cellY * stride_ + cellX + * sizeof(Ipu3AwbCell); + + /* Cast the initial IPU3 structure to simplify the reading */ + const Ipu3AwbCell *cell = + reinterpret_cast<const Ipu3AwbCell *>( + &stats->awb_raw_buffer.meta_data[cellPosition] + ); + + if (cell->satRatio == 0) { + uint8_t gr = cell->greenRedAvg; + uint8_t gb = cell->greenBlueAvg; + hist[(gr + gb) / 2]++; } } } diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h index e36797d3..64b71c65 100644 --- a/src/ipa/ipu3/algorithms/agc.h +++ b/src/ipa/ipu3/algorithms/agc.h @@ -50,6 +50,8 @@ private: Duration prevExposureNoDg_; Duration currentExposure_; Duration currentExposureNoDg_; + + uint32_t stride_; }; } /* namespace ipa::ipu3::algorithms */
Now that we know how exactly the AWB statistics are formatted, use a simplified loop in processBrightness() to parse the green values and get the histogram. Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> --- src/ipa/ipu3/algorithms/agc.cpp | 54 +++++++++++++-------------------- src/ipa/ipu3/algorithms/agc.h | 2 ++ 2 files changed, 23 insertions(+), 33 deletions(-)