7

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?

Etienne Dechamps
  • 24,037
  • 4
  • 32
  • 31

8 Answers8

11

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
  • 299,747
  • 42
  • 398
  • 622
  • 3
    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 May 26 '10 at 20:15
  • @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 May 26 '10 at 20:48
  • 1
    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). – Etienne Dechamps May 26 '10 at 21:46
  • Your solution is working. Thanks! Regarding Q, your code is yielding the same value as mine (-1.0737418e+008). – Etienne Dechamps May 26 '10 at 21:57
  • @e-t172, see my edit. I think this is the final missing piece. – Mark Ransom May 26 '10 at 22:09
  • another way to extend the numbers that can avoid branches is `(i << 8) >> 8` – phuclv Nov 05 '14 at 06:38
4

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
  • 88,407
  • 13
  • 85
  • 165
  • 1
    The byte order was defined as "little endian" in the original problem description. – Mark Ransom May 26 '10 at 20:51
  • @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 May 26 '10 at 21:53
1

The solution that works for me:

/**
 * Convert 24 byte that are saved into a char* and represent a float
 * in little endian format to a C float number.
 */
float convert(const unsigned char* src)
{
    float num_float;
    // concatenate the chars (short integers) and
    // save them to a long int
    long int num_integer = (
            ((src[2] & 0xFF) << 16) | 
            ((src[1] & 0xFF) << 8) | 
            (src[0] & 0xFF)
        ) & 0xFFFFFFFF;

    // copy the bits from the long int variable
    // to the float.
    memcpy(&num_float, &num_integer, 4);

    return num_float;
}
sbosx
  • 11
  • 1
1

Works for me:

float convert(const char* stream)
{
    int fromStream = 
        (0x00 << 24) + 
        (stream[2] << 16) + 
        (stream[1] << 8) + 
         stream[0];

    return (float)fromStream;
}
sethcall
  • 2,837
  • 1
  • 19
  • 22
1

Since it's not symmetrical, this is probably the best compromise.

Maps -((2^23)-1) to -1.0 and ((2^23)-1) to 1.0.

(Note: this is the same conversion style used by 24 bit WAV files)

float convert( const unsigned char* src )
{
    int i = ( ( src[ 2 ] << 24 ) | ( src[ 1 ] << 16 ) | ( src[ 0 ] << 8 ) ) >> 8;
    return ( ( float ) i ) / 8388607.0;
}
Grayson Lang
  • 501
  • 4
  • 2
0

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
  • 47,289
  • 11
  • 75
  • 111
0

I'm not sure if it's good programming practice, but this seems to work (at least with g++ on 32-bit Linux, haven't tried it on anything else yet) and is certainly more elegant than extracting byte-by-byte from a char array, especially if it's not really a char array but rather a stream (in my case, it's a file stream) that you read from (if it is a char array, you can use memcpy instead of istream::read).

Just load the 24-bit variable into the less significant 3 bytes of a signed 32-bit (signed long). Then shift the long variable one byte to the left, so that the sign bit appears where it's meant to. Finally, just normalize the 32-bit variable, and you're all set.

union _24bit_LE{
  char access;
  signed long _long;
}_24bit_LE_buf;

float getnormalized24bitsample(){
  std::ifstream::read(&_24bit_LE_buf.access+1, 3);
  return (_24bit_LE_buf._long<<8) / (0x7fffffff + .5);
}

(Strangely, it doesn't seem to work when you just read into the 3 more significant bytes right away).

EDIT: it turns out this method seems to have some problems I don't fully understand yet. Better don't use it for the time being.

leftaroundabout
  • 117,950
  • 5
  • 174
  • 319
0

This one, got from here, works for me.

typedef union {
    struct {
        unsigned short lo;
        unsigned short hi;
    } u16;
    unsigned int u32;
    signed int i32;
    float  f;
}Versatype32;
//Bipolar version (-1.0 to ~1.0)
void fInt24_to_float(float* dest, const char* src, size_t length) {
    Versatype32 xTemp;
    while (length--) {
        xTemp.u32 = *(int*)src;
        //Check if Negative by right shifting 8
        xTemp.u32 <<= 8; //(If it's a negative, we'll know)  (Props to Norman Wong)
        //Convert to float
        xTemp.f = (float)xTemp.i32;     
        //Skip divide down if zero
        if (xTemp.u32 != 0) {
            //Divide by (1<<31 or 2^31)
            xTemp.u16.hi -= 0x0F80; //BAM! Bitmagic!
        }
        *dest = xTemp.f;
        //Move to next set
        dest++;
        src += 3;
    }   //Are we done yet?
    //Yes!
    return;
}
Michael Chourdakis
  • 10,345
  • 3
  • 42
  • 78