Midi Import hang

[lmmsuser1001]

In version 1.1.3 (win32) the import of larger midi files hangs always at 66%.

In version 1.0.0 (win32) the import does not hang but the drum channels are not imported.

Does a workaround exist (for example converting midi files to a lmms project file with an external tool)?

[Wallacoloo]

Could you provide a midi file for which this problem occurs? That would
make it easier for us to verify and track down the error.
On Apr 17, 2015 4:20 PM, "lmmsuser1001" notifications@github.com wrote:

In version 1.1.3 (win32) the import of larger midi files hangs always at
66%.

In version 1.0.0 (win32) the import does not hang but the drum channels
are not imported.

Does a workaround exist (for example converting midi files to a lmms
project file with an external tool)?

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

[lmmsuser1001]

Unfortunately, it is not possible to attach a midi file.

But most of the files that you can download from the link below will hang (e.g. Twenty_4_Seven-Take_Me_Away.mid) on win XP.

http://www.eurokdj.com/ringtones/index_midis.php

[softrabbit]

Importing drums has improved a bit in master, but there are probably files where it still sucks badly.

[musikBear]

sometimes it help to convert the problem file to 'midi1'
Anvill-studio (free) can do that

[softrabbit]

Looks like the example file has so short notes in the drum track that LMMS rounds them down to 0 ticks and ignores them. I'm thinking a simple addition of ceil() in https://github.com/LMMS/lmms/blob/master/plugins/MidiImport/MidiImport.cpp#L398 should do it.

Didn't get a hang on Linux with master branch, but the window did go dark while importing, which might be related to the hang, e.g. LMMS not responding to some OS message or whatever.

[DeRobyJ]

I've had this problem too. Sometimes, if you're luck, you just have to wait 3 o 4 minutes. But most of the times it will stop responding forever.

Tried on Win7 64bit

[Wallacoloo]

Sounds like we aren't calling qApp->processEvents() regularly during the
import process.
On Apr 18, 2015 8:44 AM, "Roberto Giaconia" notifications@github.com
wrote:

I've had this problem too. Sometimes, if you're luck, you just have to
wait 3 o 4 minutes. But most of the times it will stop responding forever.

—
Reply to this email directly or view it on GitHub
#1971 (comment).

[softrabbit]

And the import really is awfully slow. I compared Rosegarden (3 seconds) and LMMS (25 seconds) opening the file mentioned above.

After some poking around I believe most of the time (~18 seconds in my test or some 70-ish %) is spent in this if clause: https://github.com/LMMS/lmms/blob/master/plugins/MidiImport/MidiImport.cpp#L435. Even if there is a huge number of pitch bends or something in that file, I think it's worth a closer look.

[michaelgregorius]

I did some profiling with Valgrind and it turns out that there are two main causes for the slow import of the aforementioned MIDI file (Twenty_4_Seven-Take_Me_Away.mid):

• The call to TextFloat::displayMessage in AutomationPattern::addObject. It seems like TextFloat::displayMessage is called over and over again during the import (for that file).
• The calls to createDetuning in Note::Note and Note::loadSettings. The main problem with createDetuning seems to be the sheer number of DetuningHelpers that are created during the import. The instances are created using the MemoryPool but even that seems to be of no help here. It might be that this problem can only be alleviated by changing the design. Does every note really need its own DetuningHelper?

If you comment out the aforementioned calls the import just flies.

The calls to createDetuning in the Note class also slow down the loading of LMMS project files by the way. You can reproduce this by profiling the loading of DnB.mmpz with Valgrind. So solving this problem would kill two birds with one stone and give users faster loading of projects and faster import of MIDI data.

[softrabbit]

@michaelgregorius, see softrabbit@1e29da4 for my thoughts on AutomationPattern:addObject in this context.

[Wallacoloo]

