Skip to content

CMF: Fix typo in comparison, this fixes 'slowdown' of certain CMF files#186

Merged
mywave82 merged 2 commits into
adplug:masterfrom
dmitrysmagin:cmf_fix
May 7, 2023
Merged

CMF: Fix typo in comparison, this fixes 'slowdown' of certain CMF files#186
mywave82 merged 2 commits into
adplug:masterfrom
dmitrysmagin:cmf_fix

Conversation

@dmitrysmagin

@dmitrysmagin dmitrysmagin commented Apr 28, 2023

Copy link
Copy Markdown
Contributor

There's a typo in checking the end of the song in the note off command: '>' confused with '<'.
As a result, some cmf files were played greatly slowed down, because the whole stream of events is messed up.
This is a fix.

This means that the note-off command was totally broken, because on each note-off the stream pointer was updated incorrectly, pointing to the velocity byte and not to the next delta-time value. Velocity was interpreted as an extra delta value and the next delta value as a running event (which happened still to be 0x80). That's why there appeared extra pauses in some tunes.

Incidentally, all testing tunes didn't have a note-off command apparently, because it's optional for cmf. But other tunes, converted from midi, were affected.

Tunes affected by this bug:
1001.CMF, HF048.CMF: https://file.io/fk4R228SNyaH
TMNT.CMF from this archive: http://os4depot.net/?function=showcontent&file=audio/play/adplay.lha
https://sid.ethz.ch/debian/adlib/Creative%20Music%20Format/Drum%20Blaster/MAMBO.CMF
https://sid.ethz.ch/debian/adlib/Creative%20Music%20Format/Drum%20Blaster/NEWAGE.CMF
https://sid.ethz.ch/debian/adlib/Creative%20Music%20Format/Drum%20Blaster/REGGAE.CMF
https://sid.ethz.ch/debian/adlib/Creative%20Music%20Format/Drum%20Blaster/SNDTRACK.CMF

@Malvineous

Copy link
Copy Markdown
Member

I think there's another typo here - all the other events with two data bytes have iSongLen - 2 but this one has iSongLen - 1, suggesting one data byte. But there are two data bytes so as per the other events this should probably be changed to - 2 as well.

@dmitrysmagin

Copy link
Copy Markdown
Contributor Author

Noted, fixed the comparison condition again.

@mywave82

mywave82 commented May 6, 2023

Copy link
Copy Markdown
Contributor

We should probably add a test file too

@dmitrysmagin

Copy link
Copy Markdown
Contributor Author

Okay, will add one.

@dmitrysmagin

Copy link
Copy Markdown
Contributor Author

Added a testing file.

@mywave82 mywave82 merged commit 18701aa into adplug:master May 7, 2023
@mywave82

mywave82 commented May 7, 2023

Copy link
Copy Markdown
Contributor

Thank you

@dmitrysmagin dmitrysmagin deleted the cmf_fix branch August 29, 2024 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants