Skip to content

Detect type 1 (multi-track) MIDI files.#132

Merged
binarymaster merged 12 commits into
adplug:masterfrom
mywave82:bugfix-40-midi-type1-v2
Dec 11, 2022
Merged

Detect type 1 (multi-track) MIDI files.#132
binarymaster merged 12 commits into
adplug:masterfrom
mywave82:bugfix-40-midi-type1-v2

Conversation

@mywave82

@mywave82 mywave82 commented Feb 6, 2022

Copy link
Copy Markdown
Contributor

Also increases the base-octave one up, since all test files (Duke Nukem 3D and Doom) ended up the very low bass area.

Fixes bug adplug #40

Also increases the base-octave one up, since all test files (Duke Nukem 3D and Doom) ended up the very low bass area.

Fixes bug adplug#40
Comment thread src/mid.cpp Outdated
Comment thread src/mid.cpp Outdated
Comment thread src/mid.cpp Outdated
Comment thread src/mid.cpp Outdated
Comment thread src/mid.cpp Outdated
Comment thread src/mid.cpp Outdated
* Move General MIDI type 0 vs type 1 detection into load instead of rewind
* Remove two debug printfs that are a bit redudant.
* Single track loading from "LucasArts AdLib MIDI" files where accidently blocked
@binarymaster binarymaster linked an issue Nov 26, 2022 that may be closed by this pull request
Comment thread src/mid.cpp Outdated
Comment thread src/mid.cpp Outdated
@binarymaster

binarymaster commented Dec 11, 2022

Copy link
Copy Markdown
Member

Also increases the base-octave one up, since all test files (Duke Nukem 3D and Doom) ended up the very low bass area.

Yes you're right, I can confirm this, but since it breaks LAA playback, it needs to be investigated separately.

Please create an issue for it.

Comment thread src/mid.cpp
Comment on lines +978 to +982
while ((curtrack == 0) ||
((midi_type == 1) && (curtrack < 16)))
{
/* MIDI type 0 (and LucasArts AdLib MIDI) stores all the MIDI channels in a single track,
* while MIDI type 1 splits each channel into separate tracks */

@binarymaster binarymaster Dec 11, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well actually MIDI type-1 may have more than 16 tracks, and some of them may share one channel, like drum tracks (one track per drum type, but they all use channel 10 anyway).

Looks like this MID player is yet limited to 16 channels, so I'm ok with this.

@binarymaster binarymaster left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested this PR, here are things to do:

  • Fix MIDI type-1 playback (format is detected, but only track 0 is played)
  • Add some test music files and their references

Yes, and in case of GRABBAG.MID from Duke3D, it plays only the bass line. 😄

@binarymaster binarymaster dismissed Malvineous’s stale review December 11, 2022 16:13

Adam's review has been addressed.

@binarymaster binarymaster merged commit 30bfa25 into adplug:master Dec 11, 2022
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.

Add support for type-1 MIDI files

3 participants