mod-audio / mod-plugin-builder

MOD Plugin Builder
79 stars 50 forks source link

Add suloea-labs #152

Open paulfd opened 2 years ago

paulfd commented 2 years ago

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

falkTX commented 2 years ago

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 commented 2 years ago

A few more comments:

falkTX commented 2 years ago

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 commented 2 years ago

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

falkTX commented 1 month ago

@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?