-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Switch caps to submodule #4027
base: master
Are you sure you want to change the base?
Switch caps to submodule #4027
Conversation
dfc71fd
to
05ab33a
Compare
FYI, Travis-CI checks are slightly misleading. It's passing for LMMS, but failing for tresf. https://travis-ci.org/LMMS/lmms/builds/366253352. It's just the |
First of all thanks to @tresf for initial work on the effects. The changes are immense. Here is what I can easily do:
What I may not want to do:
|
@tresf The compatibility table tells us which effects are planned to map. What, however, about the following?
|
@JohannesLorenz we either drop them or do our best to map them. Sorry I realize re-reading the original description that this is not obvious. Ideally, we'd try to maintain some form of compatibility if possible. Feel free to edit as needed. |
And as of today caps latest release reads I believe Eq10x2 is to Eq2x2 what Eq10 is to Eq. So...
|
@tresf I think we have no other chance than finishing this PR 😃 I'll try to rebase this to master and try to get the code finished now. |
Marking as TODO: Master commit ea98ba4 needs to go into CAPS. |
@mrkline Can you please open a PR of your changes of ea98ba4 against moddevices/caps-lv2? |
sorry, accidentally closed |
🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩
Linux
Windows
macOS🤖{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://13638-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.120%2Bg49eed33-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/13638?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://13637-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.120%2Bg49eed332e-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/13637?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/29d85jm07u474gsq/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/38849977"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/wfqelsgs72la7smi/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/38849977"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://13639-15778896-gh.circle-artifacts.com/0/lmms-1.3.0-alpha.1.120%2Bg49eed332e-mac10.14.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/13639?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "da9e6b1401a974781af6a2e28b0e8db71fbe8aac"} |
Thanks. Note, at some point we'll need to merge unfinished and just handle the upgrade issues as "I'm sorry" in support/Discord. Thanks for putting some time to this. I'm sure it's obvious, but I'm no longer actively maintaining my branches and PRs so please do as you need (push, close, clone, etc). |
@mrkline I submitted the patch for you now. |
df565f8
to
04263d3
Compare
I propose to find out the knob mappings by testing (as it's too complicated to analyze from the code). This is now handled in a new sub issue #5680 . |
The current version is now 0.9.26 though the latest release if more of a nudge of 0.9.25 |
Since this PR contains lots of fixes, would it make sense to merge untested and fix issues as they come by. Or we can just fix using the demo projects as reference. If everything loads, merge. |
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.
Something minor i found at first glance.
ToneStackLT0.4 ToneStackLT is pretty much (se release note from caps 0.9.1 below) the same as 0.4 ToneStack with Model value:0 . Same controls. The values are hard coded here:
Same as the first element for ToneStack: lmms/plugins/LadspaEffect/caps/ToneStack.cc Lines 44 to 46 in 32fe3e5
ToneStack wasn't removed until after 0.9 was released. https://github.com/moddevices/caps-lv2/blob/master/CHANGES Finally the modpeople have something to add
So, apart from the similar data there may also be a different method used for this setting and it seem to have been baked into the 0.9.1 version of ToneStack . Apparently it's hard coded for 44.1 kHz. |
ToneStackLT was removed in 0.9.24. It's an emulation of the tonestack in the 1959 Bassman 5F6-A amp and corresponds to ToneStack model number 0 (defaul). They do differ however but on bass/mid/treble set to all low or all high a pattern upgraded from lmms-1.2 to 1.3 will be identical.
ToneStackLT upgrades to ToneStack:model 0 in f63ec84 |
Thanks so far @zonkmachine . Just for my understanding, why did you force-push the first commit? ("Turn CAPS into submodule (#4027)") |
I had rebased again on latest master. |
CI says: - /__w/lmms/lmms/plugins/LadspaEffect/caps/caps-lv2/dsp/v4f.h:40:26:
- error: SSE vector return without SSE enabled changes the ABI [-Werror=psabi] |
@tresf Can your LMMS fork, under the "caps" branch, be used for reliably testing the 0.4-0.9 difference? If so, I'll get to work testing the missing plugins. |
I would assume so? |
What I mean is: If I were to make two projects, one in master LMMS and one in your fork, such that they contain only one plugin on the master track, would the only difference be that plugin data? Has nothing else changed in the process? |
That said, I think what you're describing is correct. We haven't done any major plugin data restructuring as a result of this PR and any upgrade routines that this PR ads should only affect upgrades from older LMMS versions, not new projects, assuming no mistakes were made. 🍻 |
The word "plugin" is in reference to the specific CAPS plugin placed on the given mixer track, for any mention of the word, you can be sure I meant it as such. Regarding the answer, that sounds great, I'll download your fork and build it, and get to testing! |
ChorusIThe input port has been moved from the first to the second-to-last position. Reordering the ports fixes the issue. The relevant part of the diff is: PortInfo
ChorusI::port_info [] =
{
+ { "in", INPUT | AUDIO },
+
{ "t (ms)", CTRL_IN, {LOG | DEFAULT_MID, 2.5, 40} },
{ "width (ms)", CTRL_IN, {DEFAULT_LOW, .5, 10} },
@@ -109,7 +111,6 @@ ChorusI::port_info [] =
{ "feedforward", CTRL_IN, {DEFAULT_LOW, 0, 1} },
{ "feedback", CTRL_IN, {DEFAULT_LOW, 0, 1} },
- { "in", INPUT | AUDIO },
{ "out", OUTPUT | AUDIO }
}; I'll do more testing and confirm. @tresf What do we do about the missing ChorusII? Are we dropping it? |
@JohannesLorenz asked a similar question, quoting:
I just created a test project to check for audible differences between My best guess is that ChorusII just didn't sound good enough to the author of CAPS plugins to justify keeping around. That's not to say people didn't take advantage of it in projects and those projects won't sound right after this upgrade. In my opinion, Replacing a plugin with one that sounds different is likely to attract some strong opinions. I'm fine with either the decision to remove or replace. In both cases, the project won't sound right. |
I feel like replacing could be possible if we make our own fork, but shouldn't be done. My argument is mainly that the maintainer of it felt that it wasn't necessary, and there's little reason to doubt such a decision. Also, it alleviates our headache and makes 1.3 happen sooner. So I'm all for dropping it, the user is gonna get a message that the plugin has not been found anyway, and we'll list the dropped plugins in the 1.3 changelog. |
That also sounds nice, but I wouldn't keep it around for longer than 1.3.x. |
Just note that all upgrade routines live essentially forever. Unless there's a justification to remove a routine, there's not much cost to keep it around. |
Alright, I'll then move on to the other plugins, to wrap it up. |
There are also some other possibilities:
|
Now that we see that this isn't as easy as thought in 2017, maybe a re-vote would be in order? I think we should just move this to v2.0, since we can then break compat without any worry. |
The CAPS LADSPA plugins -- or more commonly known in LMMS as "
C*
" -- are authored by Tim Goetze and they have changed considerably over the years. The current version as of writing this is 0.9.26.No upstream VCS exists, but moddevices keeps a viable clone of CAPS that we can safely use as an LADSPA mirror (and eventually for LV2, when we add support).
Since this involves testing presets and projects, help is needed. To compare the mapping of a plugin, export a project as
.mmp
(without the z) and compare the ladspa sections.Compatibility
Supercedes #3979