Fix non-relative samples

[tresf]

Closes #3533

This adds a fixPaths function to DataFile to attempt to:

• Provide an upgrade path for projects created with 1.2.0-rc1 or 1.2.0-rc2
• Fix non-relative samples which will occur if a project containing external samples is accidentally saved using a custom lmms working directory prior to setting the preference.

Easiest way to test this out is to re-save an existing 1.2.0-rc2 project as an .mmp file (without the "z") and validate the full paths for built-in samples or lmms working directory samples have been corrected.

[Umcaruje]

Did you test out if this affects the loading speed of projects? Running this on every project load seems to be a bit much from a quick glance.

[tresf]

DOM parsing is slow in general so I would expect a slight uptick in load time.

A version check can be added to mitigate this, but then we would move it back to the upgrade area. This would remove the usefulness it may offer in edge cases where the working directory is mucked up so it is a trade-off that requires a decision.

[tresf]

This function would eventually handle the same problem with VSTs and other plugins suffering identical edge cases.

If someone wants to benchmark it please do.

[tresf]

Did you test out if this affects the loading speed of projects?

Yes. There does not seem to be a measurable difference. Note, tests were performed against Unfa - Spoken, which contains over 50 tracks, but very few sample tracks.

I tried again with one of my own projects containing about 30 tracks, 12 of which containing sample or soundfonts in the "lmms working directory", and the upgrade benchmark had identical results to Unfa - Spoken.

Before this PR:

#### START UPGRADING:  "21:13:19"
#### FINISH UPGRADING:  "21:13:19"
DELTA:  0s

After this PR:

#### START UPGRADING:  "21:08:34"
#### FINISH UPGRADING:  "21:08:34"
DELTA: 0s

In both unit tests, the actual "Loading" of the project was not influenced by the upgrade functions (or in this case, the time to execute upgrade() combined with the time to execute fixPaths() function).

Although projects load much slower visibly, these benchmarks are real only measuring DOM parsing -- accounting for only a fraction of a second of the track loading using QTime::currentTime().toString(). Apparently it's not THAT slow after all. :) Both timestamps spit to the console long before LMMS actually load the physical tracks and plugins themselves, which is expected.

[tresf]

Ok, I noticed my benchmarks were in seconds.... Here's a proper benchmark in milliseconds...

Without this PR:

#### START UPGRADING:  1494207446807
#### START UPGRADING:  1494207446830
DELTA: 4ms
SECOND TEST: 4ms

With this PR:

#### START UPGRADING:  1494207446807
#### FINISH UPGRADING:  1494207446830
DELTA: 23ms
SECOND TEST: 12ms

This is an 3x-6x load time, resulting in an increase in about 8-18ms; about 1/50th - 1/100th of a second on a modern (i7) machine and 50+ tracks.

[curlymorphic]

20ms in a non-time critical operation, compared to non-working project files, that may take a user hours to relocate the samples, if the problem is no apparent, It's a no brainer

[Umcaruje]

Or perhaps we could put this in a context menu somewhere, though that's gonna be hard to find if you don't know what you're looking for.

I guess 20ms really isn't that much overload added. Did you try on some really sample intensive project that had this problem, e.g. was saved with 1.2 with broken paths, or was that the 2nd project you used? I couldn't make it out from the comment.

Real nice work on this issue though, we can release RC3 after this and other 3 issues in the project are merged 👍

[tresf]

Did you try on some really sample intensive project that had this problem

Not overly, no, but the overhead shouldn't be any different. The logic pretty much says...

for (every single instrument) {
     if (the src tag is not relative) {
          // make it relative;
     }
}

DOM (XML/HTML) parsing is generally more CPU intensive than a single QString/QFileInfo operation, so the benchmarks should not increase much more with different types of instruments. CPU time is going to be spent in the otherwise useless loop over elements in the DOM.

Some more information worth noting... these projects take a considerable amount of time to load. Unfa-spoken takes about 9 seconds to load on my i7 desktop, so the overall impact (as @curlymorphic suggests) would be 20ms more, or to put plainly, 9,000ms would become 9,020ms, or 1/20th of a blink of an eye.

[lukas-w]

I had a look at some of the instrument's loading code and it appears that tryToMakeRelative is already called on every load:

• GigPlayer: GigPlayer::loadSettings and GigPlayer::openFile
• AFP: audioFileProcessor::loadSettings, audioFileProcessor::setAudioFile, SampleBuffer::setAudioFile
• SampleTCO: SampleTCO::loadSettings, SampleTCO::setSampleFile

Similar code can be found in instruments Patman and SF2 Player.

A quick test on current stable-1.2 confirmed this at least with sample tracks: Opening a file with broken paths will call tryToMakeRelative and fix the path on loading, saving it will write the correct relative paths (when using the same installation the file was created with.

So it appears this PR may not even be needed…

[tresf]

So it appears this PR may not even be needed…

Confirmed. My own unit tests passed as well. Closing.