Opened 18 years ago

Closed 18 years ago

Last modified 17 months ago

#7921 closed patch (fixed)

MIDI "jumbo" patch

Reported by: SF/mattjon Owned by: SF/ender
Priority: normal Component: Audio: MT32
Keywords: Cc:
Game:

Description

Hi,

This is a new MIDI patch for ScummVM. It primarily consists of chuzwuzza's latest MIDI patch as of yesterday, plus some MT32 translation and fixes which now provide music of some sort under MI2.

This also provides a little extra debugging to explain SoundEngine::findTag errors (which are generally caused by scummvm looking for the wrong tag).

This is a unified diff against this morning's CVS.

Regards,

Matt Johnson (MattJ on IRC)

Ticket imported from: #484547. Ticket imported from: patches/26.

Attachments (1)

jumbo-midi.patch (19.3 KB ) - added by SF/mattjon 18 years ago.
MIDI "jumbo" patch.

Download all attachments as: .zip

Change History (9)

by SF/mattjon, 18 years ago

Attachment: jumbo-midi.patch added

MIDI "jumbo" patch.

comment:1 by SF/chuzwuzza, 18 years ago

Matt, not bad, except for a few minor flaws I can see

Firstly, you have removed all patch changes for the percussion channel (10). While mostly, all patch change events on channel 10 seem to be 0 (standard drum kit), what if in some weird song, the composer had put in to use the orchestral drum kit? I'll admit that's unlikely, but just in case I think you should change the exclusion to be if the patch change on channel 10 is '0' *then* don't proceed with it.

Also, just a note, when sam and max eventually does work, it uses general midi music. No need to exclude it from the list ;)

Other than that, cool. That's pretty much the exact same thign I had in mind for mt32 patch conversion too

comment:2 by SF/mattjon, 18 years ago

Hey Mike,

Yeah, that's correct. I fully agree with what you're saying, but I think (willing to be corrected though) that the Orchestral Kit patch change is a GS extension and not GMidi or MT32. Of course, if we need to allow changes to the Orchestral Kit, we need to allow it to change back to patch 0, which rather obviates the problem.

Didn't do anything particular for samnmax -- I knew tentacle needs GMIDI (It provides both GMD and ROL resources!) and should probably make the function, which determines whether mt32_translation is needed, separate.

Next thing would be to seek and destroy the things like #ifdef SAMNMAX and use Scumm._gameId instead. Using #defines for that is ugly :)

--M

comment:3 by SF/chuzwuzza, 18 years ago

Matt,

You could be right about teh extra drum patches... I first started out with midi stuff having a roland GS keyboard, so that's all I've ever known =)

as for defines being ugly, yeah i agree =) thats why i created the MidiDriver interface in the first place

I just wish I would get some feedback from the developers... they seem determined to ignore me...

comment:4 by SF/bbrox, 18 years ago

After a quick look at the patch, my biggest gripe is that it's not using at all any feature of C++.

If you want to do it 'cleanly', one should bave a MidiDriver base class with a virtual MidiOut function. Moreover, each platform specific code should be in it's respective file (for example, the Windows code in windows.cpp, Linux stuff in sdl.cpp (or even linux.cpp)).

The call to the constructor would be done in either sdl.cpp or windows.cpp. After, the iMuse code would just call MidiDriver->MidiOut() and thanks to the magic of C++, there would be no test to see which driver is active.

But this leads to another problem : once I will have written my 'record to .MID' option, you will not be able to listen and record the music at the same time (as only one driver can be loaded at one time). But well, this is maybe not such a big deal.

Finally, why do you apply the MT32 => GM translation table on games different from Tentacle ? Why not do it if you detect a 'roland_sound_tag' in the ressource file ?

comment:5 by SF/mattjon, 18 years ago

Hey Lionel,

No argument about the main bulk of the patch -- most of it is Michael (chuzwuzza)'s work integrated into the MT32 stuff unchanged. You should probably take up C++-ish stuff relating to MidiDriver:: with him. (I don't profess to knowing C++ either... I've got enough C to hack and enough Java and Ruby to do OO stuff :-)).

We say gameId != GID_TENTACLE because DOTT provides ROL tagged MIDI too! MI2 generally provides ROL and ADL (Roland, Adlib) tags and DOTT generally provides GMD, ROL and ADL, in every SOUN block. (i.e. the resource files contain 2 or 3 different midis for different platforms of the same sound).

If there's a clever way to put all of GMD, ROL and ADL into a single array and have scummvm choose in *that* priority order (which'd be ideal, since that will make the extra sounds in MI2 work too), then that'd be really nice and would tidy up the GID_specific checks.

I'm all for not doing work we don't need to do (and the fact that the GM midis sound better than the translated MT32 :-)). That gameId test really needs abstracting to a function (mainly as I understand that SamNMax is also GMIDI based).

Regards,

--M

comment:6 by SF/ender, 18 years ago

Updated to latest CVS, and applied. Roland translation is activated by the command-line option '-r'.

Geez, took a while for me to get around to this one... Sorry!

comment:7 by SF/ender, 18 years ago

Owner: set to SF/ender
Resolution: fixed
Status: newclosed

comment:8 by digitall, 17 months ago

Component: Audio: MT32
Note: See TracTickets for help on using tickets.