Solves issue #2028 (Slow MIDI import due to repeated message)

[michaelgregorius]

AutomationPattern::addObject now returns a boolean which indicates
whether the object was added or not. This change enables the removal of
the error message that is shown in the case that a model is already
connected from AutomationPattern::addObject. Instead all interactive
callers now check for the return value and show the message in case it
is needed.

This change set improves the import of MIDI files significantly. These
have been slowed down quite a lot due to the message being shown
repeatedly during the MIDI import.

[tresf]

Thanks. If we can get a someone to test, we can merge. Note also your formatting is a bit off in terms of tab spacing. Please correct this with and additional commit (or amending the existing commit) prior to the merge.

[curlymorphic]

The code looks ok to me, once the formatting is corrected. I have never imported midi files into lmms, so this could be better tested by others, as I have no experience of the expected behaviour.

iirc @softrabbit has experience in this field.

[michaelgregorius]

@tresf @curlymorphic Thanks for the quick review! I have just corrected the tab spacing.

[tresf]

Thanks! The lines under TextFloat still look like something is wrong. Can you take a look?

[michaelgregorius]

Yet another correction of the tabs. 😄

I had the tabs set to four spaces instead of eight in the settings of QtCreator. It seems that the WIKI only states that tabs should be used instead of spaces but not how many spaces should be used to display them. So you might want to add this information. Thanks!

[tresf]

WIKI only states that tabs should be used instead of spaces but not how many spaces should be used to display them

I'm confused... isn't this a user preference?

[curlymorphic]

I'm confused... isn't this a user preference?

Yes. This is just how it is displayed in QtCreator.

It would appear spaces have been changed to tabs.

In QtCreator, tools > options > text editor > display There is an option "visualize whitespace" that I find invaluable for this as it clearly shows whats a tab or a space.

[tresf]

Dave, what do we need to add to our tutorial then? Is it specific to QtCreator users mainly? Is is something non-obvious that we need to place in a wiki?

I ask because currently I use gedit and vim (and occasionally TextWranger or Notepad++ on the proprietary OSs) and none of those exhibit this tab/space behavior, although I've seen it happen on several occasions on commits.

[Wallacoloo]

@tresf many editors will respect settings contained in a ".giteditor" file
(or something similar to that name - I don't recall it off the top of my
head) placed at the root of a repository. This file can specify the tab
settings.
On May 4, 2015 1:20 PM, "Tres Finocchiaro" notifications@github.com wrote:

Dave, what do we need to add to our tutorial then? Is it specific to
QtCreator users mainly? Is is something non-obvious that we need to place
in a wiki?

I ask because currently I use gedit and vim (and occasionally TextWranger
or Notepad++ on the proprietary OSs) and none of those exhibit this
tab/space behavior, although I've seen it happen on several occasions on
commits.

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

[curlymorphic]

Is it specific to QtCreator users mainly?

QtCreator can be a bit aggressive with formatting. There are two important options that need to be correct, The red arrows below show my settings for these, the first are in the Text Editor Options

The Tab policy also has to be set in the c++ settings

There is also an option to display symbols for white space, a faded dot for space, and an arrow for tabs

[tresf]

Thanks a bunch Dave. I've placed a link to your comment in this PR in the wiki for now. Hopefully this helps others down the road.

https://github.com/LMMS/lmms/wiki/Coding-conventions

[musikBear]

Does this fix the problem with hanging and in-responsiveness at 66% import done.
The file refereed in this forum thread
http://lmms.io/forum/viewtopic.php?f=7&t=3424
Has that issue, (and btw- also a midi0 converted version hangs at 66%)
Could someone with a Master test import of the midi-file (unfortunately on mediafire, but it named jin-stage.mid

[michaelgregorius]

@musikBear When I load the file jin-stage.mid in master the progress bar stops at 66% for a while but then it eventually loads.

However, trying to play the song unfortunately leads to a crash in the SoundFont2 player code:

void sf2Instrument::noteOff( SF2PluginData * n )
{
    n->noteOffSent = true;
    m_notesRunningMutex.lock();
    const int notes = --m_notesRunning[n->midiNote];
    m_notesRunningMutex.unlock();

    if( notes <= 0 )
    {
        m_synthMutex.lock();
        fluid_synth_noteoff( m_synth, m_channel, n->midiNote );
        m_synthMutex.unlock();
    }
}

The crash occurs on the line const int notes = --m_notesRunning[n->midiNote]; due to an invalid MIDI note value of -671075328 for n->midiNote. But I guess this is something for another issue.

[musikBear]

crash occurs on the line const int notes = --m_notesRunning[n->midiNote]; due to an invalid MIDI note value of -671075328 for n->midiNote. But I guess this is something for another issue.

Believed that the 'under'-short note-length was addressed in this summery-fix
Looks like there still are issues, and also that this cant be closed yet
@michaelgregorius Thanks! for clarifying!

[midi-pascal]

@musikBear I imported the midi file with the Master branch Lmms under Ubuntu 12.04 on a Pentium4 box.

• The import stalled for 2-3 seconds at 66% then it finished successfully.
• I did not experienced the crash while playing it (did it 10 times)

[musikBear]

@midi-pascal
sounds great -this could mean that the 'under'-short note-length bug is fixed in linux, but still exists in win.. but that needs clarification.

[midi-pascal]

@musikBear I remember I encountered this crash when playing imported midi files sometimes ago in master branch but it stopped occurring for months now so I think an other fix did the job - at least under linux -

[musikBear]

@midi-pascal Sounds good 👍 , we need some windows confirming before the issue is done, but so far no binaries of 1.2
I will keep this midi, and test it specifically, when the binaries are ready.

[Umcaruje]

@musikBear are you on 32 or 64 bit windows? I could make some binaries for testing later today

[musikBear]

@Umcaruje
im on 32 (actual winXP ... :|

[Umcaruje]

@musikBear here you go: https://drive.google.com/file/d/0B49dcRGtm8vJYlpvVUZMY2xKQ3c/view?usp=sharing

[musikBear]

@Umcaruje Thanks! -i will also test the VST-plugin Blue-Cat hang and loss of settings