Load MT-32 ROMs from `romdir` using hash matches #2591

weirddan455 · 2023-06-11T00:58:14Z

Hi, first PR to this project. I ran into an issue getting MT32 emulation working on my system and realized it was due to case sensitivity. The names of the ROMs are hardcoded here:

dosbox-staging/src/midi/midi_mt32.cpp

Lines 74 to 98 in b8f72a0

    
           // Traditional ROMs 
        
           const Rom mt32_pcm_any_f = {"pcm_mt32", "MT32_PCM.ROM", unversioned}; 
        
           const Rom mt32_ctrl_any_f = {"ctrl_mt32", "MT32_CONTROL.ROM", unversioned}; 
        
           const Rom cm32l_pcm_any_f = {"pcm_cm32l", "CM32L_PCM.ROM", unversioned}; 
        
           const Rom cm32l_ctrl_any_f = {"ctrl_cm32l", "CM32L_CONTROL.ROM", unversioned}; 
        
           // MAME ROMs (versioned) 
        
           const Rom mt32_pcm_100_f = {"pcm_mt32", "r15449121.ic37.bin", versioned}; 
        
           const Rom mt32_pcm_100_l = {"pcm_mt32_l", "r15179844.ic21.bin", versioned}; 
        
           const Rom mt32_pcm_100_h = {"pcm_mt32_h", "r15179845.ic22.bin", versioned}; 
        
           const Rom mt32_ctrl_104_a = {"ctrl_mt32_1_04_a", "mt32_1.0.4.ic27.bin", versioned}; 
        
           const Rom mt32_ctrl_104_b = {"ctrl_mt32_1_04_b", "mt32_1.0.4.ic26.bin", versioned}; 
        
           const Rom mt32_ctrl_105_a = {"ctrl_mt32_1_05_a", "mt32_1.0.5.ic27.bin", versioned}; 
        
           const Rom mt32_ctrl_105_b = {"ctrl_mt32_1_05_b", "mt32_1.0.5.ic26.bin", versioned}; 
        
           const Rom mt32_ctrl_106_a = {"ctrl_mt32_1_06_a", "mt32_1.0.6.ic27.bin", versioned}; 
        
           const Rom mt32_ctrl_106_b = {"ctrl_mt32_1_06_b", "mt32_1.0.6.ic26.bin", versioned}; 
        
           const Rom mt32_ctrl_107_a = {"ctrl_mt32_1_07_a", "mt32_1.0.7.ic27.bin", versioned}; 
        
           const Rom mt32_ctrl_107_b = {"ctrl_mt32_1_07_b", "mt32_1.0.7.ic26.bin", versioned}; 
        
           const Rom mt32_ctrl_bluer_a = {"ctrl_mt32_bluer_a", "blue_ridge__mt32a.bin", versioned}; 
        
           const Rom mt32_ctrl_bluer_b = {"ctrl_mt32_bluer_b", "blue_ridge__mt32b.bin", versioned}; 
        
           const Rom mt32_ctrl_204_f = {"ctrl_mt32_2_04", "mt32_2.0.4.ic28.bin", versioned}; 
        
           const Rom cm32l_ctrl_100_f = {"ctrl_cm32l_1_00", "lapc-i.v1.0.0.ic3.bin", versioned}; 
        
           const Rom cm32l_ctrl_102_f = {"ctrl_cm32l_1_02", "cm32l_control.rom", versioned}; 
        
           const Rom cm32l_pcm_100_h = {"pcm_cm32l_h", "r15179945.ic8.bin", versioned}; 
        
           const Rom &cm32l_pcm_100_l = mt32_pcm_100_f; // Lower half of samples comes from MT-32

I figured it might be useful to search for these files in a case insensitive manner instead so that's what this patch does. It would have saved me, as a user, a bit of time diagnosing this problem in any case.

src/midi/midi_lasynth_model.cpp

src/misc/fs_utils.cpp

kcgen · 2023-06-11T06:42:14Z

looks good, @weirddan455. Just a couple suggestions, and then if you can squash on your side to reduce the intermediate commits, then that should be it! Thanks again 🚀

weirddan455 · 2023-06-11T07:05:07Z

Just did a force push with your new suggestions + squashing everything into one commit. Thanks for the fast review :)

FeralChild64

Looks OK to me.

kcgen · 2023-06-11T20:55:17Z

Looks great, @weirddan455 !

It just hit me that having unit tests for this new insensitive directory search would be helpful, too.

We use very basic tests; if you can look into the tests/ directory, checkout tests_fs_utils.cpp. Lots of existing examples to work from. We also have a couple files in the tests/files/ directory, which you can also use for testing (or add more files to it if needed).

The debug builds always have unit tests:

meson setup -Dbuildtype=debug build/debug
meson compile -C build/debug

To launch just the fs_utils tests:

./build/debug/tests/fs_utils

Or run all the tests:

meson test -C build/debug

weirddan455 · 2023-06-11T21:29:22Z

@kcgen Sure I'll take a look at that in a bit. I'm thinking if we need something reproducible like a unit test, the function might need to sort the directory contents alphabetically and return the first result. Right now it's arbitrary what gets returned if the directory contains files with the same names but different casing.

I see this function does something similar (except with the POSIX glob function):

dosbox-staging/include/fs_utils.h

Lines 53 to 70 in b8f72a0

    
           /* Convert path (possibly in format used by different OS) to a path 
        
            * native for host OS. 
        
            * 
        
            * If path (after conversion) does not correspond to an existing file or 
        
            * directory, then an empty string is returned. 
        
            * 
        
            * On Unix-like systems: 
        
            * - Expand ~ and ~name to paths in appropriate home directory. 
        
            * - Convert Windows-style path separators to Unix-style separators. 
        
            * - If case-insensitive, relative path matches an existing file in the 
        
            *   filesystem, then return case-sensitive path to that file. 
        
            * - If more than one files match, return the first one (alphabetically). 
        
            * 
        
            * On Windows: 
        
            * - If path points to an existing file, then return the path unmodified. 
        
            */ 
        
           std::string to_native_path(const std::string &path) noexcept;

kcgen · 2023-06-11T23:01:22Z

I think it's OK to keep this new function std::filesystem 'clean', which I think it is, right now.

The filesystem::path is very robust in terms of handling OS variations, and so combining a path / filename results in a logical object that can be tested, as opposed to just string manipulation (where things like 'testfile' != './testfile').

Perhaps for this function, we'll need to use filesystem's canonical handling, that can bake down nonsense like ./././testfile just to testfile. (Includeling these strange forms in the unit tests might reveal the need for the filename portion to be canonicalized too).

Regarding alphabetic sorting; it sounds reasonable, but when I think it through, for example:

MyFiLE
mYfILE
MYfiLE

... Well, we're already in random land where case doesn't matter. So sorting is trying to apply casing rules into a space where casing has no meaning.

User A might complain that it should have picked his lowercase file first, but user B might want her uppercase file instead. Both are invalid.. these users need to cleanup their ambiguous messes.

Back to the mt32 use case, we do know that if users have files 'as-is' (with casing straight from the zip files), then we want to always search for those expected filenames first, and then fallback to case insensitive.

So perhaps we can keep the direct-hit first (apologies if that's already in place but I just didn't notice😅)

johnnovak · 2023-06-12T08:31:23Z

User A might complain that it should have picked his lowercase file first, but user B might want her uppercase file instead. Both are invalid.. these users need to cleanup their ambiguous messes.

💯 fully agree.

Back to the mt32 use case, we do know that if users have files 'as-is' (with casing straight from the zip files), then we want to always search for those expected filenames first, and then fallback to case insensitive.

To me the "gold standard" solution at the high level would be this:

I shovel some files into the ROM folder, filename doesn't matter.
If DOSBox detects on the next startup that the contents of the ROM directory have changed, it identifies the valid MT-32 ROM files in it by their checksum alone — filename would not even come into play.
Naturally, on subsequent startups the checksums would not be calculated, only if we detect that the contents of the ROM folder have changed (in order not to slow things down).

This requires a little more "plumbing" and some persistent storage for sure, but it's a much more user-friendly and robust solution. WinUAE has this but with no auto-scanning: you chuck a bunch of random Kickstart ROM files you "acquired" into the ROM folder and press a button that rescans the whole folder and populates a drop-down with the names of the valid ROMs. Then the list of found ROMs is persisted across restarts in a config file, of course, until you decide to re-scan the ROM folder again. Easy, robust, and user-friendly!

ScummVM has a similar functionality where you just point it to a folder that has all your games in subfolders, then ScummVM imports all the ones it can identify.

Grounded0 · 2023-06-12T10:55:38Z

Most of my good ideas for this project have been stolen from SheepShaver. Its good WinUAE is a similar conduit for you. 🙂

weirddan455 · 2023-06-12T23:08:28Z

@johnnovak I really like that solution. I just modified the PR to do away with hard coded filenames. The mt32emu library we are using has a method identifyROMFile that internally does a SHA1 hash + compare to a known list of ROMs.

https://github.com/munt/munt/blob/813e8f0eafbfc1771a9d7704e7025915af89643e/mt32emu/src/ROMInfo.cpp#L56-L87

I also removed the idea of "unversioned ROMs" and moved everything so it has a 1 to 1 link with what mt32emu is doing.

Could still use caching in persistent storage, as you suggested. I put that a TODO comment in there for now.

johnnovak · 2023-06-13T00:35:40Z

This is great @weirddan455! I'm glad you're tackling this; I like your enthusiasm! 🎉

