OPNMIDI support

Moderator: GZDoom Developers

User avatar
Wohlstand
Posts: 73
Joined: Sun Dec 17, 2017 3:22 am
Graphics Processor: nVidia with Vulkan support
Location: Moscow, Russia
Contact:

OPNMIDI support

Post by Wohlstand »

https://github.com/Wohlstand/libOPNMIDI

Hello!
It's my one alternative FM synthesizer that uses a YM2612 chip which was widely used in Sega Megadrive/Genesis game console and in the FM Towers computer. It's was my dream to take the full-featured GM-compatible MIDI player over that chip. I have used my libADLMIDI as a base for libOPNMIDI where I have replaced emulator of the OPL3 chip with OPN2 where I also replaced logic to work with the chip. Then I created a full-GM bank based on instruments imported from various VGM files of various games and instruments ported/adapted by me from existing banks for OPL3.

As you have implemented ADLMIDI, I thought, why not to also implement OPNMIDI?

I made that on this branch of my fork: https://github.com/Wohlstand/gzdoom/tree/opnMIDI

Feel free to pull this branch into your repo and review/correct the whole stuff :wink:

Note: Unlike ADLMIDI, OPNMIDI has no embedded banks, therefore as default bank I have transformed my xg.wopn bank into byte array inside of C++ header I have passed into opn2_openBankData() function. When you want to support external bank files, use opn2_openBankFile() function but don't call one that loads default bank.

EDIT October 9, 2018: If you are interested to check out the sounding from videos, I have made some set of showcases you can find here:
https://github.com/Wohlstand/libOPNMIDI/wiki/Showcasing
Last edited by Wohlstand on Mon Oct 08, 2018 5:19 pm, edited 3 times in total.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: OPNMIDI support

Post by Graf Zahl »

Why not? I made a few changes to this and will test it a bit.

I removed the MUS and XMI converters, just like I did for ADL and put the binary data file into GZDoom.pk3. There's really no need to convert such data to an array in source form.
I'll merge once I ran a few songs through it.

I also tweaked the volume of all MIDI synths to be a bit more even. This one was particularly loud because the boost factor here needs to be quite a bit lower than for ADL:
User avatar
Wohlstand
Posts: 73
Joined: Sun Dec 17, 2017 3:22 am
Graphics Processor: nVidia with Vulkan support
Location: Moscow, Russia
Contact:

Re: OPNMIDI support

Post by Wohlstand »

I think you gave me a good idea to allow excluding XMI and MUS converters by macros are allowing you to build a code without of taking MUS/XMI converters. That will be easier for you to update ADL/OPN into newer versions as I have many plans for them to add various interesting things are also can be useful for you. Also, I will allow disabling of the embedded MIDI player which is not needed as you are using own with the passing of MIDI events individually.

Yeah, I have wondered why OPNMIDI played too loud while I tested a game... So, I'm glad you have resolved that and explained to me why that happens.

EDIT: For ADLMIDI, I made that just now, so, 'easy-to-update' way for soon updates has been provided: https://github.com/coelckers/gzdoom/pull/446, Porting same changes to OPNMIDI now...
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: OPNMIDI support

Post by Graf Zahl »

Just so you know:

MSVC emits the following warnings when compiling your code:
7>C:\Programming\Doom-Dev\GZDoom\src\sound\opnmidi\opnmidi_midiplay.cpp(2024): warning C4244: 'initializing': conversion from 'const int64_t' to 'long', possible loss of data
7>C:\Programming\Doom-Dev\GZDoom\src\sound\opnmidi\opnmidi_midiplay.cpp(2056): warning C4267: '+=': conversion from 'size_t' to 'long', possible loss of data
7>c:\programming\doom-dev\gzdoom\src\sound\opnmidi\fraction.hpp(91): warning C4146: unary minus operator applied to unsigned type, result still unsigned (compiling source file C:\Programming\Doom-Dev\GZDoom\src\sound\opnmidi\opnmidi_midiplay.cpp)
7>c:\programming\doom-dev\gzdoom\src\sound\opnmidi\fraction.hpp(87): note: while compiling class template member function 'void fraction<uint64_t>::Optim(void)' (compiling source file C:\Programming\Doom-Dev\GZDoom\src\sound\opnmidi\opnmidi_midiplay.cpp)
7>c:\programming\doom-dev\gzdoom\src\sound\opnmidi\fraction.hpp(45): note: see reference to function template instantiation 'void fraction<uint64_t>::Optim(void)' being compiled (compiling source file C:\Programming\Doom-Dev\GZDoom\src\sound\opnmidi\opnmidi_midiplay.cpp)
7>c:\programming\doom-dev\gzdoom\src\sound\opnmidi\opnmidi_private.hpp(609): note: see reference to class template instantiation 'fraction<uint64_t>' being compiled (compiling source file C:\Programming\Doom-Dev\GZDoom\src\sound\opnmidi\opnmidi_midiplay.cpp)
7>c:\programming\doom-dev\gzdoom\src\sound\opnmidi\fraction.hpp(92): warning C4146: unary minus operator applied to unsigned type, result still unsigned (compiling source file C:\Programming\Doom-Dev\GZDoom\src\sound\opnmidi\opnmidi_midiplay.cpp)
User avatar
Wohlstand
Posts: 73
Joined: Sun Dec 17, 2017 3:22 am
Graphics Processor: nVidia with Vulkan support
Location: Moscow, Russia
Contact:

Re: OPNMIDI support

Post by Wohlstand »

Graf Zahl wrote:7>C:\Programming\Doom-Dev\GZDoom\src\sound\opnmidi\opnmidi_midiplay.cpp(2024): warning C4244: 'initializing': conversion from 'const int64_t' to 'long', possible loss of data
Fixed just now and appended to current Pull-Request! ;-)
Graf Zahl wrote:7>c:\programming\doom-dev\gzdoom\src\sound\opnmidi\fraction.hpp(91): warning C4146: unary minus operator applied to unsigned type, result still unsigned (compiling source file C:\Programming\Doom-Dev\GZDoom\src\sound\opnmidi\opnmidi_midiplay.cpp)
7>c:\programming\doom-dev\gzdoom\src\sound\opnmidi\fraction.hpp(87): note: while compiling class template member function 'void fraction<uint64_t>::Optim(void)' (compiling source file C:\Programming\Doom-Dev\GZDoom\src\sound\opnmidi\opnmidi_midiplay.cpp)
7>c:\programming\doom-dev\gzdoom\src\sound\opnmidi\fraction.hpp(45): note: see reference to function template instantiation 'void fraction<uint64_t>::Optim(void)' being compiled (compiling source file C:\Programming\Doom-Dev\GZDoom\src\sound\opnmidi\opnmidi_midiplay.cpp)
7>c:\programming\doom-dev\gzdoom\src\sound\opnmidi\opnmidi_private.hpp(609): note: see reference to class template instantiation 'fraction<uint64_t>' being compiled (compiling source file C:\Programming\Doom-Dev\GZDoom\src\sound\opnmidi\opnmidi_midiplay.cpp)
7>c:\programming\doom-dev\gzdoom\src\sound\opnmidi\fraction.hpp(92): warning C4146: unary minus operator applied to unsigned type, result still unsigned (compiling source file C:\Programming\Doom-Dev\GZDoom\src\sound\opnmidi\opnmidi_midiplay.cpp)
fraction.hpp-related is a very old trouble...I had a long fight with it on CLang on macOS which was offended me lot of times from that, it's a mean thing, as it's a template and it allows any type and no way to compile-time check signness which caused a warning. However, on macOS I beated this by adding of a conditional check for signness of type, however, as we can see, MSVC still offend us with that...

For example, this code (one reported by MSVC, and also by CLang on macOS):

Code: Select all

    nn1 = std::numeric_limits<inttype>::is_signed ? (num1 >= 0 ? num1 : -num1) : num1;
    nn2 = std::numeric_limits<inttype>::is_signed ? (num2 >= 0 ? num2 : -num2) : num2;
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: OPNMIDI support

Post by Graf Zahl »

The problem is very obviously that the code still gets compiled. I think that one's a perfect candidate for explicitly disabling a warning, which I'd normally want to avoid.

I made an execption in the OpenGL code because casting everything from double to float would just hurt the code's readability. That's the downside of warning about every potential little issue.

At work I have a project on iOS which had over 1000 warnings when I started on it. I'm now down to 100, but I cannot get further because I am stuck with an old version of Boost that just floods the list with deprecation stuff about the 'register' keyword. Hard to find real problems in such a wall of noise.
User avatar
Chris
Posts: 2940
Joined: Thu Jul 17, 2003 12:07 am
Graphics Processor: ATI/AMD with Vulkan/Metal Support

Re: OPNMIDI support

Post by Chris »

Wohlstand wrote:fraction.hpp-related is a very old trouble...I had a long fight with it on CLang on macOS which was offended me lot of times from that, it's a mean thing, as it's a template and it allows any type and no way to compile-time check signness which caused a warning. However, on macOS I beated this by adding of a conditional check for signness of type, however, as we can see, MSVC still offend us with that...

For example, this code (one reported by MSVC, and also by CLang on macOS):

Code: Select all

    nn1 = std::numeric_limits<inttype>::is_signed ? (num1 >= 0 ? num1 : -num1) : num1;
    nn2 = std::numeric_limits<inttype>::is_signed ? (num2 >= 0 ? num2 : -num2) : num2;
That's simply

Code: Select all

nn1 = std::abs(num1);
nn2 = std::abs(num2);
isn't it? std::abs has overrides for signed versus unsigned types given the input type, where the signed type has the unary - applied as needed and unsigned is always pass-through, so it never generates code for an unary - on an unsigned type.

EDIT:
More generically, with C++17 there is now 'if constexpr':

Code: Select all

if constexpr (std::numeric_limits<inttype>::is_signed)
    nn1 = (num1 >= 0 ? num1 : -num1);
else
    nn1 = num1;
With this, the given condition must be a compile-time constant and the false branch is not evaluated (it must be syntactically correct, but it doesn't have to be valid code), so in this instance, the compiler wouldn't detect an unary - on an unsigned type because that branch is unevaluated when is_signed is false.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: OPNMIDI support

Post by Graf Zahl »

But we do not use C++17 yet. Should we? Rachael and I have already moved on to Visual Studio 2017 and if we can agree to change the compiler requirement I have no problems to switch to a higher language spec. Of course then the project file needs to be changed to enable those in the compilers, because they now all have switched to a mandatory manual switch to enable the new features.
_mental_
 
 
Posts: 3812
Joined: Sun Aug 07, 2011 4:32 am

Re: OPNMIDI support

Post by _mental_ »

If this table is correct, switching to C++17 will drop most of GCC versions. A few minor things are still not supported even by the latest versions of all compilers we use.
IMHO the cost here overweights the benefit. Although, if we will have several cases when C++17 simplifies or improves an implementation greatly, move to the new standard will certainly make sense.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: OPNMIDI support

Post by Graf Zahl »

Ironic, isn't it? On the one hand Microsoft gets constantly lambasted for their lax standard adherence, but what holds us back is that some platforms seem to be stuck with long outdated compilers that are far worse... ;)
_mental_
 
 
Posts: 3812
Joined: Sun Aug 07, 2011 4:32 am

Re: OPNMIDI support

Post by _mental_ »

For Linux this means mostly that the default compiler cannot build GZDoom.

We had the same situation already with GCC 4.8 which was used in many conservative and long term support distros.
That version had no support of generic lambdas and so failed compilation. For inexperienced user this was a real blocker.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: OPNMIDI support

Post by Graf Zahl »

Like I said. We also had that problem with CMake which stuck to an obsolete syntax for far too long because - God beware - upgrading would throw those 'inexperienced' Linux users under the bus because they wouldn't be able to upgrade their software without professional help.

Sorry to be harsh, but I think that people who are not capable of upgrading their system to current standards shouldn't use something where this may become an issue.
User avatar
Rachael
Posts: 13530
Joined: Tue Jan 13, 2004 1:31 pm
Preferred Pronouns: She/Her
Contact:

Re: OPNMIDI support

Post by Rachael »

That's way different in Linux, though, Graf, and I am sorry to say but already you are a strong outspoken critic of how hard it can be to use that system.

For a regular developer like you and me, that's probably no problem, as it would be easy to build a new GCC from scratch (although it would take an hour or so) - for most users, that's a serious issue. (It would not be hard for you because the process would be exactly the same as on Mac which you are already familiar with - only difference being that you'd use ./configure instead of CMake)

I would say at least allow the most major distros to adopt a compatible GCC before dropping support for the old ones. In April (next month) that should be less of an issue at least for Ubuntu users, since the new Ubuntu is coming out then.

If we were talking niche distros and outliers this would be one thing - but we're talking the major players here, and those are in common use.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: OPNMIDI support

Post by Graf Zahl »

I'd say that's a fundamental systematic problem if it can be this bothersome to update some old software. By all intents and purposes Linux should have eaten into Windows's market share by now but these are precisely the issues that keep people away from it. And nobody seems to be willing to do something about it.
User avatar
Wohlstand
Posts: 73
Joined: Sun Dec 17, 2017 3:22 am
Graphics Processor: nVidia with Vulkan support
Location: Moscow, Russia
Contact:

Re: OPNMIDI support

Post by Wohlstand »

Graf Zahl wrote:But we do not use C++17 yet. Should we? Rachael and I have already moved on to Visual Studio 2017 and if we can agree to change the compiler requirement I have no problems to switch to a higher language spec. Of course then the project file needs to be changed to enable those in the compilers because they now all have switched to a mandatory manual switch to enable the new features.
libADLMIDI and libOPNMIDI are following C++98 to be buildable everywhere. Another reason of that - Android NDK with disabled C++11 by default, and also very limited support for C++11 too. Most of the developers are using it rather some nice things like CrystaX that gives modern toolchain with a full support of modern standards.
_mental_ wrote:GCC 4.8
Things seems very old/antique... As now I see most of distros now shipping GCC 5+. However, yeah, CentOS 7 still ship GCC 4.8 (CentOS 6 had GCC 4.3 which is REALLY unsupported antique).
Last edited by Wohlstand on Sun Mar 25, 2018 4:48 am, edited 1 time in total.
Post Reply

Return to “Closed Feature Suggestions [GZDoom]”