LMMS / lmms

Cross-platform music production software
https://lmms.io
GNU General Public License v2.0
7.95k stars 991 forks source link

Add native VST support for Mac OS X (WIP) #3975

Open follower opened 6 years ago

follower commented 6 years ago

Background

I had originally assumed that the reason for LMMS using WINE for VST (non-)support on Mac was because there weren't sufficient native Mac VSTs to justify adding native support. After playing with Helio Workstation I discovered that there's plenty of native Mac VSTs so I decided to investigate adding support for them to LMMS.

Progress so far

I've reached a point[1] where I think I've made sufficient progress that I wanted to keep track of development here--meaning that if I don't get to finishing it hopefully someone else can build on it without having to re-learn all of my lessons. :) (I've discovered a lesson I just learned was already in an unmerged WIP PR from someone else. :) )

The two parts of progress so far are:

The final lesson I needed I eventually discovered had already been mentioned here: https://github.com/LMMS/lmms/pull/2393/commits/d7a63c5adda5d3878f033569453fed44e699e7e9#diff-0cfe54b7b7513b8854b14b3fb8411b78R115 (but, hey, at least I learned something along the way :) )

[[1] Context: I started writing up this issue maybe a couple of weeks ago and wanted to make it more "complete" before posting it but since that approach doesn't seem to be working I thought I'd just dump it in here now as is and then add to it as I get the opportunity.]

follower commented 6 years ago

Today's additional progress was what led me to decide to actually post what I've got so far...

Displaying a VST's GUI

I've managed to get multiple VST plugins to display their functional GUI within a standalone Qt application. What's particularly nice is that the approach I'm using doesn't require any Objective-C code to be written, we just need to pass around an opaque value retrieved via QWidget::winId().

I've got a more detailed write-up here but in summary:

// In a .ui file:
//
// <widget class="QWidget" name="vstTarget" native="true"/>

// In .cpp files:

auto wVstTarget = this->findChild<QWidget *>("vstTarget");

// ...

// Load the plugin (via QLibrary::resolve()) then...

        auto winId = wVstTarget->winId();

        _thePlugin->editOpen((void *) winId);

        wVstTarget->setFixedWidth(_thePlugin->getWidth());
        wVstTarget->setFixedHeight(_thePlugin->getHeight());

// Where `editOpen()` is roughly implemented as...

    void editOpen(void * m_window) {

        auto result = this->m_plugin->dispatcher(this->m_plugin, effEditOpen, 0, 0, m_window, 0);

        // From LMMS:
        struct ERect
        {
            short top;
            short left;
            short bottom;
            short right;
        } ;

        ERect * er;

        result = this->m_plugin->dispatcher(this->m_plugin, effEditGetRect, 0, 0, &er, 0);

        if ((er->left == 0) && (er->top == 0)) {
            this->_width = er->right - er->left;
            this->_height = er->bottom - er->top;
        }

        result = this->m_plugin->dispatcher(this->m_plugin, effEditTop, 0, 0, 0, 0);

    } 

Apparently on Mac OS X winId is (a pointer to) the type NSView.

The VSTs I tested with will quite happily allow you to interact with them, load presets, etc etc. Of course, I haven't tested anything sound-related yet but this seems like good progress :)

Here's a pretty picture:

mac-native-vst-gui-display-demo

Next steps

It would be good to get to a stage where there's some audio happening, so I'll probably keep hacking away at the VST plugin code base to try to get something happening with that. There's some Windows-specific thread/message-passing code in there so I'm not sure how straight-forward this stage will be.

Once I've managed to get some sound my plan is to look at re-implementing things in a less hacky manner in preparation for a PR (not for 1.2 maybe for 1.3 or 1.4). I think ideally the platform-specific code would be implemented as a (sub-)class or something but I suspect initially it'll be easiest to stick with the #ifdef approach of the existing code.

follower commented 6 years ago

Cross-references to related issues/PRs: https://github.com/LMMS/lmms/issues/1468#issuecomment-72732311, #698, #2393

Oh, also, I've been collecting a bunch of Mac related VST information here:

PhysSong commented 6 years ago

we just need to pass around an opaque value retrieved via QWidget::winId().

Looks similar to the way LMMS embeds VST editors on Windows. However, LMMS uses a bridge process to launch (Windows)VSTs. The process creates a window and LMMS either embeds it or lets the VST runs in a separate window. In other words, we don't use QWidget instance directly, for VST editor GUI.

PhysSong commented 6 years ago

And you may want to look into #3786.

Anonymouqs commented 6 years ago

Is anyone still working on this issue?

tresf commented 6 years ago

@Anonymouqs if you want to help you may consider reaching out to @follower directly, especially if you have the ability to help, http://rancidbacon.com/.

If you get further, here or Discord are great places to share your findings.

follower commented 6 years ago

@PhysSong Yeah, my intention was to use the exact same method the LMMS Windows VST support uses with the bridge to a separate process.

follower commented 6 years ago

@Anonymouqs Well, I haven't abandoned it entirely. :)

FWIW I got a bit discouraged with my LMMS development efforts due to how "unstable" the stable branch was. :) :/ (I apparently hit my "yak shaving limit", primarily due to the combination of the introduction of submodules and the change to an entirely new memory allocation system implementation that lead (potentially unnecessarily) to a new compiler version being required without warning...)

I'm still interested in enabling native Mac VST support but don't currently see myself getting back to it in the immediate future.

Feel free to work on it yourself if it's of interest--my suggestion for the next step would be to look at replacing the Windows-specific code in the bridge from LMMS to the external VST wrapper process.

tresf commented 6 years ago

@follower sorry for the stable troubles you've been having. Both of those changes have caused a large amount of grief but were done with the importance of stability and performance in mind. They made the build process less stable from a dependency perspective, but the effect on the binaries did not yield instability. :)

Anyway, we're always hanging out in #devtalk, #programming and #support waiting for the next question. https://lmms.io/chat

PickleBoySupreme commented 6 years ago

I was able to open up the windows version of LMMS on a mac using wine, however I did not actually run it, because I didn't want anything to break. This would probably let me run VeSTige. Not sure if this would help though...

tresf commented 6 years ago

This would probably let me run VeSTige.

Yes, it would!

Not sure if this would help though...

No, it wouldn't. šŸ˜•

follower commented 6 years ago

Dammit. :p

Okay, fine, don't get too excited but...

initial-poc-vst-mac-native-support-screenshot

Well, maybe get a little bit excited... :D

It's all hacked together on an old version and not 100% functional and has issues but...at some level it's also Mac native VST support. :)

follower commented 6 years ago

Intro

In the interests of "'imperfect & visible' is better than 'want to be perfect but it is not done'" what follows is a dump of the current changes (against 9fb74d9bffdc719cc8f060ad5e655635d0adfcf3). Yes, it'd be better as a branch but this was quicker. :)

Context

I know there's been a bunch of recent VST-related code changes but I've not yet looked closely to see how it'll affect this.

While not yet completely functional there's not actually a huge amount of code changes to get this working. A bit of CMake configuration though.

Possible implementation approaches

If some of the Windows/WINE-specific code was changed to use Qt functionality like I've done here there could be fewer #ifdefs around. I wondered about having classes rather than #ifdef but since the existing code uses the latter I've stuck with that for now.

Current state

The GUI isn't yet displayed and I've not split things into threads yet either. (This is the result of "Hack-Till-It-Works (then take a screenshot)" development methodology. :D ) There's also a heap of debugging code.

You also have to rename the Foo.vst/Contents/MacOS/Foo file to be Foo.dll. :p

[Edit: Also, it seems like there's a crash (freeing an unallocated pointer) when deleting a pattern section for a Vestige track--not sure if that's related to my hacking around or not...]

Issues with shared memory

Oh, also, I dunno if it's the same elsewhere but the shared memory use can cause problems during development if there's many crashes. These are useful commands for viewing & removing the stale shared memory:

ipcs -m -a
ipcrm

