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

Temporary workaround for ZynAddSubFx name clashes #2118

Merged
merged 1 commit into from
Dec 30, 2015

Conversation

Wallacoloo
Copy link
Member

Once Zyn 2.5 is ready to be merged, that should solve all the issues we've been having with the conflicting LMMS Engine and Zyn Engine classnames. Until then, this is the small patch that I've been using on my system in order to work around the issue (#2049, #2053, etc). It just renames Engine as LmmsCore, so that the two classes now have different symbol names, but then it typedefs it back to Engine so that the rest of LMMS doesn't need to be modified (for the most part).

I'm sharing this in case anyone else would find such a patch useful, and to see if there's demand for something like this to be merged while we work on readying Zyn 2.5. I must emphasize that this is meant to be temporary - I would not like to see this reside in the LMMS codebase for very long, if at all. Luckily, if people think this is worth merging, it's easy to revert once @curlymorphic's Zyn branch is merged.

@curlymorphic
Copy link
Contributor

I feel It may be a good idea to merge this, as lmms appears to be lacking Devs that know about windows, meaning the zasf upgrade could be a long way off.

An alternative could be to patch our zasf 2.4, although Im not sure that is any less of a hack

@Wallacoloo
Copy link
Member Author

@tresf I saw you mention this PR over in #2269. I tried rebasing this today, and for whatever reason it worked when running it from /home/colin/, but it fails inside a call to SDL_Init when I install it & run it from /usr/local/bin with the message:

Inconsistency detected by ld.so: dl-close.c: 811: _dl_close: Assertion `map->l_init_called' failed!

and the program exited with error code 127.

I'm not sure what the issue is (I made clean & tried again. Also reinstalled SDL. Master branch doesn't have the issue). I pushed the rebased version to engine-rename-min2 though.

@tresf
Copy link
Member

tresf commented Dec 26, 2015

@Wallacoloo I'm not an expert on the name clash stuff, but AFAIK, it was the remote processes that caused the hangup, so perhaps SDL uses something like that?

I assume you've tried the obvious... changing AudioSdl.cpp#L129 to use the new name (although I don't know why it would matter since AFAIK its statically linked).

@Wallacoloo
Copy link
Member Author

@tresf turns out that remnants of the old build were still sitting around even after running make clean. I removed my build directory, re-ran cmake & then make && make install and I am no longer having the SDL issue I reported earlier.

I pushed the results of the rebase to this branch, so it should merge fine if there's a concensus for this over @shimpe's #2494.

@tresf
Copy link
Member

tresf commented Dec 29, 2015

Thanks. Merging to get 1.2 back on track. Easy enough to revert if needed.

@tresf
Copy link
Member

tresf commented Dec 29, 2015

@Wallacoloo one last thing... should we include a comment about why the name differs from the header? I feel someone may change this back later in a cleanup effort if we don't explain why the discrepancy occurs.

@Wallacoloo
Copy link
Member Author

@tresf I added this blurb to the top & moved the typedef further up near that documentation:

// Note: This class is called 'LmmsCore' instead of 'Engine' because of naming
// conflicts caused by ZynAddSubFX. See https://github.com/LMMS/lmms/issues/2269
// and https://github.com/LMMS/lmms/pull/2118 for more details.
//
// The workaround was to rename Lmms' Engine so that it has a different symbol
// name in the object files, but typedef it back to 'Engine' and keep it inside
// of Engine.h so that the rest of the codebase can be oblivious to this issue
// (and it could be fixed without changing every single file).

@tresf
Copy link
Member

tresf commented Dec 29, 2015

@Wallacoloo 👍

Squash before merging?

Edit: Zyn launches on Ubuntu 12.04 x64 + engine-rename-min branch....
image

However, the console does show some strange signs.

RemotePluginClient::shmget: No such file or directory
Starting Audio: NULL
Audio Started
Starting MIDI: NULL
MIDI Started
RemotePlugin::DebugMessage: failed getting shared memory

@tmerr
Copy link

tmerr commented Dec 29, 2015

Works for me on Arch. The console output is the same as @tresf's but lacks the shared memory error.

…nflicts with ZASFx

Document the Engine renaming better & link to relevant issues/PRs
@Wallacoloo
Copy link
Member Author

@tresf Squashed. I'm not getting the shared memory error either, but I am also using Arch.

@tresf
Copy link
Member

tresf commented Dec 30, 2015

@Wallacoloo merging. Let's see how this goes. 👍 :)

tresf added a commit that referenced this pull request Dec 30, 2015
Temporary workaround for ZynAddSubFx name clashes
@tresf tresf merged commit baaed6a into LMMS:master Dec 30, 2015
@klopsi
Copy link

klopsi commented Jan 5, 2016

My entire desktop used to crash when clicking zyn's 'Show Gui'. Both in 1.1.3 branch and master (built from source).

Today, Jan 5, I did a git clone https://github.com/LMMS/lmms.git
git log shows "Temporary workaround for ZynAddSubFx name clashes"

No problems building lmms.

When I run build/lmms and click zyn's Show Gui, my Desktop (Xserver?) still crashes.

Running linux Mint 17.3 (xfce)
Graphics: Card-1: Intel 3rd Gen Core processor Graphics Controller
Display Server: X.Org 1.17.1 drivers: intel (unloaded: fbdev,vesa)

:(
EDIT: Started LMMS in a nested xserver (xserver-xephyr) and Show Gui does not crash anything there.

@tresf
Copy link
Member

tresf commented Jan 5, 2016

@klopsi please see #2100. If it is the same thing, this isn't our bug.

@shimpe
Copy link

shimpe commented Jan 5, 2016

@klopsi at the risk of stating the obvious, also make sure you don't have previous versions of lmms lingering about your system

@tresf
Copy link
Member

tresf commented Jan 5, 2016

@shimpe
Copy link

shimpe commented Jan 5, 2016

@tresf Well.. it used to log me out of KDE before it was fixed.

@klopsi
Copy link

klopsi commented Jan 6, 2016

Thanks for your comments! The bug was in Xorg! Upgrading xorg using the ubuntu xorg-edgers ppa solved the problem. https://launchpad.net/~xorg-edgers/+archive/ubuntu/ppa

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.

6 participants