@johnnovak I really like that solution. I just modified the PR to do away with hard coded filenames. The mt32emu library we are using has a method identifyROMFile that internally does a SHA1 hash + compare to a known list of ROMs.

💯 🥇

I also removed the idea of "unversioned ROMs" and moved everything so it has a 1 to 1 link with what mt32emu is doing.

I think we need to leave that in for compatibility reasons. Other DOSBox forks use the "unversioned" ROMs if they are in a set location or in the local game folder (I think, but don't quote me on the exact lookup behaviour). So basically we only want to keep this for compatibility with existing gamepacks that have whatever MT-32 ROM versions bundled with the game.

But yes, this is just for compatibility; when configuring a game from scratch, we discourage the use of non-versioned ROMs, absolutely.

Could still use caching in persistent storage, as you suggested. I put that a TODO comment in there for now.

Actually, thinking about this more deeply, it's hard to detect if the contents of a folder have changed in a cross-platform manner. So if we can get away with no caching, that's always the better and more robust solution.

Could you perhaps measure (e.g. log the time taken) how long it takes to scan the full MT-32 ROM set at startup? I'm pretty sure the limiting factor here is disk I/O; even old hardware can calculate SHA1 hashes at faster rates than reading from disk. Ideally, if this only adds a few hundred milliseconds at most, then we most definitely don't need caching.

kcgen · 2023-06-13T03:02:56Z

For the scanning, the catch will be if a user has a couple SF2 files in there as well (which might be 500+ MB). On a desktop w/ SSD, their startup time might still be quick :-) But we could probably glob *.rom, case-insensitive, as a very coarse filter.

We could also log a warning for files it scans that have nothing to do w/ mt32emu (when mt32emu return a failure core). This would encourage the user to maintain a very clean mt32emu-roms directory, and nothing more.

johnnovak · 2023-06-13T03:38:04Z

For the scanning, the catch will be if a user has a couple SF2 files in there as well (which might be 500+ MB). On a desktop w/ SSD, their startup time might still be quick :-) But we could probably glob *.rom, case-insensitive, as a very coarse filter.

We could also log a warning for files it scans that have nothing to do w/ mt32emu (when mt32emu return a failure core). This would encourage the user to maintain a very clean mt32emu-roms directory, and nothing more.

Yeah, I was gonna say, people shouldn't put all sorts of crap into the mt32emu-roms folder 😄

I propose a slightly different coarse filtering: we know for certain the expected filesizes of the valid ROM files. So let's add a whitelist of valid filesizes and only calculate the SHA1 hash for those. The side-effect is that even if some random other files have the whitelisted filesizes (extremely unlikely), calculating the hash for them won't be a problem as the ROM files are all rather small.

We could still log a warning for all the various random crap we find in the MT32 ROM folder, sure.

But I'd still like to see some concrete timing measurements for calculating the hashes for the whole ROMset.

weirddan455 · 2023-06-13T04:27:38Z

I think we need to leave that in for compatibility reasons. Other DOSBox forks use the "unversioned" ROMs if they are in a set location or in the local game folder

I believe this should still work with my patch. "unversioned" ROMs were really just versioned ROMs with specific filenames which our code excluded from version checks. However, they still went through the exact same path with regards to mt32emu so I believe they will still work now (and further more now be recognized as the real version that they are). Of course, testing would be welcome to know for sure.

Could you perhaps measure (e.g. log the time taken) how long it takes to scan the full MT-32 ROM set at startup?

Sure. Is there a particular benchmarking or profiling tool you would recommend for this?

I propose a slightly different coarse filtering: we know for certain the expected filesizes of the valid ROM files. So let's add a whitelist of valid filesizes and only calculate the SHA1 hash for those.

I had a look through the mt32emu source code as I was writing this and this is already done. Filesize gets matched before the SHA gets calculated:

Whitelist:
https://github.com/munt/munt/blob/813e8f0eafbfc1771a9d7704e7025915af89643e/mt32emu/src/ROMInfo.cpp#L56-L87

Filesize + SHA compare
https://github.com/munt/munt/blob/813e8f0eafbfc1771a9d7704e7025915af89643e/mt32emu/src/ROMInfo.cpp#L176-L185

johnnovak · 2023-06-13T04:42:00Z

However, they still went through the exact same path with regards to mt32emu so I believe they will still work now (and further more now be recognized as the real version that they are). Of course, testing would be welcome to know for sure.

You can easily test this yourself by getting hold of some of the "unversioned" ROMs. I'm a bit busy, but I can send you those files later if you're on Discord.

Could you perhaps measure (e.g. log the time taken) how long it takes to scan the full MT-32 ROM set at startup?

Sure. Is there a particular benchmarking or profiling tool you would recommend for this?

No, just measure delta time in the code and log the results.

I propose a slightly different coarse filtering: we know for certain the expected filesizes of the valid ROM files. So let's add a whitelist of valid filesizes and only calculate the SHA1 hash for those.

