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

support dynamic ports and reconsider ports in manifest #44

Open
atsushieno opened this issue Apr 14, 2020 · 6 comments
Open

support dynamic ports and reconsider ports in manifest #44

atsushieno opened this issue Apr 14, 2020 · 6 comments
Labels
API design enhancement New feature or request

Comments

@atsushieno
Copy link
Owner

atsushieno commented Apr 14, 2020

One of the kind of "unique" characteristics of LV2 and inherently AAP is that the plugin ports are exposed as part of manifest. By having them in the manifest, it is possible for hosts to retrieve parameters without instantiating the plugin. On the other hand, each plugin developer has to define parameters in both code and manifest.

It might be a good idea, but there are some use cases that a plugin cannot have a static lists of the ports. For example, a dynamic plugin wrapper doesn't statically know how many ports it will have when it is instantiated. Dexed exposes no parameter from the sound bank "Cart" until it is loaded. This was irrelevant.

LV2 has a (patchy) solution called Dynamic Manifest so that the plugin can dynamically expand the list of ports. AAP does not have that yet, but at this state I wonder if we should have such an "extension" too - this means, ports cannot be determined until a host instantiates the plugin anyways. If we simply remove ports from manifest, things become a lot simpler. JUCE does not expose parameters without instantiation either.

Another (elegant?) solution is to keep current ports in manifests but also have a flag for a plugin that indicates whether it might have additional dynamic ports or not.

Maybe as a solution, we can remove ports from manifest, and if static ports make sense then we provide static information on ports later, which will not become a breaking change.

@atsushieno atsushieno changed the title ports: should they be part of manifest? support dynamic ports and reconsider ports in manifest Apr 23, 2020
@atsushieno
Copy link
Owner Author

LV2 dynamic manifest supports supplying any data of any subject, which does not seem very obvious to me especially on plugin name etc. If they can change at runtime then DAW users will get confused if the names can be updated, or DAWs would look buggy as if they didn't show the (updated) plugin names. We should not accept such plugin information updates.

What should be allowed is just "adding" ports onto the statically defined ones. Other information should be provided via extensions.

@atsushieno
Copy link
Owner Author

There are couple of things that need design decision:

  1. Should we keep port information in aap_metadata.xml while it can be dynamically changed?
  2. How should a plugin developer return dynamic port information? Should we add port information retrieval functions in android-audio-plugin.h ?
  3. Should this dynamic port feature part of core? (or part of extensions like how LV2 deals with it?)

Thoughts:

Port information is essential to all plugins and hosts, and it is mandatory for hosts to determine if a plugin has dynamic ports or not (it will cause plugin run-time crash if it fails to correctly enumerate plugin ports). In that sense, on (3) it should be part of core framework.

And it will be more consistent if plugin developers return ports information within the plugin code.

Although, if we also require ports in aap_metadata.xml, then it is very possible that plugin developers write inconsistent code and data and that's rather due to our bad design because it is most likely an extra burden to normal plugin developers who have fixed number of ports.

An assumption here is that there is not a lot of plugin developers who need dynamic ports (we don't need it unless our plugin loads some modules that practically defines the instrument or effect characteristics). And for those who depend on this advanced feature, we can ask to be careful on keeping exposed ports in metadata and the actual ports in code consistent. In that sense, (3) it should be rather an extension.

An implication of having port retrieval functions in the core plugin API is that every single plugin developer has to implement it and it is kind of annoying, especially if we also ask developers to keep ports in aap_metadata.xml.

On the other hand, if we decided to remove ports from aap_metadata.xml, then (2) matters - it is going to be somewhat annoying that a plugin developer still has to return port information in code. Also, by having port retrieval functions in the core API, we kind of have to "freeze" port information set in the core API, otherwise the API cannot be stable. android-audio-plugin.h is in simple C API, and it is not intended to define complicated structure (especially because it will be also used in AIDL).

How LV2 resolved this issue in Dynamic Manifest extension is that the API makes use of FILE* that is passed from the host to the plugin so that the plugin writes dynamic manifest to the pointer. Therefore, a plugin developer has to first convert the ports information into TTL syntax and write to it, then host parses it back to ports information. It would seem quite annoying, but it actually makes sense by passing everything in simple-ish data (FILE*) because host should already have TTL parser so supporting this feature should not be difficult, and then the rest of the issue becomes just a matter of interpreting their standard RDF property values.

I find it simpler than defining complex structure. We should stick to simpler XML syntax though. And we should rather use simple char* for text buffer. We need AIDL-friendly cross-platform data.

atsushieno added a commit that referenced this issue Apr 25, 2020
NOTE: this is still regressive; audio processing fails even with mda-lv2 ports.

partial context: #44
atsushieno added a commit that referenced this issue Apr 25, 2020
@atsushieno atsushieno added the enhancement New feature or request label Feb 21, 2021
@atsushieno
Copy link
Owner Author

atsushieno commented Feb 22, 2021

This issue was about whether we describe ports in aap_metadata.xml, and we concluded it with "we keep doing so".

Here are what we need:

  • In aap_metadata.xml, we add an extension attribute dm:requires-dynamic-metadata (xmlns:dm="urn:org.androidaudioplugin.dynamic-metadata").
  • add dynamic-metadata.h as an extension, which defines get_dynamic_aap_metadata().
  • Since it is unclear when it should be called by host, add notification message (TBD: there is no notification framework yet) that "AAP metadata is updated" to the plugin API.

This also involves audio bus policy changes (which is being discussed and resolved at #69).

@atsushieno
Copy link
Owner Author

As an inportant part of this task, the hosting API (in both C++ and Kotlin) now provides instance based ports information (aap::PluginInstance and org.androidaudioplugin.hosting.AudioPluginInstance) instead of metadata based information (aap::PluginInformation and org.androidaudioplugin.PluginInformation).

@atsushieno
Copy link
Owner Author

Dynamic ports work is in progress for the next version (0.7.4). We already have a breaking AIDL changes (which is now marked as "V2") for some changes to support dynamic ports. Yet there are likely more breaking changes. Current problem is that we need changes in the plugin-api, or the planned port design changes have to change again.

atsushieno added a commit that referenced this issue Aug 11, 2022
…y index, not id."

This reverts commit c25defc.

context: #44 (comment)

The primary reason for the revert is that we need way more complicated
changes that would also complicate public plugin API - the plugin API
is based on array based audio/MIDI buffers just by port index, not by ID.
Dynamic mapping of the ports needs various rewrites, and even if we did that
we are not very clear on what we could bring in as the benefits.

The basic defective idea is mostly about negative ports that prevent
index-based access to buffers. They could be either appended to the last
port, or simply unbandled from user port buffers.
@atsushieno
Copy link
Owner Author

atsushieno commented Dec 6, 2022

There have been many changes in the premises.

In the latest "V2" protocol since v0.7.3 and v0.7.4, ports should be queried against aap::PluginInstance, not aap::PluginInformation. It is a foundation for dynamic ports.

Our actual port configuration is driven only by manifest, but port-config extension in the hopefully-not-so-far-future version would inject the port configuration process and give opportunity for plugins to dynamically configure it. It should be discussed at issue #69.

Another remaining task is to really support dynamic port changes (I continue it here so far, but it can be move to #69 when I find it more comfortable).

There can be two ways: (1) host asks plugin to change the port configuration, and (2) the plugin changes the expected ports (this will likely happen with plugin wrappers like DISTRHO/Ildaeil). (1) will just need port disconnection by host and notification to plugin for port changes, and (2) will need notification from plugin to host and host determines whether it disconnects or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API design enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant