Add "libvgm" target, all sub-libraries combined.

[jprjr]

Hi there, I ran into an issue recently -

I have a custom build of Music Player Daemon that uses libvgm, as well as kode54's fork of gme (the branch with vgmplay integrated). The intent is to use gme for non-vgm chiptunes, libvgm for everything else. The reason for the vgmplay branch - it seems to be the most up-to-date.

I link everything as shared libraries, and there's some conflicting symbol names, so I wind up having libvgm use functions from libgme or vice-versa.

On GCC, I can use the -Bsymbolic-functions flag to specify symbols should be resolved from the shared library first. If I do it on both libraries, I can prevent libvgm-emu functions from replacing gme functions.

But, since libvgm is split up, it doesn't work the other way around - libgme functions are able to replace libvgm-emu functions, since I link to libvgm-player, which then tries to resolve symbols with already-loaded symbols, before loading libvgm-emu. I wind up having incorrect rendering and/or segfaults.

This PR adds a single-library target, libvgm, that's everything in one library. When I build a shared library with the -Bsymbolic and -Bsymbolic-functions flags, I can load both libvgm and libgme, without conflicts.

I mentioned this in #59 , and thought I had solved the issue with just the -Bdynamic switches. I had a VGM file play incorrectly, fired up lldb and found this:

Process 534284 stopped
* thread #5, name = 'decoder:vgm', stop reason = breakpoint 1.2771
    frame #0: 0x00007ffff4dfb320 libgme.so.0.7.0`device_start_daccontrol(_info=0x00007fffeb0b91f0, param=0x00007fffeb0b9200, SampleRate=-209236032) at dac_control.c:285:1
(lldb) bt
* thread #5, name = 'decoder:vgm', stop reason = breakpoint 1.2771
  * frame #0: 0x00007ffff4dfb320 libgme.so.0.7.0`device_start_daccontrol(_info=0x00007fffeb0b91f0, param=0x00007fffeb0b9200, SampleRate=-209236032) at dac_control.c:285:1
    frame #1: 0x00007ffff3867d06 libvgm-player.so`VGMPlayer::Cmd_DACCtrl_Setup(this=0x00007fffe001ff20) at vgmplayer_cmdhandler.cpp:866:35
    frame #2: 0x00007ffff386a2a1 libvgm-player.so`VGMPlayer::ParseFile(unsigned int) at vgmplayer.cpp:1676:16
    frame #3: 0x00007ffff386a250 libvgm-player.so`VGMPlayer::ParseFile(this=0x00007fffe001ff20, ticks=<unavailable>) at vgmplayer.cpp:1666
    frame #4: 0x00007ffff386a60a libvgm-player.so`VGMPlayer::Render(this=0x00007fffe001ff20, smplCnt=2048, data=0x00007fffeb0b9320) at vgmplayer.cpp:1625:12
    frame #5: 0x000055555562caf8 mpd`vgm_stream_decode(DecoderClient&, InputStream&) at VgmDecoderPlugin.cxx:266:17
    frame #6: 0x00005555555aa7c8 mpd`decoder_stream_decode(DecoderPlugin const&, DecoderBridge&, InputStream&, std::unique_lock<std::mutex>&) at DecoderPlugin.hxx:203:16
    frame #7: 0x00005555555aada4 mpd`decoder_run(DecoderControl&) at Thread.cxx:345:31
    frame #8: 0x00005555555ab670 mpd`DecoderControl::RunThread() at Thread.cxx:568:15
    frame #9: 0x00005555555da3fd mpd`Thread::ThreadProc(void*) [inlined] Thread::Run(this=<unavailable>) at BindMethod.hxx:91:10
    frame #10: 0x00007ffff3880299 libpthread.so.0`start_thread + 233
    frame #11: 0x00007ffff2f9d053 libc.so.6`__clone + 67
(lldb) ^D

Turns out using symbolic linking isn't quite enough to prevent conflicts with libgme, but with the single-library approach, the issue goes away

[jprjr]

Other ideas, that I'm happy to help with -

Prefix/rename symbols to prevent collisions. This seems like a big undertaking, and would probably break a lot of code.

Since vgm-player depends on vgm-emu, maybe vgm-player could just include the objects from emu (and maybe utils), and not actually link against it? If somebody wants to link against just emu, they still could.

[jprjr]

Weird, I could swear this was working on my machine, but doing a fresh checkout, something seems broken

[kode54]

If you are using libvgm, you do not need the vgmplay branch of GME. In fact, for other reasons, you'd probably be better off using the other fork of GME, which has had several non-vgmplay changes, while mine has pretty much stagnated in most other features.

[jprjr]

I'll have to run some comparisons again, I recall there being some differences that your fork was better at (like famicom expansion audio support in nsfs).

My sort-of goal is to write plugins for music player daemon to reach feature-parity with your foobar plugins, which I consider the gold standard

[kode54]

Famicom extension mapper audio for NSFs should be similar to mine, and if not, they'd probably love to have feature parity there. I did implement both full Sunsoft mapper waveform length control, plus a quick hack that can try to detect and permanently repair NSF/E rips that predate emulators supporting this actual hardware behavior, by patching them to use the correct lengths. The chip support is in the vgmplay branch, but shouldn't technically be any more strictly licensed than the library it was patched into, I hope. The patcher is in the foo_gep sources, and should be easily adaptable to any platform C++ code can be compiled on, but is also simple enough to be converted to plain C. Or scripting languages.

[ValleyBell]

To be honest I'm a bit unsure about the "unified" libvgm library.

The initial idea was to have all parts (audio / emu / player) available separately to be easily reuseable. So making it one thing again feels a bit weird.

I also remember that I already removed some "objlib" stuff from libvgm in the past. (was it the attempt to do a static+shared build?) So adding object libs again leaves some uneasy feelings to me, as it clutters the project files again. (that applies especially to generated MSVC projects)
However, I had no objective reason against this PR, so I'll probably just merge it in a few days.

There is just one thing I want: No separate vgm folder, please. Please integrate the additional vgm target into the root CMakeLists.txt instead.

The way-to-go is definitely to prefix all exported functions, even though it will break existing code.
I definitely want to do that when splitting the "audio" part off of the rest of libvgm. (I haven't started with that yet mainly due to me not having a good name for the audio lib.)

[jprjr]

Maybe this should just be closed. I think adding a prefix to the c functions may be the right solution. I can also write a linker script to hide symbols for my situation

[ValleyBell]

I'll leave the decision to you, whether you want to close it or not.
In the latter case, just do the "all-in-one" target in the main CMake file and I'll merge it.