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 build for macos #55

Merged
merged 2 commits into from
May 15, 2024
Merged

Conversation

EugeneVanchugov
Copy link
Contributor

@EugeneVanchugov EugeneVanchugov commented May 13, 2024

This PR makes the build on macos possible.
Related to this issue: #53

It did not solved the issue with crash during scene running, but at least editor works.

Requires you to call make install-steam-audio or set STEAM_AUDIO_LIB_PATH env variable with your custom location.

SConstruct Outdated Show resolved Hide resolved
@stechyo
Copy link
Owner

stechyo commented May 13, 2024

LGTM, just a small comment. Thank you!

@jamosdev
Copy link

the crash on run is becuase this addon blindly allocate an embree object without checking if it worked and then passes it into the steam audio system, which checks if for NULL then dereferences it to access some members. The error code was 0x03 whatever that means, 0x03 != NULL so it passes the NULL check then it crashes inside steam audio because it just tried to pointer dereference an error code or something along those lines. Debugging this was like hell, and had to build libphonon myself to be able to debug, getting xcode to attach was a pain.

If this were a c/c++ module instead of a gdextension then building and debugging would be much easier, and you have to modify godot source anyway to add the mix buffer function, so I wished it were a c/c++ module, anyway godot already uses embree so there is embree inside steam audio's libphonon, and then embree again inside godot proper. anyway, the embree in steam audio doesn't work on MacOS unless it is x86 or x86_64, like the embree_allocate checks the CPU arch and is hardcoded to return a fail message if not x86, so embree (steam audio's version anyway) was never going to work on apple silicon anyway, and this addon specifically uses the embree device instead of some generic device or something so this addon doesn't work on apple silicon, but should work on x86_64 macOS which can run on apple silicon macs via the binary translator system.

I think building steamaudio from source in the thirdparty of the godot source tree and then putting the godot-steam-audio into the modules would be good but it seems like a bit daunting task to convert a complicated cmake project like steam audio to scons, so I'm not sure. But maybe steam audio will update embree to work on apple silicon one day or maybe this addon can be changed to not require embree device?

@stechyo
Copy link
Owner

stechyo commented May 15, 2024

the crash on run is becuase this addon blindly allocate an embree object without checking if it worked

Where does this happen? The embree device creation should log an error if it didn't work.

maybe this addon can be changed to not require embree device?

Good point, Embree is optional so setting the SteamAudio raytracer to default on SteamAudioConfig might make it work on Apple Silicon.

@stechyo stechyo merged commit ae94dd5 into stechyo:master May 15, 2024
@stechyo
Copy link
Owner

stechyo commented May 15, 2024

Merged, thanks!

@EugeneVanchugov
Copy link
Contributor Author

EugeneVanchugov commented May 17, 2024

I was trying to build Steam Audio from sources, but encountered some problems with cmake and decided to drop it.

My goal is to make some audiovisual art using spatial and occlusion sound properties of Steam Audio and found that TouchDesigner has two operators for that purpose.

They use Steam Audio SDK for spatial rendering, but there's only HRTF. It works perfectly on M2 Pro.
I wonder if that's why they decided to drop occlusion altogether. But it sound good enough for me, so I stick with TouchDesigner.

@EugeneVanchugov EugeneVanchugov deleted the fix-macos-build branch May 17, 2024 13:01
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.

3 participants