Crash when no bank was loaded when using RealTime MIDI functions

[jpcima]

When I added OPNMIDI support in the realtime player, it would crash without warning unless a bank was loaded in its memory, which isn't very nice.

The command was

adlrt -p OPNMIDI

And the crash context

==26329== Invalid read of size 1
==26329==    at 0x13E938: OPNMIDIplay::realTime_NoteOn(unsigned char, unsigned char, unsigned char) (opnmidi_midiplay.cpp:1032)
==26329==    by 0x139665: opn2_rt_noteOn (opnmidi.cpp:746)
==26329==    by 0x10E5E2: void generic_play_midi<(Player_Type)1>(unsigned char const*, unsigned int) (common.cc:99)
==26329==    by 0x10D9F2: play_midi(unsigned char const*, unsigned int) (common.cc:194)
==26329==    by 0x10BFB7: process(void*, void*, unsigned int, double, unsigned int, void*) (rtmain.cc:28)
==26329==    by 0x4E44DFF: RtApiJack::callbackEvent(unsigned long) (in /usr/lib/librtaudio.so.6.0.0)
==26329==    by 0x4E45061: ??? (in /usr/lib/librtaudio.so.6.0.0)
==26329==    by 0x5067744: ??? (in /usr/lib/libjack.so.0.1.0)
==26329==    by 0x5066EE7: ??? (in /usr/lib/libjack.so.0.1.0)
==26329==    by 0x507F445: ??? (in /usr/lib/libjack.so.0.1.0)
==26329==    by 0x5C0008B: start_thread (in /usr/lib/libpthread-2.26.so)
==26329==    by 0x67F6E7E: clone (in /usr/lib/libc-2.26.so)
==26329==  Address 0xf7c is not stack'd, malloc'd or (recently) free'd

[Wohlstand]

I think, I must to drop an error on attempt to access out of range (That because of empty array of instruments), or initialize dummy GM-set on loading to prevent possible crashes in future. I'll need to apply same to ADLMIDI as you will take same crash when you will build ADLMIDI with disabled embedded banks macro and when you will not give a custom bank to it.

[jpcima]

I agree.

[jpcima]

Can OPNMIDI also have its embedded bank? forgot to ask

[Wohlstand]

I think, a good idea to provide at least one embedded bank just for a convenience, even I'll include super-basic GM bank (for now, I working on XG bank which is incomplete yet)

Trivia: as YM2612 was never used for MIDI playing, therefore was no any GM banks created, and I made it by myself from scratch.

[freq-mod]

I think, a good idea to provide at least one embedded bank just for a convenience, even I'll include super-basic GM bank (for now, I working on XG bank which is incomplete yet)

sorry for self-promotion, but I also made a GM bank for OPN2 (with partial, incomplete GS support). If you would be interested, you can use it - YM2612.zip

I tried to make it sound as realistic as it's possible, so if you plan to have embedded banks in OPNMIDI you can give it a shot ;)

[Wohlstand]

sorry for self-promotion, but I also made a GM bank for OPN2 (with partial, incomplete GS support). If you would be interested, you can use it - YM2612.zip

Nice bank! I have checked it out and seems some instruments are needing to be polished as there are sounding a bit incorrectly, for example, ride bell must have a longer release. Keep it up! :fox_face: :+1:

Initially I made OPNMIDI to use custom-only banks, and for now I'll put the bank into bank samples folder to allow everyone use it. I think, the embedding of banks must be optional and go while build process. Anyway, as it's not an ADLMIDI with tonn of very old banks are requiring to calculate sound delays, WOPN (and WOPL on ADLMIDI) now have those values already pre-calculated, so, is much faster to generate a database of embedded banks. And yeah, about of TR-808, I have made some experiment in my xg.wopn, take my latest version for a review :wink:

[Wohlstand]

Just now I have made a fix which makes smarter management of instruments storage: it always keeps them 256 entries (full GM set), but don't clears it completely when it has less than 1024 entries. And instead to clear entire array, fills instruments with blank entries marked by "NoSound" flag.

So, when you will initialize instance and will send RT events without of bank loading, you will no more crash :wink:

[jpcima]

No more crash confirmed. And as expected no sound either. That's fine enough for now.

[Wohlstand]

Yeah, as no banks loaded, no sound is a correct behavior.

[Wohlstand]

I closing this, as the trouble has been fixed and you have confirmed that it works :fox_face: