[PATCH 0/3] vsp-tests: extend HGT tests

[PATCH 0/3] vsp-tests: extend HGT tests

[Niklas Söderlund]

Hi Laurent,

This series extends the vsp-tests with all possible setups for HGT hue 
areas. Please consider it for inclusion in the histogram branch.

Niklas Söderlund (3):
  gen-image: support overlapping hue areas for HGT frames
  tests: add hue area wraparound test for HGT
  tests: add none adjoined hue areas test for HGT

 src/gen-image.c             | 98 ++++++++++++++++++++++++++++++++++++---------
 tests/vsp-unit-test-0018.sh |  4 ++
 2 files changed, 83 insertions(+), 19 deletions(-)

-- 
2.9.3

[PATCH 1/3] gen-image: support overlapping hue areas for HGT frames

[Niklas Söderlund]

From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

The HGT can operate with hue areas which are not directly adjoined. In
this mode of operation hue values which are between two areas are
attributed to both areas with a weight for the final histogram.

Add support to generate such histograms using gen-image which can be
used to verify correct operation of the HGT. Previously gen-image could
only generate histograms with adjoined areas.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 src/gen-image.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 79 insertions(+), 19 deletions(-)

diff --git a/src/gen-image.c b/src/gen-image.c
index 688f602..9cd5eb9 100644
--- a/src/gen-image.c
+++ b/src/gen-image.c
@@ -1301,7 +1301,7 @@ static void histogram_compute_hgt(const struct image *image, void *histo,
 	uint8_t rgb[3], hsv[3], smin = 255, smax = 0;
 	uint32_t sum = 0, hist[6][32];
 	unsigned int x, y, i;
-	int m, n;
+	unsigned int hist_n, hue_pos;
 
 	memset(hist, 0x00, sizeof(hist));
 
@@ -1317,24 +1317,84 @@ static void histogram_compute_hgt(const struct image *image, void *histo,
 			smax = max(smax, hsv[1]);
 			sum += hsv[1];
 
-			/* Find m and n for hist */
-			m = n = -1;
-			for (i = 0; i < 6  && m == -1; i++)
-				if (hsv[0] >= hue_areas[i*2] && (hsv[0] <= hue_areas[i*2+1]))
-					m = i;
-			for (i = 0; i < 32 && n == -1; i++)
-				if ((hsv[1] >= 8*i) && (hsv[1] < 8 * (i+1)))
-					n = i;
+			/* Find n for hist */
+			hist_n = hsv[1] / 8;
 
 			/*
-			 * The HW supports a declining weight to be applied
-			 * when hue areas are not directly adjoined. This
-			 * test can not replicated this, the hue areas need
-			 * to be set without any gaps else the weights from HW
-			 * will be wrong. Max weight is 16.
+			 * Find position in hue areas which is greater than the
+			 * current H value. Special consideration is needed for:
+			 *
+			 * - Values inside a hue area are inclusive, values that
+			 *   are between two hue areas are exclusive.
+			 * - Hue area 0 can wrap around the H value space, for
+			 *   example include values greater then 240 but less
+			 *   then 30.
 			 */
-			if (m != -1 && n != -1)
-				hist[m][n] += 16;
+			for (i = 0; i < 12; i++) {
+
+				/* Special cases when area 0 wraps around */
+				if (hue_areas[0] > hue_areas[1]) {
+
+					/* Check if pixel is inside the wrapped area 0 */
+					if (hsv[0] > hue_areas[0] || hsv[0] <= hue_areas[1]) {
+						hue_pos = 1;
+						break;
+					}
+
+					/* Exclude first area point from normal logic */
+					if (!i)
+						continue;
+				}
+
+				/* Check if H is inside one of the hue areas */
+				if ((hsv[0] < hue_areas[i]) || (i % 2 && hsv[0] == hue_areas[i])) {
+					hue_pos = i;
+					break;
+				}
+
+				/* Check if H is larger then area 5 */
+				if (hsv[0] > hue_areas[11]) {
+					hue_pos = 0;
+					break;
+				}
+			}
+
+			/*
+			 * Figure out which areas the current H value should be
+			 * attributed to. If the H value is inside one of the
+			 * areas the max weight (16) is attributed to it else
+			 * the weight is split between them based on how close
+			 * the H value is to each area.
+			 *
+			 * If ''hue_pos'' are odd the H value is inside an area and
+			 * it should be attributed the full weight to area hue_pos/2
+			 * else it should be split between area hue_pos/2 and
+			 * hue_pos/2 - 1.
+			 */
+			if (hue_pos % 2) {
+				hist[hue_pos/2][hist_n] += 16;
+			} else {
+				unsigned int hue1, hue2;
+				unsigned int length, width, weight;
+
+				hue1 = hue_areas[hue_pos ? hue_pos - 1 : 11];
+				hue2 = hue_areas[hue_pos];
+
+				/* Calculate the total width between the two areas */
+				width = hue2 - hue1 + (hue1 > hue2 ? 256 : 0);
+
+				/* Calculate the length to the right most area */
+				if (hue1 > hue2 && hsv[0] > hue1)
+					length = width - (hsv[0] - hue1);
+				else
+					length = abs(hsv[0] - hue2);
+
+				/* Calculate weight and round up */
+				weight = (length * 16 + width - 1) / width;
+				/* Split weight between the two areas */
+				hist[hue_pos ? hue_pos/2 - 1 : 5][hist_n] += weight;
+				hist[hue_pos/2][hist_n] += 16 - weight;
+			}
 		}
 	}
 
