Add MSVC support for GigPlayer (rework)

[Veratil]

Updated form of #6595 as there are too many conflicts to try and resolve in that branch now (I tried for ~1 hour).

Notes:

• VLAs have been converted to std::array, in the event a Giga sample is loaded which will overload any buffer a std::runtime_error will be thrown; not the best solution but worked the few test gig files I have at least

[Rossmaxx]

@Monospace-V seems to have a few gig files and is looking into the spec. Can you test this one?

[Monospace-V]

One of my files seems to make it crash for some reason. I may look into it when I have more time, but I have multiple gig files, and my file "Tic Tok Men RetroDrums 1.gig" tends to cause issues if I switch to it after using a different gig file, particularly if I am on a non-zero patch before I switch. In this case LMMS seems to tend to crash.
File is available on musical artifacts as RetroDrums One. Would be useful if someone else could check this out since I'm crunched for time presently? If not, I may get to it though, eventually.

Other than this, at first glance, this seems to support pretty decently.

Thank you for altering me to the pr, Rossmaxx. I am simply trying to understand the spec tho I don't actually have much knowledge I'm trying to learn SF2 first but yes. I have a bunch of gig files and this is something I care about

[Rossmaxx]

@Monospace-V can you send that file in here, in a zip.

[Monospace-V]

can you send that file in here, in a zip.

Unfortunately not. Please find it here: https://www.tictokmen.com/sample/

[Veratil]

@Monospace-V looks like the sample buffer isn't large enough at 512, I will bump this up again.

[Veratil]

Oh actually I missed updating the size check (me being dumb and not using a constant)

[Veratil]

After bumping the size here there's no error anymore. 👍

[Monospace-V]

After bumping the size here there's no error anymore. 👍

I'm just wondering what happens if it's even bigger? Like I'm not saying it should be bigger and this might be off topic discussion but is it not possible it could be bigger, or, what stopping it from needing to be larger? Perhaps I don't understand enough.
The gig file format was created for huge instruments, and the GigaSampler just loaded the entire thing into ram iirc

[Veratil]

I'm just wondering what happens if it's even bigger? Like I'm not saying it should be bigger and this might be off topic discussion but is it not possible it could be bigger, or, what stopping it from needing to be larger?

It "slows down" from being on the heap now instead of the stack. We're safe from stack overflows in the worst case situations, but we lose the cache locality of the array. The larger the array the more needs to be moved around. The buffer is at 1Kb right now.

[Rossmaxx]

Would it be a good idea to create an arbitrarily large buffer but keep a todo comment and cleanup the calculation in a follow up pr (or do a refactor in this pr itself)

[Veratil]

There's still some cleanup in this PR left to do.

[DomClark]

This review is just of the CMake, as it sounds like the C++ changes are still in progress.

The find module isn't working on non-MSVC configurations (I've left comments on it as to why), but this doesn't cause the CI to fail as GIG support is an optional feature. It's out of scope here, but I think we need some way to catch this sort of thing automatically, to avoid problems in the future.

[Monospace-V]

It "slows down" from being on the heap now instead of the stack. We're safe from stack overflows in the worst case situations, but we lose the cache locality of the array. The larger the array the more needs to be moved around. The buffer is at 1Kb right now.

I don't quite understand, I guess I am not quite at that level yet [: It's all right. I plan to test this pr out again later with the rest of my gig files, try to induce anything. I hope I have enough time. I have a few more gig files I want to try, in the event they induce a crash somehow.

[Veratil]

Rebased on master + address Dom's review

[Monospace-V]

I can stil get this to crash using one of my gig files.
I have a file called maestro_concert_grand_v2.gig.

These are the patches.
Patches number 4 (effects) and 5 (mcg_key_release) appear to be silent for reasons I don't quite know.
Switching to patch 5 freezes and crashes LMMS.
It can be downloaded here: https://musical-artifacts.com/artifacts/73?class=related-link

[Veratil]

Alright, I'll look into them. Thanks!

[Veratil]

I confirmed I can't hear anything with patch 4, and patch 5 overloads the bounds of the arrays (by a lot, 38k sized array for example). I'm testing another idea for the buffers. 👍

[Veratil]

@Monospace-V so I've found out that when the buffers are actually sized properly, patch 4 and 5 actually do make sounds.
Patch 4: Turn gain and volume to max and play C0, D0, or E0
Patch 5: Turn gain and volume to max and play any note between A0 and C8, upon release of the key you should hear it
Patch 6: Turn gain and volume to max and play any note between A0 and C8 and it should be the same as Patch 5 but ever so slightly louder

[Veratil]

Well apparently we're not actually C++17 ready on linux and mingw builds.

[Veratil]

Looking around, seems like std::for_each_n was not added until GCC 8 GCC 9.3 (I searched through git tags until I found it). Fun stuff.

[Veratil]

mingw failures are because the CI running 18.04 has installed libgig-4.0.0, which apparently is right before 4.0.0.svn3 where RIFF::file_offset_t was added (what gig::file_offset_t is typedef'd from).

[Rossmaxx]

mingw failures are because the CI running 18.04 has installed libgig-4.0.0, which apparently is right before 4.0.0.svn3 where RIFF::file_offset_t was added (what gig::file_offset_t is typedef'd from)

Can't you use an ifdef for now, with a todo comment, atleast to get the CI working.

[Veratil]

Can't you use an ifdef for now, with a todo comment, atleast to get the CI working.

The issue is getting the libgig version. There is no define in the code to tell it.