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: remove need for explicit include path /usr/local/opus #47

Closed
wants to merge 1 commit into from

Conversation

braindigitalis
Copy link

This is not the way parent headers are supposed to be included. This has been an issue for years and nobody raised a PR yet, so i'm raising one.

This is not the way parent headers are supposed to be included. This has been an issue for years and nobody raised a PR yet, so i'm raising one.
@C0rn3j
Copy link

C0rn3j commented Dec 21, 2024

@rillian @erikd @jmvalin @MisterZeus @phschafft @smarter @tmatth

Could anyone please merge this and tag a release?

This has been an issue with a simple fix available for over 6 years - #10

There's similarly easy-to-deal-with PRs in this repository but this one is the one that keeps biting me over and over.


I need to keep silly workarounds for every single platform because of this:

include-dirs = ["/usr/include/opus", "/opt/homebrew/include/opus", "C:/msys64/mingw64/include/opus"]

And I still get bit by it anyway the moment I change my env a little -

In file included from src/phazor/phazor.c:72:
D:/a/_temp/msys64/mingw64/include/opus/opusfile.h:110:11: fatal error: opus_multistream.h: No such file or directory
  110 | # include <opus_multistream.h>
      |           ^~~~~~~~~~~~~~~~~~~~

PR should have
Fixes #41

@braindigitalis
Copy link
Author

I think this library is unfortunately dead.

@C0rn3j
Copy link

C0rn3j commented Dec 22, 2024

xiph maintains opus itself too, so I am hopeful someone gets to it, although right now it's holiday season so it may take a couple weeks.

@tmatth
Copy link
Member

tmatth commented Dec 22, 2024

@rillian @erikd @jmvalin @MisterZeus @phschafft @smarter @tmatth

Could anyone please merge this and tag a release?

This has been an issue with a simple fix available for over 6 years - #10

As @tterribe already explained this is working as intended.

Plenty of third party projects which build on multiple OSes have no problem including opus_multistream as is (see e.g., VLC or ffmpeg) It's up to your build system to ensure the include path is correct (e.g. using pkg-config --cflags).

@tmatth tmatth closed this Dec 22, 2024
@C0rn3j
Copy link

C0rn3j commented Dec 22, 2024

And as pointed out in a comment below that one, xiph is working around this issue by using double quotes in opus.

Wouldn't it then be better to use double quotes here too, and wouldn't it resolve the issue for everyone without any impact and changing import paths?

@braindigitalis
Copy link
Author

if everyone else is saying it's broken, perhaps they are right?

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