Add a "Cancel" button for Zerebus soundfont loading & audio export

(It might also be advisable to for Zerebus sound loading to first check for enough free memory to store all samples, and then prevent the user from loading the sfz if there is not enough memory, before even trying to load the first sample. Although, I'm not aware of the proper way to do this, and there doesn't seem to be a platform independent way to get this information via qt: https://forum.qt.io/topic/4301/get-available-physical-memory-multiple-p…)

thanks! I will fix now for Zerebus loading and audio export as well!

thanks...I'll look into it now.

I'm behind you...got the cancel button to appear, but yeah I see it doesn't do anything in audio or zerberus. I see those two nested for loops for audio export...i won't look at that now since you got it, so I'll look at zerberus...

I'm looking at ZInstrument::loadSfz in the while loop. I notice ZInstrument::addRegion is allocating memory with "Zone* z = new Zone;" before calling readSample, so I suppose the memory allocated needs to be deallocated upon early exit. Now the deconstructor ZInstrument::~ZInstrument does delete every Zone pointer, so I guess the early exit would be to properly call the deconstructor.

could you do me a favor and commit/share the code for your audio cancel for me to see so I know I'm doing it correctly for zerberus.

Ok...I see yours just halts the mp3 export, and I suppose there would be a file left partially completed on the drive, I suppose that is the correct interpretation of cancel.

But for Zerebus I believe do need to deallocate memory...I'm going to try breaking out of the while loop in ZInstrument::loadSfz, and properly call the deconstrutor or somehow delete the Zone* allocated pointers.

I'm going to try returning false in ZInstrument::loadSfz since it seems a false return should propagate to Zerebus::loadinstrument, which has a "delete instr" instruction...

I have ZInstrument::loadSfz polling the QProgressDialog box for wasCanceled() in the inner loop and returning false, as you can see diff here:

https://github.com/musescore/MuseScore/compare/master...ericfont:71441-…

I verified with breakpoints that the ZInstrument deconstructor is being called to delete everything in Zones* list upon cancellation**, and there is a popup that says "Cannot load soundfont" with the filename (which is the default error message when false is returned), so it seems to be working. My main concern is that I am getting a pointer to the QProgressDialog box using:

QProgressDialog* progress = reinterpret_cast(zerberus->gui())->progressDialog();

But I don't think this wouldn't be good if running musescore without a gui, since I notice zerberus->gui() will create a gui if it doesn't exists. Which meant I had to inclue zerberusgui.h in zerberus.cpp, and had to add a public function to ZerbusGui to return a pointer to the QProgressDialog, which might not be good object-oriented design. Should I instead use SIGNALS and SLOTS to propagate the cancellation message from ZerberusGui to ZInstrument? Or do you have a better way?

**I noticed my linux memory bar usage was not decreasing upon cancellation, however, upon further inspection I found that even if I delete a zerberus instrument from synthesier gui, that my memory usage for mscore doesn't decrease until after closing mscore. I believe what is happening is that linux keeps the files in memory, since if I reloaded the sfz after deleting it, that the memory usage wouldn't increases further again. I don't know for sure though. Can you think of a sure way to verify that all the memory is freed?

OK. That was your fix anyway.

Here is new branch only containing my changes regarding zerberus, so won't conflict with yours. https://github.com/musescore/MuseScore/compare/master...ericfont:71441-…

Please give feedback.

ok...fixed that...yes that text is more appropriate in the constructor.

ok. Regarding running without the gui, I am able to run mscore in converter mode "mscore -o output.mp3 scorewithzerberus.mscz" and it runs fine. I was more concerned if there was a more appropriate way to communicate the cancellation message...but the way I did it has the least amount of code changes I can think of.

PR created...I guess I will find out what other dev say: https://github.com/musescore/MuseScore/pull/2158

I compiled my 71441-zerberus-cancel branch in windows 8.1, and can confirm that the memory is indeed being freed when the user presses the cancel button. The windows resource manager showed mscore memory usage returning to previous level upon canceling or deleting a sfz from zerberus synthesizer.

(As I said in #19 and end of #12, I believe the reason the memory usage bar in linux didn't show memory decreasing after canceling or deleting a sfz is because linux was still keeping those files in memory)

Aside: if I try to load two large soundfonts (SalamanderGrandPianoV3.sfz and SalamanderGrandPianoV3Retuned.sfz) in x86-64 windows, mscore crashes halfway through loading the second one (both on offical 2.0.2 release and my git branch). I am almost certain this is because the build is 32-bit and runs out of 32-bit address space (my computer has 15GB of memory free, so I know it is not an issue with available *physical* memory). So I suppose if musescore wants to support these huge soundfonts in the future, might need to release 64-bit windows builds.

clarification: hasn't been "fixed"...the crash is "postponed" until users use even more memory.

EDIT: the "crash" that the user experienced wasn't because they ran out of addressable memory. It was because there wasn't enough *physical* memory, which caused thrash during soundfont loading, which basically brought the system to molasses, but which could have been prevented with a simple cancel

Prior to adding --large-address-aware linker flag, 32-bit mscore will crash *throw a bad_alloc exception* when try to allocate past 2GB on both 32-bit windows and 64-bit windows. After adding --large-address-aware linker flag, mscore now has the following behavior:

1. If 32-bit mscore on 64-bit windows uses more than 4GB of memory, then mscore will crash *throw a bad_alloc exception* when try to allocate past 4GB.
2. If 32-bit mscore on a 32-bit windows with "4 Gigabyte Tuning" enabled, then mscore will crash *throw a bad_alloc exception* when try to allocate past 3GB.
3. If 32-bit mscore on a 32-bit windows without "4 Gigabyte Tuning", then mscore will crash*throw a bad_alloc exception* when try to allocate past 2GB.

The proper "fix" for is to have mscore first make sure can allocate space when trying to load huge soundfonts. (And generally speaking, it would be nice if mscore always makes sure any memory allocation has succeeded and then either closes gracefully or cancels the user's operation instead of crashing.)

EDIT: I needed to make some clarifications because there are two separate but related issues:

1. One is limited address space, which will lead to memory allocations throwing std::bad_alloc exceptions. It does appear that the current master branch is handling this correctly in Zerebus, since loadInstrument is catching a std::bad_alloc exception when there was a failure to allocate memory, and loadInstrument is returning false, which does indeed deallocatte the partially loaded instrument, and which does indeed present user with popup "Unable to load Instrument". I've tested this both with and without the --large-address-aware flag, and in both cases it is behaving correctly (in that I'm able to load more soundfonts with --large-address-aware before running out of memory. Although I'm going to add a little bit more descriptive qDebug message if the bad_alloc was caught, specifically saying failed to allocate memory for that soundfont.
2. But the other issue is loading huge soundfonts on systems with limited physical memory, which causes thrashing. The only way I know how to handle this issue is by having a cancel button.

DISREGARD. regarding the crash, the suspect code is ZInstrument::addRegion which allocates a new Zone and just assumes that the memory allocation suceeded. I can fix this by putting it in a try block, printing a nice qDebugMessage, and returning false if memory allocation failed (which would send a error popup "Cannot load soundfont" and would also deallocate all the space already allocated by that ZInstrument). I'll go ahead and include this fix.

DISREGARD. ZInstrument::readSample also has unprotected new allocations, I'm going to also include in try block and will likewise return false if failed to allocated.

EDIT: The bad_alloc exception does seem be caught by loadInstrument catch (...) block.

DISREGARD. hmm...Zerberus::loadInstrument already has a try block which should catch all exceptions occuring anytime during loading the sfz, since loadInstrument would be higher in the call stack. I need to investigate why mscore is crashing, though, since mscore should be able to continue on execution even though a soundfont failed to load.

I'm going to investigate by specifically catching the std::bad_alloc exception, printing a qDebug about not having enough memory, and put a breakpoint there to see if it actually catches the exception when I exceed the memory.

EDIT: my breakpoint investigated indicated to me that the bad_alloc exception is indeed being caught by loadInstrument try block.