Re: [Lame-dev] floating point representation (was: problem with decoding on the fly)

[Aleksander K. <A.K...@el...>]

Robert Hegemann wrote:

>we have some type mismatch problem here:
>
>LAME uses "FLOAT" and "FLOAT8" which may be
>FLOAT = float	FLOAT8 = float
>FLOAT = float	FLOAT8 = double
>FLOAT = double	FLOAT8 = double
>
>mpg123 uses "real":
>#ifdef REAL_IS_FLOAT
>#  define real float
>#elif defined(REAL_IS_LONG_DOUBLE)
>#  define real long double
>#else
>#  define real double
>#endif
>
Thanks for spotting this. I think we should control the definition of 
`real' (in mpglib) and `Float_t' (in gain_analysis.h) by autoconf (that 
is <config.h>) where possible, or Makefile.MSVC and Makefile.unix 
anywhere else.

What we have now might not work on some platforms, because the code that 
executes functions from gain_analysis.h assumes that the value is of the 
type sample_t. This doesn't have to be true. Furthermore, some code that 
uses mpglib assumes that mpglib returns FLOAT8. This doesn't have to be 
true either.

Now, FLOAT and FLOAT8 are defaulted to float and double in 
"libmp3lame/machine.h". This can be overridden by <config.h> (or those 
two Makefiles).

If I may suggest two solutions:

SOLUTION 1:

I think we should remove the defaults from "libmp3lame/machine.h" and 
rely only on <config.h> or these two Makefiles. This way, if types are 
not conciously and properly set, LAME won't compile at all (which is a 
good thing, I think)

Then we could do the following:

1. Add the following into "mpglib/mpg123.h":
#ifdef HAVE_CONFIG_H
# include <config.h>
#endif
2. Then we have two alternative solutions:
2.1.
In <config.h> set REAL_IS_FLOAT or REAL_IS_LONG_DOUBLE according to 
FLOAT8, and do the same in the Makefiles.

OR

2.2
Define the following in "mpglib/mpg123.h":
typedef FLOAT8 real;
and remove:

#ifdef REAL_IS_FLOAT
#  define real float
#elif defined(REAL_IS_LONG_DOUBLE)
#  define real long double
#else
#  define real double
#endif


Any of these alternatives will acheive the same effect: the `real' value 
in mpglib will be controlled by autoconf (or the two Makefiles) and 
synchronized with what is used in the rest of the code,

As for "gain_analysis.h":

1. Remove the defaults from "libmp3lame/machine.h"
2. Add the following into "gain_analysis.h":
#ifdef HAVE_CONFIG_H
# include <config.h>
#endif
3. Define the following in "gain_analysis.h"
typedef FLOAT Float_t;
(sample_t == FLOAT)

Important: in the last release, 3.95.1, it was:
typedef sample_t Float_t;
Only now, in the CVS, it has been changed to:
typedef float Float_t;
This won't work on some platforms, because other code assumes the 
functions return sample_t.

SOLUTION 2:

The problem with solution 1 is that removing the defaults from 
"libmp3lame/machine.h" might break compilation on some untested 
platforms. This would be undesired for a stable release.

So, as a "stable" and quick fix, I suggest including <util.h> into 
"mpglib/mpg123.h" and "gain_analysis.h" and adding
typedef FLOAT8 real;
and
typedef sample_t Float_t;
to these two files.
This is how it worked for "gain_analysis.h" in 3.95.1.


Just my suggestions presented for consideration.

Regards,
AK