From bc5a53574c64eb51d2cbf31b26f4607e566eeaab Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@duempel.org>
Date: Sat, 25 Oct 2014 20:42:50 +0200
Subject: [PATCH] decoder/faad: remove workaround for ancient libfaad2 ABI bug
Many years ago, FAAD had a serious ABI bug: the NeAACDecInit()
prototype in its header declared the "samplerate" parameter to be
"unsigned long *", but internally, the function assumed it was
"uint32_t *" instead. On 32 bit machines, that was no difference, but
on 64 bit, this left one portion of the return value uninitialized;
and worse, on big-endian, the wrong word was filled. This bug had to
be worked around in MPD (commit 9c4e97a6).
A few months later, the bug was fixed in the FAAD CVS in commit 1.117
on file libfaad/decoder.c; the commit message was:
"Use public headers internally to prevent duplicate declarations"
The commit message was too brief at best; the problem was not
duplicate declarations, but a prototype mismatch. No mention of the
bug fix in the ChangeLog.
The MPD project never learned about this bug fix, and so MPD would
always pass a "uin32_t *" dressed up as a "unsigned long *". Nearly 6
years later, it's about time to fix this second ABI problem. Let's
kill the workaround!
---
NEWS | 1 +
m4/faad.m4 | 31 +---------------------
src/decoder/plugins/FaadDecoderPlugin.cxx | 12 ++-------
src/decoder/plugins/Mp4v2DecoderPlugin.cxx | 12 ++-------
4 files changed, 6 insertions(+), 50 deletions(-)
diff --git a/NEWS b/NEWS
index e1497a0ed..2a6c5c439 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,6 @@
ver 0.19.2 (not yet released)
* decoder
+ - faad: remove workaround for ancient libfaad2 ABI bug
- ffmpeg: recognize MIME type audio/aacp
* output
- fix memory leak after filter initialization error
diff --git a/m4/faad.m4 b/m4/faad.m4
index 5ca520e79..9dcb1ccab 100644
--- a/m4/faad.m4
+++ b/m4/faad.m4
@@ -62,36 +62,7 @@ int main() {
CPPFLAGS=$oldcppflags
fi
-if test x$enable_aac = xyes; then
- oldcflags=$CFLAGS
- oldlibs=$LIBS
- oldcppflags=$CPPFLAGS
- CFLAGS="$CFLAGS $FAAD_CFLAGS -Werror"
- LIBS="$LIBS $FAAD_LIBS"
- CPPFLAGS=$CFLAGS
-
- AC_MSG_CHECKING(for broken libfaad headers)
- AC_COMPILE_IFELSE([AC_LANG_SOURCE([
-#include <faad.h>
-#include <stddef.h>
-#include <stdint.h>
-
-int main() {
- unsigned char channels;
- uint32_t sample_rate;
-
- NeAACDecInit2(NULL, NULL, 0, &sample_rate, &channels);
- return 0;
-}
- ])],
- [AC_MSG_RESULT(correct)],
- [AC_MSG_RESULT(broken);
- AC_DEFINE(HAVE_FAAD_LONG, 1, [Define if faad.h uses the broken "unsigned long" pointers])])
-
- CFLAGS=$oldcflags
- LIBS=$oldlibs
- CPPFLAGS=$oldcppflags
-else
+if test x$enable_aac = xno; then
FAAD_LIBS=""
FAAD_CFLAGS=""
fi
diff --git a/src/decoder/plugins/FaadDecoderPlugin.cxx b/src/decoder/plugins/FaadDecoderPlugin.cxx
index 793ab1011..add23aaa4 100644
--- a/src/decoder/plugins/FaadDecoderPlugin.cxx
+++ b/src/decoder/plugins/FaadDecoderPlugin.cxx
@@ -255,20 +255,12 @@ faad_decoder_init(NeAACDecHandle decoder, DecoderBuffer &buffer,
}
uint8_t channels;
- uint32_t sample_rate;
-#ifdef HAVE_FAAD_LONG
- /* neaacdec.h declares all arguments as "unsigned long", but
- internally expects uint32_t pointers. To avoid gcc
- warnings, use this workaround. */
- unsigned long *sample_rate_p = (unsigned long *)(void *)&sample_rate;
-#else
- uint32_t *sample_rate_p = &sample_rate;
-#endif
+ unsigned long sample_rate;
long nbytes = NeAACDecInit(decoder,
/* deconst hack, libfaad requires this */
const_cast<unsigned char *>(data.data),
data.size,
- sample_rate_p, &channels);
+ &sample_rate, &channels);
if (nbytes < 0) {
error.Set(faad_decoder_domain, "Not an AAC stream");
return false;
diff --git a/src/decoder/plugins/Mp4v2DecoderPlugin.cxx b/src/decoder/plugins/Mp4v2DecoderPlugin.cxx
index bf97763c5..34bccd243 100644
--- a/src/decoder/plugins/Mp4v2DecoderPlugin.cxx
+++ b/src/decoder/plugins/Mp4v2DecoderPlugin.cxx
@@ -39,15 +39,7 @@ static MP4TrackId
mp4_get_aac_track(MP4FileHandle handle, NeAACDecHandle decoder,
AudioFormat &audio_format, Error &error)
{
- uint32_t sample_rate;
-#ifdef HAVE_FAAD_LONG
- /* neaacdec.h declares all arguments as "unsigned long", but
- internally expects uint32_t pointers. To avoid gcc
- warnings, use this workaround. */
- unsigned long *sample_rate_r = (unsigned long*)&sample_rate;
-#else
- uint32_t *sample_rate_r = sample_rate;
-#endif
+ unsigned long sample_rate;
const MP4TrackId tracks = MP4GetNumberOfTracks(handle);
@@ -80,7 +72,7 @@ mp4_get_aac_track(MP4FileHandle handle, NeAACDecHandle decoder,
uint8_t channels;
int32_t nbytes = NeAACDecInit(decoder, buff, buff_size,
- sample_rate_r, &channels);
+ &sample_rate, &channels);
free(buff);