Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the fallback to the internal libShout to cmake as well #2714

Merged
merged 5 commits into from
May 5, 2020

Conversation

daschuer
Copy link
Member

@daschuer daschuer commented Apr 25, 2020

This fixes lp1833225 also for cmake builds
https://bugs.launchpad.net/mixxx/+bug/1875013

I have verified that it still works and that we use the same build flags as the Ubuntu System lib.

This fixes lp1833225 also for cmake builds
@daschuer daschuer requested a review from Holzhaus April 25, 2020 21:11
Copy link
Member

@Holzhaus Holzhaus left a comment

Choose a reason for hiding this comment

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

Look good, but idn't test yet.

@Holzhaus Holzhaus added this to the 2.3.0 milestone Apr 26, 2020
@daschuer
Copy link
Member Author

OK, extra message is in place.

@daschuer
Copy link
Member Author

daschuer commented Apr 28, 2020

Ups, It has always used the system lib for linking.
Now I have uninstalled it and it still builds.

And broadcasting is still working :-)

Copy link
Member

@Holzhaus Holzhaus left a comment

Choose a reason for hiding this comment

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

Seems to work, but the printed messages could be improved because it doesn't even mention libshout.

@daschuer
Copy link
Member Author

daschuer commented May 3, 2020

Done.

@Holzhaus Holzhaus merged commit 2384465 into mixxxdj:master May 5, 2020
@Holzhaus
Copy link
Member

Holzhaus commented May 5, 2020

Thank you!

@jospezial
Copy link

https://bugs.launchpad.net/mixxx/+bug/1833225 should be fixed now with latest libshout-2.4.4 and libshout-2.4.5 releases.

https://gitlab.xiph.org/xiph/icecast-libshout/-/blob/master/NEWS
libshout 2.4.5 (20201219)

libshout 2.4.4 (20201001)

  • Fixed handling of blocking/non-blocking mode
  • Fixed ICY port increment
  • Fixed reusing of handles
  • Fixed error handling of Ogg sync layer
  • Fixed Passing of errors between connection and instance layer
    Without this fix died connections were not correctly detected.
  • Fixed and improved build scripts

libshout-2.4.5 is not tagged yet as release but downloadable:
https://ftp.osuosl.org/pub/xiph/releases/libshout/libshout-2.4.5.tar.gz

I have tried now with libshout-2.4.5 installed in my system
but configure still says it wants to use the bundled internal libshout.

-- Found Shout: /usr/lib64/libshout.so
-- Installed libshout version is suffering from bug lp1833225
-- Using internal libshout
-- Found OpenSSL: /usr/lib64/libcrypto.so (found version "1.1.1i")

/usr/lib64/pkgconfig/shout.pc has
Version: 2.4.5

lrwxrwxrwx 1 root root 17 30. Dez 12:54 /usr/lib64/libshout.so -> libshout.so.3.2.0
lrwxrwxrwx 1 root root 17 30. Dez 12:54 /usr/lib64/libshout.so.3 -> libshout.so.3.2.0
-rwxr-xr-x 1 root root 126880 30. Dez 12:54 /usr/lib64/libshout.so.3.2.0

See also Gentoo bug:
https://bugs.gentoo.org/739498#c26

@jospezial
Copy link

If I remove "QUIET" in FindShout.cmake:
-- Checking for module 'shout'
-- Found shout, version 2.4.5

@Be-ing
Copy link
Contributor

Be-ing commented Jan 1, 2021

@jospezial if you find a fix, please open a pull request.

@jospezial
Copy link

There must be something wrong with

if(Shout_FOUND AND Shout_VERSION VERSION_LESS 2.4.4)
and line
if(NOT Shout_FOUND OR Shout_VERSION VERSION_LESS 2.4.4)

but there end my skills about programming.

@fordfrog
Copy link

fordfrog commented Jan 1, 2021

@jospezial i personally would add lines just before these lines to get the output of Shout_FOUND and Shout_VERSION to see why the conditions are true.

@daschuer
Copy link
Member Author

daschuer commented Jan 1, 2021

Oh I see FindShout.cmake does not set Shout_VERSION. I will provide a fix.

@daschuer daschuer mentioned this pull request Jan 1, 2021
@daschuer
Copy link
Member Author

daschuer commented Jan 1, 2021

Fixed in: #3505

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants