audacious-media-player / audacious

A lightweight and versatile audio player
https://audacious-media-player.org
Other
842 stars 112 forks source link

Set OpenMPT priority above ModPlug #987

Closed Audacious-Bot closed 5 months ago

Audacious-Bot commented 5 months ago

Author Name: Jonathan Rubenstein Original Redmine Issue: https://redmine.audacious-media-player.org/issues/848 Original Date: 2018-12-02


I posted this before as note #303-6, however that bug is quite old (on top of being rejected) and I probably did not get noticed, so I am making a new report as a feature request.

Libmodplug is Audacious' library of choice for its official module playing input plugin, it enables playback of many old formats such as xm, it, mod, and others. Unfortunately, libmodplug appears to be only sporadically maintained, the most recent commit listed on sourceforge as being "9357867":https://sourceforge.net/p/modplug-xmms/git/ci/9357867a5cf10e601499146013270dee95acc9bd/log/, dated June 2nd, 2017, and on GitHub, "af00e21":https://github.com/Konstanty/libmodplug/commits/af00e21737839465d0dc253cf356d2b9c7554eed, dated August 2nd, 2018. As listed on #303, it does have some playback issues with some modules, one of which I've attached to this report. Generally, it also plays modules back with a low amplitude, however that could be the result of Audacious' plugin's implementation.

In #303-2, libxmp was discussed, however the mentioned audacious plugin (#303-5) is no longer maintained, and libxmp itself appears to be sporadically maintained as well. The latest commit, "2a16cdf":https://github.com/cmatsuoka/libxmp/commits/2a16cdf69e4cd1d96d51a68b65ce2304203bdc4f, is dated Oct 14th, 2018.

So, I suggest switching to "libopenmpt":https://lib.openmpt.org/libopenmpt, a cross-platform module playback library based off of "OpenMPT":https://openmpt.org, which is itself based off the open source release of ModPlug Tracker. It's actively maintained, (latest revision is "rev 10998":https://source.openmpt.org/browse/openmpt/trunk/OpenMPT/libopenmpt/?op=log&rev=10998&peg=10998 dated November 25th, 2018) and should have better support for playing back modules accurately. The types of modules it supports should be equivalent or greater than libmodplug, a list of supported modules can be found on the "features page":https://openmpt.org/features. For example, libopenmpt handles the problematic module attached to this report flawlessly. Chris Spiegel has created a working audacious plugin for this already, however his repo does not currently feature a code license. https://github.com/cspiegel/audacious-openmpt You might be able to get into a dialog with him about absorbing it into the audacious-plugins repo. His email listed in the git commit log is cspiegel at gmail.

I've used his plugin for a while and it works great in my experience, and the amplitude seems to be correct, so the modules don't sound so quiet compared to my other music files.

Audacious-Bot commented 5 months ago

Original Redmine Comment Author Name: Michael Schwendt Original Date: 2018-12-30T12:14:56Z


My advice: Don't "switch", but introduce a separate input plugin for a new MOD backend lib.

The rationale is simple. Hardly any MOD player lib is perfect. There are many thousands of MOD files. (Side-note: when I use the term "MOD" I don't refer to only the ancient .mod files from the late 80's and early 90's, but also the many derivatives that have been published later.) Typically, authors of new MOD playing libs start with a new set of test files, such as their favorite ones and new problematic ones pointed out in bug reports. A lib would need to be really popular for more users to give it a try and test it with old and new files in their own mod file collections.

Audacious runs fine with multiple MOD input plugins installed. Users may choose to disable individual plugins. No harm is done, if keeping alternative input plugins for some time.

About XMP and its plugin for Audacious: The plugin has been ported to Audacious 3 a long time ago and continues to work fine. I'm not aware of any immediate maintenance requirements. Libxmp is in a fine state and continues to receive fixes for problematic files when they come up. I don't think it supports the .dsm format of your test file. In modplug 0.8.9, some of the samples sound slightly out of tune here.

Audacious-Bot commented 5 months ago

Original Redmine Comment Author Name: Michael Schwendt Original Date: 2018-12-30T22:59:43Z


Interestingly, the "problematic module" attached to this ticket crashes openmpt123.

Going through a dir of testmods, I've found a few that don't play correctly (probably because Soundtracker detection isn't easy and some of the original mods are available as versions converted to Protracker) and one file that isn't recognized. Had files a modplug bug about it in 2011: https://sourceforge.net/p/modplug-xmms/bugs/13/

necros_-_spatial_distortion.dsm

$ openmpt123 necros_-_spatial_distortion.dsm 
openmpt123 v0.4.0, libopenmpt 0.4.0+r11103.pkg (OpenMPT 1.28.01.00 https://source.openmpt.org/svn/openmpt/tags/libopenmpt-0.4.0@11103 (2018-12-23T13:06:52.605038Z) clean-pkg)
Copyright (c) 2013-2018 OpenMPT developers <https://lib.openmpt.org/>

/usr/include/c++/8/bits/stl_vector.h:932: std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::operator[](std::vector<_Tp, _Alloc>::size_type) [with _Tp = OpenMPT::ModCommand; _Alloc = std::allocator<OpenMPT::ModCommand>; std::vector<_Tp, _Alloc>::reference = OpenMPT::ModCommand&; std::vector<_Tp, _Alloc>::size_type = long unsigned int]: Assertion '__builtin_expect(__n < this->size(), true)' failed.
Aborted (core dumped)
Audacious-Bot commented 5 months ago

Original Redmine Comment Author Name: Jonathan Rubenstein Original Date: 2018-12-31T04:23:28Z


Michael Schwendt wrote:

Interestingly, the "problematic module" attached to this ticket crashes openmpt123.

Going through a dir of testmods, I've found a few that don't play correctly (probably because Soundtracker detection isn't easy and some of the original mods are available as versions converted to Protracker) and one file that isn't recognized. Had files a modplug bug about it in 2011: https://sourceforge.net/p/modplug-xmms/bugs/13/

necros_-_spatial_distortion.dsm

[...]

The problematic file I submitted doesn't crash openmpt123 for me, even using the same version

openmpt123 v0.4.0, libopenmpt 0.4.0+r11103.pkg (OpenMPT 1.28.01.00 https://source.openmpt.org/svn/openmpt/tags/libopenmpt-0.4.0@11103 (2018-12-23T13:06:52.605038Z) clean-pkg)
Copyright (c) 2013-2018 OpenMPT developers <https://lib.openmpt.org/>

I've tested on a freshly installed and up to date Debian Testing amd64 install in a VM, with only the 'standard' (no graphics) packages and openmpt123. I've attached the result of a @dpkg-query -l@ on the VM. It works on the host too, but I made the VM just to be sure it wasn't my specific configuration that resolves the crash.

I think a new lib alongside libmodplug would work fine as well, as Michael suggests.

I believe libopenmpt is a better pick. But regardless, deprecating libmodplug, whether by replacing it completely, or offering an additional plugin on the side, would help with module playback immensely regardless of which one is chosen, since they are still actively maintained.

Audacious-Bot commented 5 months ago

Original Redmine Comment Author Name: Michael Schwendt Original Date: 2018-12-31T12:12:51Z


Debian Testing is somewhat behind Fedora because of Red Hat's involvement with GCC.

$ rpm -q gcc-c++ libstdc++
gcc-c++-8.2.1-6.fc29.x86_64
libstdc++-8.2.1-6.fc29.x86_64

It's the only file that crashes openmpt123 so far while parsing the pattern data.

Audacious-Bot commented 5 months ago

Original Redmine Comment Author Name: John Lindgren Original Date: 2018-12-31T21:52:14Z


Chris Spiegel has created a working audacious plugin for this already

I will wait for a request from Chris himself before "absorbing" his plugin into audacious-plugins. There is nothing wrong with him maintaining it out-of-tree if he prefers to do so (and it is probably less work for him that way).

Audacious-Bot commented 5 months ago

Original Redmine Comment Author Name: Jörn Heusipp Original Date: 2019-01-17T20:12:49Z


(I am the libopenmpt maintainer)

necros_-_spatial_distortion.dsm Interestingly, the "problematic module" attached to this ticket crashes openmpt123.

This is a libopenmpt bug, I added a ticket to our bug tracker at https://bugs.openmpt.org/view.php?id=1191 . We will release a fix ASAP.

Audacious-Bot commented 5 months ago

Original Redmine Comment Author Name: Jörn Heusipp Original Date: 2019-01-25T08:08:29Z


necros_-_spatial_distortion.dsm Interestingly, the "problematic module" attached to this ticket crashes openmpt123.

This is a libopenmpt bug, I added a ticket to our bug tracker at https://bugs.openmpt.org/view.php?id=1191 . We will release a fix ASAP.

Fixed in libopenmpt 0.4.2: https://lib.openmpt.org/libopenmpt/2019/01/22/security-updates-0.4.2-0.3.15-0.2.11253-beta37-0.2.7561-beta20.5-p13-0.2.7386-beta20.3-p16/

Audacious-Bot commented 5 months ago

Original Redmine Comment Author Name: Chris Spiegel Original Date: 2019-01-31T01:11:49Z


John Lindgren wrote:

Chris Spiegel has created a working audacious plugin for this already

I will wait for a request from Chris himself before "absorbing" his plugin into audacious-plugins. There is nothing wrong with him maintaining it out-of-tree if he prefers to do so (and it is probably less work for him that way).

Hi, I'm the author of this plugin. If you'd like it integrated with audacious-plugins, I'd be happy to move it there. It's 2-clause BSD licensed (I've just added an explicit LICENSE file to clarify this) so should be no issues there. If you're interested, let me know what I can do to help get it moved over.

Audacious-Bot commented 5 months ago

Original Redmine Comment Author Name: Chris Spiegel Original Date: 2019-02-04T05:04:19Z


Chris Spiegel wrote:

John Lindgren wrote:

Chris Spiegel has created a working audacious plugin for this already

I will wait for a request from Chris himself before "absorbing" his plugin into audacious-plugins. There is nothing wrong with him maintaining it out-of-tree if he prefers to do so (and it is probably less work for him that way).

Hi, I'm the author of this plugin. If you'd like it integrated with audacious-plugins, I'd be happy to move it there. It's 2-clause BSD licensed (I've just added an explicit LICENSE file to clarify this) so should be no issues there. If you're interested, let me know what I can do to help get it moved over.

I decided to just take a look and see the level of effort needed to incorporate the plugin into audacious-plugins, and it was very easy. So I've now got a preliminary pull request at https://github.com/audacious-media-player/audacious-plugins/pull/68.

As noted in the pull request, I didn't replace the libmodplug plugin, instead just making it lower priority, so that anybody without access to libopenmpt can still have a tracker plugin.

Audacious-Bot commented 5 months ago

Original Redmine Comment Author Name: John Lindgren Original Date: 2019-02-24T13:53:34Z


The OpenMPT plugin is now merged but has lower priority than ModPlug for now. As discussed on GitHub, that's to give people a chance to test it out in the next release before dropping it on everyone as the default. I'm leaving this ticket open as a reminder to swap the priorities for the release after next.

Audacious-Bot commented 5 months ago

Original Redmine Comment Author Name: John Lindgren Original Date: 2020-12-05T17:38:51Z


OpenMPT is now the default in Git master.