CODEWITHSUNDEEP
c++algorithmc++11floating-point
iksemyonov
iksemyonov

Reputation: 4196

How can I correctly implement this operation with averaging unsigned chars - C++

Consider the following, seemingly simple, problem, that I've come across in solving assignments in CV involving masks and implementing AA in ray tracing.

An color in the RGB format has three channels, each channel represented by an unsigned char, hence 0 <= channel value <= 255. For reasons of symmetry, let's consider a single channel. I want to sum up channel values over a certain set of points, then average and round that (not just truncate) and store back into an unsigned char. Note that in all the problems, the resulting averaged value is guaranteed to be in the range [0, 255], it's an invariant of the algorithm.

This looks like an easy task, but I'm concerned with the following:

p.1. The most important: the type choice, see pseudocode below for which types I need. More precisely, I'm getting a narrowing conversion error, which turns into an error with -pedantic-errors set, and I don't want to simply wave it away.

p.2. The idea of going from a wider type that can accumulate many uchar's, back to the uchar type. I'm confident that the algorithm is guaranteed to produce a value in the uchar range, but is it enough? Should I worry about this?

Pseudocode (cv::Vec3b is a fixed size 3x1 array of unsigned char in OpenCV which iI use for image storing and output):

// this returns a value in the range [0, 255]
cv::Vec3b processing_func(const point&) { ... }

accum_type acc_r = 0, acc_g = 0 , acc_b = 0;
for (point p: point_set) {
    const cv::Vec3b color = processing_func(p);
    acc_r += color[0]; acc_g += color[1]; acc_b += color[2];
}
cv::Vec3b result_color{roundf(acc_r / (floating_type)set_size),
                       roundf(acc_g / (floating_type)set_size),
                       roundf(acc_b / (floating_type)set_size)};
// hello narrowing conversion ^^^

Refer to p.1, what should the accum_type and floating_type be (the latter required to divide correctly without truncation), or would you maybe code it differently?

Edit after the first comment: set_size is an integer above 0, it can be hardcoded if required, but in general is bound to change. It's e.g. AA factor, squared; or mask size, and so on.

Upvotes: 2

Views: 242

Answers (4)

knivil
knivil

Reputation: 807

p.1: Use an explicit cast.

p.2: In general it is not enough. Every time you operate on floating point types you will lose precision. That depends on your operation in respect to your floating point type. To prevent this in your case, you can check for saturation, e.g.:

uchar v = (uchar)(value < 255.0 ? value : 255);

But I don't think that you will need it.

Upvotes: 2

Adl A
Adl A

Reputation: 183

I would use unsigned int

cv::Vec3b processing_func(const point&) { ... }

unsigned int acc_r, acc_g, acc_b;
    for (point p: point_set) {
    const cv::Vec3b color = processing_func(p);
    acc_r += color[0]; acc_g += color[1]; acc_b += color[2];
}

then for the rounding i would make us of % operator in the following manner

cv::Vec3b result_color{acc_r / set_size + acc_r % set_size * 2 / set_size,
                       acc_g / set_size + acc_g % set_size * 2 / set_size,
                       acc_b / set_size + acc_b % set_size * 2 / set_size};

notice that 0 <= a % b * 2 < 2b, so a % b * 2 / b is 0 when a%b < b/2 and 1 when a%b >= b/2

Upvotes: 1

Mark B
Mark B

Reputation: 96281

I would use unsigned for the accumulator type (assuming you're accumulating fewer than 2^24 values), and double for the floating point type (as it's the type of literal decimal values).

Then you're just missing the static_cast<unsigned char>(rounded_floating_point_value) to get your floating point rounded results back to the desired type warning-free.

Upvotes: 1

Skizz
Skizz

Reputation: 71090

I'd be tempted to use unsigned integers:-

uint acc_r = set_size / 2, acc_g = set_size / 2, acc_b = set_size / 2;

for (point p: point_set) {
    const cv::Vec3b color = processing_func(p);
    acc_r += color[0]; acc_g += color[1]; acc_b += color[2];
}

cv::Vec3b result_color{acc_r / set_size, acc_g / set_size, acc_b / set_size}

Notice that I initialise the RGB values to 1/2 (set_size / 2) so that the division rounds rather than truncates.

Upvotes: 1

Related Questions