FLAC-Free Lossless Audio Codec

Tracker: Patches

8 Correct FLAC inline and fix compiling with Sun Studio - ID: 1701960

Last Update: Comment added ( jcoalson )

Details:

This patch fixes a number of problems with FLAC. Note this bug is a new
patch that works with FLAC 1.4.0 and replaces bug 1214424. I couldn't
figure out how to update that bug with a new patch.

1) Mainly it fixes flac so it properly tests for inline
in the configure script and consistently uses
FLAC_INLINE everywhere. This properly sets inline
to work on different compilers by using AC_C_INLINE.
Note you need to run autoheader and autoconf after
applying the patch for config.h to have the define
FLAC_INLINE.

2) Fix src/share/replaygain_analysis so it compiles
with Sun Studio. Sun Studio doesn't like floating
points in array values. So I fixed this by changing
RMS_WINDOW_TIME from 0.050 to 50 (so it is specified
in ms rather than s). Then in the two places where
it is referenced I add " / 1000" so it gives the
same value. I think this is cleaner. Plus it works
with more compilers.

3) Note that it was necessary to redefine four things
as extern instead of static. This is because
Sun Studio does not allow you to reference static
elements from within extern inline functions.

bitreader_read_from_client and byte_to_unary_table
in src/libFLAC/bitreader.c
bitwriter_grow_ in src/libFLAC/bitwriter.c
is_big_endian_host_ in src/libFLAC/md5.c.

This was necessary because Sun Studio doesn't
allow you to reference static variables from
extern inline functions.

Also byteSwap in src/libFLAC/md5.c needed to be
more clearly defined as "extern" to make the
linker happy.

4) Sun Studio doesn't like the ^M characters at the
end of each line in
include/share/replaygain_analysis.h, so the
patch removes these.

Submitted:

Brian Cameron ( yippi ) - 2007-04-17 04:53

Priority:

Status:

Open

Resolution:

Fixed

Assigned:

Josh Coalson

Category:

None

Group:

None

Visibility:

Public

Comments ( 10 )

Date: 2009-01-03 02:13 Sender: jcoalson ok, I checked in fixes for everything I think. can you try with cvs head?
Date: 2008-12-08 23:39 Sender: yippi ping Can this go upstream? This bug report seems to have languished for over a year.
Date: 2007-11-07 01:20 Sender: jcoalson it will eventually get in... I haven't had a lot of free time to work on FLAC lately
Date: 2007-11-06 22:51 Sender: yippi Can this patch go upstream? It's been a long time since this bug has been updated. If you want me to change the patch to use some other string than FLAC_INLINE, I can change that if you want. It would be nice if FLAC had better inline support.
Date: 2007-08-24 19:07 Sender: jengelh Can't you use -Dinline=__inline__, if the compiler does not know about 'inline', instead of using shouting macros like FLAC_INLINE? e.g. configure.ac: AC_C_INLINE if [ "$ac_cv_c_inline" != no ]; then AC_CFLAGS="$AC_CFLAGS -Dinline=$ac_cv_c_inline"; fi; ... AC_SUBST(AC_CFLAGS) Makefile.am: AM_CFLAGS = $(AC_CFLAGS) $(libogg_CFLAGS) $(etc_CFLAGS)
Date: 2007-08-01 18:59 Sender: yippi Here is an updated patch that applies cleanly against 1.4.0. This patch addresses the 4 above issues. Note that in 1.4.0 you did do some work to fix issue #2. However, I don't think that fix is so great. It's not good to hardcode the value only on Sun. So I changed this again so that RMS_WINDOW_TIME is changed from 0.050 to 50 (so it is specified in ms rather than s). Then in the two places where it is referenced I add " / 1000". This way it gives the same value. This compiles fine with Sun Studio. Sun Studio just doesn't like the numeric values with the decimal points (it likes 1 but doesn't like 1."). If you really want to keep the "." characters in there when compiling with GCC, then perhaps you should "#ifdef __GNUC__" and in the else use the code without the ".". Note that #ifdef __sun isn't so good because users can use the GCC compiler on Sun. Though I'd still recommend changing RMS_WINDOW_TIME to 50 and its usage to "RMS_WINDOW_TIME / 1000" just because it's nicer. I think this is a cleaner fix for issues #2, since it doesn't hardcode the value when using Sun Studio. I know you are talking about changing the AC_INLINE to a different string, but I'll let you do that since you said you would. However AC_INLINE works fine with the latest autotools on Solaris. Perhaps this is a bug with older versions of the autotools that we don't need to support anymore? File Added: flac-01-forte.diff
Date: 2007-07-31 22:31 Sender: jcoalson I'll do it
Date: 2007-07-30 20:45 Sender: nobody Logged In: NO Seems to work okay for me as FLAC_INLINE, though changing this string to something else that avoids the "AC_" should be a trivial change. Do you need me to change this, or do you want to? This is Brian.Cameron@sun.com (user yippi).
Date: 2007-07-26 07:07 Sender: jcoalson I'm a little bit wary about applying this, the reason I had FLaC__INLINE before was because of an autoconf bug that 'owned' any symbol that had AC_ anywhere in it and caused all kinds of problems... let me take a look more closely
Date: 2007-05-01 02:35 Sender: yippi Can this go upstream? I think this patch makes FLAC much more portable and shouldn't contain anything controversial.

Attached Files ( 2 )

Filename	Description	Download
flac-01-forte.diff	patch fixing FLAC	Download
flac-01-forte.diff	updated patch to fix this issue.	Download

Changes ( 7 )

Field	Old Value	Date	By
resolution_id	Accepted	2009-01-03 02:13	jcoalson
priority	6	2009-01-01 01:15	jcoalson
File Added	239518: flac-01-forte.diff	2007-08-01 18:59	yippi
assigned_to	nobody	2007-04-21 05:43	jcoalson
resolution_id	None	2007-04-21 05:43	jcoalson
priority	5	2007-04-21 05:43	jcoalson
File Added	225244: flac-01-forte.diff	2007-04-17 04:53	yippi

FLAC-Free Lossless Audio Codec

Tracker: Patches

Comments ( 10 )

Attached Files ( 2 )

Changes ( 7 )

About SourceForge

Find Software

Develop Software

Community

Help