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

Plugin audio ports #3

Merged
merged 9 commits into from
Jan 29, 2025
Merged

Plugin audio ports #3

merged 9 commits into from
Jan 29, 2025

Conversation

messmerd
Copy link
Owner

@messmerd messmerd commented Jan 29, 2025

Fixes some of the jank left over from the big refactor.

Changes

  • PluginAudioPort - New interface class that joins a pin connector together with a buffer interface.
    • The pin connector, as the base class, can now call the virtual bufferPropertiesChanged to directly update the buffer in the derived class. This coupling of two closely related components eliminates the need for janky signal/slot connections.
    • DefaultPluginAudioPort is the default implementation and it uses DefaultAudioPluginBuffer.
    • The plugin audio port design is very flexible in its capabilities while also being very easy to use by plugin implementations. For a custom audio port, simply implement CustomPluginAudioPort.
  • RemotePluginAudioPort is a custom plugin audio port implementation for plugins that use RemotePlugin (VSTs and Zyn). It has an "inactive" state where the buffer is unavailable (such as when Vestige has no VST loaded).
  • ConfigurableAudioPort is a custom plugin audio port implementation that builds on top of RemotePluginAudioPort to offer the choice between a remote buffer or a local buffer at runtime. This is used by ZynAddSubFX which uses a remote buffer when the GUI is open and a local buffer when it is closed. In the future, ConfigurableAudioPort should also be helpful in adding sandboxing support for plugins to run either in a separate process or directly in the LMMS process.
  • AudioDataKind - All supported sample types are encoded into this enum. Allows all plugin config information to be stored in AudioPluginConfig - no more SampleT type parameter that must accompany it.
    • Using the new AudioPluginConfig I was able to greatly simplify a lot of the template parameters throughout the pin connector PR
  • Removed the IdChangeInputCount and IdChangeOutputCount remote plugin messages, because it is simpler to always pass both the input and output counts.
  • Removed AudioDataType because it was clunky to use, bloated the PR, and without a strong typedef it did nothing that couldn't be accomplished with a comment.
  • Various small changes

Future directions

  • RemotePluginAudioPort is kind of strange because it partially implements an audio buffer for RemotePlugin rather than having its own RemotePluginAudioBuffer class which carries that responsibility. In the future a RemotePluginAudioBuffer class which implements PluginAudioBufferInterface using shared memory should be created. This would require a refactor of RemotePlugin, including turning it into a class template. The corresponding RemotePluginClient side could receive similar changes. I've explained this idea further in a comment in RemotePluginAudioPort.h.

Refactors how the pin connector and audio buffers are incorporated into
AudioPlugin, how buffer updates occur, and how buffer customization
works.

Also uses more class type NTTP to simplify more of the template
parameters.
Reasons:
- It's clunky to use
- It bloats the PR
- Without a strong typedef it does nothing that can't be accomplished
with a comment
@messmerd messmerd merged commit eadeabf into pin-connector Jan 29, 2025
11 of 20 checks passed
@messmerd messmerd mentioned this pull request Jan 29, 2025
Copy link

@JohannesLorenz JohannesLorenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, only 5 editorials/questions.

As usual with such PRs, I didn't check every single template parameter, but rather tried to understand the common sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants