-
-
Notifications
You must be signed in to change notification settings - Fork 172
Rework the MT-32/CM-32L ROM lookup and MIXER /LISTMIDI
improvements
#3011
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
c0f7679
to
ff54768
Compare
ff54768
to
6bf55ff
Compare
MIXER /LISTMIDI
improvements
MIXER /LISTMIDI
improvementsMIXER /LISTMIDI
improvements
TODO: We'll need to update the MIDI section of the wiki that details the MT-32/CM-32L ROM lookup mechanism. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a big improvement, @johnnovak - very nice!
Just a couple suggestions.
6bf55ff
to
322b086
Compare
Had a bit of time @kcgen so I've addressed your comments & have rebased to latest main. Thanks for the thorough review ❤️ |
322b086
to
3c9a7dd
Compare
3c9a7dd
to
2046e60
Compare
No functional changes.
Treating CM-32LN as the "best" CM-32* model is the wrong choice as the LN models feature a faster vibrato than the older L models. Most game music was composed for the older L models, which makes some soundtracks sound a bit weird on the newer LN models.
Now that we have moved from filename-based MT-32/CM-32L ROM lookup to hash-based, the unversioned vs versioned ROM differentiation doesn't make much sense. `mt32_old` and `mt32_new` linking to specific v1.x and v2.x MT-32 versions, respectively, was also a pretty arbitrary decision and was in conflict with how the auto model worked. This change makes the lookup behaviour of "symbolic" models uniform and consistent (`auto`, `mt32`, `cm32l`, `mt32_old`, and `mt32_new`). The lookup mechanism of models that specify the version number (e.g., `mt32_107`, `cm32l_102`) is unchanged. The output of `MIXER /LISTMIDI` is also improved.
2046e60
to
2e1d443
Compare
No remarks from my side. When testing the command I have discovered an issue #3046, but this is not a regression, at least comparing to 0.80.1. |
Description
Now that we have moved from filename-based MT-32/CM-32L ROM lookup to hash-based, the unversioned vs versioned ROM differentiation doesn't make much sense.
mt32_old
andmt32_new
linking to specific v1.x and v2.x MT-32 versions, respectively, was also a pretty arbitrary decision and was in conflict with how theauto
model worked.This change makes the lookup behaviour of "symbolic" models uniform and consistent (
auto
,mt32
,cm32l
,mt32_old
, andmt32_new
). The lookup mechanism of models that specify the version number (e.g.,mt32_107
,cm32l_102
) is unchanged.The output of
MIXER /LISTMIDI
is also improved.Previous behaviour
MT32_CONTROL.ROM
&MT32_PCM.ROM
filenames for MT-32, andMT-32 CM32L_CONTROL.ROM
&CM32L_PCM.ROM
for the CM-32L.model = auto
loaded the first existing ROM in priority order as specified in the config description ofmodel
.model = mt32
attempted to load the unversioned MT-32 ROM and fail the ROM file did not exist.model = cm32l
attempted to load the unversioned CM-32L ROM and fail the ROM file did not exist.model = mt32_new
attempted to load the versioned v2.04 MT-32 ROM and fail if that exact ROM file did not exist.model = mt32_old
attempted to load the versioned v1.07 MT-32 ROM and fail if that exact ROM file did not exist.New revised behaviour
model = auto
loads the first existing ROM in priority order as specified in the config description ofmodel
. This is the same behaviour as before.model = mt32
loads the first MT-32 ROM using the same priority order (it fails if only CM-32L ROMs are present or no ROMs at all).model = cm32l
loads the first CM-32L ROM in priority order (it fails if only MT-32 ROMs are present or no ROMs at all).model = mt32_new
loads the first v2.x MT-32 ROM in priority order (it fails only if no v2.x MT-32 ROMs are present).model = mt32_old
loads the first v1.x MT-32 ROM in priority order (it fails only if no v1.x MT-32 ROMs are present).New
model
setting descriptionBasically, a succinct version of the above:
Backward compatibility
In some weird edge cases (point 3), the behaviour can be different than before (meaning a different ROM model might be picked up while the contents of the
romdir
is exactly the same and the user's config is exactly the same too). However, I think this is a complete non-issue as most people would fall into the sane 1) and 2) use cases. People doing weird shit as described in 3) will need to fix up their configs 😎romdir
has the "unversioned" ROM files only. In this legacy use case,model = mt32
will pick the MT-32 ROM andmodel = cm32l
the CM-32L ROM. Easy!romdir
has all versioned ROM files (recommended setup). In this case, everything works as expected, even a. bit better (e.g., formt32_new
we'll still pick the "best" available "new" MT-32 ROM, even if v2.04 is missing).Mixed case when
romdir
contains both versioned and unversioned ROM files, and the person usesmt32
orcm32l
which were previously referring to the "unversioned" files—well, depending on what version the "unversioned" ROMs are, and what "versioned" ROMs the person has, the behaviour can be... pretty much anything 😄 However, this use case was always a bit of a clusterfuck and normally people who still use "legacy unversioned ROMs" fall into the first category. The behaviour with mixed versioned and unversioned ROMs was pretty confusing previously and it wasn't a good idea to do that anyway.Related issues
Fixes:
model
setting for the MT-32 emulation #2969 (except for actually changing the MT-32 when setting a differentmodel
value)Manual testing
Tested for regressions when using exact model versions and the symbolic models. The symbolic model resolution is more interesting, so here's the test evidence for that:
Hercules output
Checklist
Please tick the items as you have addressed them. Don't remove items; leave the ones that are not applicable unchecked.
I have: