| Message ID | 20210913132803.54881-3-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jean-Michel, Thank you for the patch. On Mon, Sep 13, 2021 at 03:28:00PM +0200, Jean-Michel Hautbois wrote: > The IspStatsRegion structure was introduced as an attempt to prepare for > a generic AWB algorithm structure. The structure name by itself is not > explicit and it is too optimistic to try and make a generic one for now. > > Its role is to accumulate the pixels in a given zone. Rename it to > accumulator, and remove the uncounted field at the same time. It is > always possible to know how many pixels are not relevant for the > algorithm by calculating total-counted. The uncounted field was only > declared and not used. Amend the documentation accordingly. > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/ipa/ipu3/algorithms/awb.cpp | 80 +++++++++++++++++++++++++-------- > src/ipa/ipu3/algorithms/awb.h | 5 +-- > 2 files changed, 64 insertions(+), 21 deletions(-) > > diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp > index e05647c9..d993e21e 100644 > --- a/src/ipa/ipu3/algorithms/awb.cpp > +++ b/src/ipa/ipu3/algorithms/awb.cpp > @@ -21,26 +21,71 @@ static constexpr uint32_t kMinZonesCounted = 16; > static constexpr uint32_t kMinGreenLevelInZone = 32; > > /** > - * \struct IspStatsRegion > - * \brief RGB statistics for a given region > + * \struct Accumulator > + * \brief RGB statistics for a given zone > * > - * The IspStatsRegion structure is intended to abstract the ISP specific > - * statistics and use an agnostic algorithm to compute AWB. > + * - Cells are defined in Pixels > + * - Zones are defined in Cells > + * - The total zones may overlap the image width and height to meet hardware > + * constraints Quoting your reply from v6: "[This] means that the zones will be adapted to the BDS output size which may be greater than the rendered frame (asking for 1280x720 may lead to a 1296x720 BDS output and you will have 81 cells of 16 pixels width and not 80)." From a statistics point of view, the zones match the output of the BDS. The image is then slightly cropped to achieve the required resolution. I think that's out of scope here. I would thus drop the last point. The image and text below need to be adapted to replace 1280 with 1296. > * > - * \var IspStatsRegion::counted > - * \brief Number of pixels used to calculate the sums > + * 81 cells > + * /───────────── 1280 pixels ───────────\ > + * 16 zones > + * 16 > + * ┌────┬────┬────┬────┬────┬─ ──────┬────┐ \ > + * │Cell│ │ │ │ │ | │ │ │ > + * 16 │ px │ │ │ │ │ | │ │ │ > + * ├────┼────┼────┼────┼────┼─ ──────┼────┤ │ > + * │ │ │ │ │ │ | │ │ > + * │ │ │ │ │ │ | │ │ 7 > + * │ ── │ ── │ ── │ ── │ ── │ ── ── ─┤ ── │ 1 2 4 > + * │ │ │ │ │ │ | │ │ 2 0 5 > * > - * \var IspStatsRegion::uncounted > - * \brief Remaining number of pixels in the region > + * │ │ │ │ │ │ | │ │ z p c > + * ├────┼────┼────┼────┼────┼─ ──────┼────┤ o i e > + * │ │ │ │ │ │ | │ │ n x l > + * │ │ | │ │ e e l > + * ├─── ───┼─ ──────┼────┤ s l s > + * │ │ | │ │ s > + * │ │ | │ │ > + * ├─── Zone of Cells ───┼─ ──────┼────┤ │ > + * │ (5 x 4) │ | │ │ │ > + * │ │ | │ │ │ > + * ├── ───┼─ ──────┼────┤ │ > + * │ │ │ | │ │ │ > + * │ │ │ │ │ │ | │ │ │ > + * └────┴────┴────┴────┴────┴─ ──────┴────┘ / > * > - * \var IspStatsRegion::rSum > - * \brief Sum of the red values in the region > + * The algorithm works with a fixed number of zones \a kAwbStatsSizeX x > + * \a kAwbStatsSizeY. For example, a frame of 1280x720 is divided into 81x45 > + * cells of [16x16] pixels with a BDS output size of 1296x720 to account for the > + * 16x16 pixel alignment restrictions. In the case of \a kAwbStatsSizeX=16 and > + * \a kAwbStatsSizeY=12 the zones are made of [5x4] cells. The cells are > + * left-aligned and calculated by IPAIPU3::calculateBdsGrid(). > * > - * \var IspStatsRegion::gSum > - * \brief Sum of the green values in the region > + * Each statistics cell represents the average value of the pixels in that cell > + * split by colour components. > * > - * \var IspStatsRegion::bSum > - * \brief Sum of the blue values in the region > + * The Accumulator structure stores the sum of the average of each cell in a > + * zone of the image, as well as the number of cells which were unsaturated and > + * therefore included in the average. > + * \todo move this description and structure into a common header > + * > + * Cells which are saturated beyond the threshold defined in > + * ipu3_uapi_awb_config_s are not included in the average. > + * > + * \var Accumulator::counted > + * \brief Number of unsaturated cells used to calculate the sums > + * > + * \var Accumulator::rSum > + * \brief Sum of the average red values of each unsaturated cell in the zone > + * > + * \var Accumulator::gSum > + * \brief Sum of the average green values of each unsaturated cell in the zone > + * > + * \var Accumulator::bSum > + * \brief Sum of the average blue values of each unsaturated cell in the zone > */ > > /** > @@ -157,7 +202,7 @@ uint32_t Awb::estimateCCT(double red, double green, double blue) > return 449 * n * n * n + 3525 * n * n + 6823.3 * n + 5520.33; > } > > -/* Generate an RGB vector with the average values for each region */ > +/* Generate an RGB vector with the average values for each zone */ > void Awb::generateZones(std::vector<RGB> &zones) > { > for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) { > @@ -174,7 +219,7 @@ void Awb::generateZones(std::vector<RGB> &zones) > } > } > > -/* Translate the IPU3 statistics into the default statistics region array */ > +/* Translate the IPU3 statistics into the default statistics zone array */ > void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats, > const ipu3_uapi_grid_config &grid) > { > @@ -215,7 +260,6 @@ void Awb::clearAwbStats() > awbStats_[i].rSum = 0; > awbStats_[i].gSum = 0; > awbStats_[i].counted = 0; > - awbStats_[i].uncounted = 0; > } > } > > @@ -304,7 +348,7 @@ void Awb::prepare(IPAContext &context, ipu3_uapi_params *params) > > /* > * Optical center is column start (respectively row start) of the > - * region of interest minus its X center (respectively Y center). > + * cell of interest minus its X center (respectively Y center). > * > * For the moment use BDS as a first approximation, but it should > * be calculated based on Shading (SHD) parameters. > diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h > index cc848060..ac8ccc84 100644 > --- a/src/ipa/ipu3/algorithms/awb.h > +++ b/src/ipa/ipu3/algorithms/awb.h > @@ -33,9 +33,8 @@ struct Ipu3AwbCell { > unsigned char padding[3]; > } __attribute__((packed)); > > -struct IspStatsRegion { > +struct Accumulator { > unsigned int counted; > - unsigned int uncounted; > unsigned long long rSum; > unsigned long long gSum; > unsigned long long bSum; > @@ -82,7 +81,7 @@ private: > uint32_t estimateCCT(double red, double green, double blue); > > std::vector<RGB> zones_; > - IspStatsRegion awbStats_[kAwbStatsSizeX * kAwbStatsSizeY]; > + Accumulator awbStats_[kAwbStatsSizeX * kAwbStatsSizeY]; > AwbStatus asyncResults_; > }; >
Hi Laurent, On 13/09/2021 17:56, Laurent Pinchart wrote: > Hi Jean-Michel, > > Thank you for the patch. > > On Mon, Sep 13, 2021 at 03:28:00PM +0200, Jean-Michel Hautbois wrote: >> The IspStatsRegion structure was introduced as an attempt to prepare for >> a generic AWB algorithm structure. The structure name by itself is not >> explicit and it is too optimistic to try and make a generic one for now. >> >> Its role is to accumulate the pixels in a given zone. Rename it to >> accumulator, and remove the uncounted field at the same time. It is >> always possible to know how many pixels are not relevant for the >> algorithm by calculating total-counted. The uncounted field was only >> declared and not used. Amend the documentation accordingly. >> >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> --- >> src/ipa/ipu3/algorithms/awb.cpp | 80 +++++++++++++++++++++++++-------- >> src/ipa/ipu3/algorithms/awb.h | 5 +-- >> 2 files changed, 64 insertions(+), 21 deletions(-) >> >> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp >> index e05647c9..d993e21e 100644 >> --- a/src/ipa/ipu3/algorithms/awb.cpp >> +++ b/src/ipa/ipu3/algorithms/awb.cpp >> @@ -21,26 +21,71 @@ static constexpr uint32_t kMinZonesCounted = 16; >> static constexpr uint32_t kMinGreenLevelInZone = 32; >> >> /** >> - * \struct IspStatsRegion >> - * \brief RGB statistics for a given region >> + * \struct Accumulator >> + * \brief RGB statistics for a given zone >> * >> - * The IspStatsRegion structure is intended to abstract the ISP specific >> - * statistics and use an agnostic algorithm to compute AWB. >> + * - Cells are defined in Pixels >> + * - Zones are defined in Cells >> + * - The total zones may overlap the image width and height to meet hardware >> + * constraints > > Quoting your reply from v6: > > "[This] means that the zones will be adapted to the BDS output size > which may be greater than the rendered frame (asking for 1280x720 may > lead to a 1296x720 BDS output and you will have 81 cells of 16 pixels > width and not 80)." > > From a statistics point of view, the zones match the output of the BDS. > The image is then slightly cropped to achieve the required resolution. I > think that's out of scope here. I would thus drop the last point. The > image and text below need to be adapted to replace 1280 with 1296. > >> * >> - * \var IspStatsRegion::counted >> - * \brief Number of pixels used to calculate the sums >> + * 81 cells >> + * /───────────── 1280 pixels ───────────\ >> + * 16 zones >> + * 16 >> + * ┌────┬────┬────┬────┬────┬─ ──────┬────┐ \ >> + * │Cell│ │ │ │ │ | │ │ │ >> + * 16 │ px │ │ │ │ │ | │ │ │ >> + * ├────┼────┼────┼────┼────┼─ ──────┼────┤ │ >> + * │ │ │ │ │ │ | │ │ >> + * │ │ │ │ │ │ | │ │ 7 >> + * │ ── │ ── │ ── │ ── │ ── │ ── ── ─┤ ── │ 1 2 4 >> + * │ │ │ │ │ │ | │ │ 2 0 5 >> * >> - * \var IspStatsRegion::uncounted >> - * \brief Remaining number of pixels in the region >> + * │ │ │ │ │ │ | │ │ z p c >> + * ├────┼────┼────┼────┼────┼─ ──────┼────┤ o i e >> + * │ │ │ │ │ │ | │ │ n x l >> + * │ │ | │ │ e e l >> + * ├─── ───┼─ ──────┼────┤ s l s >> + * │ │ | │ │ s >> + * │ │ | │ │ >> + * ├─── Zone of Cells ───┼─ ──────┼────┤ │ >> + * │ (5 x 4) │ | │ │ │ >> + * │ │ | │ │ │ >> + * ├── ───┼─ ──────┼────┤ │ >> + * │ │ │ | │ │ │ >> + * │ │ │ │ │ │ | │ │ │ >> + * └────┴────┴────┴────┴────┴─ ──────┴────┘ / >> * >> - * \var IspStatsRegion::rSum >> - * \brief Sum of the red values in the region >> + * The algorithm works with a fixed number of zones \a kAwbStatsSizeX x >> + * \a kAwbStatsSizeY. For example, a frame of 1280x720 is divided into 81x45 >> + * cells of [16x16] pixels with a BDS output size of 1296x720 to account for the Replacing 1280 by 1296 except this sentence, right :-) ? >> + * 16x16 pixel alignment restrictions. In the case of \a kAwbStatsSizeX=16 and >> + * \a kAwbStatsSizeY=12 the zones are made of [5x4] cells. The cells are >> + * left-aligned and calculated by IPAIPU3::calculateBdsGrid(). >> * >> - * \var IspStatsRegion::gSum >> - * \brief Sum of the green values in the region >> + * Each statistics cell represents the average value of the pixels in that cell >> + * split by colour components. >> * >> - * \var IspStatsRegion::bSum >> - * \brief Sum of the blue values in the region >> + * The Accumulator structure stores the sum of the average of each cell in a >> + * zone of the image, as well as the number of cells which were unsaturated and >> + * therefore included in the average. >> + * \todo move this description and structure into a common header >> + * >> + * Cells which are saturated beyond the threshold defined in >> + * ipu3_uapi_awb_config_s are not included in the average. >> + * >> + * \var Accumulator::counted >> + * \brief Number of unsaturated cells used to calculate the sums >> + * >> + * \var Accumulator::rSum >> + * \brief Sum of the average red values of each unsaturated cell in the zone >> + * >> + * \var Accumulator::gSum >> + * \brief Sum of the average green values of each unsaturated cell in the zone >> + * >> + * \var Accumulator::bSum >> + * \brief Sum of the average blue values of each unsaturated cell in the zone >> */ >> >> /** >> @@ -157,7 +202,7 @@ uint32_t Awb::estimateCCT(double red, double green, double blue) >> return 449 * n * n * n + 3525 * n * n + 6823.3 * n + 5520.33; >> } >> >> -/* Generate an RGB vector with the average values for each region */ >> +/* Generate an RGB vector with the average values for each zone */ >> void Awb::generateZones(std::vector<RGB> &zones) >> { >> for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) { >> @@ -174,7 +219,7 @@ void Awb::generateZones(std::vector<RGB> &zones) >> } >> } >> >> -/* Translate the IPU3 statistics into the default statistics region array */ >> +/* Translate the IPU3 statistics into the default statistics zone array */ >> void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats, >> const ipu3_uapi_grid_config &grid) >> { >> @@ -215,7 +260,6 @@ void Awb::clearAwbStats() >> awbStats_[i].rSum = 0; >> awbStats_[i].gSum = 0; >> awbStats_[i].counted = 0; >> - awbStats_[i].uncounted = 0; >> } >> } >> >> @@ -304,7 +348,7 @@ void Awb::prepare(IPAContext &context, ipu3_uapi_params *params) >> >> /* >> * Optical center is column start (respectively row start) of the >> - * region of interest minus its X center (respectively Y center). >> + * cell of interest minus its X center (respectively Y center). >> * >> * For the moment use BDS as a first approximation, but it should >> * be calculated based on Shading (SHD) parameters. >> diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h >> index cc848060..ac8ccc84 100644 >> --- a/src/ipa/ipu3/algorithms/awb.h >> +++ b/src/ipa/ipu3/algorithms/awb.h >> @@ -33,9 +33,8 @@ struct Ipu3AwbCell { >> unsigned char padding[3]; >> } __attribute__((packed)); >> >> -struct IspStatsRegion { >> +struct Accumulator { >> unsigned int counted; >> - unsigned int uncounted; >> unsigned long long rSum; >> unsigned long long gSum; >> unsigned long long bSum; >> @@ -82,7 +81,7 @@ private: >> uint32_t estimateCCT(double red, double green, double blue); >> >> std::vector<RGB> zones_; >> - IspStatsRegion awbStats_[kAwbStatsSizeX * kAwbStatsSizeY]; >> + Accumulator awbStats_[kAwbStatsSizeX * kAwbStatsSizeY]; >> AwbStatus asyncResults_; >> }; >> >
On Mon, Sep 13, 2021 at 06:45:21PM +0200, Jean-Michel Hautbois wrote: > Hi Laurent, > > On 13/09/2021 17:56, Laurent Pinchart wrote: > > Hi Jean-Michel, > > > > Thank you for the patch. > > > > On Mon, Sep 13, 2021 at 03:28:00PM +0200, Jean-Michel Hautbois wrote: > >> The IspStatsRegion structure was introduced as an attempt to prepare for > >> a generic AWB algorithm structure. The structure name by itself is not > >> explicit and it is too optimistic to try and make a generic one for now. > >> > >> Its role is to accumulate the pixels in a given zone. Rename it to > >> accumulator, and remove the uncounted field at the same time. It is > >> always possible to know how many pixels are not relevant for the > >> algorithm by calculating total-counted. The uncounted field was only > >> declared and not used. Amend the documentation accordingly. > >> > >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> --- > >> src/ipa/ipu3/algorithms/awb.cpp | 80 +++++++++++++++++++++++++-------- > >> src/ipa/ipu3/algorithms/awb.h | 5 +-- > >> 2 files changed, 64 insertions(+), 21 deletions(-) > >> > >> diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp > >> index e05647c9..d993e21e 100644 > >> --- a/src/ipa/ipu3/algorithms/awb.cpp > >> +++ b/src/ipa/ipu3/algorithms/awb.cpp > >> @@ -21,26 +21,71 @@ static constexpr uint32_t kMinZonesCounted = 16; > >> static constexpr uint32_t kMinGreenLevelInZone = 32; > >> > >> /** > >> - * \struct IspStatsRegion > >> - * \brief RGB statistics for a given region > >> + * \struct Accumulator > >> + * \brief RGB statistics for a given zone > >> * > >> - * The IspStatsRegion structure is intended to abstract the ISP specific > >> - * statistics and use an agnostic algorithm to compute AWB. > >> + * - Cells are defined in Pixels > >> + * - Zones are defined in Cells > >> + * - The total zones may overlap the image width and height to meet hardware > >> + * constraints > > > > Quoting your reply from v6: > > > > "[This] means that the zones will be adapted to the BDS output size > > which may be greater than the rendered frame (asking for 1280x720 may > > lead to a 1296x720 BDS output and you will have 81 cells of 16 pixels > > width and not 80)." > > > > From a statistics point of view, the zones match the output of the BDS. > > The image is then slightly cropped to achieve the required resolution. I > > think that's out of scope here. I would thus drop the last point. The > > image and text below need to be adapted to replace 1280 with 1296. > > > >> * > >> - * \var IspStatsRegion::counted > >> - * \brief Number of pixels used to calculate the sums > >> + * 81 cells > >> + * /───────────── 1280 pixels ───────────\ > >> + * 16 zones > >> + * 16 > >> + * ┌────┬────┬────┬────┬────┬─ ──────┬────┐ \ > >> + * │Cell│ │ │ │ │ | │ │ │ > >> + * 16 │ px │ │ │ │ │ | │ │ │ > >> + * ├────┼────┼────┼────┼────┼─ ──────┼────┤ │ > >> + * │ │ │ │ │ │ | │ │ > >> + * │ │ │ │ │ │ | │ │ 7 > >> + * │ ── │ ── │ ── │ ── │ ── │ ── ── ─┤ ── │ 1 2 4 > >> + * │ │ │ │ │ │ | │ │ 2 0 5 > >> * > >> - * \var IspStatsRegion::uncounted > >> - * \brief Remaining number of pixels in the region > >> + * │ │ │ │ │ │ | │ │ z p c > >> + * ├────┼────┼────┼────┼────┼─ ──────┼────┤ o i e > >> + * │ │ │ │ │ │ | │ │ n x l > >> + * │ │ | │ │ e e l > >> + * ├─── ───┼─ ──────┼────┤ s l s > >> + * │ │ | │ │ s > >> + * │ │ | │ │ > >> + * ├─── Zone of Cells ───┼─ ──────┼────┤ │ > >> + * │ (5 x 4) │ | │ │ │ > >> + * │ │ | │ │ │ > >> + * ├── ───┼─ ──────┼────┤ │ > >> + * │ │ │ | │ │ │ > >> + * │ │ │ │ │ │ | │ │ │ > >> + * └────┴────┴────┴────┴────┴─ ──────┴────┘ / > >> * > >> - * \var IspStatsRegion::rSum > >> - * \brief Sum of the red values in the region > >> + * The algorithm works with a fixed number of zones \a kAwbStatsSizeX x > >> + * \a kAwbStatsSizeY. For example, a frame of 1280x720 is divided into 81x45 > >> + * cells of [16x16] pixels with a BDS output size of 1296x720 to account for the > > Replacing 1280 by 1296 except this sentence, right :-) ? I'd write "For example, a frame of 1296x720 is divided into 81x45 cells of [16x16] pixels." We shouldn't mention the BDS here, this belongs to the documentation of the ImgU, not the Accumulator structure. > >> + * 16x16 pixel alignment restrictions. In the case of \a kAwbStatsSizeX=16 and > >> + * \a kAwbStatsSizeY=12 the zones are made of [5x4] cells. The cells are > >> + * left-aligned and calculated by IPAIPU3::calculateBdsGrid(). > >> * > >> - * \var IspStatsRegion::gSum > >> - * \brief Sum of the green values in the region > >> + * Each statistics cell represents the average value of the pixels in that cell > >> + * split by colour components. > >> * > >> - * \var IspStatsRegion::bSum > >> - * \brief Sum of the blue values in the region > >> + * The Accumulator structure stores the sum of the average of each cell in a > >> + * zone of the image, as well as the number of cells which were unsaturated and > >> + * therefore included in the average. > >> + * \todo move this description and structure into a common header > >> + * > >> + * Cells which are saturated beyond the threshold defined in > >> + * ipu3_uapi_awb_config_s are not included in the average. > >> + * > >> + * \var Accumulator::counted > >> + * \brief Number of unsaturated cells used to calculate the sums > >> + * > >> + * \var Accumulator::rSum > >> + * \brief Sum of the average red values of each unsaturated cell in the zone > >> + * > >> + * \var Accumulator::gSum > >> + * \brief Sum of the average green values of each unsaturated cell in the zone > >> + * > >> + * \var Accumulator::bSum > >> + * \brief Sum of the average blue values of each unsaturated cell in the zone > >> */ > >> > >> /** > >> @@ -157,7 +202,7 @@ uint32_t Awb::estimateCCT(double red, double green, double blue) > >> return 449 * n * n * n + 3525 * n * n + 6823.3 * n + 5520.33; > >> } > >> > >> -/* Generate an RGB vector with the average values for each region */ > >> +/* Generate an RGB vector with the average values for each zone */ > >> void Awb::generateZones(std::vector<RGB> &zones) > >> { > >> for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) { > >> @@ -174,7 +219,7 @@ void Awb::generateZones(std::vector<RGB> &zones) > >> } > >> } > >> > >> -/* Translate the IPU3 statistics into the default statistics region array */ > >> +/* Translate the IPU3 statistics into the default statistics zone array */ > >> void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats, > >> const ipu3_uapi_grid_config &grid) > >> { > >> @@ -215,7 +260,6 @@ void Awb::clearAwbStats() > >> awbStats_[i].rSum = 0; > >> awbStats_[i].gSum = 0; > >> awbStats_[i].counted = 0; > >> - awbStats_[i].uncounted = 0; > >> } > >> } > >> > >> @@ -304,7 +348,7 @@ void Awb::prepare(IPAContext &context, ipu3_uapi_params *params) > >> > >> /* > >> * Optical center is column start (respectively row start) of the > >> - * region of interest minus its X center (respectively Y center). > >> + * cell of interest minus its X center (respectively Y center). > >> * > >> * For the moment use BDS as a first approximation, but it should > >> * be calculated based on Shading (SHD) parameters. > >> diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h > >> index cc848060..ac8ccc84 100644 > >> --- a/src/ipa/ipu3/algorithms/awb.h > >> +++ b/src/ipa/ipu3/algorithms/awb.h > >> @@ -33,9 +33,8 @@ struct Ipu3AwbCell { > >> unsigned char padding[3]; > >> } __attribute__((packed)); > >> > >> -struct IspStatsRegion { > >> +struct Accumulator { > >> unsigned int counted; > >> - unsigned int uncounted; > >> unsigned long long rSum; > >> unsigned long long gSum; > >> unsigned long long bSum; > >> @@ -82,7 +81,7 @@ private: > >> uint32_t estimateCCT(double red, double green, double blue); > >> > >> std::vector<RGB> zones_; > >> - IspStatsRegion awbStats_[kAwbStatsSizeX * kAwbStatsSizeY]; > >> + Accumulator awbStats_[kAwbStatsSizeX * kAwbStatsSizeY]; > >> AwbStatus asyncResults_; > >> }; > >>
diff --git a/src/ipa/ipu3/algorithms/awb.cpp b/src/ipa/ipu3/algorithms/awb.cpp index e05647c9..d993e21e 100644 --- a/src/ipa/ipu3/algorithms/awb.cpp +++ b/src/ipa/ipu3/algorithms/awb.cpp @@ -21,26 +21,71 @@ static constexpr uint32_t kMinZonesCounted = 16; static constexpr uint32_t kMinGreenLevelInZone = 32; /** - * \struct IspStatsRegion - * \brief RGB statistics for a given region + * \struct Accumulator + * \brief RGB statistics for a given zone * - * The IspStatsRegion structure is intended to abstract the ISP specific - * statistics and use an agnostic algorithm to compute AWB. + * - Cells are defined in Pixels + * - Zones are defined in Cells + * - The total zones may overlap the image width and height to meet hardware + * constraints * - * \var IspStatsRegion::counted - * \brief Number of pixels used to calculate the sums + * 81 cells + * /───────────── 1280 pixels ───────────\ + * 16 zones + * 16 + * ┌────┬────┬────┬────┬────┬─ ──────┬────┐ \ + * │Cell│ │ │ │ │ | │ │ │ + * 16 │ px │ │ │ │ │ | │ │ │ + * ├────┼────┼────┼────┼────┼─ ──────┼────┤ │ + * │ │ │ │ │ │ | │ │ + * │ │ │ │ │ │ | │ │ 7 + * │ ── │ ── │ ── │ ── │ ── │ ── ── ─┤ ── │ 1 2 4 + * │ │ │ │ │ │ | │ │ 2 0 5 * - * \var IspStatsRegion::uncounted - * \brief Remaining number of pixels in the region + * │ │ │ │ │ │ | │ │ z p c + * ├────┼────┼────┼────┼────┼─ ──────┼────┤ o i e + * │ │ │ │ │ │ | │ │ n x l + * │ │ | │ │ e e l + * ├─── ───┼─ ──────┼────┤ s l s + * │ │ | │ │ s + * │ │ | │ │ + * ├─── Zone of Cells ───┼─ ──────┼────┤ │ + * │ (5 x 4) │ | │ │ │ + * │ │ | │ │ │ + * ├── ───┼─ ──────┼────┤ │ + * │ │ │ | │ │ │ + * │ │ │ │ │ │ | │ │ │ + * └────┴────┴────┴────┴────┴─ ──────┴────┘ / * - * \var IspStatsRegion::rSum - * \brief Sum of the red values in the region + * The algorithm works with a fixed number of zones \a kAwbStatsSizeX x + * \a kAwbStatsSizeY. For example, a frame of 1280x720 is divided into 81x45 + * cells of [16x16] pixels with a BDS output size of 1296x720 to account for the + * 16x16 pixel alignment restrictions. In the case of \a kAwbStatsSizeX=16 and + * \a kAwbStatsSizeY=12 the zones are made of [5x4] cells. The cells are + * left-aligned and calculated by IPAIPU3::calculateBdsGrid(). * - * \var IspStatsRegion::gSum - * \brief Sum of the green values in the region + * Each statistics cell represents the average value of the pixels in that cell + * split by colour components. * - * \var IspStatsRegion::bSum - * \brief Sum of the blue values in the region + * The Accumulator structure stores the sum of the average of each cell in a + * zone of the image, as well as the number of cells which were unsaturated and + * therefore included in the average. + * \todo move this description and structure into a common header + * + * Cells which are saturated beyond the threshold defined in + * ipu3_uapi_awb_config_s are not included in the average. + * + * \var Accumulator::counted + * \brief Number of unsaturated cells used to calculate the sums + * + * \var Accumulator::rSum + * \brief Sum of the average red values of each unsaturated cell in the zone + * + * \var Accumulator::gSum + * \brief Sum of the average green values of each unsaturated cell in the zone + * + * \var Accumulator::bSum + * \brief Sum of the average blue values of each unsaturated cell in the zone */ /** @@ -157,7 +202,7 @@ uint32_t Awb::estimateCCT(double red, double green, double blue) return 449 * n * n * n + 3525 * n * n + 6823.3 * n + 5520.33; } -/* Generate an RGB vector with the average values for each region */ +/* Generate an RGB vector with the average values for each zone */ void Awb::generateZones(std::vector<RGB> &zones) { for (unsigned int i = 0; i < kAwbStatsSizeX * kAwbStatsSizeY; i++) { @@ -174,7 +219,7 @@ void Awb::generateZones(std::vector<RGB> &zones) } } -/* Translate the IPU3 statistics into the default statistics region array */ +/* Translate the IPU3 statistics into the default statistics zone array */ void Awb::generateAwbStats(const ipu3_uapi_stats_3a *stats, const ipu3_uapi_grid_config &grid) { @@ -215,7 +260,6 @@ void Awb::clearAwbStats() awbStats_[i].rSum = 0; awbStats_[i].gSum = 0; awbStats_[i].counted = 0; - awbStats_[i].uncounted = 0; } } @@ -304,7 +348,7 @@ void Awb::prepare(IPAContext &context, ipu3_uapi_params *params) /* * Optical center is column start (respectively row start) of the - * region of interest minus its X center (respectively Y center). + * cell of interest minus its X center (respectively Y center). * * For the moment use BDS as a first approximation, but it should * be calculated based on Shading (SHD) parameters. diff --git a/src/ipa/ipu3/algorithms/awb.h b/src/ipa/ipu3/algorithms/awb.h index cc848060..ac8ccc84 100644 --- a/src/ipa/ipu3/algorithms/awb.h +++ b/src/ipa/ipu3/algorithms/awb.h @@ -33,9 +33,8 @@ struct Ipu3AwbCell { unsigned char padding[3]; } __attribute__((packed)); -struct IspStatsRegion { +struct Accumulator { unsigned int counted; - unsigned int uncounted; unsigned long long rSum; unsigned long long gSum; unsigned long long bSum; @@ -82,7 +81,7 @@ private: uint32_t estimateCCT(double red, double green, double blue); std::vector<RGB> zones_; - IspStatsRegion awbStats_[kAwbStatsSizeX * kAwbStatsSizeY]; + Accumulator awbStats_[kAwbStatsSizeX * kAwbStatsSizeY]; AwbStatus asyncResults_; };