HDRI - Statistic bias in convertion16bit.ppm to 8bit.ppm

[magick]

Here is our conversion method:

Code: Select all

static inline unsigned char ScaleQuantumToChar(const Quantum quantum)
{
#if !defined(MAGICKCORE_HDRI_SUPPORT)
  return((unsigned char) (quantum/257U));
#else
  if (quantum <= 0.0)
    return(0);
  if ((quantum/256.0) >= 255.0)
    return(255);
  return((unsigned char) (quantum/256.0+0.5));
#endif
}

Is it mathematically correct? Note that Quantum for Q16 is unsigned short and float for Q16 HDRI.

[rnbc]

Well, the integer part is certainly correct since 65535/255=257

The floating point part is more tricky...

For example quantum/256.0+0.5 rounds up at 0.5 but it's also correct to round down at 0.5, although it's not usual. So don't change this.

If you want the same exact results at the integer version try, as I just did:

Code: Select all

#elif (MAGICKCORE_QUANTUM_DEPTH == 16)
static inline unsigned char ScaleQuantumToChar(const Quantum quantum)
{
#if !defined(MAGICKCORE_HDRI_SUPPORT)
  return((unsigned char) (quantum/257U));
#else
  if (quantum <= 0.0)
    return(0);
  if ((quantum/257.0) >= 255.0)
    return(255);
  return((unsigned char) (quantum/257.0+0.5));
#endif
}
#elif (MAGICKCORE_QUANTUM_DEPTH == 32)
static inline unsigned char ScaleQuantumToChar(const Quantum quantum)
{
#if !defined(MAGICKCORE_HDRI_SUPPORT)
  return((unsigned char) ((quantum+MagickULLConstant(8421504))/
    MagickULLConstant(16843009)));
#else
  if (quantum <= 0.0)
    return(0);
  if ((quantum/16843009.0) >= 255.0)
    return(255);
  return((unsigned char) (quantum/16843009.0+0.5));
#endif
}

I think this is more correct than your version, at least the error is more symmetrical, if not smaller. You might also want to check the other functions for 32 bits and 16 bits quanta.

The function for 64 bits seems strange to me, but the correct value should be (2^64-1)/255=72340172838076673, not 71777214294589694, and the structure should be the same as the others. Maybe I'm just not understanding something in that function.

As a general rule bear in mind that 16 bits quantum goes from 0 to 65535, not 65536. And 8 bits goes from 0 to 255, not 256, etc... I know this leads to weird factors like 257 and 16843009, but it's how it is...

I was checking the ScaleCharToQuantum: why the 256.0*value+0.5 ? Shouldn't it be 257.0*value, if quantum is 65535? I think it should... I changed a few values in that file.

Your integer factors are all correct, it seems to me. Just use the same factors in the float part, and all will just work.

I'm not sure about what to do in ScaleMapToQuantum and ScaleQuantumToMap.

Some functions seem not to clamp correctly, like ScaleQuantumToLong, and the factor in that one should be 4294967297.0, not 4294967296.0 IMHO.

Check my changes in here:

http://rnbc.dyndns.org/pub/imagemagick_quanta/000/

There are probably other functions needing a rework in there... but I just checked the results with those changes made and they seem quite OK. The error is symmetrical, there is no bias... the results are the same as with the integer version, or as close as it gets considering calculation errors.

Hope this helps!

[rnbc]

Ops, I think broke the conversion from 8-->16

Could you check out?

[magick]

Thanks for the detailed response. We'll apply these patches to ImageMagick 6.6.1-8 and verify ImageMagick is returning correct results for HDRI. We'll need another day to confirm.

[rnbc]

I'm a bit confused, perhaps because it's 4am... but I think the general rule is:

int --> float, return the exact value in float, only then convert into the correct quantum.

float --> int, first convert into the correct quantum, then return trunc(float+0.5) [and clamp if needed after]

8 --> 16, multiply by 257 or 257.0, no rounding

8 --> 32, multiply by 16843009 or 16843009.0, no rounding

8 --> 64, multiply by 72340172838076673 or 72340172838076673.0, no rounding

16 --> 8, divide by 257 or 257.0, no rounding

16 --> 32, multiply by 65537 or 65537.0, no rounding

16 --> 64, multiply by 281479271743489 or 281479271743489.0, no rounding

32 --> 8, divide by 16843009 or 16843009.0, no rounding

32 --> 16, divide by 65537 or 65537.0, no rounding

32 --> 64, multiply by 4294967297 or 4294967297.0, no rounding

64 --> 8, divide by 72340172838076673 or 72340172838076673.0, no rounding

64 --> 16, divide by 281479271743489 or 281479271743489.0, no rounding

64 --> 32, divide by 4294967297 or 4294967297.0, no rounding

[rnbc]

Thanks for the detailed response. We'll apply these patches to ImageMagick 6.6.1-8 and verify ImageMagick is returning correct results for HDRI. We'll need another day to confirm.

No please... the patch is broken in the conversion from 8-->16 (at least!). I just checked with a PPM_8bit into PPM_16bit...

But see the rules I wrote in the previous post.

[rnbc]

I changed the code again, it at http://rnbc.dyndns.org/pub/imagemagick_quanta/001/, but don't use it because that one has the same error we had one week ago

I think the rules I stated for conversion are sound, we just need to apply them correctly, and I seem unable to do it right now... tomorrow I'll look at it again.

Concerning the current code I don't understand:

• Why are we adding 0.5 when converting from int to float?
• Why are we adding 0.5 when converting from float to float, in a different quantum?
• What's the "Map" type? If it's a float from 0.0 to 1.0 the HDRI conversion is 1:1, with no arithmetic whatsoever. And then you might multiply by 255 or 65535 or whatever quantum is needed, but no need to add 0.5!

We should only add 0.5 exactly before converting from float to int, after any divisions, etc... it's a rounding to avoid truncation bias, not a conversion factor.

See you tomorrow

[rnbc]

By the current definition in http://rnbc.dyndns.org/pub/imagemagick_quanta/001/ when I go from 8_bit_pgm to 16_bit_pgm, in a quantum=65535 version this should happen:

From magick/quantum-private.h

Code: Select all

static inline Quantum ScaleCharToQuantum(const unsigned char value)
{
#if !defined(MAGICKCORE_HDRI_SUPPORT)
  return((Quantum) (257U*value));
#else
  return((Quantum) (257.0*value+0.0));
#endif
}

From magick/quantum-private.h

Code: Select all

static inline unsigned short ScaleQuantumToShort(const Quantum quantum)
{
#if !defined(MAGICKCORE_HDRI_SUPPORT)
  return((unsigned short) quantum);
#else
  if (quantum <= 0.0)
    return(0);
  if (quantum >= 65535.0)
    return(65535);
  return((unsigned short) (quantum+0.5));
#endif
}

(this seems to work as it should from the source code)

0 --> 0 --> 0 --> 0.5 --> 0
1 --> 257 --> 257.5 --> 257
254 --> 65278 --> 65278.5 --> 65278
255 --> 65535 --> 65535.5 --> 65535

And when going from 16bit_pgm to 8bit_pgm:

From magick/quantum-private.h

Code: Select all

static inline Quantum ScaleShortToQuantum(const unsigned short value)
{
  return((Quantum) value);
}

From magick/quantum.h

Code: Select all

static inline unsigned char ScaleQuantumToChar(const Quantum quantum)
{
#if !defined(MAGICKCORE_HDRI_SUPPORT)
  return((unsigned char) (quantum/257U));
#else
  if (quantum <= 0.0)
    return(0);
  if ((quantum/257.0) >= 255.0)
    return(255);
  return((unsigned char) (quantum/257.0+0.5));
#endif
}

(in italics what I am seeing, and don't understand because it's not what's in the source!)

0 --> 0 --> 0 --> 0.5 --> 0 (0)
1 --> 1 --> .0038910505 --> .5038910505 --> 0 (0)
126 --> 126 --> .4902723735 --> .9902723735 --> 0 (0)
127 --> 127 --> .4941634241 --> .9941634241 --> 0 (0)
128 --> 128 --> .4980544747 --> .9980544747 --> 0 (0)
129 --> 129 --> .5019455252 --> 1.001945525 --> 1 (0)
254 --> 254 --> .9883268482 --> 1.488326848 --> 1 (0)
255 --> 255 --> .9922178988 --> 1.492217898 --> 1 (0)
256 --> 256 --> .9961089494 --> 1.496108949 --> 1 (0)
257 --> 257 --> 1.000000000 --> 1.500000000 --> 1 (1)
65406 --> 65406 --> 254.4980544 --> 254.9980544 --> 254 (254)
65407 --> 65407 --> 254.5019455 --> 255.0019455 --> 255 (254)
65408 --> 65408 --> ... 255 (254)
65409 --> 65409 --> ... 255 (254)
65534 --> 65534 --> ... 255 (254)
65535 --> 65535 --> ... 255 (255)

Could you shed some light over this?

[magick]

Grab ftp://ftp.imagemagick.org/pub/ImageMagi ... 1-8.tar.gz. We get the expected results for HDRI:

• # ImageMagick pixel enumeration: 1,16,255,rgb
  0,0: ( 0, 0, 0) #000000 rgb(0,0,0)
  0,1: ( 0, 0, 0) #000000 rgb(0,0,0)
  0,2: ( 0, 0, 0) #000000 rgb(0,0,0)
  0,3: ( 0, 0, 0) #000000 rgb(0,0,0)
  0,4: ( 0, 0, 0) #000000 rgb(0,0,0)
  0,5: ( 1, 1, 1) #010101 rgb(1,1,1)
  0,6: ( 1, 1, 1) #010101 rgb(1,1,1)
  0,7: ( 1, 1, 1) #010101 rgb(1,1,1)
  0,8: ( 1, 1, 1) #010101 rgb(1,1,1)
  0,9: ( 1, 1, 1) #010101 rgb(1,1,1)
  0,10: (254,254,254) #FEFEFE rgb(254,254,254)
  0,11: (255,255,255) #FFFFFF rgb(255,255,255)
  0,12: (255,255,255) #FFFFFF rgb(255,255,255)
  0,13: (255,255,255) #FFFFFF rgb(255,255,255)
  0,14: (255,255,255) #FFFFFF rgb(255,255,255)
  0,15: (255,255,255) #FFFFFF rgb(255,255,255)

[rnbc]

Seems to be working now, and the code makes sense. Both good things

I did a small cleanup in quantum-private.h, you can check it at http://rnbc.dyndns.org/pub/imagemagick_quanta/002/

It's only a cleanup, except for line 587.

I also checked conversion to/from floating point EXR, and it works just fine, albeit within the limited precision of 16bit half-float, of course.

I still have some doubts about Map<-->Quantum and Any<-->Quantum conversions, mostly because I have a very limited knowledge of the inner workings of ImageMagick:

• Is "Any" supposed to be an integer type? If yes the code seems OK to me.
• Is "Map" supposed to be a real type? Then no rounding (+0.5) is needed.

Cheers!