@@ -1349,9 +1409,9 @@ static void histogram_compute_hgt(const struct image *image, void *histo,
 	histo += 4;
 
 	/* Weighted Frequency of Hue Area-m and Saturation Area-n */
-	for (m = 0; m < 6; m++) {
-		for (n = 0; n < 32; n++) {
-			*(uint32_t *)histo = hist[m][n];
+	for (x = 0; x < 6; x++) {
+		for (y = 0; y < 32; y++) {
+			*(uint32_t *)histo = hist[x][y];
 			histo += 4;
 		}
 	}
-- 
2.9.3

[PATCH 2/3] tests: add hue area wraparound test for HGT

[Niklas Söderlund]

From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

One configuration for HGT histograms are where hue area 0 is wrapped
around the hue input value space (0-255), for example area 0 can cover
hue values above 220 but bellow 40. Hue area 0 is the only area which
supports this mode where the areas lower limit uses a higher value then
it's higher limit hence the area wraparound the max hue value.

Add a test to verify this works now that gen-image supports generating
reference frames with wrapped around hue areas.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 tests/vsp-unit-test-0018.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/vsp-unit-test-0018.sh b/tests/vsp-unit-test-0018.sh
index 2b49b54..944f7ac 100755
--- a/tests/vsp-unit-test-0018.sh
+++ b/tests/vsp-unit-test-0018.sh
@@ -31,6 +31,7 @@ test_histogram() {
 test_main() {
 	test_histogram "0,255,255,255,255,255,255,255,255,255,255,255"
 	test_histogram "0,40,40,80,80,120,120,160,160,200,200,255"
+	test_histogram "220,40,40,80,80,120,120,160,160,200,200,220"
 }
 
 test_init $0 "$features"
-- 
2.9.3

[PATCH 3/3] tests: add none adjoined hue areas test for HGT

[Niklas Söderlund]

From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

The HGT can be configured in such a way that the 6 user configurable hue
areas are not joined next to each other. In such cases a hue value
outside of a hue area should be attributed with a weight to both its
adjoining hue areas.

Add a test to verify this works now that gen-image supports generating
reference frames for none adjoined hue areas.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 tests/vsp-unit-test-0018.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/vsp-unit-test-0018.sh b/tests/vsp-unit-test-0018.sh
index 944f7ac..c0aba4b 100755
--- a/tests/vsp-unit-test-0018.sh
+++ b/tests/vsp-unit-test-0018.sh
@@ -32,6 +32,9 @@ test_main() {
 	test_histogram "0,255,255,255,255,255,255,255,255,255,255,255"
 	test_histogram "0,40,40,80,80,120,120,160,160,200,200,255"
 	test_histogram "220,40,40,80,80,120,120,160,160,200,200,220"
+	test_histogram "0,10,50,60,100,110,150,160,200,210,250,255"
+	test_histogram "10,20,50,60,100,110,150,160,200,210,230,240"
+	test_histogram "240,20,60,80,100,120,140,160,180,200,210,220"
 }
 
 test_init $0 "$features"
-- 
2.9.3

Re: [PATCH 1/3] gen-image: support overlapping hue areas for HGT frames

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Tuesday 04 Oct 2016 15:09:13 Niklas Söderlund wrote:
> From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> The HGT can operate with hue areas which are not directly adjoined. In
> this mode of operation hue values which are between two areas are

s/which/that/g

> attributed to both areas with a weight for the final histogram.
> 
> Add support to generate such histograms using gen-image which can be
> used to verify correct operation of the HGT. Previously gen-image could
> only generate histograms with adjoined areas.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
>  src/gen-image.c | 98 +++++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 79 insertions(+), 19 deletions(-)
> 
> diff --git a/src/gen-image.c b/src/gen-image.c
> index 688f602..9cd5eb9 100644
> --- a/src/gen-image.c
> +++ b/src/gen-image.c
> @@ -1301,7 +1301,7 @@ static void histogram_compute_hgt(const struct image
> *image, void *histo, uint8_t rgb[3], hsv[3], smin = 255, smax = 0;
>  	uint32_t sum = 0, hist[6][32];
>  	unsigned int x, y, i;
> -	int m, n;
> +	unsigned int hist_n, hue_pos;
> 
>  	memset(hist, 0x00, sizeof(hist));
> 
> @@ -1317,24 +1317,84 @@ static void histogram_compute_hgt(const struct image
> *image, void *histo, smax = max(smax, hsv[1]);
>  			sum += hsv[1];
> 
> -			/* Find m and n for hist */
> -			m = n = -1;
> -			for (i = 0; i < 6  && m == -1; i++)
> -				if (hsv[0] >= hue_areas[i*2] && (hsv[0] <= 
hue_areas[i*2+1]))
> -					m = i;
> -			for (i = 0; i < 32 && n == -1; i++)
> -				if ((hsv[1] >= 8*i) && (hsv[1] < 8 * (i+1)))
> -					n = i;
> +			/* Find n for hist */

How about "Compute the n coordinate of the histogram bucket" ?

> +			hist_n = hsv[1] / 8;
> 
>  			/*
> -			 * The HW supports a declining weight to be applied
> -			 * when hue areas are not directly adjoined. This
> -			 * test can not replicated this, the hue areas need
> -			 * to be set without any gaps else the weights from HW
> -			 * will be wrong. Max weight is 16.
> +			 * Find position in hue areas which is greater than 
the

s/which/that/

https://en.oxforddictionaries.com/usage/that-or-which

> +			 * current H value. Special consideration is needed 
for:
> +			 *
> +			 * - Values inside a hue area are inclusive, values 
that
> +			 *   are between two hue areas are exclusive.
> +			 * - Hue area 0 can wrap around the H value space, for
> +			 *   example include values greater then 240 but less

s/then/than/
s/but/and/

> +			 *   then 30.
>  			 */
> -			if (m != -1 && n != -1)
> -				hist[m][n] += 16;
> +			for (i = 0; i < 12; i++) {
> +
> +				/* Special cases when area 0 wraps around */
> +				if (hue_areas[0] > hue_areas[1]) {
> +
> +					/* Check if pixel is inside the 
wrapped area 0 */
> +					if (hsv[0] > hue_areas[0] || hsv[0] <= 
hue_areas[1]) {
> +						hue_pos = 1;
> +						break;
> +					}
> +
> +					/* Exclude first area point from 
normal logic */
> +					if (!i)
> +						continue;
> +				}
> +
> +				/* Check if H is inside one of the hue areas 
*/
> +				if ((hsv[0] < hue_areas[i]) || (i % 2 && 
hsv[0] == hue_areas[i])) {
> +					hue_pos = i;
> +					break;
> +				}
> +
> +				/* Check if H is larger then area 5 */
> +				if (hsv[0] > hue_areas[11]) {
> +					hue_pos = 0;
> +					break;
> +				}
> +			}

What would you think about precomputing the hue pos values for hue values 0 to 
255 and just indexing that table ? That should be faster at runtime, with an 
additional memory consumption of 256 bytes, which seems quite negligible to 
me. I can fix that as an additional patch.

> +
> +			/*
> +			 * Figure out which areas the current H value should 
be
> +			 * attributed to. If the H value is inside one of the
> +			 * areas the max weight (16) is attributed to it else
> +			 * the weight is split between them based on how close
> +			 * the H value is to each area.
> +			 *
> +			 * If ''hue_pos'' are odd the H value is inside an

s/are/is/

> area and
> +			 * it should be attributed the full weight to area 
hue_pos/2
> +			 * else it should be split between area hue_pos/2 and

s/area/areas/

> +			 * hue_pos/2 - 1.
> +			 */
> +			if (hue_pos % 2) {
> +				hist[hue_pos/2][hist_n] += 16;
> +			} else {
> +				unsigned int hue1, hue2;
> +				unsigned int length, width, weight;

I'd name the variable dist(ance) as it's not a length.

I can fix all this when applying, no need to resubmit.

> +
> +				hue1 = hue_areas[hue_pos ? hue_pos - 1 : 11];
> +				hue2 = hue_areas[hue_pos];
> +
> +				/* Calculate the total width between the two 
areas */
> +				width = hue2 - hue1 + (hue1 > hue2 ? 256 : 0);
> +
> +				/* Calculate the length to the right most area 
*/
> +				if (hue1 > hue2 && hsv[0] > hue1)
> +					length = width - (hsv[0] - hue1);
> +				else
> +					length = abs(hsv[0] - hue2);
> +
> +				/* Calculate weight and round up */
> +				weight = (length * 16 + width - 1) / width;
> +				/* Split weight between the two areas */
> +				hist[hue_pos ? hue_pos/2 - 1 : 5][hist_n] += 
weight;
> +				hist[hue_pos/2][hist_n] += 16 - weight;
> +			}
>  		}
>  	}
> 
> @@ -1349,9 +1409,9 @@ static void histogram_compute_hgt(const struct image
> *image, void *histo, histo += 4;
> 
>  	/* Weighted Frequency of Hue Area-m and Saturation Area-n */
> -	for (m = 0; m < 6; m++) {
> -		for (n = 0; n < 32; n++) {
> -			*(uint32_t *)histo = hist[m][n];
> +	for (x = 0; x < 6; x++) {
> +		for (y = 0; y < 32; y++) {
> +			*(uint32_t *)histo = hist[x][y];
>  			histo += 4;
>  		}
>  	}

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/3] tests: add hue area wraparound test for HGT

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Tuesday 04 Oct 2016 15:09:14 Niklas Söderlund wrote:
> From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> One configuration for HGT histograms are where hue area 0 is wrapped

s/is/

> around the hue input value space (0-255), for example area 0 can cover
> hue values above 220 but bellow 40. Hue area 0 is the only area which

s/bellow/below/

> supports this mode where the areas lower limit uses a higher value then

s/areas/area's/
s/then/than/

> it's higher limit hence the area wraparound the max hue value.

s/it's/its/
s/wraparound/wraps around/

I'll fix this while applying.

> Add a test to verify this works now that gen-image supports generating
> reference frames with wrapped around hue areas.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  tests/vsp-unit-test-0018.sh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/vsp-unit-test-0018.sh b/tests/vsp-unit-test-0018.sh
> index 2b49b54..944f7ac 100755
> --- a/tests/vsp-unit-test-0018.sh
> +++ b/tests/vsp-unit-test-0018.sh
> @@ -31,6 +31,7 @@ test_histogram() {
>  test_main() {
>  	test_histogram "0,255,255,255,255,255,255,255,255,255,255,255"
>  	test_histogram "0,40,40,80,80,120,120,160,160,200,200,255"
> +	test_histogram "220,40,40,80,80,120,120,160,160,200,200,220"
>  }
> 
>  test_init $0 "$features"

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] tests: add none adjoined hue areas test for HGT

[Laurent Pinchart]

On Tuesday 04 Oct 2016 15:09:15 Niklas Söderlund wrote:
> From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> The HGT can be configured in such a way that the 6 user configurable hue
> areas are not joined next to each other. In such cases a hue value
> outside of a hue area should be attributed with a weight to both its
> adjoining hue areas.
> 
> Add a test to verify this works now that gen-image supports generating
> reference frames for none adjoined hue areas.

s/none/non/

I'll fix this when applying.

> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  tests/vsp-unit-test-0018.sh | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tests/vsp-unit-test-0018.sh b/tests/vsp-unit-test-0018.sh
> index 944f7ac..c0aba4b 100755
> --- a/tests/vsp-unit-test-0018.sh
> +++ b/tests/vsp-unit-test-0018.sh
> @@ -32,6 +32,9 @@ test_main() {
>  	test_histogram "0,255,255,255,255,255,255,255,255,255,255,255"
>  	test_histogram "0,40,40,80,80,120,120,160,160,200,200,255"
>  	test_histogram "220,40,40,80,80,120,120,160,160,200,200,220"
> +	test_histogram "0,10,50,60,100,110,150,160,200,210,250,255"
> +	test_histogram "10,20,50,60,100,110,150,160,200,210,230,240"
> +	test_histogram "240,20,60,80,100,120,140,160,180,200,210,220"
>  }
> 
>  test_init $0 "$features"

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/3] gen-image: support overlapping hue areas for HGT frames

[Niklas Söderlund]

Hi Laurent,

Thank you for the feedback.

On 2016-11-11 16:53:15 +0200, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Tuesday 04 Oct 2016 15:09:13 Niklas S�derlund wrote:
> > From: Niklas S�derlund <niklas.soderlund+renesas@ragnatech.se>
> > 
> > The HGT can operate with hue areas which are not directly adjoined. In
> > this mode of operation hue values which are between two areas are
> 
> s/which/that/g
> 
> > attributed to both areas with a weight for the final histogram.
> > 
> > Add support to generate such histograms using gen-image which can be
> > used to verify correct operation of the HGT. Previously gen-image could
> > only generate histograms with adjoined areas.
> > 
> > Signed-off-by: Niklas S�derlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> >  src/gen-image.c | 98 +++++++++++++++++++++++++++++++++++++++++++-----------
> >  1 file changed, 79 insertions(+), 19 deletions(-)
> > 
> > diff --git a/src/gen-image.c b/src/gen-image.c
> > index 688f602..9cd5eb9 100644
> > --- a/src/gen-image.c
> > +++ b/src/gen-image.c
> > @@ -1301,7 +1301,7 @@ static void histogram_compute_hgt(const struct image
> > *image, void *histo, uint8_t rgb[3], hsv[3], smin = 255, smax = 0;
> >  	uint32_t sum = 0, hist[6][32];
> >  	unsigned int x, y, i;
> > -	int m, n;
> > +	unsigned int hist_n, hue_pos;
> > 
> >  	memset(hist, 0x00, sizeof(hist));
> > 
> > @@ -1317,24 +1317,84 @@ static void histogram_compute_hgt(const struct image
> > *image, void *histo, smax = max(smax, hsv[1]);
> >  			sum += hsv[1];
> > 
> > -			/* Find m and n for hist */
> > -			m = n = -1;
> > -			for (i = 0; i < 6  && m == -1; i++)
> > -				if (hsv[0] >= hue_areas[i*2] && (hsv[0] <= 
> hue_areas[i*2+1]))
> > -					m = i;
> > -			for (i = 0; i < 32 && n == -1; i++)
> > -				if ((hsv[1] >= 8*i) && (hsv[1] < 8 * (i+1)))
> > -					n = i;
> > +			/* Find n for hist */
> 
> How about "Compute the n coordinate of the histogram bucket" ?
> 
> > +			hist_n = hsv[1] / 8;
> > 
> >  			/*
> > -			 * The HW supports a declining weight to be applied
> > -			 * when hue areas are not directly adjoined. This
> > -			 * test can not replicated this, the hue areas need
> > -			 * to be set without any gaps else the weights from HW
> > -			 * will be wrong. Max weight is 16.
> > +			 * Find position in hue areas which is greater than 
> the
> 
> s/which/that/
> 
> https://en.oxforddictionaries.com/usage/that-or-which

Thanks, you learn something new everyday. I'm truly grateful for this 
kind of feedback.

> 
> > +			 * current H value. Special consideration is needed 
> for:
> > +			 *
> > +			 * - Values inside a hue area are inclusive, values 
> that
> > +			 *   are between two hue areas are exclusive.
> > +			 * - Hue area 0 can wrap around the H value space, for
> > +			 *   example include values greater then 240 but less
> 
> s/then/than/
> s/but/and/
> 
> > +			 *   then 30.
> >  			 */
> > -			if (m != -1 && n != -1)
> > -				hist[m][n] += 16;
> > +			for (i = 0; i < 12; i++) {
> > +
> > +				/* Special cases when area 0 wraps around */
> > +				if (hue_areas[0] > hue_areas[1]) {
> > +
> > +					/* Check if pixel is inside the 
> wrapped area 0 */
> > +					if (hsv[0] > hue_areas[0] || hsv[0] <= 
> hue_areas[1]) {
> > +						hue_pos = 1;
> > +						break;
> > +					}
> > +
> > +					/* Exclude first area point from 
> normal logic */
> > +					if (!i)
> > +						continue;
> > +				}
> > +
> > +				/* Check if H is inside one of the hue areas 
> */
> > +				if ((hsv[0] < hue_areas[i]) || (i % 2 && 
> hsv[0] == hue_areas[i])) {
> > +					hue_pos = i;
> > +					break;
> > +				}
> > +
> > +				/* Check if H is larger then area 5 */
> > +				if (hsv[0] > hue_areas[11]) {
> > +					hue_pos = 0;
> > +					break;
> > +				}
> > +			}
> 
> What would you think about precomputing the hue pos values for hue values 0 to 
> 255 and just indexing that table ? That should be faster at runtime, with an 
> additional memory consumption of 256 bytes, which seems quite negligible to 
> me. I can fix that as an additional patch.

Yes I think that would be an improvement, both for runtime speed and 
code readability.

> 
> > +
> > +			/*
> > +			 * Figure out which areas the current H value should 
> be
> > +			 * attributed to. If the H value is inside one of the
> > +			 * areas the max weight (16) is attributed to it else
> > +			 * the weight is split between them based on how close
> > +			 * the H value is to each area.
> > +			 *
> > +			 * If ''hue_pos'' are odd the H value is inside an
> 
> s/are/is/
> 
> > area and
> > +			 * it should be attributed the full weight to area 
> hue_pos/2
> > +			 * else it should be split between area hue_pos/2 and
> 
> s/area/areas/
> 
> > +			 * hue_pos/2 - 1.
> > +			 */
> > +			if (hue_pos % 2) {
> > +				hist[hue_pos/2][hist_n] += 16;
> > +			} else {
> > +				unsigned int hue1, hue2;
> > +				unsigned int length, width, weight;
> 
> I'd name the variable dist(ance) as it's not a length.
> 
> I can fix all this when applying, no need to resubmit.

Thanks. If you get overloaded or some other issues emerge let me know 
and I will do what I can to help out.

> 
> > +
> > +				hue1 = hue_areas[hue_pos ? hue_pos - 1 : 11];
> > +				hue2 = hue_areas[hue_pos];
> > +
> > +				/* Calculate the total width between the two 
> areas */
> > +				width = hue2 - hue1 + (hue1 > hue2 ? 256 : 0);
> > +
> > +				/* Calculate the length to the right most area 
> */
> > +				if (hue1 > hue2 && hsv[0] > hue1)
> > +					length = width - (hsv[0] - hue1);
> > +				else
> > +					length = abs(hsv[0] - hue2);
> > +
> > +				/* Calculate weight and round up */
> > +				weight = (length * 16 + width - 1) / width;
> > +				/* Split weight between the two areas */
> > +				hist[hue_pos ? hue_pos/2 - 1 : 5][hist_n] += 
> weight;
> > +				hist[hue_pos/2][hist_n] += 16 - weight;
> > +			}
> >  		}
> >  	}
> > 
> > @@ -1349,9 +1409,9 @@ static void histogram_compute_hgt(const struct image
> > *image, void *histo, histo += 4;
> > 
> >  	/* Weighted Frequency of Hue Area-m and Saturation Area-n */
> > -	for (m = 0; m < 6; m++) {
> > -		for (n = 0; n < 32; n++) {
> > -			*(uint32_t *)histo = hist[m][n];
> > +	for (x = 0; x < 6; x++) {
> > +		for (y = 0; y < 32; y++) {
> > +			*(uint32_t *)histo = hist[x][y];
> >  			histo += 4;
> >  		}
> >  	}
> 

-- 
Regards,
Niklas S�derlund