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

Add suloea-labs #152

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add suloea-labs #152

wants to merge 1 commit into from

Conversation

paulfd
Copy link

@paulfd paulfd commented Jun 18, 2022

It's a LV2 wrapper around the Aeolus engine, trimmed down to a single division and with the builtin reverb.

@falkTX
Copy link
Member

falkTX commented Jun 21, 2022

Hi there, thanks for the pull request! (and the nice little plugin too)

2 things to do here

  1. SULOEA_LABS_POST_INSTALL_TARGET_TTLFILES define is not needed, and unused in your mk file. please remove it
  2. there is a mismatch new/free usage within the plugin. seems started by suloea.cpp:204, by using C++ new operator and freeing that memory with C-style free instead of delete.
==3651137== Mismatched free() / delete / delete []
==3651137==    at 0x4E05288: free (vg_replace_malloc.c:538)
==3651137==    by 0x4AD869: CarlaBackend::CarlaPluginLV2::~CarlaPluginLV2() (CarlaPluginLV2.cpp:742)
==3651137==    by 0x4ADF43: CarlaBackend::CarlaPluginLV2::~CarlaPluginLV2() (CarlaPluginLV2.cpp:846)
==3651137==    by 0x4CBD6F: std::_Sp_counted_ptr<CarlaBackend::CarlaPluginLV2*, (__gnu_cxx::_Lock_policy)2>::_M_dispose() (shared_ptr_base.h:376)
==3651137==    by 0x41A637: std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() (shared_ptr_base.h:154)
==3651137==    by 0x419DE2: std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() (shared_ptr_base.h:684)
==3651137==    by 0x41991D: std::__shared_ptr<CarlaBackend::CarlaPlugin, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() (shared_ptr_base.h:1123)
==3651137==    by 0x419939: std::shared_ptr<CarlaBackend::CarlaPlugin>::~shared_ptr() (shared_ptr.h:93)
==3651137==    by 0x41BED3: void __gnu_cxx::new_allocator<std::shared_ptr<CarlaBackend::CarlaPlugin> >::destroy<std::shared_ptr<CarlaBackend::CarlaPlugin> >(std::shared_ptr<CarlaBackend::CarlaPlugin>*) (new_allocator.h:140)
==3651137==    by 0x41B3F8: void std::allocator_traits<std::allocator<std::shared_ptr<CarlaBackend::CarlaPlugin> > >::destroy<std::shared_ptr<CarlaBackend::CarlaPlugin> >(std::allocator<std::shared_ptr<CarlaBackend::CarlaPlugin> >&, std::shared_ptr<CarlaBackend::CarlaPlugin>*) (alloc_traits.h:487)
==3651137==    by 0x43849A: std::vector<std::shared_ptr<CarlaBackend::CarlaPlugin>, std::allocator<std::shared_ptr<CarlaBackend::CarlaPlugin> > >::_M_erase(__gnu_cxx::__normal_iterator<std::shared_ptr<CarlaBackend::CarlaPlugin>*, std::vector<std::shared_ptr<CarlaBackend::CarlaPlugin>, std::allocator<std::shared_ptr<CarlaBackend::CarlaPlugin> > > >) (vector.tcc:159)
==3651137==    by 0x438237: std::vector<std::shared_ptr<CarlaBackend::CarlaPlugin>, std::allocator<std::shared_ptr<CarlaBackend::CarlaPlugin> > >::erase(__gnu_cxx::__normal_iterator<std::shared_ptr<CarlaBackend::CarlaPlugin> const*, std::vector<std::shared_ptr<CarlaBackend::CarlaPlugin>, std::allocator<std::shared_ptr<CarlaBackend::CarlaPlugin> > > >) (stl_vector.h:1180)
==3651137==  Address 0x557f510 is 0 bytes inside a block of size 2,712 alloc'd
==3651137==    at 0x4E045E4: operator new(unsigned long) (vg_replace_malloc.c:342)
==3651137==    by 0x7002C66: SuloeaPlugin::instantiate(double, char const*, _LV2_Feature const* const*) (suloea.cpp:204)
==3651137==    by 0x7003810: instantiate(_LV2_Descriptor const*, double, char const*, _LV2_Feature const* const*) (suloea.cpp:606)
==3651137==    by 0x4C4272: CarlaBackend::CarlaPluginLV2::init(std::shared_ptr<CarlaBackend::CarlaPlugin>, char const*, char const*, unsigned int, char const*&) (CarlaPluginLV2.cpp:6764)
==3651137==    by 0x4A687D: CarlaBackend::CarlaPlugin::newLV2(CarlaBackend::CarlaPlugin::Initializer const&) (CarlaPluginLV2.cpp:8238)
==3651137==    by 0x40D193: CarlaBackend::CarlaEngine::addPlugin(CarlaBackend::BinaryType, CarlaBackend::PluginType, char const*, char const*, char const*, long, void const*, unsigned int) (CarlaEngine.cpp:693)
==3651137==    by 0x4DEDE0: carla_add_plugin (CarlaStandalone.cpp:1247)
==3651137==    by 0x408D40: main (CarlaBridgePlugin.cpp:675)
==3651137== 

@falkTX
Copy link
Member

falkTX commented Jun 21, 2022

A few more comments:

  • plugin does not respond to all-notes-off, which can lead to some stuck notes
  • it should have at least 1 preset, could be "default"
  • takes a while to load, because "retuning" is done on start. but any subsequent change pushes such changes to worker thread. this is a little inconsistent, and if loading a PB with some tweaked parameters makes the plugin take 2x what it should before it produces sound. a suggestion is to make plugin load right away and use worker thread on 1st run.
  • retuning LED is a bit too simplistic, I suggest to make it blink/animate (can be done with pure CSS classes)
  • related to previous point, would be great to have it use https://github.com/moddevices/mod-lv2-extensions/blob/main/mod-hmi.lv2/mod-hmi.h to let user know when retuning is taking place (assuming a suloea parameter is addressed to the device/HMI). see https://github.com/DISTRHO/DIE-Plugins/blob/main/plugins/a-fluidsynth.lv2/a-fluidsynth.cc for inspiration, look around MOD_EXTENDED bits

@falkTX
Copy link
Member

falkTX commented Jun 21, 2022

That said, we can already push the plugin to beta if you wish, so it gets wider testing.
Just let us know if that is something you want.

@paulfd
Copy link
Author

paulfd commented Jun 26, 2022

I'll look into it, thanks for the comments! No need to publish it until it's in shape, no worries.

@falkTX
Copy link
Member

falkTX commented Sep 27, 2024

@paulfd I would like to move with this plugin, do you have time to have a look at the points I mentioned before? If not, would a patch/PR be acceptable?

@falkTX falkTX force-pushed the master branch 2 times, most recently from 93ebd76 to e0b798c Compare October 26, 2024 09:55
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.

2 participants