Skip to content

ase/Makefile.mk: fix libsndfile support for flac, ogg and opus files#78

Open
swesterfeld wants to merge 1 commit intotrunkfrom
stw/fix-flac-ogg-opus
Open

ase/Makefile.mk: fix libsndfile support for flac, ogg and opus files#78
swesterfeld wants to merge 1 commit intotrunkfrom
stw/fix-flac-ogg-opus

Conversation

@swesterfeld
Copy link
Copy Markdown
Collaborator

I found that LiquidSFZ in Anklang does not load .flac or .ogg files. This PR fixes libsndfile support for these formats for me.

Signed-off-by: Stefan Westerfeld <stefan@space.twc.de>
greptile-apps[bot]

This comment was marked as spam.

@gemini-code-assist

This comment was marked as spam.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request aims to fix libsndfile support for FLAC, Ogg, and Opus files by updating header paths and using pkg-config for linking. The changes are a good step towards a more robust build process. I've found one issue with the list of libraries passed to pkg-config where a required library is missing and others are redundant. My suggestion should resolve this.

Comment thread ase/Makefile.mk
Copy link
Copy Markdown
Owner

@tim-janik tim-janik left a comment

Choose a reason for hiding this comment

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

.

Comment thread ase/Makefile.mk
$(LIBSNDFILE_OBJECTS), \
ase/Makefile.mk | $>/lib/, \
-lmpg123 -lmp3lame, \
$(shell $(PKG_CONFIG) --libs \
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Take a look at config-checks.mk, that's where we do all pkg checks in one place and cache them.
Once the packages are determined present there its just a matter of adding *_CFLAGS or *_LIBS elsewhere in the Makefiles.

Comment thread ase/Makefile.mk
$Q echo " __has_include(<vorbisfile.h>) && \\" >> $@.tmp
$Q echo " __has_include(<vorbisenc.h>) && \\" >> $@.tmp
$Q echo " __has_include(<opus.h>) )" >> $@.tmp
$Q echo " __has_include(<FLAC/all.h>) && \\" >> $@.tmp
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

<flac.h> looks indeed odd, i don't have that in externals/ or /usr/include/.

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