Skip to content

Commit

Permalink
Fix link failure with gcc-9 and wl,asneeded flags
Browse files Browse the repository at this point in the history
Author: Gianfranco Costamagna <locutusofborg@debian.org>
Last-Update: 2019-06-11
  • Loading branch information
fabiangreffrath committed Jun 11, 2019
1 parent f1f8e00 commit 920ec98
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,9 @@ AC_DEFUN([AC_C99_FUNC_LRINTF],
ac_cv_c99_lrintf,
[
lrintf_save_CFLAGS=$CFLAGS
CFLAGS="-O -lm"
lrintf_save_LIBS=$LIBS
CFLAGS="-O"
LIBS="-lm"
AC_TRY_LINK([
#define _ISOC9X_SOURCE 1
#define _ISOC99_SOURCE 1
Expand All @@ -103,6 +105,7 @@ AC_TRY_LINK([
], if (!lrintf(3.14159)) lrintf(2.7183);, ac_cv_c99_lrintf=yes, ac_cv_c99_lrintf=no)
CFLAGS=$lrintf_save_CFLAGS
LIBS=$lrintf_save_LIBS
])
Expand Down

6 comments on commit 920ec98

@wildstar84
Copy link

@wildstar84 wildstar84 commented on 920ec98 Nov 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow this commit? BREAKS playback of many AAC streams causing broken play and floods of errors in Audacious media player (v2.8.8-3 works fine, v2.8.8-3.2+ FAIL) and this is the only thing mentioned in the changelog for v2.8.8-3.2. Please investigate.

ERROR aac.cc:449 [play]: Channel coupling not yet implemented.
ERROR aac.cc:449 [play]: Unexpected channel configuration change.

Example stream: https://c4icy.prod.playlists.ihrhls.com/3379_icy

Thanks,

Jim (turnerjw784@yahoo.com)

@fabiangreffrath
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v2.8.8-3 works fine, v2.8.8-3.2+

With these I guess you are referring to Debian package revisions? Well, there was 2.8.8-3.1 between them and this contained far more invasive changes than the changed linker line in 2.8.8-3.2. You might want to check this one out first: https://snapshot.debian.org/package/faad2/2.8.8-3.1/

@hlef Do you think one of your security patches might have introduced this behavior?

@hlef
Copy link
Contributor

@hlef hlef commented on 920ec98 Nov 8, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is probably a consequence of 3b80a57.

In short, this commit adds a check to reject specific broken AAC streams. Those streams previously "seemed" to be supported by FAAD2, but this support heavily relied on undefined behavior, and contained security relevant issues. I don't think the output was valid / reliable.

Now I hope we're not rejecting valid streams with this check. I spent a lot of time on this patch, this seems quite unlikely to me. But still, I might be mistaken. I will try to take a look at this during the week-end.

thanks for reporting this.

@wildstar84
Copy link

@wildstar84 wildstar84 commented on 920ec98 Nov 8, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick responses!

Yes, I'm referring to Deb. package#s. I'm using Antix, a derivitive rolling distro of Debian Testing and it skipped v2.8.8-3.1. I went ahead and installed v2.8.8-3.1 from the snapshot link above (libfaad2_2.8.8-3.1_amd64.deb) and, as I suspected, it failed too (I doubted that the simple 2.8.8-3.2 patch was the culprit). Anyway, the example stream I included was a commercial radio station on IHeartRadio (https://www.iheart.com/live/lone-star-925-3379/: Lone Star 92.5 - Dallas-Ft. Worth's Classic Rock). Here are a cpl. more commercial station examples from IHeartRadio: https://c10icy.prod.playlists.ihrhls.com/2241_icy (97.1 the EAGLE) and https://c11icy.prod.playlists.ihrhls.com/7094_icy (Bloomberg Radio - 24/7 Global Business Coverage).

All 3 seem to play fine under v2.8.8-3, which I've rolled back to for now.

Hope this helps and let me know if I can be of further assistance.

@uklotzde
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm the issue for Mixxx after we received the first error report from an Arch Linux user: https://www.mixxx.org/forums/viewtopic.php?f=3&t=13157

@wildstar84
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see v2.9.2-1 has come out, but b4 I upgrade, has anything been done a/b this yet? (I didn't see anything related in any of the commits since)

Please sign in to comment.