Fix vestige by removing linux shm and semaphores and use Qt ones. Fixes #1875

[Narfinger]

As mentioned in the bug thread, there is still a problem.
Compiling on a Linux x64 without qt4 32bit version installed will fail with a linker error. Nothing I tried really worked to fix this so I just added a big warning in the main cmake file.

Perhaps somebody else who knows cmake better can take a stab at it.

[tresf]

@Narfinger we use travis to meet our Ubuntu, Win32 and Mac dependencies. Can you see if you can get the Linux build to pass by putting the appropriate dependency here: https://github.com/LMMS/lmms/blob/master/.travis/linux..install.sh#L10
-Tres

[tresf]

@Narfinger Thanks for your help on this issue. Some feedback on this Pull Request:

• The PR title can simply say "Fix VST on Arch Linux"
• The body should mention Fix/Close of VST freezes on Arch #1875 so that that bug auto-closes when this is merged.
• Don't mention the bug report by number in the title please as it is confusing to read Title #foo #bar from email
• The body should also mention briefly what we're doing rather than mentioning "the bug thread" so that someone reviewing this PR can quickly decide if its is ready for merging without having to read an entire bug report. e.g. removing linux shm and semaphores and use Qt ones should probably be in the both rather than in the title, or the title should plainly say VST: Replace all Linux shm.h and semaphores with QSharedMemory or something quick and to the point. 👍
• If this is a work in progress and shouldn't be merged, that is good to mention too so that a project admin doesn't click merge without checking with you first.

Hope that feedback is well received. Thanks again for your continued work on this issue.

---

@Wallacoloo @lukas-w @Umcaruje @tobydox what I'm about to say is probably best left for the bug report, but I'm mentioning it here where the code is. This change brings with it a 32-bit Qt dependency for all platforms as it moves our shared memory logic to a pure Qt implementation. This simplifies our codebase but does add the new dependency. This may also make VST available on Mac down the road (disabled via CMakeLists.txt currently), once we fix the winegcc problems ( #698 (comment))

[tresf]

Via this thread, we may need to temporarily switch out the path to qmake, do a find_package and then switch back. If someone can find a command line switch for locating the 32-bit libs, I can help write up a CMake command to execute and return the value as well.

[tobydox]

I strongly vote against merging this. We developed a dedicated non-Qt-based version of RemotePlugin in order to allow building RemotePlugins for foreign platforms without additional dependencies. Adding Qt dependency on the WINE side adds complexity and increases chances, things will fail in combination with WINE. When using native mechanisms in the winegcc-based binaries (which run as normal Linux processes), we can be sure, SHM/IPC mechanisms do work. Furthermore building will be almost impossible on most distributions where wine-dev is all we have. No distribution ships Qt binaries for Win32 or Win64 and no distribution will accept source packages where these binaries are bundled.

[tresf]

we can be sure, SHM/IPC mechanisms do work

Fair enough, but what about these cases where SHM/IPC doesn't work? That is what sparked this change to begin with.

[tobydox]

Then we could switch to the proposed behaviour manually using a cmake define such as "REMOTE_PLUGIN_QT_IPC_ONLY". This define could be used in CMake files as well as source files. The default build behaviour should not be touched for the reasons mentioned.

[tresf]

Then we could switch to the proposed behaviour manually using a cmake define

But if I'm reading your previous post correctly, we'll still have to distribute win32 QT libraries with releases for affected distros (assumng they want VST) right?

[tobydox]

No idea how this will be handled. Shipping binaries is a no-go either way and will lead us to big problems with all major distribution's source package policies (such as Debian/Ubuntu, Fedora etc.). There will have to be a documentation for manually grabbing this file (+ all includes...!) from somewhere and putting things at the proper place.

[tresf]

Shipping binaries is a no-go either way and will lead us to big problems with all major distribution's source package policies (such as Debian/Ubuntu, Fedora etc.)

This is already a problem for VST tho, no? RemoteVSTPlugin.exe.so falls into this category, right? Not trying to be antagonistic, just trying to understand.

[tobydox]

It's about source packages not binary packages. Our source package does not contain any compiled files but only source files ;-) Of course the resulting binary packages contain binaries of all kinds but even here I guess the Debian people would blame us for including a big Windows binary which somehow got in.

[tresf]

Our source package does not contain any compiled files but only source files

So the fear from a packaging perspective is that source would get a DLL bundled? I wouldn't allow that 😉 but if this is the case, how is it working on Arch for this PR? The issue which sparked this move claims to have Sylenth working, but the report said nothing about cherry-picking QtCore.dll from PPA. How can wine use this without having it to link against? What am I missing?

Debian people would blame us for including a big Windows binary which somehow got in.

Understood.

[tobydox]

I'm not sure whether RemoteVstPlugin.exe is linked against native QtCore binary (32 bit) or QtCore.dll. In either case, the native mechanism could should be kept and the Qt-only version only used where neccessary.

[tobydox]

Ok, I didn't know you could link against native i386 ELF libraries with winegcc. This changes my opinion about the new approach a little. I still think the current approach should be kept because usually there won't be a 32 bit Qt installation while basic syscalls are available as soon as you have winegcc. But using Qt-only for IPC as an alternative method seems OK and is mainly a question of properly setting up the build system. I don't know whether CMake supports multi-arch builds.

[tresf]

I still think the current approach should be kept because usually there won't be a 32 bit Qt installation

From what I'm gathering, libqt4-core:i386 on a non-KDE Ubuntu system weighs in at about 27MB (fetch) and 30MB installed as compared to Wine which weighs in at about 213MB (fetch) 500MB installed..

Fedora's qt.i686 package weighs in at 16MB (fetch) 55MB installed.

On a KDE Ubuntu system, the 32-bit libs weigh in at 3.5MB (fetch) and 12MB installed.

From the OSX side of the fence, I looked this up on Homebrew and MacPorts and from what I'm gathering, they both require building Qt from source to obtain the 32-bit QtCore library (not sure how relevant this is since we haven't made it far enough to test wine + IPC on Apple). So adding a 32-bit Qt dependency could make our apple build environment quite difficult to create if it ever sees win32 VST support.

