-
-
Notifications
You must be signed in to change notification settings - Fork 160
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
Make mpg123, fluidsynth and sndfile not link directly to lower-level audio playback libs #2471
Conversation
9419211
to
10783c3
Compare
Here is the diff of the libasound-e2127b4b.so.2.0.0
- libattr-4f2a9577.so.1.1.0
libbrotlicommon-c828db13.so.1.1.0
libbrotlidec-3b276a64.so.1.1.0
- libbz2-a273e504.so.1.0.6
- libcap-79733b59.so.2.22
- libdbus-1-5d678da9.so.3.14.14
- libdw-0-780627ce.176.so
- libelf-0-3d9de92a.176.so
- libFLAC-9c16284b.so.12.1.0
+ libFLAC-11b10a77.so.12.1.0
- libfluidsynth-78c3029a.so.3.2.2
+ libfluidsynth-13e12a66.so.3.2.2
libfreetype-ce7af30f.so.6.20.1
- libgcrypt-18957bae.so.11.8.2
libgomp-a34b3233.so.1.0.0
- libgpg-error-3f09c3c7.so.0.10.0
libharfbuzz-b5bd39f9.so.0.60821.0
- libjpeg-1944207f.so.62.4.0
+ libjpeg-034e96d7.so.62.4.0
- liblz4-af1653fb.so.1.8.3
- liblzma-004595ca.so.5.2.2
libmodplug-8c1b3b53.so.1.0.0
libmpg123-7a05540a.so.0.48.0
libogg-3f4a1572.so.0.8.5
libopus-4c85b165.so.0.9.0
libopusfile-df4005ff.so.0.4.5
- libpcre-9513aab5.so.1.2.0
libpng16-92b73642.so.16.40.0
libportmidi-bc2a3536.so.2.0.3
- libpulse-5b5d1e90.so.0.23.0
- libpulsecommon-14-9b36c523.2.so
- libpulse-simple-4ba3beb5.so.0.1.1
libSDL2-2-cdcf49c3.0.so.0.2600.5
libSDL2_image-2-d19463c5.0.so.0.2.3
libSDL2_mixer-2-705164cf.0.so.0.600.3
libSDL2_ttf-2-e58b8489.0.so.0.2000.2
- libselinux-0922c95c.so.1
libsharpyuv-5ed6246f.so.0.0.1
libsndfile-bfa200bd.so.1.0.37
- libsystemd-9be61d21.so.0.6.0
libtiff-61e60d59.so.6.0.2
libvorbis-ac399090.so.0.4.9
libvorbisenc-bb396c4a.so.2.0.12
libvorbisfile-8aced0b7.so.3.3.8
libwebp-3bee5bfa.so.7.1.8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Tested on mac |
Is this okay? @ankith26 I believe all the audio output backends of mpg123 and fluidsynth should be turned off, because they're not relevant to SDL_mixer. This also means the SDL + libs build order doesn't need to be rearranged, so this PR will get simpler and shave even more off of dependency size? |
Regarding flac-in-ogg support, I realised that I will take starbucks suggestions to simplify the diffs in this PR, and move this flac fix stuff out too in the process |
10783c3
to
8f9fec0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
I really like how this turned out, better buildconfig -> smaller wheels.
I just checked and it looks like this takes the Linux wheels down 1.4 mb each
Much less significant effect on the Mac wheels, only down about 2 kb each
Depends on #2470 (it should be merged before this)
EDITED description to what this PR does now (taking starbucks suggestions):
Previously,
mpg123
,fluidsynth
andsndfile
would link directly to lower-level librariesalsa
,pulseaudio
and others and all these would get included in the wheels. But these links aren't required for audio playbackAs a side effect of these changes, our manylinux wheels should get a bit lighter (maybe 1-2 mb)
To test this PR, we need to make sure that fluidsynth-based music and mp3 music still work as before on linux (which I can test) and mac (for which I need a volunteers help)