LMMS / lmms

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

Bug on Gigasampler Player 100% CPU #3825

Open gdd71 opened 6 years ago

gdd71 commented 6 years ago

This is confirmed by another user Test used in project "RGD Kit 5.gig" sample link + Video demo: https://drive.google.com/file/d/0B8RY2vt8PBm8NV9qN3Y2LU0tRUU/view Tested in RC3 latest code and older RC3 versions also.

ghost commented 6 years ago

You might just have a weak PC. I remember saying you couldnt play some lsp lmms project since it also was 100% but for me it was fine.

gdd71 commented 6 years ago

This is not weak PC - Read info in LSP! High quality SF2 samples work fine, Any GIG not. You can see in the video how it gets, gradual accumulation in CPU load to full blocking.

michaelgregorius commented 6 years ago

I can confirm this problem. On my machine the CPU load also increases over time which really baffles me because if someone told me to explicitly implement such a behaviour I would have to think really hard how to accomplish this. 😉

I have also done some profiling with valgrind and got the following result (please click on the image for full size): 3825 - valgrind adsr

Much of the time is spent in ADSR::value() in the file GigPlayer.cpp around a line where the following comment can be found: "Maybe not the best way of doing this, but it appears to be about right". This is also the place where the call to expf can be found (also visible in the image above) which in fact is not the best way to compute an exponential decay (I assume that this is done in that line).

If you want to have a decay from let's say 1 to 0.001 in a certain amount of time you can compute the number of samples that span that time, e.g. n samples, and then compute a constant factor that you will multiply each sample by. For the given example you would do this by solving 1x^n = 0.001 for x. This formula translates to: "I want to multiply the start value of 1 n times with the constant x so that I reach 0.001 after n steps." Once you have found x you initialize a variable (let's call it s) with 1 and then multiply that variable each sample with x, so `s = x. This will be much faster then repeated calls to the costlyexpf` function.

However, even if I replace the implementation of ADSR::value() with return 1; the gradual CPU load still occurs so that problem seems to be caused somewhere else.

michaelgregorius commented 6 years ago

Here's a profiling result when replacing ADSR::value() with return 1.;: 3825 - valgrind adsr quick return

In this case it's dominated by calls to GigInstrument::play and GigInstrument::loadSample.

michaelgregorius commented 6 years ago

Here are the full profiling results:

softrabbit commented 6 years ago

On my machine the CPU load also increases over time which really baffles me because if someone told me to explicitly implement such a behaviour I would have to think really hard how to accomplish this.

This is a guess, based on the load increasing, but are the notes being disposed of properly? From reading the code it looks to me like a note that ends up in the PlayingKeyUp state won't be erased until all its samples have ended. Which I'm not sure happens every time.

gdd71 commented 6 years ago

RC4 testted, Windows 8.1 and Ubuntu 16.04 64bit. -> The problem is still available. What makes an impression yet: Load 200MB SF2 file -> RAM increases by 200MB -> Play OK Load 500MB SF2 file -> RAM increases by 500MB -> Play OK Load 200MB GIG file -> RAM increases by 10MB -> Play not OK Load 500MB GIG file -> RAM increases by 10MB -> Play not OK I do not know if it matters but GIG does not load in to RAM completely..

michaelgregorius commented 6 years ago

@softrabbit You are right that it has to do with the disposal of the GigNotes. The problem seems to be that every note is played for 10 seconds before it is disposed of although the samples in the example that I have used only sound for a very short time because it is a drum set. I found this out by adding debugging code to the place where notes are added to the QList m_notes and where they are removed.

In GigInstrument::playNote I have adjusted the last lines of the method as follows (added the last two lines):

QMutexLocker locker( &m_notesMutex );
m_notes.push_back( GigNote( midiNote, velocity, _n->unpitchedFrequency(), pluginData ) );
GigNote const & noteAtBack = m_notes.back();
qDebug() << "Pushed note " << &noteAtBack << "at " << QTime::currentTime().toString() << ", Thread: " << QThread::currentThreadId();

In GigInstrument::play I have adjusted the erasing code as follows (added the first two lines in the if block):

// Delete ended notes (either in the completed state or all the samples ended)
if( it->state == Completed || it->samples.empty() )
{
    GigNote const & iteratorNote = *it;
    qDebug() << "Removing note " << &iteratorNote << "at " << QTime::currentTime().toString() << ", Thread: " << QThread::currentThreadId();
    it = m_notes.erase( it );

    if( it == m_notes.end() )
    {
        break;
    }
}

This yields results like the following. Please note that I have cut out repetitive blocks:

Pushed note  0x7fff84005690 at  "22:29:20" , Thread:  140736061802240 
Pushed note  0x7fffd8005250 at  "22:29:20" , Thread:  140736963454720 
Pushed note  0x7fffd8005220 at  "22:29:20" , Thread:  140736061802240 
Pushed note  0x7fffd8005280 at  "22:29:20" , Thread:  140736963454720 
Pushed note  0x7fffd0004e10 at  "22:29:20" , Thread:  140736955062016 
Pushed note  0x7fffd4004aa0 at  "22:29:20" , Thread:  140736946669312 
----------------- [ Big block of only note pushing without removal ] -----------------
Pushed note  0x7fffd4011fc0 at  "22:29:30" , Thread:  140736946669312 
Pushed note  0x7fff84020ca0 at  "22:29:30" , Thread:  140736061802240 
Pushed note  0x7fffd80136d0 at  "22:29:30" , Thread:  140736963454720 
Removing note  0x7fff84005690 at  "22:29:30" , Thread:  140736963454720 
Removing note  0x7fffd8005220 at  "22:29:30" , Thread:  140736963454720 
Removing note  0x7fffd4004aa0 at  "22:29:30" , Thread:  140736963454720 
Removing note  0x7fffd8005250 at  "22:29:30" , Thread:  140736955062016 
Pushed note  0x7fffd400d400 at  "22:29:30" , Thread:  140736963454720 
Pushed note  0x7fff84020d90 at  "22:29:30" , Thread:  140736061802240 
Pushed note  0x7fffd4011ff0 at  "22:29:30" , Thread:  140736946669312 
Pushed note  0x7fffd8005250 at  "22:29:30" , Thread:  140736955062016 
Pushed note  0x7fffd00129a0 at  "22:29:30" , Thread:  140736061802240 
Pushed note  0x7fffd8013730 at  "22:29:30" , Thread:  140736963454720 
----------------- [ Block of notes which are pushed and removed ] -----------------
Removing note  0x7fffd8012cd0 at  "22:29:38" , Thread:  140736955062016 
Removing note  0x7fffd0011680 at  "22:29:38" , Thread:  140736955062016 
Removing note  0x7fff8401d2e0 at  "22:29:38" , Thread:  140736963454720 
Pushed note  0x7fff8401d680 at  "22:29:38" , Thread:  140736061802240 
Pushed note  0x7fffd0015ae0 at  "22:29:38" , Thread:  140736955062016 
Pushed note  0x7fff84026990 at  "22:29:38" , Thread:  140736061802240 
Pushed note  0x7fff840269c0 at  "22:29:38" , Thread:  140736061802240 
Pushed note  0x7fff84026a40 at  "22:29:38" , Thread:  140736061802240 
Pushed note  0x7fffd0011680 at  "22:29:38" , Thread:  140736955062016 
Pushed note  0x7fff8401d720 at  "22:29:38" , Thread:  140736955062016 
Pushed note  0x7fffd8012cd0 at  "22:29:38" , Thread:  140736955062016 
Pushed note  0x7fff8401d390 at  "22:29:38" , Thread:  140736955062016 
Pushed note  0x7fffd0015c30 at  "22:29:38" , Thread:  140736955062016 
Pushed note  0x7fff8401d2e0 at  "22:29:38" , Thread:  140736963454720 
Pushed note  0x7fffd8010d20 at  "22:29:38" , Thread:  140736946669312 
Removing note  0x7fffd0011770 at  "22:29:38" , Thread:  140736946669312 
Removing note  0x7fff8401e0c0 at  "22:29:38" , Thread:  140736946669312 
Removing note  0x7fff8401e0f0 at  "22:29:38" , Thread:  140736946669312 
Removing note  0x7fffd0011830 at  "22:29:38" , Thread:  140736955062016 
Removing note  0x7fff8401ce00 at  "22:29:38" , Thread:  140736963454720 
----------------- [ A block where notes are only removed ] -----------------
Removing note  0x7fffd8012cd0 at  "22:30:08" , Thread:  140736955062016 
Removing note  0x7fffd0015c30 at  "22:30:08" , Thread:  140736946669312 
Removing note  0x7fff84026a40 at  "22:30:08" , Thread:  140736061802240

So at the start it is only pushing notes into the list because they all last 10 seconds. Then 10 seconds after the first note was played we get a mixture of removing and pushing because new notes are still being played while the older ones are removed. Finally after a while notes are only removed because no new notes are being played.

The GIG Player plugin states on its envelope tab that no envelopes can be set for this kind of instrument so I assume that the envelopes are stored in the GIG format and that they are taken from there. The next thing that we therefore need to find out whether it is really a problem on our side or whether the GIG file used is just inefficient, i.e. if the length of 10 seconds for each sample is stored in that file.

michaelgregorius commented 6 years ago

I have replaced the original GIG file used in my testing (found here: https://drive.google.com/file/d/0B8RY2vt8PBm8NV9qN3Y2LU0tRUU/view) with another GIG file and cannot reproduce the problem anymore. Here's what I did:

  1. Download the archive of GIG files provided here: https://musical-artifacts.com/artifacts/77/g-town-church-samples-gig.zip (source: https://musical-artifacts.com/artifacts/77).
  2. Decompress the ZIP archive which holds mostly RAR files.
  3. Now decompress the file G-Town_Church_Dual_Snare_01.rar. This yields the file G-Town_Church_Dual_Snare_01.gig which we will use as a replacement.
  4. Open the original file Gigasampler Player Bug - real drums gig.mmpz provided with the Google Drive file mentioned above.
  5. Open both "GIGA Player" instances and set G-Town_Church_Dual_Snare_01.gig as their files.
  6. Set the root / base note (the rectangle above the instrument's keyboard) to A3 so that notes will sound when the song is played.
  7. Play the song.

Because the problem cannot be reproduced with the alternative GIG file I therefore conclude that it's a problem with the GIG file RGD Kit 5.gig and that we can mark this as "not our bug".

@gdd71 Can you please check the GIG file RGD Kit 5.gig or try to reproduce the steps above?

gdd71 commented 6 years ago

@michaelgregorius, "G-Town_Church_Dual_Snare_01.gig" works same as you.

Рroblem not with the GIG file.. entire collection "RGD Kit 2.3.4.5" (RGD Kit 5.gig has been submitted for testing ) and "FS-Gibson Les Paul Neck Pick-Up Electric Guitar Direct In" work great in Ardour - 20% CPU, same files in LMMS not. ...we are talking about high quality large size files I have also tested high quality SF2 long notes files and they work well with LMMS. Must somehow make a GIG player to work as a SF2 player without any restrictions, which would raise the "rating bar" a few steps up.. or another alternative .. I think to look for GIG to SF2 converter software to convert same RGD Kit fite to SF2 and test to see what the outcome will be..

gdd71 commented 6 years ago

So.. I converted same used "RGD Kit 5.gig" file to SF2 with "Extreme Sample Converter" software. Tested "RGD Kit 5 DRY.sf2" work great! I clone 15 tracks worked togeder.. without video recording works fine! Another feature that can be seen is that the high CPU load is available even with turn-off GIG plugins. I also provide a detailed video file for more clarity (download file for better 1920x1080 resolution - 15MB):

https://drive.google.com/file/d/0B8RY2vt8PBm8b294MWRfSHJXRmc/view

So I still think that this is 100% a bug in LMMS.

michaelgregorius commented 6 years ago

So I still think that this is 100% a bug in LMMS.

I did some further investigations. The problem is caused in part by the file and in part by LMMS.

The file's part

The file defines a release time of 10 seconds for the sample. This can be seen when setting a break point in the method ADSR::ADSR as defined in the file GigPlayer.cpp after the following statement:

release = region->EG1Release;

This sets the release of the ADSR to 9.99999905 seconds. Sometimes even the value 29.9999962 shows up, i.e. a release of half a minute. Please note that this value is read directly from the GIG file (documentation of EG1Release).

LMMS' part

The GigNote instances are only deleted once all their samples have been played. The samples in turn are only removed from a note when their ADSR is in the state "done". Once the key is released it takes 10 seconds for the ADSR to reach that state (due to the release defined above). This of course fits very nice with the observations made above that GigNote instances are removed with a delay of 10 seconds.

Potential solution

It seems that LMMS does not remove non-looping samples as soon as they have been played completely. It should be investigated if this can be fixed somehow.

michaelgregorius commented 6 years ago

There is already a test that checks if all samples have been played. It can be found on this line. However, it's coupled with the additional condition it->isRelease == true which never seems to become true (line above the other one).

If I remove the condition it->isRelease == true from the overall condition the performance problems are largely gone, i.e. with a debug build my CPU hovers around 20% which I find a bit heavy for just playing some samples. Also all samples play to their end. However, I don't understand yet why the condition is there. I assume that it is there to ensure that samples with loops are not removed if the current position happens to coincide with the last sample because the loop might go back from there.

gdd71 commented 6 years ago

Test File check info: Real.Giga.Drums.GIGA-DELiRiUM by Best Service http://www.bestservice.de Release Type ... GIGA Format ... Bin / Cue Release Date ... 01-19-2002 Release Size ...
CD1: 19 x 15mb CD2: 23 x 15mb CD3: 23 x 15mb This revolutionary 3 CD-ROM collection arms you with hundreds of first-class drum and cymbal samples, recorded with the finest instruments and equipment available. ....

\Real.Giga.Drums.GIGA-DELiRiUM\CD3\

RGD Kit 4 economic.gig 107221114 4/25/01 RGD Kit 4.gig 246889644 4/24/01 RGD Kit 5 economic.gig 72847296 4/24/01 RGD Kit 5.gig 154746960 4/24/01 4 file(s) 581,705,014 bytes

floft commented 6 years ago

My guess would also be it->isRelease == true was for when we're looping. I can't find my .gig files with loops though, so I can't test it.

@michaelgregorius As for expf, I did not think about that. That's definitely a better way.

Rossmaxx commented 4 months ago

@Veratil does #7162 fix this by any chance?

Veratil commented 4 months ago

@Veratil does #7162 fix this by any chance?

Most likely not.

michaelgregorius commented 4 months ago

The links are all dead. So it's a bit of a question if the issue can be reproduced at all. I cannot find the files on my local machine anymore.