views:

658

answers:

3

I'm programming in C++. I need to convert a 24-bit signed integer (stored in a 3-byte array) to float (normalizing to [-1.0,1.0]).

The platform is MSVC++ on x86 (which means the input is little-endian).

I tried this:

float convert(const unsigned char* src)
{
    int i = src[2];
    i = (i << 8) | src[1];
    i = (i << 8) | src[0];

    const float Q = 2.0 / ((1 << 24) - 1.0);

    return (i + 0.5) * Q;
}

I'm not entirely sure, but it seems the results I'm getting from this code are incorrect. So, is my code wrong and if so, why?

+4  A: 

You are not sign extending the 24 bits into an integer; the upper bits will always be zero. This code will work no matter what your int size is:

if (i & 0x800000)
    i |= ~0xffffff;

Edit: Problem 2 is your scaling constant. In simple terms, you want to multiply by the new maximum and divide by the old maximum, assuming that 0 remains at 0.0 after conversion.

const float Q = 1.0 / 0x7fffff;

Finally, why are you adding 0.5 in the final conversion? I could understand if you were trying to round to an integer value, but you're going the other direction.

Edit 2: The source you point to has a very detailed rationale for your choices. Not the way I would have chosen, but perfectly defensible nonetheless. My advice for the multiplier still holds, but the maximum is different because of the 0.5 added factor:

const float Q = 1.0 / (0x7fffff + 0.5);

Because the positive and negative magnitudes are the same after the addition, this should scale both directions correctly.

Mark Ransom
The range of 24 ints is not symmetrical around 0. There are more negative ints (by one) than positive ints. The OP wants to map onto closed interval [-1.0, 1.0], and I guess that's what the added 0.5 is for.
Maciej Hehl
@Maciej Hehl, indeed the input range is not symmetrical, but I suspect the practical range is. If it is truly necessary to map the full range to [-1.0,1.0] then you have two choices: different multipliers for positive and negative numbers, or 0 != 0.0. I wouldn't choose either one.
Mark Ransom
Maciej is right, see the comments at the beginning of this file: http://bit.ly/bog0nN (that's the file I'm working on, by the way - 24-bit support is broken in numerous ways, and I have to fix it).
e-t172
Your solution is working. Thanks! Regarding Q, your code is yielding the same value as mine (-1.0737418e+008).
e-t172
@e-t172, see my edit. I think this is the final missing piece.
Mark Ransom
A: 

Looks like you're treating it as an 24-bit unsigned integer. If the most significant bit is 1, you need to make i negative by setting the remaining 8 bits to 1 as well.

Thorarin
+2  A: 

Since you are using a char array, it does not necessarily follow that the input is little endian by virtue of being x86; the char array makes the byte order architecture independent.

Your code is somewhat over complicated. A simple solution is to shift the 24 bit data to scale it to a 32bit value (so that the machine's natural signed arithmetic will work), and then use a simple ratio of the result with the maximum possible value (which is INT_MAX less 256 because of the vacant lower 8 bits).

#include <limits.h>

float convert(const unsigned char* src)
{
    int i = src[2] << 24 | src[1] << 16 | src[0] << 8 ;
    return i / (float)(INT_MAX - 256) ;
}

Test code:

unsigned char* makeS24( unsigned int i, unsigned char* s24 )
{
    s24[2] = (unsigned char)(i >> 16) ;
    s24[1] = (unsigned char)((i >> 8) & 0xff);
    s24[0] = (unsigned char)(i & 0xff);
    return s24 ;
}

#include <iostream>

int main()
{
    unsigned char s24[3] ;
    volatile int x = INT_MIN / 2 ;

    std::cout << convert( makeS24( 0x800000, s24 )) << std::endl ;  // -1.0
    std::cout << convert( makeS24( 0x7fffff, s24 )) << std::endl ;  //  1.0
    std::cout << convert( makeS24( 0, s24 )) << std::endl ;         //  0.0
    std::cout << convert( makeS24( 0xc00000, s24 )) << std::endl ;  // -0.5
    std::cout << convert( makeS24( 0x400000, s24 )) << std::endl ;  //  0.5

}
Clifford
The byte order was defined as "little endian" in the original problem description.
Mark Ransom
@Mark Ransom: I did miss that, but the conclusion "which means the input is little-endian" does not follow from "The platform is MSVC++ on x86"; because a char array is used rather than an int, the byte order could be independent of the architecture. Nonetheless you are right, I have removed the statment.
Clifford