(Don't ask me how long it took me to discover that...)

The code diff...

Click here to show the diff ```diff diff --git a/CMakeLists.txt b/CMakeLists.txt index ee47d12..c9b9f56 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -73,7 +73,9 @@ OPTION(WANT_DEBUG_FPE "Debug floating point exceptions" OFF) IF(LMMS_BUILD_APPLE) SET(WANT_ALSA OFF) SET(WANT_PULSEAUDIO OFF) - SET(WANT_VST OFF) + #SET(WANT_VST OFF) + SET(WANT_VST ON) + SET(WANT_VST_NOWINE ON) SET(STATUS_ALSA "") SET(STATUS_PULSEAUDIO "") SET(STATUS_APPLEMIDI "OK") diff --git a/include/RemotePlugin.h b/include/RemotePlugin.h index 2d81148..7240a6e 100644 --- a/include/RemotePlugin.h +++ b/include/RemotePlugin.h @@ -25,6 +25,8 @@ #ifndef REMOTE_PLUGIN_H #define REMOTE_PLUGIN_H +#include + #include "export.h" #include "MidiEvent.h" #include "VstSyncData.h" @@ -150,6 +152,8 @@ public: m_data = (shmData *) m_shmObj.data(); #else + qDebug("sizeof shmData: %d\n", sizeof( shmData )); + while( ( m_shmID = shmget( ++m_shmKey, sizeof( shmData ), IPC_CREAT | IPC_EXCL | 0600 ) ) == -1 ) { @@ -1030,6 +1034,7 @@ RemotePluginBase::~RemotePluginBase() int RemotePluginBase::sendMessage( const message & _m ) { + // qDebug("sendMessage _m.id: %d", _m.id); #ifdef SYNC_WITH_SHM_FIFO m_out->lock(); m_out->writeInt( _m.id ); @@ -1218,6 +1223,7 @@ RemotePluginClient::RemotePluginClient( const char * socketPath ) : } else { // connect to shared memory segment + qDebug() << "key: " << key << "\n"; if( ( m_shmID = shmget( key, 0, 0 ) ) == -1 ) { perror( "RemotePluginClient::shmget" ); diff --git a/plugins/vestige/CMakeLists.txt b/plugins/vestige/CMakeLists.txt index 21803a9..0b8ac70 100644 --- a/plugins/vestige/CMakeLists.txt +++ b/plugins/vestige/CMakeLists.txt @@ -9,7 +9,18 @@ IF(LMMS_SUPPORT_VST) ELSE() SET(CMAKE_INSTALL_RPATH "${CMAKE_INSTALL_PREFIX}/${PLUGIN_DIR}") ENDIF() - BUILD_PLUGIN(vestige vestige.cpp vestige.h MOCFILES vestige.h EMBEDDED_RESOURCES "${CMAKE_CURRENT_SOURCE_DIR}/*.png") + + # via + IF(LMMS_BUILD_WIN32) + BUILD_PLUGIN(vestige vestige.cpp vestige.h MOCFILES vestige.h EMBEDDED_RESOURCES "${CMAKE_CURRENT_SOURCE_DIR}/*.png") + ELSE() + IF(LMMS_BUILD_APPLE) + # via + BUILD_PLUGIN(vestige vestige.cpp vestige.h MOCFILES vestige.h EMBEDDED_RESOURCES "${CMAKE_CURRENT_SOURCE_DIR}/*.png" LINK) + ELSE() + BUILD_PLUGIN(vestige vestige.cpp vestige.h MOCFILES vestige.h EMBEDDED_RESOURCES "${CMAKE_CURRENT_SOURCE_DIR}/*.png" LINK SHARED) + ENDIF() + ENDIF() TARGET_LINK_LIBRARIES(vestige -lvstbase) ADD_DEPENDENCIES(vestige vstbase) ENDIF(LMMS_SUPPORT_VST) diff --git a/plugins/vst_base/CMakeLists.txt b/plugins/vst_base/CMakeLists.txt index a3f919a..242b52c 100644 --- a/plugins/vst_base/CMakeLists.txt +++ b/plugins/vst_base/CMakeLists.txt @@ -2,6 +2,8 @@ IF(LMMS_SUPPORT_VST) INCLUDE(BuildPlugin) +set(CMAKE_AUTOMOC ON) + IF(LMMS_BUILD_WIN32) ADD_DEFINITIONS(-DPTW32_STATIC_LIB) ADD_EXECUTABLE(RemoteVstPlugin "${CMAKE_CURRENT_SOURCE_DIR}/RemoteVstPlugin.cpp") @@ -24,10 +26,100 @@ IF(LMMS_BUILD_WIN32) ENDIF(LMMS_BUILD_WIN64 AND NOT LMMS_BUILD_MSYS) ENDIF(LMMS_BUILD_WIN32) +IF(LMMS_BUILD_APPLE) + #ADD_DEFINITIONS(-DPTW32_STATIC_LIB) + ADD_EXECUTABLE(RemoteVstPlugin "${CMAKE_CURRENT_SOURCE_DIR}/RemoteVstPlugin.cpp") + + #IF(QT5) + TARGET_LINK_LIBRARIES(RemoteVstPlugin Qt5::Core) + #ELSE() + # TARGET_LINK_LIBRARIES(RemoteVstPlugin -lQtCore4) + #ENDIF() + #TARGET_LINK_LIBRARIES(RemoteVstPlugin -lpthread -lgdi32 -lws2_32) + TARGET_LINK_LIBRARIES(RemoteVstPlugin -lpthread) + SET_TARGET_PROPERTIES(RemoteVstPlugin PROPERTIES COMPILE_FLAGS "${COMPILE_FLAGS} -O0") + ADD_CUSTOM_COMMAND(TARGET RemoteVstPlugin POST_BUILD COMMAND "${STRIP}" "$") + INSTALL(TARGETS RemoteVstPlugin RUNTIME DESTINATION "${PLUGIN_DIR}") + + ## FIXME: 32-bit VST fails on MSYS + #IF(LMMS_BUILD_WIN64 AND NOT LMMS_BUILD_MSYS) + # # build 32 bit version of RemoteVstPlugin for Win64 so we can load + # # 32 bit VST plugins + # ADD_SUBDIRECTORY(Win64) + #ENDIF(LMMS_BUILD_WIN64 AND NOT LMMS_BUILD_MSYS) +ENDIF(LMMS_BUILD_APPLE) + + SET(REMOTE_VST_PLUGIN_FILEPATH "RemoteVstPlugin" CACHE STRING "Relative file path to RemoteVstPlugin") ADD_DEFINITIONS(-DREMOTE_VST_PLUGIN_FILEPATH="${REMOTE_VST_PLUGIN_FILEPATH}") -BUILD_PLUGIN(vstbase vst_base.cpp VstPlugin.cpp VstPlugin.h communication.h MOCFILES VstPlugin.h) + +# via +IF(LMMS_BUILD_WIN32) + BUILD_PLUGIN(vstbase vst_base.cpp VstPlugin.cpp VstPlugin.h communication.h MOCFILES VstPlugin.h) +ELSE() +IF(LMMS_BUILD_APPLE) + # via + # BUILD_PLUGIN(vstbase vst_base.cpp VstPlugin.cpp VstPlugin.h communication.h MOCFILES VstPlugin.h LINK) + # BUILD_PLUGIN(vstbase vst_base.cpp VstPlugin.cpp VstPlugin.h communication.h MOCFILES VstPlugin.h LINK SHARED) + + # via <../cmake/modules/BuildPlugin.cmake> + #QT5_WRAP_CPP(plugin_MOC_out ${PLUGIN_MOCFILES}) + #QT5_WRAP_UI(plugin_UIC_out ${PLUGIN_UICFILES}) + QT5_WRAP_CPP(vstbase_MOC_out VstPlugin.h) + + # via + + ADD_FILE_DEPENDENCIES(VstPlugin.cpp ${vstbase_MOC_out}) + + INCLUDE_DIRECTORIES(${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_BINARY_DIR} ${CMAKE_SOURCE_DIR}/include) + ADD_DEFINITIONS(-DPLUGIN_NAME="vstbase") + + #IF(LMMS_BUILD_APPLE) + LINK_DIRECTORIES(${CMAKE_BINARY_DIR}) + LINK_LIBRARIES(${QT_LIBRARIES}) + #ENDIF() + + # via + ADD_LIBRARY(vstbase SHARED VstPlugin.cpp VstPlugin.h) + ##ADD_LIBRARY(vstbase MODULE VstPlugin.cpp VstPlugin.h) + + TARGET_LINK_LIBRARIES(vstbase Qt5::Widgets Qt5::Xml) + #TARGET_LINK_LIBRARIES(vstbase lmms) + #TARGET_LINK_LIBRARIES(vstbase -llmms) + + INSTALL(TARGETS vstbase DESTINATION "${PLUGIN_DIR}") + + #SET_TARGET_PROPERTIES(vstbase PROPERTIES LINK_FLAGS "-bundle_loader ${CMAKE_BINARY_DIR}/lmms") + #ADD_DEPENDENCIES(vstbase lmms) + + SET_TARGET_PROPERTIES(vstbase PROPERTIES LINK_FLAGS "-undefined dynamic_lookup") + + #ADD_DEPENDENCIES(vstbase lmms) +ELSE() + BUILD_PLUGIN(vstbase vst_base.cpp VstPlugin.cpp VstPlugin.h communication.h MOCFILES VstPlugin.h LINK SHARED) +ENDIF() +ENDIF() + + +IF(LMMS_BUILD_APPLExxxx) + +ADD_CUSTOM_COMMAND( + SOURCE "${CMAKE_CURRENT_SOURCE_DIR}/RemoteVstPlugin.cpp" + COMMAND ${CXX} + ARGS -I${CMAKE_BINARY_DIR} + -I${CMAKE_SOURCE_DIR}/include + ${CMAKE_CURRENT_SOURCE_DIR}/RemoteVstPlugin.cpp + -ansi -lpthread ${EXTRA_FLAGS} -fno-omit-frame-pointer + ${WINE_BUILD_FLAGS} + -o ../RemoteVstPlugin + # Ensure correct file extension + COMMAND sh -c "mv ../RemoteVstPlugin.exe ../RemoteVstPlugin || true" + TARGET vstbase + OUTPUTS ../RemoteVstPlugin +) + +ENDIF() IF(LMMS_BUILD_LINUX AND NOT WANT_VST_NOWINE) diff --git a/plugins/vst_base/RemoteVstPlugin.cpp b/plugins/vst_base/RemoteVstPlugin.cpp index 09f8569..abb3ae1 100644 --- a/plugins/vst_base/RemoteVstPlugin.cpp +++ b/plugins/vst_base/RemoteVstPlugin.cpp @@ -27,6 +27,8 @@ * */ +#include +#include #include "lmmsconfig.h" @@ -61,7 +63,9 @@ #endif #define USE_WS_PREFIX +#ifndef LMMS_BUILD_APPLE #include +#endif #if defined(LMMS_BUILD_WIN32) || defined(LMMS_BUILD_WIN64) #include "basename.c" @@ -118,8 +122,9 @@ class RemoteVstPlugin; RemoteVstPlugin * __plugin = NULL; +#ifndef LMMS_BUILD_APPLE HWND __MessageHwnd = NULL; - +#endif class RemoteVstPlugin : public RemotePluginClient @@ -296,12 +301,15 @@ public: void idle(); void processUIThreadMessages(); +#ifndef LMMS_BUILD_APPLE static DWORD WINAPI processingThread( LPVOID _param ); static bool setupMessageWindow(); static DWORD WINAPI guiEventLoop(); static LRESULT CALLBACK messageWndProc( HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam ); - +#else + int processingThread(); +#endif private: enum GuiThreadMessages @@ -347,10 +355,18 @@ private: std::string m_shortName; +#ifndef LMMS_BUILD_APPLE HINSTANCE m_libInst; +#else + void * m_libInst; +#endif AEffect * m_plugin; +#ifndef LMMS_BUILD_APPLE HWND m_window; +#else + void * m_window; +#endif intptr_t m_windowID; int m_windowWidth; int m_windowHeight; @@ -519,7 +535,9 @@ RemoteVstPlugin::~RemoteVstPlugin() if( m_libInst != NULL ) { +#ifndef LMMS_BUILD_APPLE FreeLibrary( m_libInst ); +#endif m_libInst = NULL; } @@ -535,6 +553,11 @@ RemoteVstPlugin::~RemoteVstPlugin() bool RemoteVstPlugin::processMessage( const message & _m ) { + + if ((_m.id != 0) && (_m.id != 6 /* Don't forget this in future. :D */)) { + qDebug() << "id: " << _m.id << "\n"; + } + switch( _m.id ) { case IdVstLoadPlugin: @@ -636,6 +659,8 @@ bool RemoteVstPlugin::processMessage( const message & _m ) void RemoteVstPlugin::init( const std::string & _plugin_file ) { + qDebug("init: %s\n", _plugin_file.c_str()); + if( load( _plugin_file ) == false ) { sendMessage( IdVstFailedLoadingPlugin ); @@ -659,14 +684,20 @@ void RemoteVstPlugin::init( const std::string & _plugin_file ) pluginDispatch( effMainsChanged, 0, 1 ); +#ifndef LMMS_BUILD_APPLE debugMessage( "creating editor\n" ); initEditor(); debugMessage( "editor successfully created\n" ); +#else + debugMessage( "skipping editor creation\n" ); +#endif // now post some information about our plugin sendMessage( message( IdVstPluginWindowID ).addInt( m_windowID ) ); + debugMessage( "post-window-id\n" ); + sendMessage( message( IdVstPluginEditorGeometry ). addInt( m_windowWidth ). addInt( m_windowHeight ) ); @@ -682,6 +713,8 @@ void RemoteVstPlugin::init( const std::string & _plugin_file ) sendMessage( IdInitDone ); + debugMessage( "post-IdInitDone\n" ); + m_initialized = true; } @@ -696,6 +729,7 @@ void RemoteVstPlugin::initEditor() } +#ifndef LMMS_BUILD_APPLE HMODULE hInst = GetModuleHandle( NULL ); if( hInst == NULL ) { @@ -720,7 +754,9 @@ void RemoteVstPlugin::initEditor() { return; } +#endif +#ifndef LMMS_BUILD_APPLE #ifdef LMMS_BUILD_LINUX //m_window = CreateWindowEx( 0, "LVSL", m_shortName.c_str(), // ( WS_OVERLAPPEDWINDOW | WS_THICKFRAME ) & ~WS_MAXIMIZEBOX, @@ -741,6 +777,7 @@ void RemoteVstPlugin::initEditor() debugMessage( "initEditor(): cannot create editor window\n" ); return; } +#endif pluginDispatch( effEditOpen, 0, 0, m_window ); @@ -751,13 +788,17 @@ void RemoteVstPlugin::initEditor() m_windowWidth = er->right - er->left; m_windowHeight = er->bottom - er->top; +#ifndef LMMS_BUILD_APPLE SetWindowPos( m_window, 0, 0, 0, m_windowWidth + 8, m_windowHeight + 26, SWP_NOACTIVATE | SWP_NOMOVE | SWP_NOZORDER ); +#endif pluginDispatch( effEditTop ); +#ifndef LMMS_BUILD_APPLE ShowWindow( m_window, SW_SHOWNORMAL ); UpdateWindow( m_window ); +#endif #ifdef LMMS_BUILD_LINUX m_windowID = (intptr_t) GetProp( m_window, "__wine_x11_whole_window" ); @@ -769,6 +810,7 @@ void RemoteVstPlugin::initEditor() bool RemoteVstPlugin::load( const std::string & _plugin_file ) { +#ifndef LMMS_BUILD_APPLE if( ( m_libInst = LoadLibrary( _plugin_file.c_str() ) ) == NULL ) { // give VstPlugin class a chance to start 32 bit version of RemoteVstPlugin @@ -782,9 +824,14 @@ bool RemoteVstPlugin::load( const std::string & _plugin_file ) char * tmp = strdup( _plugin_file.c_str() ); m_shortName = basename( tmp ); free( tmp ); +#else + // TODO: Handle this in a less hacky way? +#define __stdcall +#endif typedef AEffect * ( __stdcall * mainEntryPointer ) ( audioMasterCallback ); +#ifndef LMMS_BUILD_APPLE mainEntryPointer mainEntry = (mainEntryPointer) GetProcAddress( m_libInst, "VSTPluginMain" ); if( mainEntry == NULL ) @@ -797,6 +844,11 @@ bool RemoteVstPlugin::load( const std::string & _plugin_file ) mainEntry = (mainEntryPointer) GetProcAddress( m_libInst, "main" ); } +#else + // TODO: Also look for other variations of the main entry name? + mainEntryPointer mainEntry = (mainEntryPointer) QLibrary::resolve(_plugin_file.c_str(), "VSTPluginMain"); +#endif + if( mainEntry == NULL ) { debugMessage( "could not find entry point\n" ); @@ -1061,6 +1113,7 @@ void RemoteVstPlugin::saveChunkToFile( const std::string & _file ) const int len = pluginDispatch( 23, 0, 0, &chunk ); if( len > 0 ) { +#ifndef LMMS_BUILD_APPLE int fd = open( _file.c_str(), O_WRONLY | O_BINARY ); if ( ::write( fd, chunk, len ) != len ) { @@ -1068,6 +1121,7 @@ void RemoteVstPlugin::saveChunkToFile( const std::string & _file ) "Error saving chunk to file.\n" ); } close( fd ); +#endif } } } @@ -1387,6 +1441,7 @@ void RemoteVstPlugin::loadChunkFromFile( const std::string & _file, int _len ) { char * chunk = new char[_len]; +#ifndef LMMS_BUILD_APPLE const int fd = open( _file.c_str(), O_RDONLY | O_BINARY ); if ( ::read( fd, chunk, _len ) != _len ) { @@ -1395,6 +1450,7 @@ void RemoteVstPlugin::loadChunkFromFile( const std::string & _file, int _len ) close( fd ); pluginDispatch( effSetChunk, 0, _len, chunk ); +#endif delete[] chunk; } @@ -1489,7 +1545,9 @@ intptr_t RemoteVstPlugin::hostCallback( AEffect * _effect, int32_t _opcode, SHOW_CALLBACK ("amc: audioMasterIdle\n" ); // call application idle routine (this will // call effEditIdle for all open editors too) +#ifndef LMMS_BUILD_APPLE PostMessage( __MessageHwnd, WM_USER, GiveIdle, 0 ); +#endif return 0; case audioMasterPinConnected: @@ -1673,10 +1731,12 @@ intptr_t RemoteVstPlugin::hostCallback( AEffect * _effect, int32_t _opcode, } __plugin->m_windowWidth = _index; __plugin->m_windowHeight = _value; +#ifndef LMMS_BUILD_APPLE SetWindowPos( __plugin->m_window, 0, 0, 0, _index + 8, _value + 26, SWP_NOACTIVATE | SWP_NOMOVE | SWP_NOOWNERZORDER | SWP_NOZORDER ); +#endif __plugin->sendMessage( message( IdVstPluginEditorGeometry ). addInt( __plugin->m_windowWidth ). @@ -1790,7 +1850,9 @@ intptr_t RemoteVstPlugin::hostCallback( AEffect * _effect, int32_t _opcode, case audioMasterUpdateDisplay: SHOW_CALLBACK( "amc: audioMasterUpdateDisplay\n" ); // something has changed, update 'multi-fx' display +#ifndef LMMS_BUILD_APPLE PostMessage( __MessageHwnd, WM_USER, GiveIdle, 0 ); +#endif return 0; #if kVstVersion > 2 @@ -1860,9 +1922,16 @@ void RemoteVstPlugin::processUIThreadMessages() +#ifndef LMMS_BUILD_APPLE DWORD WINAPI RemoteVstPlugin::processingThread( LPVOID _param ) { RemoteVstPlugin * _this = static_cast( _param ); +#else +int RemoteVstPlugin::processingThread() { +// TODO: Handle this better? +#define _this this +// RemoteVstPlugin * _this = &this; +#endif RemotePluginClient::message m; while( ( m = _this->receiveMessage() ).id != IdQuit ) @@ -1878,15 +1947,33 @@ DWORD WINAPI RemoteVstPlugin::processingThread( LPVOID _param ) } else { +#ifndef LMMS_BUILD_APPLE PostMessage( __MessageHwnd, WM_USER, ProcessPluginMessage, (LPARAM) new message( m ) ); +#else + // TODO: !!! + qDebug("processingThread m.id: %d (TODO: Handle this.)", m.id); + + // Based on `messageWndProc()`: + // TODO: Actually have in guievent loop? + _this->queueMessage( m ); + if( !_this->isProcessing() ) + { + _this->processUIThreadMessages(); + } + +#endif } } +#ifndef LMMS_BUILD_APPLE // notify GUI thread about shutdown PostMessage( __MessageHwnd, WM_USER, ClosePlugin, 0 ); +#else + // TODO: !!!? +#endif return 0; } @@ -1894,6 +1981,7 @@ DWORD WINAPI RemoteVstPlugin::processingThread( LPVOID _param ) +#ifndef LMMS_BUILD_APPLE bool RemoteVstPlugin::setupMessageWindow() { HMODULE hInst = GetModuleHandle( NULL ); @@ -1972,12 +2060,16 @@ LRESULT CALLBACK RemoteVstPlugin::messageWndProc( HWND hwnd, UINT uMsg, } return DefWindowProc( hwnd, uMsg, wParam, lParam ); } - +#endif int main( int _argc, char * * _argv ) { + + qDebug() << "_argc: " << _argc << "\n"; + + #ifdef SYNC_WITH_SHM_FIFO if( _argc < 3 ) #else @@ -1988,6 +2080,9 @@ int main( int _argc, char * * _argv ) return -1; } + qDebug() << "_argv[1]: " << _argv[1] << "\n"; + + #ifdef LMMS_BUILD_WIN32 #ifndef __WINPTHREADS_VERSION // (non-portable) initialization of statically linked pthread library @@ -2023,6 +2118,8 @@ int main( int _argc, char * * _argv ) if( __plugin->isInitialized() ) { + qDebug("post-__plugin->isInitialized()"); +#ifndef LMMS_BUILD_APPLE if( RemoteVstPlugin::setupMessageWindow() == false ) { return -1; @@ -2035,11 +2132,17 @@ int main( int _argc, char * * _argv ) return -1; } RemoteVstPlugin::guiEventLoop(); +#else + __plugin->processingThread(); // TODO: Create thread? + // TODO: Handle guiEventLoop? +#endif } + qDebug("pre-delete"); delete __plugin; + qDebug("post-delete"); #ifdef LMMS_BUILD_WIN32 #ifndef __WINPTHREADS_VERSION diff --git a/src/core/RemotePlugin.cpp b/src/core/RemotePlugin.cpp index df6a0d9..e6190ed 100644 --- a/src/core/RemotePlugin.cpp +++ b/src/core/RemotePlugin.cpp @@ -28,6 +28,9 @@ #include #endif +#include + + #include "BufferManager.h" #include "RemotePlugin.h" #include "Mixer.h" @@ -98,8 +101,15 @@ RemotePlugin::RemotePlugin() : m_socketFile = QDir::tempPath() + QDir::separator() + QUuid::createUuid().toString(); - const char * path = m_socketFile.toUtf8().constData(); + auto path_utf8 = m_socketFile.toUtf8(); + const char * path = path_utf8.constData(); + // qDebug() << "m_socketFile path: " << path << "\n"; + qDebug() << "m_socketFile: " << m_socketFile.toUtf8() << "\n"; + qDebug("m_socketFile path: %s", path); size_t length = strlen( path ); + qDebug("m_socketFile path length: %zu", length); + qDebug("m_socketFile path length: %zu", strlen(path)); + qDebug("m_socketFile path 1st character: %c", path[0]); if ( length >= sizeof sa.sun_path ) { length = sizeof sa.sun_path - 1; @@ -108,12 +118,19 @@ RemotePlugin::RemotePlugin() : memcpy( sa.sun_path, path, length ); sa.sun_path[length] = '\0'; + qDebug("sa.sun_path: %s", sa.sun_path); + + qDebug() << "PF_LOCAL: " << PF_LOCAL << "\n"; + m_server = socket( PF_LOCAL, SOCK_STREAM, 0 ); + + qDebug() << "m_server: " << m_server << "\n"; + if ( m_server == -1 ) { qWarning( "Unable to start the server." ); } - remove( path ); + //remove( path ); int ret = bind( m_server, (struct sockaddr *) &sa, sizeof sa ); if ( ret == -1 || listen( m_server, 1 ) == -1 ) { @@ -209,6 +226,12 @@ bool RemotePlugin::init( const QString &pluginExecutable, #else args << m_socketFile; #endif + + qDebug() << "exec: " << exec << "\n"; + qDebug() << "args: " << args << "\n"; + + qDebug() << "exists: " << QFile(m_socketFile).exists() << "\n"; + #ifndef DEBUG_REMOTE_PLUGIN m_process.setProcessChannelMode( QProcess::ForwardedChannels ); m_process.setWorkingDirectory( QCoreApplication::applicationDirPath() ); @@ -411,6 +434,8 @@ void RemotePlugin::resizeSharedProcessingMemory() #endif } + qDebug("size: %zu\n", s); + static int shm_key = 0; #ifdef USE_QT_SHMEM do @@ -421,11 +446,20 @@ void RemotePlugin::resizeSharedProcessingMemory() m_shm = (float *) m_shmObj.data(); #else + //qDebug("sizeof shmData: %d\n", sizeof( shmData )); while( ( m_shmID = shmget( ++shm_key, s, IPC_CREAT | IPC_EXCL | 0600 ) ) == -1 ) { + //qDebug("errno: %d", errno); + perror("bad shmget"); + qDebug("(%d): ", shm_key); + if (shm_key > 48) { + abort(); + } } + qDebug("post-shmget\n"); + m_shm = (float *) shmat( m_shmID, 0, 0 ); #endif m_shmSize = s; ```
GoAm commented 6 years ago

Looking forward to (at some point) see a native VST implementation for Mac OS running in LMMS.

follower commented 6 years ago

@tresf I was wondering if you/lmms have a preference in terms of integrating the changes required to make VST cross-platform? The #ifdef approach "works" but feels pretty inelegant.

I wonder if having specific sub-classes implementing the platform-specific parts might work better in terms of keeping related code together while still sharing as much as possible. But anything other than the #ifdef approach seems like it would require modifying existing working code on other platforms.

Seems like the potential target support matrix looks something like:

A potentially related factor is that on Mac VST plugins are "Bundles" which are directories containing a bunch of files rather than, e.g. a single .dll as on Windows. This would mean that the project files wouldn't be entirely cross-platform compatible. I'm not sure if this would really be an issue though.

tresf commented 6 years ago

@tresf I was wondering if you/lmms have a preference in terms of integrating the changes required to make VST cross-platform? The #ifdef approach "works" but feels pretty inelegant.

Agreed. I've felt pretty strongly that the existing #ifdef approach is difficult to read but having touched very little of the code, I don't feel qualified to make a call on this. Instead I'd defer this to @DomClark, @lukas-w and @PhysSong. Dom's been doing the most of our VST fixes lately whereas Lukas and Phys have been making the X11 stuff work properly with Qt5. Sorry to defer this one, but I think they're better judges with this codebase.

Also, somewhat related... https://lists.linuxaudio.org/archives/linux-audio-dev/2018-June/037131.html. The aeffectx.h is under some (possibly unwarranted) scrutiny right now. Not directly on-topic, but I wanted to mention it.

A potentially related factor is that on Mac VST plugins are "Bundles" which are directories containing a bunch of files rather than, e.g. a single .dll as on Windows. This would mean that the project files wouldn't be entirely cross-platform compatible. I'm not sure if this would really be an issue though.

This is an excellent point. I'm not sure what you mean by "cross-platform"... but to infer by experience... I think what I'm reading is that for /Library/Audio/Plug-Ins/VST/Synth1.component (a MacOS "package" containing a bunch of other files), Windows and Linux would (hypothetically) use C:\Program Files\Steinberg\VstPlugins\Synth1.dll. I've never seen the remote process logic in C++ open a bundle (instead of a binary) so I can't speak to the technicalities, but we already obfuscate the plugin extensions in the Plugin and LADSPA areas of LMMS (e.g. .dll versus .so) so that's a good place to start. I know on the Mac desktop, it generally doesn't care if you call open LMMS.app vs. open LMMS.app/Contents/MacOS/lmms but how C++ handles this is not something I'm familiar with.

The way we handle VSTs cross-platform today (despite possible hard-coding of file extensions) is that we use the same antiquated LMMS WORKING DIRECTORY concept, and it works quite well. If you can honor that, we'll be in great shape.

DomClark commented 6 years ago

I don't feel I've been with the project long enough yet to prescribe a solution here, but here's my opinion, along with some details on where I'd like to take VST support in future versions:

One potential issue with this approach is handling 32-bit native VSTs on 64-bit Mac and Linux. I'm not sure how much of an issue this will be however - even if we don't tackle this we will already have expanded VST support a lot, and VST3, should we get that far (which I would like to), is 64-bit only if I recall correctly.

Regarding loading VST bundles on Mac, I'm not sure what kind of support Qt offers in this area (QLibrary, etc.), but should we choose to do it directly ourselves, a quick Google search brought up this documentation from Apple. At the bottom, under "Loading Plug-in Code With Bundles", there are indications towards techniques for loading functions from bundles at run-time.

Once the stable-1.2 VST support is reasonably bug free, I'd like to move on to implementing in-process VST support, and have it work for native VSTs across Windows, Linux and Mac. I don't, however, own a Mac, nor have any experience with them, so someone familiar in that area would be invaluable in fully achieving this.

I realise this basically summarises to "I think it should be done differently to how you've done it", and I don't at all want your work to go to waste, so I will wait and let others chime in with ideas. However, should we go down the path I suggested, it would be great if you could try and get Windows VSTs going under Wine, and later on help with native in-process VSTs.

lukas-w commented 6 years ago

Sorry for the late response.

I agree with most points made here. The current #ifdef approach is really hard to read and to maintain, efforts such as #2390 are needed to improve this.

Native VSTs will likely be more stable and certainly easier to implement than the remote approach. However, I think we should at least delay its implementation. I'm not sure if that's what you're suggesting @DomClark so I wanted to point this out. The remote implementation will always be there because we need it for Windows VSTs through Wine on Linux or macOS. I believe we should spend our efforts on simplifying it and reducing its complexity. Wine VSTs will likely be a very common use case for a long time to come, so it will continue to receive bug fixes and stability improvements. Adding a in-process implementation is not going to make those problems go away. If we're lucky, we manage to make the remote-process implementation so good that an in-process one offers too little benefit to be worth the added complexity. If well done, a remote-process solution can be more stable then the traditional implementation because it isolates the host from the guest (LMMS doesn't crash if the plugin crashes).

So I think we should implement native VSTs using the remote-process solution we already have ā€“ at least for now ā€“ because it makes for less code complexity, and thus less code paths to test and maintain.

follower commented 5 years ago

As an FYI, I have pushed a couple of branches to my fork against a semi-recent LMMS master:

Obligatory screenshot:

screenshot-vst-with-gui-wip-01

tresf commented 5 years ago

if you dig into the .vst bundle to select the actual executable

Can we do some .app detection logic to fix that? Although this may be hard to do with the file section dialog, right? Because it's a folder?

e.g. ...

    QString p = m_plugin;

        if(p.endsWith(".app")) {
            // traverse, set p to the executable
        }
        if( QFileInfo( p ).dir().isRelative() )
        {
            p = ConfigManager::inst()->vstDir()  + p;
        }
follower commented 5 years ago

if you dig into the .vst bundle to select the actual executable

Can we do some .app detection logic to fix that?

Thanks for the code suggestion.

I've not responded on this aspect specifically before now (in part because it's somewhat a "solved problem" technically but has some "platform integration" aspects that are susceptible to bike-shedding :D so I've left it unaddressed) but I do have some notes related to how to approach this here: http://www.labradoc.com/i/follower/p/notes-lmms#20180502.

Essentially, my impression is that the "platform native" approach is to auto-scan the standard platform VST directory locations (e.g. /Library/Audio/Plug-Ins/VST/ & ~/Library/Audio/Plug-Ins/VST/) and then provide a list of the available VSTs by name with no explicit reference to the bundle file path. So, strictly speaking, I suspect providing a file dialog is non-native behavior.

But I suspect for initial ease of implementation I'll probably just adapt the existing file dialog code and use one of the approaches referenced in the link above.

follower commented 5 years ago

Quick update: I've just hacked up the threads-side of things so that the VST GUIs are fully interactive & update/respond to clicks properly and it seems mostly straight-forward--still need to handle program exit cleanly though.

tresf commented 5 years ago

Can we do some .app detection logic to fix that?

I do have some notes related to how to approach this here: http://www.labradoc.com/i/follower/p/notes-lmms#20180502.

QFileDialog::Directory combined with QFileDialog::ShowDirsOnly however it appears setNameFilter(...) strips out the directories. Fortunately we do roll out our own FileDialog.cpp so there's some customization options.

"platform native" approach is to auto-scan the standard platform VST directory locations (e.g. /Library/Audio/Plug-Ins/VST/ & ~/Library/Audio/Plug-Ins/VST/) and then provide a list of the available VSTs by name with no explicit reference to the bundle file path. So, strictly speaking, I suspect providing a file dialog is non-native behavior.

Agreed. This is how @falkTX does it in Carla, which we'll be offering support for in stable-1.2.

follower commented 5 years ago

@tresf Thanks for the pointer about the approach Carla uses.

What's the likely minimum level of VST-selection functionality that would be accepted into master? (Even if Mac-native VST support is not enabled by default I'd like to avoid bit-rot by getting a MVP into master sooner rather than later.)

Latest update

I've just pushed the changes that make the plugin GUIs properly interactive: e7a29357a677602f397b64f65066acf159a27ca7

It also includes some code that is meant to avoid occurrences of this error on file close/application exit:

QProcess: Destroyed while process ("/<path>/lmms/build/plugins/RemoteVstPlugin") is still running.
Process error:  QProcess::ProcessError(Crashed)
Remote plugin crashed

And while it helps reduce the number of occurrences it doesn't completely eliminate them so either I'm not handling plugin shutdown/exit requests properly or it's slower to do so than expected on other platforms so the calling code needs to be modified.

It's still hacky rather than production code but should at least be buildable & work for someone else.

Use of cross-platform Qt code vs platform-native code

It seems like some duplication/extra code could be prevented if Windows and/or Linux+WINE code used the cross-platform Qt code where appropriate (e.g. library loading & threading?) but I don't know whether the current non-Qt approach is required in order for the WINE side of things to work correctly (can anyone comment on that?).

tresf commented 5 years ago

I don't know whether the current non-Qt approach is required in order for the WINE side of things to work correctly (can anyone comment on that?).

@follower The best explanation I've seen is here: https://github.com/LMMS/lmms/pull/2304#issuecomment-136302025

follower commented 5 years ago

For (my) future reference, it may be worth investigating if any of these Launch Services Keys are useful in this situation:

PhysSong commented 5 years ago

I don't know whether the current non-Qt approach is required in order for the WINE side of things to work correctly

If we use Qt with wine, some unusual dependencies are required, like 32bit Qt on 64bit Linux. BTW, it'd be nice to add an abstraction layer and separate platform-specific part from RemoteVstPlugin. It will make the code cleaner and easier to read.

follower commented 5 years ago

For possible future reference here's another FLOSS project's cross-platform VST-related implementation:

follower commented 5 years ago

BTW I've been using my VST support off & on since September-ish with success.

Main issues I've encountered have been reliability related:

It's been really nice being able to use VSTs but I think dealing with some of the above reliability issues will take a non-trivial amount of work--in addition to any work required to refactor the cross-platform implementation to be "nicer".

FWIW if we were able to add support piece-by-piece it'd probably be easier to prioritize making some PRs.

tresf commented 5 years ago

@follower we're fine with WIP code because it increases the likelihood of others getting in and fixing stuff.

Need to refer to executable within the .vst bundle. (So, not native behaviour.)

IIRC, A .vst is mirrored after an .app bundle, so we can infer the executable path, right? My guess, we'd just need to allow draggable folders when they end in .vst. This should be doable. Ideally we'd just execute the .vst folder directly (Mac allows this -- for example -- with the open command), but I'm not sure how we'd handle IPC. Perhaps MacOS has a proprietary technique for this we can leverage? Launch Services may also have it registered too.

Sometimes processes and/or shared memory segments get left behind after an "abnormal exit" which can in theory prevent VSTs from loading on application start due to exhausting available resources (i.e. shared memory).

This isn't my wheelhouse, but if @DomClark is interested, the project can obtain a Mac Mini for him for testing. This offer extends to anyone capable BTW.

PhysSong commented 5 years ago

@follower Are you still on this? Also, could you create a PR from the WIP branch so we can review and/or make suggestions?

follower commented 5 years ago

Short answer is sort-of (i.e. I haven't abandoned it) but also I've found that the Carla plugin in 1.2 makes VSTs usable on Mac (although not perfect) in the interim.

spiderworm commented 4 years ago

@follower I don't think Carla is supported on Mac https://lmms.io/wiki/index.php?title=Carla

Any updates on this? Would love even rudimentary support in OSX.

PhysSong commented 4 years ago

@spiderworm The wiki article is outdated. Carla is supported on Mac since 1.2.0-RC7.

spiderworm commented 4 years ago

@PhysSong I have the official release v 1.2.1 installed, but I don't see Carla anywhere in the UI. I don't see Carla Rack and Carla Patchbay in the Instruments panel, like the wiki says (which I get you already mentioned is out of date).

Are there any instructions anywhere for using LMMS Carla?

PhysSong commented 4 years ago

You need to install the Carla app separately. You can get it from from https://github.com/falkTX/Carla/releases.

tresf commented 4 years ago

Are there any instructions anywhere for using LMMS Carla?

Not currently, but if LMMS and Carla are both installed in /Applications/ LMMS will find it. It's the best we have right now.

iburunat commented 2 years ago

I'm checking on the state of VST support for Mac. Is this something implemented at this point? Thanks.

tresf commented 2 years ago

I'm checking on the state of VST support for Mac. Is this something implemented at this point? Thanks.

No.

luzpaz commented 1 year ago

Ticket is missing macos label