[Patch] Add NukedOPL

Discussion of development and patch submission.
Post Reply
Battler
Posts: 793
Joined: Sun 06 Jul, 2014 7:05 pm

[Patch] Add NukedOPL

Post by Battler »

The three required files are attached. I also fixed the SB OPL volume so it uses the correct level (provided by James-F) when using both types of OPL. The nukedopl.* files go to src/dosbox . I also added the ability to not use NukedOPL which will make it fall back to DOSBox OPL3 emulation. NukedOPL is enabled by default.
Attachments
nukedopl.h
(2.39 KiB) Downloaded 352 times
nukedopl.cpp
(38.45 KiB) Downloaded 338 times
nukedopl.patch
(11.81 KiB) Downloaded 342 times
User avatar
James-F
Posts: 88
Joined: Tue 30 May, 2017 10:26 am

Re: [Patch] Add NukedOPL

Post by James-F »

Thank you very much Battler!
Please continue contributing to PCem, your skills are invaluable.
ecksemmess
Posts: 183
Joined: Wed 18 Mar, 2015 5:27 am

Re: [Patch] Add NukedOPL

Post by ecksemmess »

Good thinking including that option to switch back to the original OPL3 emulation, with NukedOPL as the default. Last I checked, the speed hit was considerable enough to be an issue when emulating faster machines, but NukedOPL's improvements are major enough to demand its use in all other circumstances IMO. This is the right solution.
User avatar
James-F
Posts: 88
Joined: Tue 30 May, 2017 10:26 am

Re: [Patch] Add NukedOPL

Post by James-F »

On my machine nukedopl only adds 2% to cou time wheb emulating p75, what 'speed hit' are you talking about?

I definitely agree that any performance hit is justifiable to have a bit perfect emulation of the OPL3.
ecksemmess
Posts: 183
Joined: Wed 18 Mar, 2015 5:27 am

Re: [Patch] Add NukedOPL

Post by ecksemmess »

Depending on your host machine and guest configuration, it's possible to get a very significant speed hit from NukedOPL. The accuracy is excellent, but of course that comes at a price. Battler's patch makes NukedOPL the default but leaves open the option to use the previous, less-accurate OPL emulation, which will be necessary for people who are trying to run games that are near the borderline of what their hardware can handle. It's the best of both worlds. I think he made the right call.
Battler
Posts: 793
Joined: Sun 06 Jul, 2014 7:05 pm

Re: [Patch] Add NukedOPL

Post by Battler »

On the emulated Pentium 75, I used Little Big Adventure 2 as reference. With NukedOPL disabled, it runs at solid 100%, while with NukedOPL enabled, it runs at 85-90%. That's a 10-15% difference. And the game doesn't even use any sort of synth music, so the mere attachment of NukedOPL to the guest machine reduces performance.
User avatar
SarahWalker
Site Admin
Posts: 2054
Joined: Thu 24 Apr, 2014 4:18 pm

Re: [Patch] Add NukedOPL

Post by SarahWalker »

Committed at rev 756. I moved the configuration option into the per-device config, as there's no reason for it to be global.

BTW, if you're going to assign 'copyright holders' to each file, could you at least try to get it right? I don't see why the DOSBox team get 'copyright' over sound_dbopl.cc/h, given that I wrote it. Interfacing with someone else's code doesn't give them copyright over the interface code.
Battler
Posts: 793
Joined: Sun 06 Jul, 2014 7:05 pm

Re: [Patch] Add NukedOPL

Post by Battler »

- SarahWalker: Yeah, I messed up there, sorry. :p
User avatar
leilei
Posts: 1039
Joined: Fri 25 Apr, 2014 4:47 pm

Re: [Patch] Add NukedOPL

Post by leilei »

FYI dosbox/nukedopl.h and dosbox/nukedopl.cpp are missing from the commit
User avatar
James-F
Posts: 88
Joined: Tue 30 May, 2017 10:26 am

Re: [Patch] Add NukedOPL

Post by James-F »

Yes those two are missing from the source, but when included everything works as expected; big big thank you Sarah..
I don't understand why the "per-device" option because the user can't actually use more then one device, but oh well.

There are crashes with the latest commit when minimizing the window to taskbar and returning to it, I think D3D related.
But, there is still a problem with oplenAL32.dll, I'll report in the sound lag thread.
Greatpsycho
Posts: 151
Joined: Tue 22 Mar, 2016 10:03 am
Location: Korea
Contact:

Re: [Patch] Add NukedOPL

Post by Greatpsycho »

This patch solves compile error on linux.
Attachments
Makefile.in.patch
Patch to solve compile error on linux.
(13.26 KiB) Downloaded 311 times
User avatar
leilei
Posts: 1039
Joined: Fri 25 Apr, 2014 4:47 pm

Re: [Patch] Add NukedOPL

Post by leilei »

Those wide humming synths in Doom E1M2 and E1M5 sound strange.......
User avatar
James-F
Posts: 88
Joined: Tue 30 May, 2017 10:26 am

Re: [Patch] Add NukedOPL

Post by James-F »

leilei wrote:Those wide humming synths in Doom E1M2 and E1M5 sound strange.......
Compared to what?
User avatar
leilei
Posts: 1039
Joined: Fri 25 Apr, 2014 4:47 pm

Re: [Patch] Add NukedOPL

Post by leilei »

NukedOPL elsewhere.

but I realize it's apparently a bug in 1.666. Sounds normal in 1.9 and 1.2
User avatar
SarahWalker
Site Admin
Posts: 2054
Joined: Thu 24 Apr, 2014 4:18 pm

Re: [Patch] Add NukedOPL

Post by SarahWalker »

Added missing files at rev 757. Not sure how they got missed off - I definitely added them to the commit...
User avatar
James-F
Posts: 88
Joined: Tue 30 May, 2017 10:26 am

Re: [Patch] Add NukedOPL

Post by James-F »

A small fix is still needed.
The dbopl value for OPL3 in sound_sb.c remained 55000, it should be 51000 like opl2. -> 47000: 51000.
Battler probably miss typed it manually.
Battler
Posts: 793
Joined: Sun 06 Jul, 2014 7:05 pm

Re: [Patch] Add NukedOPL

Post by Battler »

- James-F: Yeah, I forgot to update that one. Yes, it's supposed to be 47000 : 51000.
User avatar
SarahWalker
Site Admin
Posts: 2054
Joined: Thu 24 Apr, 2014 4:18 pm

Re: [Patch] Add NukedOPL

Post by SarahWalker »

Fixed in rev 761. Also fixed the AWE32, which was broken in the original patch.
User avatar
James-F
Posts: 88
Joined: Tue 30 May, 2017 10:26 am

Re: [Patch] Add NukedOPL

Post by James-F »

Battler, I should point that DOSBox uses the dbOPL core for OPL2 and OPL3
NukedOPL patch for DOSBox is like dbOPL applied to both OPL2 and OPL3.
According to NukedOPL author: http://www.vogons.org/viewtopic.php?f=4 ... 27#p590414
Since you ported the patch to PCem, I suggest enabling NukedOPL on OPL2 too, and setting its level to 47000 like OPL3.

Also, Doom1/2 doesn't seem to recognize OPL3 with "SET DMXOPTION=-opl3" environment variable, but it does in DOSBox and real hardware.
Battler
Posts: 793
Joined: Sun 06 Jul, 2014 7:05 pm

Re: [Patch] Add NukedOPL

Post by Battler »

- James-F: The NukedOPL author even told you using NukedOPL for OPL 2 can break things since NukedOPL only emulates the OPL 3.
User avatar
James-F
Posts: 88
Joined: Tue 30 May, 2017 10:26 am

Re: [Patch] Add NukedOPL

Post by James-F »

No, you misunderstood.
DOSBox uses OPL3 core with ANY core (Fast, Compat, Nuked) to emulate OPL2, Dual-OPL2 and OPL3.
In other words, it doesn't have special OPL2 emulation, it uses the same core as for the OPL3, so when SBPro1 is selected DOSBox simply translates the Dual-OPL2 commands to OPL3 addresses.
The only thing that might be broken is Dual-OPL2 in SBPro1 that has two independent Rhythm channels but the OPL3 has only one, and so does DOSBox has only one.
I am not sure how Sarah implemented the dbOPL core in PCem, but if it it's exactly like DOSBox, then the above statement is true.
Since PCem uses dbOPL core, NukedOPL should be used as one of the cores in OPL2 and OPL3 just like DOSBox, no separation is needed.
Post Reply