openAVproductions / openAV-ArtyFX

A repository for the ArtyFX plugin bundle
GNU General Public License v2.0
80 stars 19 forks source link

Mark Atom port as Optional in Vihda, Panda, and Bitta #31

Open giuliomoro opened 5 years ago

giuliomoro commented 5 years ago

in the .cxx files, it looks like there is some code left over from using other plugins code as a template?

harryhaaren commented 5 years ago

Hi Giulio, Correct yes.

From a technical perspective it would be nice if all hosts supported Atom ports, and sent specific messages or Atoms in to achieve control port changes. Control ports are basically "leftover" from LADSPA, and handling them in TTL as well as code is more work than Atoms. Hence, I was hoping to do all control for ArtyFX with Atom at some point, so I kept the atom port implementation for many plugins (even if they didn't need it).

Is there a reason to remove them? If so, we can look at doing that - however we need to be sure to not break existing projects. I'm not sure if its worth changing the URI just for removing the Atom port..

Thanks for noting this, -Harry

giuliomoro commented 5 years ago

ok I see your point. I guess there is no need to remove them, I was just noting it because many hosts (including lv2apply and the one I am working on) don't support AtomPort and therefore couldn't load these plugins, but then in my case the fix was as simple as loading them without doing the atom port and they worked just fine, which made me look into the source code and I realized that the port was unused, so I thought I would let you know in case it was an oversight.

harryhaaren commented 5 years ago

Ah ok, well if its an issue that's stopping you / your-host from loading, perhaps we can mark it ConnectionOptional? Do you check/support that?

giuliomoro commented 5 years ago

Yes, we do check that: if it's optional, it is simply ignored. I guess it would make sense to mark it ConnectionOptional.

harryhaaren commented 5 years ago

Cool, I'll rename this issue to reflect that, and try to fix this at some point soon-ish. Marking a port as ConnectionOptional doesn't break the plugin's backward compatibility, so this is a good solution for the problem you're having. THere's a few code-changes required to ensure we don't actually reference the port itself in the code if it hasn't been connected by the host - but that's trivial.

Thanks again for reporting & discussion!

giuliomoro commented 5 years ago

thank you !

harryhaaren commented 3 years ago

HI @giuliomoro, are you sure the issue is with Atom ports? I'm investigating, and it seems that the URID Map feature is not supported in lv2apply? (I have lv2apply --version = lv2apply (lilv) 0.24.8).

On investigating further, yes this LV2 URID Map support is the issue. The Atom port (both with and without connectionOptional) works fine with lv2apply.

Fixing issue to run these plugins with lv2apply can be achieved by removing required-dependency on LV2 URID Map.

giuliomoro commented 3 years ago

hi there, unfortunately I don't have any memory of this issue right now so I am afraid I cannot be of much help here :(