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

fix(main/openal-soft): disable dlopen() of opensles for luanti #23149

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

robertkirkman
Copy link
Contributor

@robertkirkman robertkirkman commented Feb 10, 2025

@Biswa96
Copy link
Member

Biswa96 commented Feb 10, 2025

Instead of reverting the patches, would it be possible to disable the HAVE_DYNLOAD constant in alc/backends/opensl.cpp file?

@robertkirkman robertkirkman changed the title fix(main/openal-soft): revert opensl loading change for luanti fix(main/openal-soft): disable dlopen() of opensles for luanti Feb 11, 2025
@robertkirkman
Copy link
Contributor Author

robertkirkman commented Feb 11, 2025

Thank you, I did not really notice that earlier, but yes, adding -DHAVE_DLFCN_H=OFF to TERMUX_PKG_EXTRA_CONFIGURE_ARGS appears to have an identical effect, and it also solves the problem, while being much fewer lines, so I have changed it to that.

@Biswa96
Copy link
Member

Biswa96 commented Feb 11, 2025

I did not mean to add that option globally. There are many source files which depend on that option. So, I am not sure if -DHAVE_DLFCN_H=OFF breaks any existing stuff.

@robertkirkman
Copy link
Contributor Author

robertkirkman commented Feb 11, 2025

Well, I did not think that other stuff will be affected because it doesn't seem used in the Termux build of this package,

but I do not fully understand the problem, since I have no idea why only one app is affected by the problem and not other apps,

and yes you are correct, so I will write this exactly how you suggested. Also, the exact way you suggested does also work to fix the problem.

@robertkirkman robertkirkman marked this pull request as draft February 11, 2025 10:11
@robertkirkman
Copy link
Contributor Author

robertkirkman commented Feb 11, 2025

@Biswa96 something still seemed confusing to me, even more so than this problem already is, so I carefully double checked everything, and unfortunately, I have to report the following:

  • my original bisection and whole commit reversion was correct, and -DHAVE_DLFCN_H=OFF or -DALSOFT_DLOPEN=OFF by themselves were both also able to work
  • but, very unfortunately, I am regretfully saddened to have to report that when you sent me your suggestion about HAVE_DYNLOAD, I made a mistake while testing it and I did not test it correctly, so I did not realize that it didn't work as written. When I double check the exact method you suggested, unfortunately it results in this error:
CANNOT LINK EXECUTABLE "mplayer": cannot locate symbol "SL_IID_ENGINE" referenced by "/data/data/com.termux/files/usr/lib/libopenal.so"...

I have now thoroughly tested this, and it appears that at the very least, in order for the HAVE_DYNLOAD reversion to work, this single line in the CMakeLists.txt must also be reverted. I have just pushed a version that is currently able to fix the problem for me.

@robertkirkman robertkirkman marked this pull request as ready for review February 11, 2025 10:44
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.

[Bug]: after openal-soft bump to 1.24.2, sound stopped working in x11/luanti
2 participants