ispstats zone_idx calculation wrong?

[obruendl]

I believe we found a bug in the ispstats. Because our code is modifieda bit to work only in one zone out fix ins not aplicable to the general case where multiple zones are supported.

The results come out of a debugging session with @tasgomes

The bug we found is here:

Vitis_Libraries/vision/L1/include/imgproc/xf_ispstats.hpp

Lines 128 to 130 in b2c657d

	int zone_col = int((col - roi_tlx) / zone_width);
	int zone_row = int((row - roi_tly) / zone_height);
	zone_idx = (zone_row * zone_col_num) + zone_col;

The problem is that col is running one value further than the range that would be natural to expect. As a result, with roi_tlx=0 , the zone_col goes outside of the allowed range (for 1 zone it jumps from 0 to 1), so does the zone_idx and as a result the bin update logic fails.

Vitis_Libraries/vision/L1/include/imgproc/xf_ispstats.hpp

Line 113 in b2c657d

for (uint16_t col = 0; col < (imgwidth + 1); col++) {

We could fix the issue easily by hardcoding zone_idx=0 - but of course this solution only works specifically for us with only one zone.

Could someone confirm if what we found is really a but? I am a bit surprised beause to me it seems like this is not a edge-case at all and anybody using the ispstats on a full frame. So I can hardly estimate nobody ran into this before.

[tasgomes]

The problem is not in the zone_idx statement. The real problem is yet another mix up between X and Y axis and their mapping to rows and columns.

The correct fix for this is the following:

Then zone_idx will be computed accordingly, even when max zones is set to 1.

This core of this function uses X to denote rows and Y to denote columns. The choice of "X" and "Y" to specify the ROI coordinates is indeed a very poor one. The designer was able to mix X, Y, rows and columns in several parts of this function.

For example:

The assertions at the very beginning consider (X -> COL, Y -> ROW).
The calculation of zone_width and zone_height consider (X -> COL, Y -> ROW).
The for-loops consider (X -> ROW, Y -> COL).

All of this could be avoided if the registers to configure the ROI would be named Top-Lelt Row & Column coordinate and Bottom-Right Row & Column coordinate.