Replace libmodplug with libopenmpt
Hello, I am an OpenMPT / libopenmpt developer. As you might be aware, libmodplug (which VLC uses for module playback) is based on the playback code of the original ModPlug Tracker. Its playback capabilities have been left pretty much untouched since the original code release, meaning that it has many deficiencies when it comes to playing back module files correctly.
ModPlug Tracker was open-sourced more than ten years ago, and OpenMPT is the result of this. In 2014, libopenmpt has finally been released. It is a library based on OpenMPT's much improved playback code which supports a couple of more formats than libmodplug (minus a few formats that are not actually module files, such as MIDI and ABC), it supports them better (less playback bugs), supports more platforms than libmodplug and in general is still being developed very actively. To keep things simple, you can use the libmodplug emulation layer, but of course it would be nicer to directly talk to libopenmpt's own library interface.
There are already libopenmpt-based input plugins for XMPlay, Winamp and foobar2k, so it would be nice to see VLC finally updating its module player to the current decade as well. libopenmpt can be found at http://lib.openmpt.org/libopenmpt/ - if you have any questions, please let me know.
Activity
added Component::Demuxers Type::feature labels
changed milestone to %Features paradize
So, is it a fork of libmodplug or not?
If it's compatible with libmodplug, then it should be easy to replace in the contribs.
However, as usual, we should be careful about security issues (is this library fuzzed correctly? It's not like we had CVE with modplug...), about regressions in formats, and of course if it correctly cross-compiles (statistics says it does not).
It's not a fork. libopenmpt is based on OpenMPT, which is based on ModPlug Tracker, on which libmodplug is based. For this reason libopenmpt's codebase is very different from libmodplug's, and in many ways it is written in a more modern and more secure way than libmodplug; I am well aware of the CVEs you are talking about - some of them never affected OpenMPT because I rewrote the code in question to be more secure, others were fixed in OpenMPT at the same time as they were fixed in libmodplug. We don't do fuzz testing, but it's definitely harder to break libopenmpt's file loaders than libmodplug's as we moved away from the error-prone memory mapping which was responsible for many crashes in libmodplug. I'm not sure what "regressions in formats" refers to but if anything, libopenmpt plays files better than libmodplug (I have spent a lot of time on playback compatibility). I don't think cross-compiling should be an issue - I'm not aware what the usual pitfalls for that would be, but given that it already compiles on MSVC, gcc and clang on several platforms (not just x86), I don't think there would be any issues with cross-compiling?
libmodplug could be replaced simply by using libopenmpt's libmodplug emulation layer, but in the long run it would be nicer to directly talk to libopenmpt, especially since the new API offers e.g. access to sub tunes in module files.
-
Replying to [comment:3 sagamusix]:
I don't think cross-compiling should be an issue - I'm not aware what the usual pitfalls for that would be, but given that it already compiles on MSVC, gcc and clang on several platforms (not just x86), I don't think there would be any issues with cross-compiling?
This is usually not enough: notably most libraries have test and binary programs that tend to break in cross-compilation, notably when compiled as static libraries. Usually, we need to patch makefiles to only compile the part that actually works, and often hack around them, because there is no configure switch to not compile those parts.
I don't know if it's relevant, but libopenmpt can already be cross-compiled to JS using emscripten, and while this works in general, there was for example one test case that would fail for some mysterious reason on JS (something related floating point rounding, nothing too dramatic) so some precompiler switches have been added to simply not compile that part. If any more types of these issues would arise, we would of course be more than happy to add them to our own makefiles, if applicable.
By the way:
However, as usual, we should be careful about security issues (is this library fuzzed correctly? It's not like we had CVE with modplug...)
I have used afl to fuzz libopenmpt over the last few months, and while a few crashes came up, it can be safely said that none of them were critical to security, i.e. no heap overflows that could reach critical data or stack overflows.
FFmpeg has recently received a libopenmpt demuxer (https://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=d52dd768a3228081e85b15b93d92b3ccb1df278f) - like the old libmodplug demuxer it's optional and must be enabled with --enable-libopenmpt when compiling. Maybe this will simplify things regarding this issue, since you wouldn't have to write your own decoder?
As sagamusix wrote, libopenmpt is a much newer and more secure code base than libmodplug. libmodplug has low activity (since 2014, only two recent commits by well-known security researcher Hanno Böck, hinting at small memory safety issues), while libopenmpt is an active project which undergoes fuzzing rounds on a regular basis.
Though it's much better than libmikmod, libmodplug produces noticeable artifacts when rendering some module files, for instance Saga Musix's own "Suburban Streets", such as some notes not fading out properly.
Debian packages for libopenmpt and openmpt123 recently entered the sid and testing distros, so they will be in Stretch. The 6 packages don't seem to contain the libmodplug compat layer yet, though.
assigned to @chouquette
added Component::Build system: Contribs label and removed Component::Demuxers label
-
So a few remarks:
- the configure defaults to trying to find libdl to open 3rd party libs,
- the configure defaults to trying to find libltdl to open 3rd party libs,
- the configure cannot deal with absent libraries like pulseaudio or portaudio, everything has to be specified... Or even flac, sndfile and so many other...
For a library that cares about "security" this does not give a good impression.
No to add the WTF about having different tarballs depending on the buildsystem, that gives stuff like libopenmpt-0.2.7561-beta20.5-autotools tarballs...
Then, there is no way of disabling compilation of openmpt123 program, which of course, does not cross-compile correctly.
Finally, the modplug.pc|.h is not installed on make install, breaking the current build.
-
I am the libopenmpt maintainer.
Replying to [comment:15 jb]:
- the configure defaults to trying to find libdl to open 3rd party libs,
- the configure defaults to trying to find libltdl to open 3rd party libs,
libdl and/or libltdl are used to load the closed source unmo3 library from un4seen.com in order to load MO3 files that are unsupported by the internal libopenmpt MO3 loader (either because of missing features in our loader (currently highly unlikely bacause I think we implement all known features) or because vorbis or mp3 sample decoding is not available in libopenmpt). As VLC probably wants to link libopenmpt to libmpg123 and libvorbisfile directly, there is no real need to support the Un4seen unmo3 library. libdl and libltdl can thus be disabled for VLC.
- the configure cannot deal with absent libraries like pulseaudio or portaudio, everything has to be specified... Or even flac, sndfile and so many other...
libflac, libsndfile, and others are only required for the openmpt123 command line player and can thus also be just disabled for VLC.
For the VLC use case you only want the following libraries enabled: zlib, libmpg123, libogg, libvorbis, libvorbisfile.
configure failing with missing default dependencies is by design. See https://wiki.gentoo.org/wiki/Project:Quality_Assurance/Automagic_dependencies for the reasoning.
For a library that cares about "security" this does not give a good impression.
I fail to see what explicit dependencies instead of automagic dependencies has to do wth library security. Can you please explain your concerns?
No to add the WTF about having different tarballs depending on the buildsystem, that gives stuff like libopenmpt-0.2.7561-beta20.5-autotools tarballs...
This again is a conscious decision. The -windows archive containes all library dependencies in addition to libopenmpt itself, which makes it easier for MSVC users to get started. The -autotools lacks a few features compared to the plain tar.gz, mainly explicit build and test suite support for weird platforms like emscripten, as well as the possibility to replace libmpg123 with minimp3 and libvorbisfile with stb_vorbis which can be useful in some constrained situations but is not useful or desirable for Linux distributions or projects like VLC. In theory, we could merge the -autotools and the plain tar.gz, but frankly, I do not see any advantage in doing this when we have to provide a separate archive for Windows anyway. The tarball situation probably needs more detailed documentation on the libopenmpt side, I agree on that part.
Then, there is no way of disabling compilation of openmpt123 program, which of course, does not cross-compile correctly.
Valid point, I'll add a switch to disable building openmpt123 in the next version. I created a libopenmpt issue at https://bugs.openmpt.org/view.php?id=922 .
Finally, the modplug.pc|.h is not installed on make install, breaking the current build.
libmodplug emulation must be enabled explicitly via
./configure --enable-modplug(which will install the library as a drop-in replacement for libmodplug, including the library itself, the header and pkgconfig files) or via./configure --enable-libopenmpt_modplug(which will install libopenmpt_modplug.so which has to be used explicitly by client code and just implements the libmodplug ABI/API and can be installed in parallel to the original libmodplug on a system). VLC probably wants --enable-modplug here.In any case, the better long term solution would be to implement explicit support for the libopenmpt API instead of relying on the libopenmpt emulation of the libmodplug interface. I'll happily assist with any questions regarding the libopenmpt API when they arise (like I already did for libopenmpt support in ffmpeg). Although I'm a VLC user myself, I am not familiar with VLC code internals and thus do not feel qualified to comment on VLC-specific integration issues.
Also, it would be nice if you reported libopenmpt bugs (and build system problems) in the libopenmpt bug tracker.
Replying to [comment:12 sagamusix]:
The libmodplug emulation layer will not make it into Stretch, so for proper support on the next Debian distro, VLC would need its own libopenmpt-based decoder rather than a libmodplug-based decoder with compatibility layer.
The libmodplug actually made it intro Stretch and Debian Stretch VLC already uses libopenmpt via the libmodplug emulation (at the moment, Stretch is not yet released of course, thus things might change).