I had a look through the mt32emu source code as I was writing this and this is already done. Filesize gets matched before the SHA gets calculated:

Whitelist: https://github.com/munt/munt/blob/813e8f0eafbfc1771a9d7704e7025915af89643e/mt32emu/src/ROMInfo.cpp#L56-L87

Filesize + SHA compare https://github.com/munt/munt/blob/813e8f0eafbfc1771a9d7704e7025915af89643e/mt32emu/src/ROMInfo.cpp#L176-L18

Nice, these guys have thought of everything! We only need to use these functions then! 🚀

weirddan455 · 2023-06-13T05:53:29Z

So it's pretty fast on my system. I wrapped this load_model function with clock_gettime(CLOCK_MONOTONIC) to get a quick benchmark with my full ROM set. Here's some numbers:

dosbox-staging/src/midi/midi_mt32.cpp

Line 562 in d178d1e

auto loaded_model_and_dir = load_model(mt32_service, selected_model, rom_dirs);

67ms - Debug build pre-patch
143ms - Debug build post-patch

11ms - Release build pre-patch
22ms - Release build post-patch

As far as the "unversioned" ROMs, I can confirm the following 2 sets are working (after the latest commit where I found one was missing).

CM32L "unversioned"
a439fbb390da38cada95a7cbb1d6ca199cd66ef8 cm32l_control.rom
289cc298ad532b702461bfc738009d9ebe8025ea cm32l_pcm.rom

MT32 "unversioned"
b083518fffb7f66b03c23b7eb4f868e62dc5a987 mt32_control.rom
f6b1eebc4b2d200ec6d3d21d51325d5b48c60252 mt32_pcm.rom

Lastly, the code is already calling the identifyROMFile function in mt32emu which in turn calls the functions with the optimizations I mentioned above if that wasn't clear.

kcgen · 2023-06-13T06:21:43Z

@sergm , nice to see @weirddan455 leveraging mt32emu's ROM loading features even more directly in this PR!

This will open up access to all the models currently supported by mt32emu:

Model	Version	Staging support
MT-32 'old'	1.04	0.78+
MT-32 'old'	1.05	0.78+
MT-32 'old'	1.06	0.78+
MT-32 'old'	1.07	0.78+
MT-32 'new'	2.03	0.78+
MT-32 'new'	2.04	0.78+
MT-32 'new'	2.06	0.81-alpha (PR)
MT-32 'new'	2.07	0.81-alpha (PR)
CM-32L	1.00	0.78+
CM-32L	1.02	0.78+
CM-32LN	1.00	0.81-alpha (PR)

weirddan455 · 2023-06-13T06:29:05Z

@kcgen Yes, that is the idea. I haven’t added all of those yet. I believe 2.06 and 2.07 are missing. Those probably need new config options as well since they were never supported by us.

kcgen · 2023-06-13T07:07:29Z

I believe 2.06 and 2.07 are missing. Those probably need new config options as well since they were never supported by us.

Contacted @sergm, who might be able to help in this regard. I will follow up via email.

hmm - there's also an 'M9 enhanced' firmware referenced in the MAME MT-32 sources.

for general ref, I believe this is where active MT-32/CM32 version and ROM status is being held: https://en.wikipedia.org/wiki/User:Lord_Nightmare

johnnovak · 2023-06-13T08:01:22Z

Those are great results @weirddan455 regarding the timings. I suspect the detection of the full ROM-set won't take too long even on underpowered devices then such as the Raspberry Pi4.

I have uploaded a while ago the full MT-32 ROM set to archieve.org along with instructions how to create the "unversioned" files out of them, if that helps with testing. I will add 2.06 and 2.07 to it once we get hold of them.

https://archive.org/details/Roland-MT-32-ROMs

We also need to update the output of the MIXER command for the new ROMs, but let's treat that separately:

We can simply move the path to the current MT32 ROM below the little table that lists the found MT32 versions. That would be an improvement for other reasons too because then the ROM path could use the full width of the screen.

johnnovak · 2023-06-13T08:03:49Z

By the way, the output of MIXER /LISTMIDI is not hardcoded to 80-columns; it takes full advantage of all available area horizontally. Whoever wrote it did a good job! 🏅 This is how it looks like in 132-column VESA modes:

src/midi/midi_lasynth_model.cpp

kcgen · 2023-06-14T19:28:34Z

@kcgen Yes, that is the idea. I haven’t added all of those yet. I believe 2.06 and 2.07 are missing. Those probably need new config options as well since they were never supported by us.

Partially resolved, messaged in Discord.

weirddan455 · 2023-06-15T00:36:50Z

I pushed a commit yesterday for the items @johnnovak pointed out (added logging of unknown files and removed the TODO comment).

I just added support for the rest of the ROMs supported by mt32emu. Thanks to @kcgen for providing the 2.07 ROM to test with.

The mixer command now also lists the additional models. I believe @kcgen wrote the mt32 ListAll interface for that. I just added the new models to an array and it worked 😄

kcgen · 2023-06-15T13:46:51Z

Tested it and it's working great! The PR is also a surprisingly small modification for such a big (and welcome) change in the loading functionality!

Three comments:

For the /listmidi output, it was previously:

The columns start with the model followed by its versions.

So users can type:
- model = cml32l, and they get the first available CM-32L from left-to-right (priority-order), or
- model = cm32l_102, and get a specific version.
That's how the PR works too! But visually we've lost the model names, which makes the versions ambiguous:

So I think we need to put back the model prefix names (with cm32ln, now too), this is going to make the table quite wide, so after this, we probably need to rethink a new way to present the information (can be a follow on PR!).
Version ordering

When the model = .. doesn't have a version, we pick the first (best) available version, because this provides the most bug fixes. A small nuance is that the MT-32 'old' (1.x) series were more commonly used by composers than the short-lived 'new' (2.x versions), so for MT-32, we order its versions starting with 'old' then 'new'.

Regarding the newly added CM-32LN, it was like bug-fixed CM-32L before it evolved further into the D-models. Also, keeping with the time order of the models, I think the new ordering should be:

CM-32LN 100; CM-32L 102, 100; MT-32 107, 106, ... bluer, 207, 206, ... 203
For the error logging, I get a lot of repetition when changing the model.
try:
```
Z:\> model mt32_207
Z:\> mididevice mt32
```

2023-06-15 06:03:10.458 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.462 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/cm32l_pcm.rom
2023-06-15 06:03:10.463 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/a__m-9.27c256.ic27.bin
2023-06-15 06:03:10.466 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/b__m-9.27c256.ic26.bin
2023-06-15 06:03:10.468 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.472 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/cm32l_pcm.rom
2023-06-15 06:03:10.473 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/a__m-9.27c256.ic27.bin
2023-06-15 06:03:10.475 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/b__m-9.27c256.ic26.bin
2023-06-15 06:03:10.478 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.484 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.488 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/cm32l_pcm.rom
2023-06-15 06:03:10.489 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/a__m-9.27c256.ic27.bin
2023-06-15 06:03:10.492 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/b__m-9.27c256.ic26.bin
2023-06-15 06:03:10.495 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.499 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/cm32l_pcm.rom
2023-06-15 06:03:10.500 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/a__m-9.27c256.ic27.bin
2023-06-15 06:03:10.502 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/b__m-9.27c256.ic26.bin
2023-06-15 06:03:10.505 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.509 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.513 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/cm32l_pcm.rom
2023-06-15 06:03:10.514 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/a__m-9.27c256.ic27.bin
2023-06-15 06:03:10.518 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/b__m-9.27c256.ic26.bin
2023-06-15 06:03:10.521 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.525 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/cm32l_pcm.rom
2023-06-15 06:03:10.526 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/a__m-9.27c256.ic27.bin
2023-06-15 06:03:10.529 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/b__m-9.27c256.ic26.bin
2023-06-15 06:03:10.531 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.536 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.540 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/cm32l_pcm.rom
2023-06-15 06:03:10.540 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/a__m-9.27c256.ic27.bin
2023-06-15 06:03:10.543 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/b__m-9.27c256.ic26.bin
2023-06-15 06:03:10.550 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.555 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.559 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.564 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/cm32l_pcm.rom
2023-06-15 06:03:10.564 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/a__m-9.27c256.ic27.bin
2023-06-15 06:03:10.567 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/b__m-9.27c256.ic26.bin
2023-06-15 06:03:10.571 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.575 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.579 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/cm32l_pcm.rom
2023-06-15 06:03:10.580 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/a__m-9.27c256.ic27.bin
2023-06-15 06:03:10.584 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/b__m-9.27c256.ic26.bin
2023-06-15 06:03:10.587 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.591 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.596 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/cm32l_pcm.rom
2023-06-15 06:03:10.599 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.603 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.607 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/cm32l_pcm.rom
2023-06-15 06:03:10.608 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/a__m-9.27c256.ic27.bin
2023-06-15 06:03:10.610 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/b__m-9.27c256.ic26.bin
2023-06-15 06:03:10.614 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.619 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/sed1200-a.bin
2023-06-15 06:03:10.623 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/cm32l_pcm.rom
2023-06-15 06:03:10.624 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/a__m-9.27c256.ic27.bin
2023-06-15 06:03:10.627 | MT32: Unknown file in ROM folder: /usr/share/mt32-rom-data/b__m-9.27c256.ic26.bin

Some users are going to point romdir at a common area used by Munt and ScummVM, so I think we should just print one warning per file, without repeats. So we probably need a new member std::set to hold std_fs::path objects that we've already warned about.

Grounded0 · 2023-06-15T15:19:09Z

I don't want to bloat this PR too much but can you throw this in while you're at it. Also can be included in a follow-up PR that categorises the different ROM sets as indicated above.

Mainly the green highlight colouring of the active munt ROM like it was before and is with fluidsynth. It also acts as a great way to ascertain are you on mididevice auto, fluidsynth or mt32:

Small improvements to 'mixer /listmidi' #2484

johnnovak · 2023-06-15T22:00:12Z

I don't want to bloat this PR too much but can you throw this in while you're at it. Also can be included in a follow-up PR that categorises the different ROM sets as indicated above.

Mainly the green highlight colouring of the active munt ROM like it was before and is with fluidsynth. It also acts as a great way to ascertain are you on mididevice auto, fluidsynth or mt32:
* [Small improvements to 'mixer /listmidi' #2484](https://github.com/dosbox-staging/dosbox-staging/issues/2484)

Good point @Grounded0 but let's treat that separately as restoring the highlighting is already half-implemented on a branch of mine. I'll finish it after this has been merged.

johnnovak · 2023-06-15T22:53:24Z

Before you guys go into redesigning the MIXER /LISTMIDI output @kcgen @weirddan455, I have some ideas too how it could be improved. I'll post a mockup in a hour or two, you might wanna hold off any extensive changes until then.

But probably it's best to merge this as is, then the visual refactoring can be done in a separate ticket (I'm happy to do it).

weirddan455 · 2023-06-15T23:00:23Z

Before you guys go into redesigning the MIXER /LISTMIDI output @kcgen @weirddan455, I have some ideas too how it could be improved. I'll post a mockup in a hour or two, you might wanna hold off any extensive changes until then.

But probably it's best to merge this as is, then the visual refactoring can be done in a separate ticket (I'm happy to do it).

Cool. I just pushed commits fixing the duplicate logging and re-ordering the ROMs as @kcgen suggested. I tried setting the mixer output to show the full name but it failed an assertion and crashed because there was not enough room on the terminal to display so I didn't do any changes there.

Agreed that the mixer output should probably be on a separate ticket.

johnnovak · 2023-06-15T23:22:05Z

Before you guys go into redesigning the MIXER /LISTMIDI output @kcgen @weirddan455, I have some ideas too how it could be improved. I'll post a mockup in a hour or two, you might wanna hold off any extensive changes until then.
But probably it's best to merge this as is, then the visual refactoring can be done in a separate ticket (I'm happy to do it).

Cool. I just pushed commits fixing the duplicate logging and re-ordering the ROMs as @kcgen suggested. I tried setting the mixer output to show the full name but it failed an assertion and crashed because there was not enough room on the terminal to display so I didn't do any changes there.

Agreed that the mixer output should probably be on a separate ticket.

If @kcgen agrees, I think now that the order and the logging is fixed, this is good to go. Thanks for your help, much appreciated! ❤️

Then @kcgen and myself can brainstorm about the UI improvements and treat it separately.

kcgen

thanks for working through several iterations, @weirddan455 ! Let's get this merged; it's a big improvement, and nicely scoped just to the loading.

kcgen · 2023-06-16T00:23:47Z

Compilation issue on macOS:

https://github.com/dosbox-staging/dosbox-staging/actions/runs/5284347668/jobs/9562708411

kcgen · 2023-06-16T02:03:35Z

@weirddan455 - I think we're set to merge!

One small suggestion, if you can reorder and combine (squash) a couple commits, I think there are three groups of changes:

Also suggest naming the commits using the active voice like you've done in the first four and last commits (without the MT32: prefix).

PS- apologies for the bloated image (on a 4k monitor..)

kcgen · 2023-06-16T02:51:00Z

Looks great, @weirddan455. All commits build and run clean.
Inbound as soon as CI goes green.

johnnovak · 2023-06-16T03:15:00Z

Thanks @weirddan455! 😎

Regression from PR #2591. The logic for these still works. It's a simple reverse string search. They were just accidently removed from the list of valid config values.

kcgen reviewed Jun 11, 2023

View reviewed changes

src/midi/midi_lasynth_model.cpp Show resolved Hide resolved

kcgen reviewed Jun 11, 2023

View reviewed changes

src/midi/midi_lasynth_model.cpp Outdated Show resolved Hide resolved

kcgen added enhancement New feature or enhancement of existing features audio Audio related issues or enhancements labels Jun 11, 2023

kcgen assigned weirddan455 Jun 11, 2023

kcgen reviewed Jun 11, 2023

View reviewed changes

src/misc/fs_utils.cpp Outdated Show resolved Hide resolved

kcgen reviewed Jun 11, 2023

View reviewed changes

src/misc/fs_utils.cpp Outdated Show resolved Hide resolved

kcgen requested review from johnnovak and FeralChild64 June 11, 2023 06:45

weirddan455 force-pushed the mt32-casing branch from 425be17 to 8bcff93 Compare June 11, 2023 07:02

FeralChild64 reviewed Jun 11, 2023

View reviewed changes

weirddan455 force-pushed the mt32-casing branch from 8bcff93 to ed60fc8 Compare June 12, 2023 22:59

johnnovak reviewed Jun 13, 2023

View reviewed changes

src/midi/midi_lasynth_model.cpp Show resolved Hide resolved

johnnovak reviewed Jun 13, 2023

View reviewed changes

src/midi/midi_lasynth_model.cpp Show resolved Hide resolved

kcgen changed the title ~~Make searching for MT32 Roms case insensitive~~ Load MT-32 ROMs from romdir using hash matches Jun 13, 2023

kcgen self-requested a review June 16, 2023 00:21

kcgen approved these changes Jun 16, 2023

View reviewed changes

weirddan455 added 3 commits June 15, 2023 21:28

Improve searching for MT32 ROMs

b1cf9ac

Add support for additional MT32 ROMs

86efa68

Only log once per unknown MT32 ROM file

c1da78b

weirddan455 force-pushed the mt32-casing branch from 273d78e to c1da78b Compare June 16, 2023 02:31

johnnovak merged commit 6b4c273 into dosbox-staging:main Jun 16, 2023
44 checks passed

weirddan455 deleted the mt32-casing branch June 16, 2023 18:54

weirddan455 mentioned this pull request Oct 15, 2023

Various problems with the model setting for the MT-32 emulation #2969

Closed

weirddan455 added a commit that referenced this pull request Oct 15, 2023

Re-add "mt32" and "cm32l" alias models

6428001

Regression from PR #2591. The logic for these still works. It's a simple reverse string search. They were just accidently removed from the list of valid config values.

weirddan455 mentioned this pull request Oct 15, 2023

Re-add "mt32" and "cm32l" alias models #3010

Closed

10 tasks

johnnovak mentioned this pull request Oct 19, 2023

Rework the MT-32/CM-32L ROM lookup and MIXER /LISTMIDI improvements #3011

Merged

10 tasks

johnnovak mentioned this pull request Nov 13, 2023

DOSBox Staging 0.81 release steering ticket #3091

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Load MT-32 ROMs from `romdir` using hash matches #2591

Load MT-32 ROMs from `romdir` using hash matches #2591

weirddan455 commented Jun 11, 2023

kcgen commented Jun 11, 2023

weirddan455 commented Jun 11, 2023

FeralChild64 left a comment

kcgen commented Jun 11, 2023 •

edited

weirddan455 commented Jun 11, 2023

kcgen commented Jun 11, 2023 •

edited

johnnovak commented Jun 12, 2023 •

edited

Grounded0 commented Jun 12, 2023 •

edited

weirddan455 commented Jun 12, 2023

johnnovak commented Jun 13, 2023

kcgen commented Jun 13, 2023 •

edited

johnnovak commented Jun 13, 2023 •

edited

weirddan455 commented Jun 13, 2023

johnnovak commented Jun 13, 2023

weirddan455 commented Jun 13, 2023 •

edited

kcgen commented Jun 13, 2023 •

edited

weirddan455 commented Jun 13, 2023

kcgen commented Jun 13, 2023 •

edited

johnnovak commented Jun 13, 2023

johnnovak commented Jun 13, 2023 •

edited

kcgen commented Jun 14, 2023

weirddan455 commented Jun 15, 2023

kcgen commented Jun 15, 2023 •

edited

Grounded0 commented Jun 15, 2023 •

edited

johnnovak commented Jun 15, 2023

johnnovak commented Jun 15, 2023

weirddan455 commented Jun 15, 2023

johnnovak commented Jun 15, 2023

kcgen left a comment

kcgen commented Jun 16, 2023 •

edited

kcgen commented Jun 16, 2023 •

edited

kcgen commented Jun 16, 2023

