LMMS / lmms

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

Updating Zynaddsubfx to v2.5 #1860

Closed curlymorphic closed 6 years ago

curlymorphic commented 9 years ago

I have started work on updating our embedded Zynaddsubfx to version 2.5.

@fundamental has been kind enough to answer my questions on IRC :)

I really started this issue so we can keep track of progress.

It does add a new dependancy however , http://liblo.sourceforge.net/ I am unfimular with what is needed for this on a cross platform basis.

tresf commented 9 years ago

Liblo seems to support Windows.

From my understanding, we'll generally have @tobydox add it as a new library to the MINGW repository for Windows.

For Mac, I'd be building and bundling it (if Homebrew/MacPorts offers it, I add it to the bundle script, if not, I'd have to add it to our Apple build instructions).

For Linux, we'd probably just add it to our package dependency listing (perhaps something like Israel's packaging instructions)

fundamental commented 9 years ago

It might be possible to update Zyn for the 1.2.0 milestone. The 2.5.1 release for zyn is almost purely a bug fixing release and that will be hopefully finalized in the next few weeks, so any bugs/issues found in upstream code can be fixed in pretty short order at the moment.

tresf commented 9 years ago

It might be possible to update Zyn for the 1.2.0 milestone.

Agreed. Really dependent on our team's progress, mingw included. :)

curlymorphic commented 9 years ago

Is it a problem to work on this for 1.2? Im not really sure of the time frame, If so I will do all that I can to help this issue.

curlymorphic commented 9 years ago

@tres sorry didnt see your last post

badosu commented 9 years ago

@tresf We really can't release 1.2.0 without -O3 back, I/we need to figure out what's going on.

tresf commented 9 years ago

@badosu, that is an unrelated winegxx/mingw combination that only applies to the vst_base plugin, specifically the RemoteVSTPlugin process (and dependent plugins -- VstEffect, vestige).

This -O3 (temporarily set to -O0)setting you are referring to is minor because it is only the process which interacts with the VST that suffers the bug.

Are you aware of performance issues related to the 1.1.3 build? 1.1.3 is a production release and it has that -O0 setting on for the VST stuff, so I'd think 1.2.x would follow suit (until we get it working again)

badosu commented 9 years ago

@tresf So are you saying only the compilation of vsteffect is unoptimized? If this is the case then it's fine.

I am unaware of complaints yet, so I really can't back my concerns.

tresf commented 9 years ago

Correct. My commit description is a bit misleading now that I re-read it... https://github.com/LMMS/lmms/commit/d14f4511b2235906ad66c375ed9d4ad34db0283c

badosu commented 9 years ago

@tresf Fine, sorry. I should have seen better, I read the description literally and was a bit over the top, lol.

tobydox commented 9 years ago

@curlymorphic : didn't read you're already working on this so maybe I did duplicate work in the master-zynaddsubfx-experimental branch. Nevertheless maybe it still helps you to figure out some integration issues. Alternatively you can help by working on this branch.

tobydox commented 9 years ago

@tresf : I'll take care of packaging rtosc and liblo for MinGW as soon as required.

tresf commented 9 years ago

@tresf : I'll take care of packaging rtosc and liblo for MinGW as soon as required.

:tada:

curlymorphic commented 9 years ago

Alternatively you can help by working on this branch.

@tobydox Let me know if theres anything you want me to look at.

tresf commented 9 years ago

@curlymorphic can you check out the branch he's referring to and see what else needs to be done?

curlymorphic commented 9 years ago

Sure will get on this today

curlymorphic commented 9 years ago

Still Wip.

curlymorphic commented 9 years ago

Been battling with this for days now. still cant get lmms to see zyn as a plugin. I think Its a linking issue.

curlymorphic commented 9 years ago

Ive not made progress on this in 10's of hours, so Im asking if anyone has any ideas.

What I have so far

git clone -b mZyn1 https://github.com/curlymorphic/lmms.git installed liblo sudo apt-get install liblo-dev

cloned built and installed rtosc

https://github.com/fundamental/rtosc

I have added the above 2 dependancy to

https://github.com/curlymorphic/lmms/blob/mZyn1/plugins/zynaddsubfx/CMakeLists.txt

With the build i linked above, When running lmms, zyn is not listed as a synth.

Experimanting, I have found that if in https://github.com/curlymorphic/lmms/blob/mZyn1/plugins/zynaddsubfx/ZynAddSubFx.cpp LocalZynAddSubFx is used, this is the result.

I have commented out large chunks of this file in order to get lmms to atleast see zyn as a plugin.

If https://github.com/curlymorphic/lmms/blob/mZyn1/plugins/zynaddsubfx/ZynAddSubFx.cpp#L460 is commented out, and rebuilt, lmms will list zyn as a synth, but obviously it does not run.

Im not sure what the problem is, and have spent hours experimenting trying to find out.

I would be really helpful if someone has some pointers please, maybe checking that I have linked it correctly would be helpful :)

@tresf @tobydox @Lukas-W

I

tresf commented 9 years ago

Here's the diff against master for those technically inclined.

Edit: @curlymorphic, I'm starting to wrap my head round the idea of updating by first looking back on previous commits per (none of which appear to be major)

Some changes that may or may not have made it into 2.5 upstream release:

And after that we seem to fall back into upstream commits per https://github.com/LMMS/zynaddsubfx/commit/ae6824cd9601cc41796b277d365f931a56e7721a, and so on.

-Tres

tresf commented 9 years ago

cloned built and installed rtosc

FYI for anyone trying to do this on precise, minor bug filed upstream. rtosc/#17

curlymorphic commented 9 years ago

@tresf can I ask for you help please to work out what needs to be to done and hows best to approach it?

Are you saying go thru all the commit logs for LMMS/zynaddsubfx and see if the patch has been aplied to the new code, if not apply it?

If that is the case, how should i do that, can that be done with git, or is it a manual process?

tresf commented 9 years ago

Are you saying go thru all the commit logs for LMMS/zynaddsubfx and see if the patch has been aplied to the new code, if not apply it?

Sorry for the confusion... No, I'm only stepping back on others' previous work to understand the plugin code and how we've integrated it a bit better.

tresf commented 9 years ago

can I ask for you help please to work out what needs to be to done and hows best to approach it?

I think if we compare 2.4.4 upstream against our 2.4.4 we could learn a bit about what was done to integrate Zyn in the first place and go from there. Furthermore, any major upgrades to Zyn would also result in some changes on our end, so I think we can learn from the past integration points and go from there.

But I'm entering into this blind as well, so take anything I say with a grain of salt. I'll continue researching and let you know what I find. :)

P.S. On precise curlymorphic/[..]/zynaddsubfx/CMakeLists.txt#L57 had to switch it to c++0x just like rtosc/#17

tresf commented 9 years ago

Ok... I'm googling my own problems now... got stuck on building ADnoteUI.cxx, which according to an old conversation with Mark, turned out to be a build problem with precise, fixed by building with 14.04. This is going to be a fun one. :)

fundamental commented 9 years ago

if you're having a ntk specific issue there was just a bug reported in regards to linking, so that may end up solving that particular issue once it is fixed.

curlymorphic commented 9 years ago

P.S. On precise curlymorphic/[..]/zynaddsubfx/CMakeLists.txt#L57 had to switch it to c++0x just like rtosc/#17

cheers. will change that.

turned out to be a build problem with precise, fixed by building with 14.04.

I though the vm I had been using for this the other week was 14.4, but my memory failed me, it was 14.10 :(.

thanks @fundamental will keep that in mind

curlymorphic commented 9 years ago

We can ignore my attempts at link rtosc and liblo to LocalZyn, because they are not needed there, just on Remote as per

https://github.com/LMMS/lmms/compare/master-zynaddsubfx-experimental#diff-75559ad7a36e29cd9d13ed002f9738abR148

Ive got a diff of lmms/zynaddsubfx/master-upstream to lmms/zynaddsubfx/master

https://github.com/LMMS/lmms/compare/master-zynaddsubfx-experimental#diff-75559ad7a36e29cd9d13ed002f9738abR148.

Im not sure what has and hasnt been done in @tobydox branch, master-zynaddsubfx-experimental branch, when I do i diff it's huge, with no real hints in the commit log. https://github.com/LMMS/lmms/compare/master-zynaddsubfx-experimental

Im going to try on another fork of master-zynaddsubfx-experimental, and work through the above diff on 2.4, to see where we stand

curlymorphic commented 9 years ago

Some changes that may or may not have made it into 2.5 upstream release:

0d76dc8 XML config file working directory 050af40 XML wrapper customization (reduce deps) 640f081 mingw64 fixes

These fixes are all in the master-zynaddsubfx-experimental branch

curlymorphic commented 9 years ago

I have managed to break out of the rutt that I was in, and made progress, had to add the dependencies to zynAddSubFxCore. Now Lmms correctly lists zyn :) There is more work to go, but I have feedback now in the form of an error, so I have an idea of how to progress.

musikBear commented 9 years ago

Now Lmms correctly lists zyn :)

big :+1: !

badosu commented 9 years ago

Congrats @curlymorphic :-)

curlymorphic commented 9 years ago

The is now audio :) Still more work before the ui,

tresf commented 9 years ago

There is now audio :)

:tada:

curlymorphic commented 9 years ago

zyn is now running as a remote process, with the ui.

Im not saying it's fully working, I need to test it for more than 10 mins, and do some clearing up, but initial signs look good.

image

tresf commented 9 years ago

Should testers snag a copy from your branch?

curlymorphic commented 9 years ago

not yet. give me a day or two to test it myself first. I have already found 1 problem with the gui callback, and wont be surprised if there is more i have to tweak yet.

Im just so happy to have the remote process stable for more than 3 seconds :)

tresf commented 9 years ago

Im just so happy to have the remote process stable for more than 3 seconds :)

:+1:

curlymorphic commented 9 years ago

The Ui now works and displays correct values, working vu meter, correct patch name etc.

image

atm I am aware of 2 issues that need sorting, and a clean up, before this is ready for initial testing.

crash on multiple internal instances, @fundamental has already kindly written a patch for zynAddSubFx, to allow it to run multiple instances, I think I just have to adjust our wrapper now.

crashes when loading specific presets, always the same presets, every time. I have not even started looking into this yet.

tresf commented 9 years ago

@curlymorphic if you want some help debugging, point us to the partially working branch. :+1:

curlymorphic commented 9 years ago

@curlymorphic if you want some help debugging, point us to the partially working branch.

sure, but I must warn you I would need a big clean up. there is still hack code in there

git clone -b "mZyn3Compare2.4Diff" https://github.com/curlymorphic/lmms.git

I keeping all changes to the lmms code, with the following exception.

In allocator.cpp I have temporarily raised the limit from 5mib to 100mib

merged the patch to allow multiple instances.

you will need to install the following

http://liblo.sourceforge.net/ , can be found in the ubuntu repo

rtosc https://github.com/fundamental/rtosc

tresf commented 9 years ago
git clone -b "mZyn3Compare2.4Diff" https://github.com/curlymorphic/lmms.git

Thanks!

I just wanted to offer because people like @mikobuntu, @Umcaruje or myself may be able to help debug a few things (like why the presets are crashing).

curlymorphic commented 9 years ago

(like why the presets are crashing)

:) that would be really helpful, I think I know what to do about the multiple instances. Ive just been starting at that code for far too long today. I will look into that tomorrow.

curlymorphic commented 9 years ago

As for some presets not loading @fundamental has been really helpful on irc. I am going to start work on this now. I want a "working build" that can start being tested this weekend. It will probably still need some cleaning up. We will need windows dependancys before I will be able to make windows installers.

<fundamental> presets with effects?
<curlymorphic> erm ,not yet worked out the common denominator.
<curlymorphic> I have put zero time into that so far. I am going to start have an investigation now
<fundamental> both of those have extra extensions in the initialization path
<fundamental> well, those are very very likely to be the common denominator
<curlymorphic> nice, that could be saving me alot of time :)
<fundamental> effects for instance are not initialized until the part has been passed to the realtime thread
<fundamental> as they make use of the allocator which is RT thread only
curlymorphic commented 9 years ago

Finally at a stage with a build for testing. I am expecting bugs and it would be really good if they can be listed here.

@softrabbit @mikobuntu @Umcaruje

https://github.com/curlymorphic/lmms/releases/tag/zyn25.00

@unfa what os are you using? as your input on this will be of great use.

There is no windows build yet.

Although it is possible to load and play unfa-Spoken there is an issue with 1 of the 34 instances that stops the remote process(ui)'s from being opened, and I have not narrowed it down yet.

mikobuntu commented 9 years ago

Ok dl'd the source so I will get onto testing this. We have to remember that ZASF 2.4 is still buggy in the current stable build, so maybe this will address those issues aswell.

thanks Mikobuntu ;)

Date: Sat, 11 Apr 2015 13:59:25 -0700 From: notifications@github.com To: lmms@noreply.github.com CC: chrissy.mc.1@hotmail.co.uk Subject: Re: [lmms] Updating Zynaddsubfx to v2.5 (#1860)

Finally at a stage with a build for testing. I am expecting bugs and it would be really good if they can be listed here.

@softrabbit @mikobuntu @Umcaruje

https://github.com/curlymorphic/lmms/releases/tag/zyn25.00

@unfa what os are you using? as your input on this will be of great use.

There is no windows build yet.

Although it is possible to load and play unfa-Spoken there is an issue with 1 of the 34 instances that stops the remote process(ui)'s from being opened, and I have not narrowed it down yet.

— Reply to this email directly or view it on GitHub.

tresf commented 9 years ago

We have to remember that ZASF 2.4 is still buggy in the current stable build, so maybe this will address those issues aswell.

Agreed. Would be nice to close out some 2.4 items with this effort. :+1:

curlymorphic commented 9 years ago

Todays update.

Unfa spoken, now full loads, plays, can open all the ui's. afaik It now works as well as zyn 2.4

There is still the same instability when opening or closing the ui as 2.4.

The issue is caused by lmms hosting zyn internally, when there is no ui, when the ui is opened, the zyn parameters as saved to a file, the the internal zyn is deleted, then zyn is opened as an external process, then loads the saved parameters. It is this huge time game that I feel is causing the audio connections to be lost.

I have spoken with @fundamental on irc about this situation, and have been informed that this was necessary due to the integration of the ui and core code. This has nearly finished being separated.

I have suggested and fundamental confirmed that once this separation is complete, the best way to proceed is just to open the ui in the remote process, and leave the core audio in lmms, this should eliminate the current slackness of opening the ui.

I have an idea for a tempory fix that I am going to try tomorrow. It may not work, but i feel is worth the effort, even if it is only for this release.

btw the preset names now work (dont think i fixed this, If it was me its a pleasant side effect).

@tobydox you mentioned about sorting out the dependencies. I have absolutely no clue how to go about making the windows versions. It would be really nice to let our windows users test this before a merge.

@tresf

Agreed. Would be nice to close out some 2.4 items with this effort.

Are there any bugs other than the 2 above that really need to be included in this upgrade. I am more than happy to take a look at them, but would prefer to handle them as separate issues. both to keep the commits cleaner, and for my sanity.

tresf commented 9 years ago

Are there any bugs other than the 2 above

Not too many surprisingly.... #703 #1001 #1912

I know you can't readily test the Apple one... Myself or @eagles051387 would likely try that, but it suffers similar packaging issues of Windows, so that might have to wait... :)

curlymorphic commented 9 years ago

703 could well be down to the way we swap internal and external zyn's. I will just have to do the best I can, and hope it works on apple. Are there any cost effective ways for me to test apple builds?

1001 if im reading the bug reports correctly is now fixed

image

1912 time will tell. up until now I have assumed all segfaults were down to me not having zyn integrated correctly