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

Implement SPA support (enabling zyn-fusion) #4662

Open
wants to merge 36 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@JohannesLorenz
Copy link
Contributor

commented Oct 13, 2018

What's this?

This is an inofficial LMMS branch which provides zynaddsubfx 3.x support. There's an installer linked below (see "How to run") for installing this LMMS branch, coupled with a zynaddsubfx 3.0.3 plugin.

LMMS and zyn are connected by using a new, simple plugin API ("spa"). spa is like an abstraction of LADSPA, allowing ports to be not just operating on floats, but anything represented as a class, e.g. integers, booleans, or even ringbuffers. This makes it easy to add OSC ports.

What can it do?

What should work:

  • load and save files
  • preview XMZ files from 'My Presets' (left sidebar in LMMS)
  • drag-drop XMZ files from 'My Presets' over songs
  • drag-drop the zyn instrument on tracks from old zyn to convert them
  • drag-drop zyn widgets on automation patterns (you must
    keep the F1 key pressed, in contrast to LMMS, where it's the control
    key)
  • drag-drop zyn widgets (keep F1 pressed) on the controller rack's controllers
  • remove controller connections by dragging them on the controller rack in an area where no controller is (keep F1 pressed)
  • exporting songs, e.g. to WAV

What still needs to be done:

  • LMMS can crash if project loading time takes more than 10 seconds
  • reviews for
    • the spa concept in general
    • the LMMS implementation
    • the zyn implementation

How to run?

There are two ways to install from source, both require cloning from the lmms-zyn-fusion-test repo and following the README.

Currently, there are no binaries.

Bug reports

Please not in this PR, please submit at lmms-zyn-fusion-test repo instead.

Why another plugin API if we can have LV2?

There may be reasons for both APIs. From the spa FAQ.md:

  • Possible advantages of LV2 over SPA:
    • Currently a way larger user base, both plugins and hosts
    • Does not depend on C++11 (and less on the ABI)
    • LV2 is stable for years, SPA is still very experimental
  • Possible advantages of SPA over LV2
    • (Simple) C++ only, you don't need to learn anything else
    • Easy to implement hosts
    • Quick and easy to implement plugins (compare the zyn SPA and LV2 plugins:
      SPA are a few hundred LOC, LV2 comes with a whole folder plus the DPF lib)

What to do with this?

  • Don't merge! Reviews are required (also for the zyn plugin and the API
    itself) if it will ever be merged.
  • If it won't be merged, I'm really not sad:
    • It may not be an easy decision to
      adopt a plugin API that's only supported by a small number of plugins
    • There won't be a conflict to future LV2 implementations, but the savefiles for spa and LV2 will look
      different, requiring at least some kind of converter

Thanks to...

... everyone helping with this. Especially:

  • @fundamental for support with zyn, zest and general music advice
  • @tresf and @PhysSong for helping with merging some of the branche's features to main
  • @Bleuzen for alpha testing
Show resolved Hide resolved include/PluginFactory.h
Show resolved Hide resolved include/SpaOscModel.h Outdated
Show resolved Hide resolved include/StringPairDrag.h Outdated
Show resolved Hide resolved src/core/PluginFactory.cpp Outdated
Show resolved Hide resolved src/core/spa/SpaInstrument.cpp Outdated
@PhysSong

This comment has been minimized.

Copy link
Member

commented Oct 14, 2018

@JohannesLorenz I can't drop OSC models to existing automation patterns because it's not implemented yet. Are you going to implement it?

@PhysSong

This comment has been minimized.

Copy link
Member

commented Oct 14, 2018

I also think we should eventually integrate the build script into the LMMS repository prior to merging this, though it has less priority for now.

PhysSong and others added some commits Oct 19, 2018

Refactor getting Automatable Models to Engine
This is solely code moving, no functional changes.
The reason is explained by the following commit.
Allow dropping automation in more ways
This enables DnD from internal and spa knobs on

* Controller views (from the controller rack)
* Existing Automation Patterns
* Automation Editors
Allow drag-dropping onto Controller Rack
* Implement connecting automatable model to controller by
  dragging its widget onto a controller in the controller rack
* Implement saving of automatable models with controller
  connections when their names must be quoted
* Let Engine::getAutomatableModel() return existing osc connections, too
  (in order to overwrite them)
@JohannesLorenz

This comment has been minimized.

Copy link
Contributor Author

commented Nov 1, 2018

@JohannesLorenz I can't drop OSC models to existing automation patterns because it's not implemented yet. Are you going to implement it?

Yep, it's implemented now in 7e7044d. Also implemented: Drag-dropping on the controller rack (da9fcfc) to be able to connect to controllers.

JohannesLorenz added some commits Nov 1, 2018

