A few fixes for linux VST


#1

Hi Jules,

I’ve been working a bit on the linux VST code, trying to get my plugin as robust as possible. Here is the list of changes that I have made:

in ~JuceVSTWrapper() destructor : it is very important to lock the messagemanager during shutdown so that it does not continue to pump out timer messages – or crashes do happen… Maybe adding a timer in the JuceDemoPlugin AudioProcessor would be a good idea as it is quite efficient to spot issues with the message thread !

Here are my changes to ~JuceVSTWrapper():

begin the function with

#if JUCE_LINUX
      { MessageManagerLock mmLock;
#endif

end it with (just releases the lock before deleting the message thread – deleting it while inside the lock would avoid any race condition, but how ?)

#if JUCE_LINUX
      }
      if (activePlugins.size() == 0) SharedMessageThread::deleteInstance(); 
#endif
      if (activePlugins.size() == 0) shutdownJuce_GUI();

Making window resize work: the current code does not work in any host. Despite the comment in the source, linux hosts should be trusted regarding the canHostDo(“sizeWindow”) – at least energyxt and renoise can be trusted. The code calling XResizeWindow on the host top-level window is really not host-friendly and should be removed (and it does not work). Here is my modified resizeHostWindow function:

    void resizeHostWindow (int newWidth, int newHeight)
    {
        if (editorComp != 0)
        {
          //YES linux hosts should be trusted, at least energyxt and renoise
#if 1 //! JUCE_LINUX // linux hosts shouldn't be trusted!
            if (! (canHostDo (const_cast <char*> ("sizeWindow")) && sizeWindow (newWidth, newHeight)))
#endif
            {
                // some hosts don't support the sizeWindow call, so do it manually..
#if JUCE_MAC
                setNativeHostWindowSize (hostWindow, editorComp, newWidth, newHeight);
#elif JUCE_LINUX
                // the code below is really not host friendly -- I replace it with 
                // editorComp->setSize(newWidth, newHeight); but I don't know a linux vst host
                // that does not support sizeWindow
                /*
                Window root;
                int x, y;
                unsigned int width, height, border, depth;

                XGetGeometry (display, hostWindow, &root,
                              &x, &y, &width, &height, &border, &depth);

                newWidth += (width + border) - editorComp->getWidth();
                newHeight += (height + border) - editorComp->getHeight();

                XResizeWindow (display, hostWindow, newWidth, newHeight);
                */
                editorComp->setSize(newWidth, newHeight);
#else
...etc..

childBoundsChanged must also be adjusted to react correctly to resizing:

void EditorCompWrapper::childBoundsChanged (Component* child)
{
    child->setTopLeftPosition (0, 0);

    const int cw = child->getWidth();
    const int ch = child->getHeight();

    wrapper->resizeHostWindow (cw, ch);
#if !JUCE_LINUX /* the setSize call on linux will cause renoise and energyxt to fail. Just issuing a XResizeWindow is sufficient, and works */
    setSize (cw, ch);
#else
    XResizeWindow (display, (Window)getWindowHandle(), cw, ch);
#endif

#if JUCE_MAC
    wrapper->resizeHostWindow (cw, ch);  // (doing this a second time seems to be necessary in tracktion)
#endif
}

I have also added a MessageManagerLock in pluginEntryPoint , just before the call to createPluginFilter. I have not encountered a crash there but it seems very possible that the SharedMessageThread would start pumping messages before the createPluginFilter had time to finish. So that seems quite healthy to protect it:

          // extra safety measure to make sure the msg manager will not start pumpimp message to an initialized plugin
          // I have not run into a crash because of that, but it seems to me quite possible
#if JUCE_LINUX
          MessageManagerLock mmLock;
#endif

            AudioProcessor* const filter = createPluginFilter();

            if (filter != 0)

Small fix: the function VSTPluginMain is not declared with the visibility attribute:

extern "C" __attribute__ ((visibility("default"))) AEffect* VSTPluginMain (audioMasterCallback audioMaster)

I have also made these changes in juce_linux_Messaging: add

 if (JUCEApplication::getInstance()) return; // ioerror handlers should not be enable when we are running as a plugin

at the beginning of installXErrorHandlers() and removeXErrorHandlers(), and installKeyboardBreakHandler() . Because they are global stuff, the plugin is not supposed to touch to that.

In doPlatformSpecificInitialisation() as well, all the init block for XInitThreads() should be wrapped in a “if (JUCEApplication::getInstance()) {}” for the same reasons. Not doing that caused crashes in Qt hosts.

On a side note, gcc outputs this annoying warning each time the juce header is included:
"juce_amalgamated.h:2889: warning: dereferencing type-punned pointer will break strict-aliasing rules"
it does not like the casts “castFrom32Bit” “castTo32Bit” etc (I’m not sure what their purpose is – the usual way to avoid these warnings is to use a union instead of casting pointers)


#2

Excellent stuff, thanks!

The castTo32Bit stuff doesn’t give me any warnings, but I don’t like it. I avoided a union because I wasn’t confident that the compiler would completely optimise it away - those methods are really just a way to coerce the compiler into treating e.g. a 32-bit float as a 32-bit int for the purposes of passing it to an atomic function that can only take an int. Maybe I’m being too pessimistic about the compiler’s ability - perhaps a union would work too…