johnnovak commented Jun 16, 2023

	// Traditional ROMs
	const Rom mt32_pcm_any_f = {"pcm_mt32", "MT32_PCM.ROM", unversioned};
	const Rom mt32_ctrl_any_f = {"ctrl_mt32", "MT32_CONTROL.ROM", unversioned};
	const Rom cm32l_pcm_any_f = {"pcm_cm32l", "CM32L_PCM.ROM", unversioned};
	const Rom cm32l_ctrl_any_f = {"ctrl_cm32l", "CM32L_CONTROL.ROM", unversioned};

	// MAME ROMs (versioned)
	const Rom mt32_pcm_100_f = {"pcm_mt32", "r15449121.ic37.bin", versioned};
	const Rom mt32_pcm_100_l = {"pcm_mt32_l", "r15179844.ic21.bin", versioned};
	const Rom mt32_pcm_100_h = {"pcm_mt32_h", "r15179845.ic22.bin", versioned};
	const Rom mt32_ctrl_104_a = {"ctrl_mt32_1_04_a", "mt32_1.0.4.ic27.bin", versioned};
	const Rom mt32_ctrl_104_b = {"ctrl_mt32_1_04_b", "mt32_1.0.4.ic26.bin", versioned};
	const Rom mt32_ctrl_105_a = {"ctrl_mt32_1_05_a", "mt32_1.0.5.ic27.bin", versioned};
	const Rom mt32_ctrl_105_b = {"ctrl_mt32_1_05_b", "mt32_1.0.5.ic26.bin", versioned};
	const Rom mt32_ctrl_106_a = {"ctrl_mt32_1_06_a", "mt32_1.0.6.ic27.bin", versioned};
	const Rom mt32_ctrl_106_b = {"ctrl_mt32_1_06_b", "mt32_1.0.6.ic26.bin", versioned};
	const Rom mt32_ctrl_107_a = {"ctrl_mt32_1_07_a", "mt32_1.0.7.ic27.bin", versioned};
	const Rom mt32_ctrl_107_b = {"ctrl_mt32_1_07_b", "mt32_1.0.7.ic26.bin", versioned};
	const Rom mt32_ctrl_bluer_a = {"ctrl_mt32_bluer_a", "blue_ridge__mt32a.bin", versioned};
	const Rom mt32_ctrl_bluer_b = {"ctrl_mt32_bluer_b", "blue_ridge__mt32b.bin", versioned};
	const Rom mt32_ctrl_204_f = {"ctrl_mt32_2_04", "mt32_2.0.4.ic28.bin", versioned};
	const Rom cm32l_ctrl_100_f = {"ctrl_cm32l_1_00", "lapc-i.v1.0.0.ic3.bin", versioned};
	const Rom cm32l_ctrl_102_f = {"ctrl_cm32l_1_02", "cm32l_control.rom", versioned};
	const Rom cm32l_pcm_100_h = {"pcm_cm32l_h", "r15179945.ic8.bin", versioned};
	const Rom &cm32l_pcm_100_l = mt32_pcm_100_f; // Lower half of samples comes from MT-32

Load MT-32 ROMs from romdir using hash matches #2591

Load MT-32 ROMs from romdir using hash matches #2591

Conversation

weirddan455 commented Jun 11, 2023

kcgen commented Jun 11, 2023

weirddan455 commented Jun 11, 2023

FeralChild64 left a comment

Choose a reason for hiding this comment

kcgen commented Jun 11, 2023 • edited

weirddan455 commented Jun 11, 2023

kcgen commented Jun 11, 2023 • edited

johnnovak commented Jun 12, 2023 • edited

Grounded0 commented Jun 12, 2023 • edited

weirddan455 commented Jun 12, 2023

johnnovak commented Jun 13, 2023

kcgen commented Jun 13, 2023 • edited

johnnovak commented Jun 13, 2023 • edited

weirddan455 commented Jun 13, 2023

johnnovak commented Jun 13, 2023

weirddan455 commented Jun 13, 2023 • edited

kcgen commented Jun 13, 2023 • edited

weirddan455 commented Jun 13, 2023

kcgen commented Jun 13, 2023 • edited

johnnovak commented Jun 13, 2023

johnnovak commented Jun 13, 2023 • edited

kcgen commented Jun 14, 2023

weirddan455 commented Jun 15, 2023

kcgen commented Jun 15, 2023 • edited

Grounded0 commented Jun 15, 2023 • edited

johnnovak commented Jun 15, 2023

johnnovak commented Jun 15, 2023

weirddan455 commented Jun 15, 2023

johnnovak commented Jun 15, 2023

kcgen left a comment

Choose a reason for hiding this comment

kcgen commented Jun 16, 2023 • edited

kcgen commented Jun 16, 2023 • edited

kcgen commented Jun 16, 2023

johnnovak commented Jun 16, 2023

Load MT-32 ROMs from `romdir` using hash matches #2591

Load MT-32 ROMs from `romdir` using hash matches #2591

kcgen commented Jun 11, 2023 •

edited

kcgen commented Jun 11, 2023 •

edited

johnnovak commented Jun 12, 2023 •

edited

Grounded0 commented Jun 12, 2023 •

edited

kcgen commented Jun 13, 2023 •

edited

johnnovak commented Jun 13, 2023 •

edited

weirddan455 commented Jun 13, 2023 •

edited

kcgen commented Jun 13, 2023 •

edited

kcgen commented Jun 13, 2023 •

edited

johnnovak commented Jun 13, 2023 •

edited

kcgen commented Jun 15, 2023 •

edited

Grounded0 commented Jun 15, 2023 •

edited

kcgen commented Jun 16, 2023 •

edited

kcgen commented Jun 16, 2023 •

edited