[libcamera-devel] [PATCH v7 2/5] ipa: ipu3: Rename IspStatsRegion to Accumulator
Jean-Michel Hautbois
jeanmichel.hautbois at ideasonboard.com
Mon Sep 13 18:45:21 CEST 2021
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 at ideasonboard.com>
>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart at 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_;
>> };
>>
>
More information about the libcamera-devel
mailing list