-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Effect modules #6232
base: master
Are you sure you want to change the base?
Effect modules #6232
Conversation
cfd5e9e
to
1a1d5eb
Compare
I understand that modules allow making some functionality shippable independently on the rest of the application. The only real use cases I know of would be closed-source modules or modules that are just too heavy to be shipped with the normal executable. |
Audacity must depend on libraries for them to work. This makes little sense for features like effects or importers/exporters. They usually register themselves using a well known Registry. They don't need to have any public symbols and Audacity doesn't depend on them. However, if Audacity doesn't have a symbol that it imports directly from the library - it will not load it. Modules are loaded at runtime and do not have this limitation. They clearly state, that they extend Audacity functionality, but not the set of public APIs |
Factoring functionality into modules shrinks the executable and its link dependencies on third party libraries. It enforces non cyclic dependencies as the program and module evolve. It could allow an advanced user to find the modules preference page and disable modules not needed for their work, reducing the program’s memory footprint and startup time. It was already decided to put each import/export format in its own module. Why not do the same for each effect protocol? This is work I did in 2020-2021 still waiting for merge. |
I can make sense of that, thanks for clarifying. |
It is not true that it modularizes the "framework" of built-in effects. This PR only moves all the definitions of particular effect subclasses. I don't think it is too much. What ends up remaining in the executable -- that is isolated as the difficult stuff still needing its rewrite for toolkit neutrality. |
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.
LGTM, but as you might guess I don't have a strong opinion here :D
Thanks @Paul-Licameli for your contribution!
static auto item = std::shared_ptr{ | ||
// Delayed evaluation | ||
// Stereo to Mono is an oddball command that is also subject to control | ||
// by the plug-in manager, as if an effect. Decide whether to show or |
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.
subject to control by the plug-in manager
Is it still the case? It doesn't show up when I open the plugin-manager window
I would like @crsib ’s concurring opinion of this PR first |
... Delete pluginregistry.cfg before testing this
... Delete pluginregistry.cfg before testing this
1a1d5eb
to
fdf53f8
Compare
Resolves: (direct link to the issue)
Depends on
Take user interface for the various third-party effect protocols, Vamp, and Nyquist out of the executable.
Take out the built-in effects and analyzers (but not the non-modal dialogs in the Analyze menu).
Audacity links fewer third-party libraries.
The graph of modules will be thus, with one new cluster, and addition to the Nyquist cluster:
(short description of the changes and the motivation to make the changes)
Recommended: