This package is marked stable under amd64 despite the niggling little detail of it not actually working. It compiles and installs but cannot decode m4a's. Reproducible: Always Steps to Reproduce: 1. emerge /usr/portage/media-libs/faad2/faad2-2.0-r2.ebuild 2. Try and play an m4a in xmms 3. "!$^&*, it doesn't work!" Actual Results: It cannot play m4a's, giving the error anyone who's ever tried faad2 under amd64 recognizes and loathes: "MP4: Pulse coding not allowed in short blocks". Expected Results: It ought to be able to understand m4a files. Portage 2.0.50-r9 (default-amd64-2004.0, gcc-3.3.3, glibc-2.3.4.20040808-r0, 2.6.4-gentoo-r1) ================================================================= System uname: 2.6.4-gentoo-r1 x86_64 5 Gentoo Base System version 1.5.1 Autoconf: sys-devel/autoconf-2.59-r4 Automake: sys-devel/automake-1.8.5-r1 ACCEPT_KEYWORDS="amd64 ~amd64" AUTOCLEAN="yes" CFLAGS="-O2 -pipe" CHOST="x86_64-pc-linux-gnu" COMPILER="gcc3" CONFIG_PROTECT="/etc /usr/X11R6/lib/X11/xkb /usr/kde/2/share/config /usr/kde/3.1/share/config /usr/kde/3/share/config /usr/lib/mozilla/defaults/pref /usr/share/config /var/qmail/control" CONFIG_PROTECT_MASK="/etc/gconf /etc/terminfo /etc/env.d" CXXFLAGS="" DISTDIR="/usr/portage/distfiles" FEATURES="autoaddcvs ccache" GENTOO_MIRRORS="http://mirrors.tds.net/gentoo ftp://mirrors.tds.net/gentoo ftp://ftp.ndlug.nd.edu/pub/gentoo/" MAKEOPTS="-j3" PKGDIR="/usr/portage/packages" PORTAGE_TMPDIR="/var/tmp" PORTDIR="/usr/portage" PORTDIR_OVERLAY="/usr/local/portage/" SYNC="rsync://rsync.gentoo.org/gentoo-portage" USE="X aalib alsa amd64 apm arts avi berkdb cdr crypt cups dvd encode esd foomaticdb gdbm ggi gif gnome gpm gtk gtk2 guile imlib jpeg kde libg++ libwww mikmod motif mozilla mpeg multilib mysql ncurses nls nogcj oggvorbis opengl oss pam pdflib perl png ppds python qt quicktime readline scanner sdl slang spell ssl tcltk tcpd truetype usb xml2 xmms xv zlib"
It's been a month, and faad2 is still broken. Call me a purist, but I don't think well-known borked packages deserve to be flagged 'stable' just because they compile.
Yep..this problem still exists despite the patches this is the original now closed bug http://bugs.gentoo.org/show_bug.cgi?id=58780
It's a whole new year, and this broken package is still marked as "stable". I admit it does not crash, but it is completely useless under amd64 for it's stated function of decoding MPEG-4 and MPEG-2 audio files.
well, it's also used to make mplayer play mpeg4 video files, and that works fine. could you provide an example m4a file so we can test it? i don't have such a file
Here's a sample audio file that doesn't play properly: http://burningsmell.org/crash_powermac_lc.m4a mp4 video seemingly works, but mp4 audio is broken. Most of the time they're rendered as pure silence, sometimes clicks, and once in a blue moon mplayer plays them properly, but xmms never works, and this audio file in particular doesn't work under either.
I can confirm that the above file will decode properly with an x86 compiled version of faad2. It also will NOT decode properly with a amd64 compiled version of faad2. It will the produce the same "Error: Pulse coding not allowed in short blocks Error: Pulse coding not allowed in short blocks Error: Pulse coding not allowed in short blocks Segmentation fault" that has been refrenced before.
just did some more scrounging....the latest CVS of faad2 is fixed and decodes perfectly for amd64 the xmms plugin however does not build
Finally got to the root of this obnoxious bug! It seems faad2 has two different headers. There's the ones it uses internally, which are strictly defined as 32-bit everything; then there's the one it installs in the system which have longs all over the place, which of course become 64-bit under amd64. So faad2 itself was working, but anything linking to faad2 would call these 32-bit functions with 64-bit types. This led to all kinds of miscellaneous hilarity, like the XMMS plugin failing with faad's 'pulse coding not allowed in short blocks' error code when faad was actually returning success! It also had some ugliness where it didn't define headers for some functions and didn't #include <string.h> -- which of course the linker can fix for the programmer under a 32-bit system, but breaks horribly under 64-bit. After posting this I'll attach a patch that applies directly to faad2-2.0.tar.gz. It allows faad2 to compile and work(as long as you fix the broken makefile ./bootstrap and ./configure produce, but I think that's covered by other patches). It's been able to play every mp4 on my system flawlessly, and has been 100% stable so far.
Created attachment 50096 [details, diff] amd64 patch for faad2's inconsistent headers and misc nastiness
This patch works for me. Thanks
created a combined patch from the noext and your patch, otherwise it didn't work. i've added this patch to the tree, -r5 applies it. thanks for your effort.
i'm trying to use faad2 on x86 and these amd64 changes have changed the API in an incompatible way. mythmusic will not compile against it: aacdecoder.cpp: In member function `bool aacDecoder::initializeMP4()': aacdecoder.cpp:298: error: invalid conversion from `long unsigned int*' to ` unsigned int*' can these changes me made specific to the amd64?
They are legit also on x86, the changes needs to be done upstream and the apps which relies on the old behaviour should be fixed.
reopening, other programs which rely on the old behaviour dont like the patch. removing...
We NEED that patch, or something like it. Things will not work without it under amd64 because the old behavior is flagrantly wrong. I'll try and make a less bothersome patch but some breakage might be inevitable -- it was always broken, the compiler just didn't know it.
the problem is that upstream has to accept these changes, i'll keep bugging them. i've send the patch upstream and commited it on the same day. other projects which rely up on the old (bad or broken) behaviour have to change their programs, too.
Was there a change in policy recently? It's a hell of a lot harder to get and keep fixes in portage than it used to be. A *year* without any official releases is more than enough justification for including our own patches imho. Anyway I hope you'll like this one better, since it doesn't break compilation of anything -- the installed header is not modified. The patch and ebuild (which I'll attach shortly) build a working xmms plugin etc, and apply all the r4 patches as well as the amd64 one. I've tested it in my portage overlay. Any linking on vanilla x86 wouldn't break since there's no effective difference between an int and a long there. Anyone that built r5 on amd64 would need to rebuild things that linked to faad unfortunately.
it's wasnt taken out of the tree because of a policy. the problem is that the patch changes the api, even though this change is needed and sane. other programs, atleast 2 which are actually in the tree break due to this patch. I've removed it temporaly until i get the change to talk to upstream. foser, or better another user who talked to foser, pointed me to this fact. it really seems that upstream is dead or inactive, something has to be done. i will try to get this issue fixed next week.
Created attachment 58217 [details, diff] improved amd64 faad2 patch Modifies faad's internal headers instead of the external ones so as to not break compilation of anything depending on faad.
Created attachment 58218 [details] ebuild using amd64 patch
That patch is bogus. The redefinition of uintX_t isn't working as the logic is wrong, and anyway stdint.h takes care of defining them correctly depeding on arch. Removing references to standard ints is not good at all. They are used to avoid size misunderstanding. That can't be applied, sorry. The right way is to change the public api and fix the apps accordling. Avidemux is already fixed, both mplayer and xine-lib uses internal copies.
> That patch is bogus. The redefinition of uintX_t isn't working as the logic > is wrong, and anyway stdint.h takes care of defining them correctly depeding > on arch. If the logic is wrong, how about telling me *how* it's wrong so I don't make that mistake again? Works perfectly for me. I know perfectly well those types won't be used unless it can't find stdint.h but I put them there to be thorough. As for stdint.h "properly defining the integer types", it sure does, but faad2 fails to use them for the external library interface! Lot of good that does. > Removing references to standard ints is not good at all. They are used to > avoid size misunderstanding. HAHAHAHAHAHA! XD 'avoid size misunderstanding'. That's what you call a year+ of muddling around with "Pulse coding not allowed in short blocks" errors when faad2 was actually returning SUCCESS but the interfaces were so mangled we never knew it. The actual bits that NEED specific integer sizes are all internal, confusion is caused by USING those types internally because the external header does NOT. Anyway we already tried that solution with R5 and it was summarily rejected. Granted the way faad2 handles these interfaces is ugly -- why can't it just use the same damn header for both? :( -- but unless you feel like completely rewriting faad to unify the internal and external headers, we have to work with what we have. Let me suggest a compromise: Bring back R5, or R7 now I guess, but *mask it* for anything but amd64 and ppc64. I'll submit a improved improved patch for it that uses the holy standard integer types. We can likewise mask fixes for for whatever R5 breaks, additionally adding a >=faad-2.0r7 dependency in them. Then once we've got most everything working with R7, THEN we unmask all those for x86 and the like. amd64ers get what we want, a working faad2. x86ers get what they want, a working faad2 and working libs with it. We'll have to work quickly so we don't block new versions of things that use faad, but I am completely willing to *personally* do all the patchwork fixes if anyone will tell me what things R5 broke.
Created attachment 58582 [details, diff] improved improved amd64 faad2 patch Uses standard integer types in faad.h to match faad's internal headers. Replacement for previous patch, works properly with ebuild above
This patch seems ok to me.. I tested it on amd64 and x86 .. but I think its only necessary to play around with the long... bug I guess the others can't hurt (until the next version).
alright, no one complains so I put it back in (into -r6)... blame me if it breaks again
this patch got removed in -r7 because it breaks (and putting the patch in -r6 without revbump caused even more headaches), fresh suggestions would be appreciated.
(removing myself from cc, I already receive this through amd64@g.o) The most of headaches is when the patch is added and then removed and then readded. I think the patch currently in -r7 (and added in -r6) is good enough, *but* we must stop adding and removing it and just fix the broken apps. I can fix the ones in video/sound, but we must find a stable API for the next versions. By the way, currently upstream seams quite dead or at least doesn't care too much from users; xine-lib devs seems also in trouble with its license. So whichever decision we'll make, should be quite safe for now that no new faad2 version will come up.
Which apps does the current patch break? I'll see if I can fix them..
it breaks the fadd plugin in gstreamer, I don't se how this can bee fixed without a upstream release of fadd2. The gst-plugin can't be changed as then it won't build elsewhere.
I'm not following you. Where won't it build if we don't leave it broken?
Chalk up media-video/vlc as another package that doesn't seem to like the patched faad2. When playing an h264/aac mp4 file, the video plays properly, but the sound comes out sounding with a horrible static sound.
ok, this has been open for quite some time. faad2 is broken on amd64 and needs fixing to work correctly. taking the patch in and out doesn't solve the issue. upstream seems to be dead and we have devs willing to patch packages using faad2 to work with the changed api. plan: put in a new package.mask'ed revision of faad2 including the patch and have all current ebuilds using the old faad2 api depend on below this new revision. then our devs can start to patch packages and have them depend on at least the fixed revision of faad2. if you commit them to the tree, they need to be package.mask'ed too to not cause broken deps. if you unmask the new faad2 ebuild please make sure that every package using it also gets unmasked and that a new patched revision is available, else portage might do the upgrade and downgrade thing because of dependencies not sharing common matches. add a postinst message to the new faad2 ebuilds that the api has changed and that users need to update all packages that use faad2. how does that sound?
faad2 took over by sound herd, the patch is in place, I think the problem is solved now.