@michaelgregorius That's very useful info - thanks for sharing it here. I don't know enough about the internals of LMMS to answer the DetuningHelper question, but all I can say is to be careful when using Valgrind as a profiler. On top of executing each instruction, it does a whole host of dependency tracking (in order to catch uses of uninitialized variables) and such for every instruction. This means that because of the overhead, an integer division might take only 3% longer than an integer add, vs 300% longer when run outside of valgrind. Similar issues occur with its heap allocator, which results in new/delete operations taking much longer than they typically would in normal execution.

So it's quite possible that the two bottlenecks you mentioned could be anomalies that only become significant under valgrind and not in regular use. I'd recommend an alternative, but my experience with profiling is fairly minimal beyond that.

[tresf]

FYI, in regards to the MIDI files.... for uploading files, it looks like GitHub will have a storage solution very soon....

https://github.com/early_access/large_file_storage/

Thought it was slightly on-topic in regards to hosting some of these files. :)

[michaelgregorius]

@Wallacoloo I think Valgrind was pretty accurate in this case. Here is what I did in my profiler run using a release build:

1. Start the application
2. Import the MIDI file
3. Close LMMS

After the analysis of the results I then commented out the calls to TextFloat::displayMessage and the two calls to createDetuning in Note::Note, recompiled and tested the import in a release version without Valgrind. It became a lot quicker immediately. So these two code areas in fact seem to be some bottlenecks.

The way I understand it Valgrind simulates the CPU (instructions) and should therefore know exactly which instructions are executed and what cost are associated with each instruction. However, I don't what exact models they use, i.e. whether they model the different microprocessor architectures, etc.

Does anyone know a good free stochastic profiler by the way? These usually don't have that much impact on the execution time and performance but still provide a good picture of what's happening.

[Espyo]

I just want to drop in some help as well. An example of a MIDI file I used to be able to open just fine, but now can't, as per all that's been discussed so far, is the Koopa Cape MIDI on this page http://www.vgmusic.com/music/console/nintendo/wii/ (check under Mario Kart Wii).

Oh, and while importing, LMMS reports quite a lot of "Volume / Model is already connected to this pattern." warnings on the bottom-left.

[michaelgregorius]

@Espyo Do you mean that the Koopa Cape MIDI file just opens slowly or that you cannot open it at all? Just asking because on my local master build the file is imported successfully.

Again if I comment out the call to TextFloat::displayMessage in AutomationPattern::addObject the performance during import is a difference like night and day.

I assume that the current implementation of AutomationPattern::addObject was meant to be used in interactive cases where the user wants to create a connection that is already there and that it was not meant to be called from "non-interactive" code like the MIDI import. The question is whether the information provided by the displayed message is really useful to the user in the first place. If he connects something that's already connected and the system is able to handle this case gracefully, i.e. doing nothing will have the exact effect that the user intended, then why tell him that the connection is already there? It almost comes across like "You did something really stupid and I will even tell you." 😄 So the simplest solution might be to remove the message altogether.

If the message should be kept for interactive cases (and my assumption is correct in the first place) there are two ways I can think of to handle the problem:

1. Let AutomationPattern::addObject return a boolean that indicates whether the model was added and remove the call to TextFloat::displayMessage. In case the model is not added the caller may decide what to do. In an interactive context the message could still be shown whereas in an non-interactive context, e.g. the MIDI import, the result is just ignored and no message is shown (and performance is good).
2. Add a boolean parameter to AutomationPattern::addObject which indicates whether it's called from an interactive context which means that the message should be shown or a non-interactive context which means that no messages are shown.

I would prefer the first solution because it keeps the usage of AutomationPattern::addObject more flexible, i.e. it would be simpler to implement some third, forth, etc. kind of behaviours in other callers. It also provides a better separation between presentation and functionality.

[Espyo]

@michaelgregorius Sorry, should've been more detailed. Yeah, that MIDI is one of the cases where it slowly imports, and then hangs on the 66% mark.

[michaelgregorius]

@Espyo No problem. :) The problem with the failing import should be fixed in master.

[tresf]

@michaelgregorius, ok to close?

[michaelgregorius]

@tresf I guess it should be OK to close the issue. I have create two new issues which cover the performance problems.

[Umcaruje]

I have create two new issues which cover the performance problems.

👍