Automized formatting changes only
No functional changes at all!
Manual formatting changes only
No functional changes at all!
The changes in Enginge.cpp are also just refactoring.
@JohannesLorenz

This comment has been minimized.

Copy link
Contributor Author

commented Nov 2, 2018

I also think we should eventually integrate the build script into the LMMS repository prior to merging this, though it has less priority for now.

The build script compiles zyn and mruby-zest, too, so I think we don't need the full script. It should be enough to change the travis/circle files, such that they

  • clone SPA
  • compile and install it
  • use PKG_CONFIG_PATH=... in front of the LMMS cmake call

For travis, I think it should suffice to add the changes to .travis/script.rb. Do the non-linux builds all support the PKG_CONFIG_PATH?
For circle, I must check how this will work.

@PhysSong

This comment has been minimized.

Copy link
Member

commented Nov 3, 2018

It should be enough to change the travis/circle files, such that they

  • clone SPA
  • compile and install it
  • use PKG_CONFIG_PATH=... in front of the LMMS cmake call

There's a CMake alternative of it, ExternalProject_Add.

@PhysSong

This comment has been minimized.

Copy link
Member

commented Nov 3, 2018

Some more issues I've found so far:

  • InstrumentTrackWindow::dropEvent needs some changes to fix compiler warnings and memory leaks, see PhysSong@e81f9b0
  • SPA fails to build on Mac and for Windows, due to linking errors on building examples(after making CMake accept any compilers)
  • SPA is a hard dependency of LMMS now due to unchecked #includes.

We should either bundle SPA or soften the dependency to fix the build.

JohannesLorenz added some commits Nov 3, 2018

Make all spa includes optional
* Add a -DWANT_SPA to control inclusion of SPA
* Remove compiler errors about missing assert
@JohannesLorenz

This comment has been minimized.

Copy link
Contributor Author

commented Nov 5, 2018

For your most recent comment:

  • I merged your patch in, thanks for that.
  • The hard dependencies should now all be removed in 2a4ad86.
  • I can't reproduce it, since I have no Windows or Mac. Can you please provide some logs showing what went wrong when building SPA (and maybe the mentioned CMake fixes)?
@JohannesLorenz

This comment has been minimized.

Copy link
Contributor Author

commented Nov 7, 2018

@PhysSong Concerning the CI, I'd rather not use ExternalProject_Add, since it means building LMMS twice (and building zyn and mruby zest unnecessarily). I'd suggest to compile all CI tests with SPA support, but first, I'd like to get your Windows issues solved.

@PhysSong

This comment has been minimized.

Copy link
Member

commented Nov 7, 2018

I'd rather not use ExternalProject_Add, since it means building LMMS twice

Can you elaborate it? I meant building SPA using it.

@JohannesLorenz

This comment has been minimized.

Copy link
Contributor Author

commented Nov 9, 2018

Can you elaborate it? I meant building SPA using it.

Oh, I think I got you completely wrong. That sounds reasonable, I'll try it out.

JohannesLorenz added some commits Nov 28, 2018

Abstrahize SpaInstrument into "SpaPluginBase" and "SpaControlBase"
! Don't read the code diff, it's just refactoring, no features or fixes. !

This takes the code from SpaInstrument and refactors those code parts out
that will be common for SpaInstrument and the new SpaEffect in the next commit.
As LMMS effects are split into plugin and controls, we abstrahize into two
classes.

The description should be enough to not need/want to read the code changes.
Turn SPA core plugins into sub plugins
* Turn the core-coded SPA plugin into an LMMS plugin with sub plugins for
the individual SPA effects. This removes *tons* of spa code from the core
* Like for LADSPA, a SPA manager is added.
* The list of AutomatableModels for OSC and the ability of a network port is
  abstrahized and put into Plugin (`Plugin::modelAtPort`, `Plugin::net_port`)
* Fix SPA nodeName in XML savefiles

Functional changes

* Use SPA's delete functions rather then deleting at the host

JohannesLorenz added some commits Dec 31, 2018

Use QString for SubPluginFeatures' virtuals
The former virtuals returned `const char*`, which lead to invalid reads when
`LadspaSubPluginFeatures` returned pointers to temporary `QByteArray::data`.
Set models when model changed
Before the commit, models were only set at construction.
Now, they are also changed after the model of the Plugin changed.
Add Control classes
Add labeled controls for different types with a common base class
Add linked model groups
Implement a container for multiple equal groups of linked models and
suiting views. Such groups are suited for representing mono effects where each
Model occurs twice. A group provides Models for one mono processor and is
visually represented with a group box.

This concept is common for LADSPA and Lv2, and useful for any mono effect.
Bugfix for SubPluginFeature's return types
Fix bugs after using QString for SubPluginFeature's virtuals. This had been
done fore LADSPA, but forgotten for SPA.
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.