[IPOL discuss] libpng

[Nicolas Limare]

On Mon, Jul 25, 2011 at 09:42:34PM +0200, Julien Rabin wrote:
> Dear all,
> 
> Nicolas told me that this has been corrected a month ago.

The corrected code is available from the tools page:
http://tools.ipol.im/wiki/author/code/tools/

The fix is here:
http://dev.ipol.im/git/?p=nil/io_png.git;a=blob;f=io_png.c#l313

I also dropped the pointer arithmetics, the marginal gain of this
idiom (use less registers) is not much relevant anymore on amd64 CPUs,
and for loops are easier to understand and parallelize.

> while (ptr_gray < ptr_end)
>             *ptr_gray++ = (unsigned char) ((6969.f * *ptr_r++
>                                            + 23434.f * *ptr_g++
>                                            + 2365.f * *ptr_b++) /
> 32768.f);

Why do you want to use float numbers? the R, G and B values are
unsigned chars, (6969 R + 23434 G + 2365 B ) / 32768 will be performed
in the internal integer representation (~ size_t), then casted. I
don't see the need to use floating-point values.

> Indeed, I think that to be 100% correct we should apply a round() function
> before the (unsigned char) cast to avoid just truncating the value instead
> of rounding it to the nearest integer.

Maybe. But the result of an integer division is an integer, so there
is nothing to round after the division. If we want sopmething exact,
then the RGB->gray conversion must be done with float values, but I
want to check if it makes sense, because the (6969 R + 23434 G + 2365
B ) / 32768 formula already is an approximation of the Rec. 709
weighting of RGB components[1], based on perception estimations. I'll
check if roundoing instead of truncating makes sense.

[1]http://www.poynton.com/notes/colour_and_gamma/ColorFAQ.html#RTFToC9

-- 
Nicolas LIMARE - CMLA - ENS Cachan    http://www.cmla.ens-cachan.fr/~limare/
IPOL - image processing on line                          http://www.ipol.im/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://tools.ipol.im/mailman/archive/discuss/attachments/20110726/969c1009/attachment.pgp>