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

Remove duplicate CMake code when adding backends. #96

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

adrianbroher
Copy link
Contributor

The PR moves some duplicated CMake code like option definitions, set-ting variables or checking for availability of a specific backend to a common CMake macro. As far as I could check this seems to work properly for the backends ALSA, OSS, PORTAUDIO, PULSEAUDIO, JACK but I haven't no ways to test the other backends, maybe the Travis/AppVeyor can give me some answers here.

@adrianbroher
Copy link
Contributor Author

adrianbroher commented Mar 5, 2017

Looks like WinMM, DSound and MMDevAPI are detected properly on Windows, but the TravisCI Linux build almost enables no backends (except OSS) because the dependencies are not installed. Is this intended?

CMakeLists.txt Outdated
IF(ALSOFT_REQUIRE_ALSA AND NOT HAVE_ALSA)
MESSAGE(FATAL_ERROR "Failed to enabled required ALSA backend")
ADD_BACKEND(ALSA ALSA Alc/backends/alsa.c)
IF(ALSOFT_BACKEND_ALSA)
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure these should check the ALSOFT_BACKEND_* options, since they may be cached values and will stay even if the backend is no longer available. I think it makes more sense to check the HAVE_* variables.

# WINMM_INCLUDE_DIRS - Set when WINMM_INCLUDE_DIR is found
#
# WINMM_INCLUDE_DIR - where to find dsound.h, etc.
# WINMM_LIBRARY - the dsound library
Copy link
Owner

Choose a reason for hiding this comment

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

Seems to be a bit of a copy-paste error here.

@kcat
Copy link
Owner

kcat commented Mar 5, 2017

Looks like WinMM, DSound and MMDevAPI are detected propertly, but the TravisCI build almost enables no backends because the dependencies are not installed. Is this intended?

I don't really know. I'm not familiar with using TravisCI, so I'm not sure if I can or how to enable any dependencies.

@adrianbroher
Copy link
Contributor Author

I'm not sure these should check the ALSOFT_BACKEND_* options, since they may be cached values and will stay even if the backend is no longer available. I think it makes more sense to check the HAVE_* variables.

Yeah, checking against a user option isn't the best idea. I replaced the ALSOFT_BACKEND_* variables with HAVE_* counterparts.

Seems to be a bit of a copy-paste error here.

Yes, it was a copy-paste error.

@kcat
Copy link
Owner

kcat commented Mar 7, 2017

Seems the Android build fails, unable to find the OpenSLES lib: https://travis-ci.org/kcat/openal-soft/jobs/208520864. Can't exactly see why though, since it seems to be looking for the same lib name. Perhaps a default search path isn't being set for find_library?

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

Successfully merging this pull request may close these issues.

2 participants