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 #208006: Add built-in MP3 support for Windows #3395

Merged
merged 1 commit into from Feb 27, 2018

Conversation

Projects
None yet
2 participants
@Jojo-Schmitz
Copy link
Contributor

commented Jan 15, 2018

expects lame_enc.dll (from https://lame.buanzo.org/#lamewindl) to be found at the same place as libogg.dll, libsndfile-1.dll, libvorbis.dll, libvorbisfile.dll and portaudio.dll

AppVeyor build expected to fail, lame_enc.dll needs to get added to http://utils.musescore.org.s3.amazonaws.com/musescore_dependencies_win32.7z first

Not sure, we may want to use the same approach for master too, at least until there's MP3 support in libsndfile?

@lasconic

This comment has been minimized.

Copy link
Member

commented Jan 15, 2018

I added lame_enc.dll to the 7z, cleared the cache and restarted the build.

@Jojo-Schmitz

This comment has been minimized.

Copy link
Contributor Author

commented Jan 15, 2018

And it worked ;-)

@Jojo-Schmitz

This comment has been minimized.

Copy link
Contributor Author

commented Jan 16, 2018

Should apply to master too, but I guess with a merge conflict due to ${CROSSQT}/bin/icudt53.dll vs. ${QtInstallLibraries}

@lasconic

This comment has been minimized.

Copy link
Member

commented Jan 16, 2018

I will try it first. Since we have the luxury of downloading binaries for PR now : https://ci.appveyor.com/project/MuseScore/musescore/build/1.0.129/artifacts

If it works, it's probably worth doing something cleaner on master, and remove the whole lame loading scheme. That would mean having lame deployed on all platforms. On linux, it's probably just adding a dependency but on mac... we need to have a version compiled on an old mac system to be compatible...

@Jojo-Schmitz

This comment has been minimized.

Copy link
Contributor Author

commented Jan 16, 2018

Removing the loading scheme would require a parallel fix for Mac

@Jojo-Schmitz Jojo-Schmitz force-pushed the Jojo-Schmitz:mp3-windows-2.2 branch from 5db12f1 to fa2f612 Feb 9, 2018

@@ -421,6 +421,7 @@ if (MINGW)
${CROSS}/lib/libvorbis.dll
${CROSS}/lib/libvorbisfile.dll
${CROSS}/lib/portaudio.dll
${CROSS}/lib/lame_enc.dll

This comment has been minimized.

Copy link
@Jojo-Schmitz

Jojo-Schmitz Feb 9, 2018

Author Contributor

We could even make it OPTIONAL, only install it if it exists.

@Jojo-Schmitz Jojo-Schmitz force-pushed the Jojo-Schmitz:mp3-windows-2.2 branch 2 times, most recently from a5ebf2f to 465080b Feb 14, 2018

Fix #208006: Add built-in MP3 support for Windows
expects lame_enc.dll (from https://lame.buanzo.org/#lamewindl) to be
found at the same place as libogg.dll, libsndfile-1.dll, libvorbis.dll,
libvorbisfile.dll and portaudio.dll

@Jojo-Schmitz Jojo-Schmitz force-pushed the Jojo-Schmitz:mp3-windows-2.2 branch from 465080b to 51448ec Feb 23, 2018

@lasconic

This comment has been minimized.

Copy link
Member

commented Feb 27, 2018

Ok. Let's do it on windows at least.

@lasconic lasconic merged commit 2d30b14 into musescore:2.2 Feb 27, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Jojo-Schmitz Jojo-Schmitz deleted the Jojo-Schmitz:mp3-windows-2.2 branch Feb 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.