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

Add built-in FMMIDI support to Autotools #881

Closed
wants to merge 1 commit into from
Closed

Add built-in FMMIDI support to Autotools #881

wants to merge 1 commit into from

Conversation

fdelapena
Copy link
Contributor

As requested by Ghabry (#752), an opted-out by default FMMIDI option for Autotools.

The --enable-* option differs from --with-* because this is a built-in feature, not an external package. That's why I suggest to name it --enable-fmmidi without the "builtin", as it feels redundant.

This will need #ifdef HAVE_FMMIDI guards (don't forget #include "system.h") where used.

CMake users: Just declare -DHAVE_FMMIDI as usual.

@Ghabry
Copy link
Member

Ghabry commented Apr 30, 2016

Iirc someone also suggested a way to fallback to use sdl midi by default and fallback to fmmidi when it fails. But that needs another audio-sdl code change. Not sure if its currently worth it.

@carstene1ns
Copy link
Member

I also had some commit for it prepared: carstene1ns@94fa946
We talked about allowing fallback mode for fmmidi if sdl_mixer does not play midi. WANT_FMMIDI can be 1 or 2.
Also i really dislike having empty objects compiled and linked in, so i disabled them if unneeded.

@carstene1ns
Copy link
Member

And a friendly reminder to guard the fmmidi sources properly.
Currently it is bloat and will stay bloat, even if someone -disable[s]-fmmidi.
(I know you also wrote it above, but this is important IMHO)

@Ghabry
Copy link
Member

Ghabry commented Apr 30, 2016

👍 for your fmmidi want (fallback) and guarding.

@fdelapena
Copy link
Contributor Author

carstene1ns' autotools branch is definitely better, could I discard this PR?

@Ghabry
Copy link
Member

Ghabry commented May 2, 2016

Okay. I will cherry pick carstene1ns commit into my branch instead.

@fdelapena fdelapena closed this May 2, 2016
@fdelapena fdelapena deleted the fmmidi branch May 13, 2016 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants