Malvineous / dro2midi

Convert DRO files into MIDI
11 stars 5 forks source link

Overzealous instrument detection #3

Closed snoopin1 closed 2 years ago

snoopin1 commented 2 years ago

The method DRO2MIDI uses to detect OPL2 instruments seems to be different from other OPL tools.

As an example, XCITY.IMF from Bio Menace is shown to contain 6 distinct instruments when opened with tools like Wohlstand's OPL3BankEditor, as shown below:

xcity ins import

The instruments can be individually exported to SBI, although this is a slow process. Using DRO2MIDI's -s option to export instruments is much faster; however, this results in a huge number of SBIs being exported for just one song. For XCITY, DRO2MIDI creates 46 SBIs. The SBIs all have different hashes, meaning "duplicates" (in terms of sound) cannot be automatically removed. I am curious as to why other tools detect far fewer instruments in OPL2 songs, even songs that do use more than one instrument on the same OPL2 channel (like some of Bio Menace's other songs).

Malvineous commented 2 years ago

You can see the code that checks whether the instrument has changed in dro2midi.cpp:564. It's been a long time since I looked at this but it looks like it's checking all registers except for the note pitch, when it should probably skip checking the volume/velocity registers too. Likely this is one possible source for the extra instruments - they are all the same but with a different volume.

snoopin1 commented 2 years ago

Disabling the check for register 40 on the carrier seems to have made the instrument detection match OPL3BankEditor's. Tested with MUTCMPUT.IMF, which OPL3BankEditor detects as containing 13 instruments (7 of which are played on OPL2 channel 5).

Edit - some information that may be useful for other users: Making exported SBIs automatically have offset 0x27 set to 0x00 (or any other volume value) also makes it possible to identify duplicate SBIs from different song files (the same instrument may appear in different game songs but at different volumes). This also requires that the internal instrument titles be the same.

rofl0r commented 2 years ago

@PoniestPonypony would you care to open a PR with your improvement ? i guess others would also benefit from it.

snoopin1 commented 2 years ago

@rofl0r My change is only necessary for songs that make heavy use of instrument velocity variations, like Bobby Prince's IMFs.

rofl0r commented 2 years ago

well, could you at least attach your diff here so other people can follow along ?

snoopin1 commented 2 years ago

Simply removing the 2 instances of the following code from the lines Malvineous highlighted will fix the SBI output: difference(a.reg40[1], b.reg40[1], 1) +

The code that writes the SBIs is at dro2midi.cpp:619. Here you can change dro2midi.cpp:629 to just snprintf(title, 32, "dro2midi"); so that duplicate instruments from different songs can be easily detected (if they have the same volume).

Lastly, for best duplicate detection results (if you have exported several SBIs from multiple songs), you can use a script to automatically clear the lower 6 bits of the byte at offset 0x27 (OPL base register 0x43) in all SBIs. This will give all instruments a carrier volume value of 0.00 dB. This should be the only register that needs to be modified, as it only affects the instrument's volume and not its timbre.

rofl0r commented 2 years ago

that sounds complicated. can't you just run git diff in your checkout and paste the output here ?

snoopin1 commented 2 years ago

Edit: DRO2MIDI's SBI export function does not seem to work with the the 1-operator rhythm mode instruments (everything but the bass drum). While the code below improves the melodic instrument detection, it still does not allow for 1-op rhythm instrument export. I have confirmed this with Apogee's Monster Bash.


@@ -578,7 +578,6 @@ long compareinstr(INSTRUMENT& a, INSTRUMENT& b, RHYTHM_INSTRUMENT ri)
                difference(a.reg20[0], b.reg20[0], 2) +
                difference(a.reg20[1], b.reg20[1], 2) +
                difference(a.reg40[0], b.reg40[0], 1) +
[-              difference(a.reg40[1], b.reg40[1], 1) +-]
                difference(a.reg60[0], b.reg60[0], 2) +
                difference(a.reg60[1], b.reg60[1], 2) +
                difference(a.reg80[0], b.reg80[0], 2) +
@@ -609,7 +608,6 @@ long compareinstr(INSTRUMENT& a, INSTRUMENT& b, RHYTHM_INSTRUMENT ri)
            return
                difference(a.eRhythmInstrument, ri, 4) +
                difference(a.reg20[1], b.reg20[1], 2) +
[-              difference(a.reg40[1], b.reg40[1], 1) +-]
                difference(a.reg60[1], b.reg60[1], 2) +
                difference(a.reg80[1], b.reg80[1], 2) +
                difference(a.regE0[1], b.regE0[1], 1);
@@ -626,7 +624,7 @@ void writesbi(const char* filename, int instrno, int chanOPL) {
    } else {
        fwrite("SBI\x1a", sizeof(char), 4, f_sbi);
        memset(title, 0, 32);
        snprintf(title, 32, [-"dro2midi_%03d", instrno);-]{+"dro2midi");+}
        fwrite(title, sizeof(char), 32, f_sbi);
        unsigned char instr[16];
        memset(instr, 0, 16);
rofl0r commented 2 years ago

thanks for your effort. since what you pasted doesn't quite look like a real diff, would you confirm that the below is what git diff really outputs for your changes ?

diff --git a/dro2midi.cpp b/dro2midi.cpp
index af5fe45..816c8ee 100644
--- a/dro2midi.cpp
+++ b/dro2midi.cpp
@@ -578,7 +578,6 @@ long compareinstr(INSTRUMENT& a, INSTRUMENT& b, RHYTHM_INSTRUMENT ri)
                difference(a.reg20[0], b.reg20[0], 2) +
                difference(a.reg20[1], b.reg20[1], 2) +
                difference(a.reg40[0], b.reg40[0], 1) +
-               difference(a.reg40[1], b.reg40[1], 1) +
                difference(a.reg60[0], b.reg60[0], 2) +
                difference(a.reg60[1], b.reg60[1], 2) +
                difference(a.reg80[0], b.reg80[0], 2) +
@@ -609,7 +608,6 @@ long compareinstr(INSTRUMENT& a, INSTRUMENT& b, RHYTHM_INSTRUMENT ri)
            return
                difference(a.eRhythmInstrument, ri, 4) +
                difference(a.reg20[1], b.reg20[1], 2) +
-               difference(a.reg40[1], b.reg40[1], 1) +
                difference(a.reg60[1], b.reg60[1], 2) +
                difference(a.reg80[1], b.reg80[1], 2) +
                difference(a.regE0[1], b.regE0[1], 1);
@@ -626,7 +624,7 @@ void writesbi(const char* filename, int instrno, int chanOPL) {
    } else {
        fwrite("SBI\x1a", sizeof(char), 4, f_sbi);
        memset(title, 0, 32);
-       snprintf(title, 32, "dro2midi_%03d", instrno);
+       snprintf(title, 32, "dro2midi");
        fwrite(title, sizeof(char), 32, f_sbi);
        unsigned char instr[16];
        memset(instr, 0, 16);
snoopin1 commented 2 years ago

I used the --word-diff option and outputted the result to a text file (I don't have much experience with posting comments on GitHub, hence my post was not the most readable). But yes, those are the changes I made.

rofl0r commented 2 years ago

thanks for the confirmation. you could just run git diff > file.txt and then paste its contents into a formatted block using the triple-backtick syntax, followed by diff for the opening statement, like this:

```diff
...
```

the diff filetype association makes it so you get that nice syntax coloring.