-Tres

[tobydox]

You still need WINE for building RemoteVstPlugin even when linking against 32 bit Qt4Core.

[tresf]

You still need WINE

I was trying to illustrate the dependency severity of using 32-bit Qt libraries.

[tresf]

there won't be a 32 bit Qt installation

You still need WINE

I'll elaborate a bit...

I was only trying to illustrate the dependency impact/severity of using 32-bit Qt libraries since your previous statement claimed they won't have it to begin with. From my research, they CAN meet this dependency with relative ease; hence spelling out the footprint that the 32-bit Qt libraries have on various platforms. Mac was the only platform I researched without an easy QtCore-32-bit installation candidate.

Yes, I understand Wine is needed and this comparison against Wine wasn't meant to be a mutually exclusive comparison but rather as a benchmark comparison in terms of meeting dependencies from a bandwidth and disk space perspective.

I guess what I'm saying is, if someone wants VST on Linux, the additional 32-bit QtCore dependency is moot in nearly all cases because the packaging providers have it ready to download and install.

I understand the desire to keep things native and perhaps @Narfinger can do the best of both worlds and do a flag such as REMOTE_PLUGIN_QT_IPC_ONLY as suggested, however, from a code complexity and compatibility standpoint, I think there is some measurable benefit to making 32-bit QtCore a hard dependency as well, or at a minimum, making it the default option for most platforms.

[Narfinger]

Sorry I did not reply earlier. I will try to incorporate some changes discussed here when I have time.

But I am not sure keeping the old approach in there is a good idea. The code is not very readable as it is and with additional ifdefs this is still not very readable.

Secondly, the problem seems to crop up in more distributions than just arch as the bug report mentions xubuntu 15.04. I strongly assume there is some bug and changes in some of the core libraries trigger it. Or there was an implicit assumption somewhere in the code which is not valid anymore with new libraries.
So I do not see the point in keeping code around that does not work. And unless somebody is brave enough to try to fix the code in the native semaphore and native shared memory I would vote to remove it.

[tresf]

And unless somebody is brave enough to try to fix the code in the native semaphore and native shared memory I would vote to remove it.

Or perhaps we separate the logic entirely (or attempt to subclass it). I agree, the #ifdefs make the code very difficult to read and troubleshoot, but Toby's points have merit, so we should meet half way.

[Narfinger]

Ok I will see if I can split the logic. That at least is easier for the moment than handling cmake/qmake issue.