From f5e896e3f20e7b412c096ea16b3e8f2e2abb818e Mon Sep 17 00:00:00 2001 From: Johannes Schultz Date: Mon, 4 Nov 2024 17:38:39 +0000 Subject: [PATCH] [Mod] Change NOTE_MAX so that there are 128 valid notes instead of 120, like in the instrument keyboard mapping. Fixes bad notes in some Future Composer files (https://www.un4seen.com/forum/?topic=15448.msg144041#msg144041). [Var] Update MPTM/XM tests to verify correct writing of special notes and notes at the upper/lower note limit. git-svn-id: https://source.openmpt.org/svn/openmpt/trunk/OpenMPT@22093 56274372-70c3-4bfc-bfc3-4c3a0b034d27 --- mptrack/Ctrl_ins.cpp | 53 ++++++++++++++++++++------------ soundlib/InstrumentSynth.cpp | 2 +- soundlib/Load_fc.cpp | 1 + soundlib/Load_it.cpp | 37 ++++++++++++++-------- soundlib/Load_s3m.cpp | 13 ++++---- soundlib/Load_xm.cpp | 9 ++++-- soundlib/Sndfile.cpp | 5 +-- soundlib/mod_specifications.cpp | 2 +- soundlib/modcommand.h | 2 +- test/test.cpp | 16 +++++++++- test/test.mptm | Bin 11804 -> 11900 bytes test/test.xm | Bin 6308 -> 6279 bytes 12 files changed, 94 insertions(+), 46 deletions(-) diff --git a/mptrack/Ctrl_ins.cpp b/mptrack/Ctrl_ins.cpp index e693bdbf6b2..9c540fe925e 100644 --- a/mptrack/Ctrl_ins.cpp +++ b/mptrack/Ctrl_ins.cpp @@ -175,6 +175,13 @@ void CNoteMapWnd::OnPaint() dc.IntersectClipRect(&rcClient); const CSoundFile &sndFile = m_modDoc.GetSoundFile(); + const auto &modSpecs = sndFile.GetModSpecifications(); + int noteMin = 0, noteMax = NOTE_MAX - NOTE_MIN; + if(modSpecs.instrumentsMax) + { + noteMin = modSpecs.noteMin - NOTE_MIN; + noteMax = modSpecs.noteMax - NOTE_MIN; + } if (m_cxFont > 0 && m_cyFont > 0) { const bool focus = (::GetFocus() == m_hWnd); @@ -185,11 +192,11 @@ void CNoteMapWnd::OnPaint() int nPos = m_nNote - (nNotes/2); int ypaint = 0; mpt::winstring s; - for (int ynote=0; ynote= 0) && (nPos < NOTE_MAX - NOTE_MIN + 1); - if (isValidPos) + const bool isValidPos = mpt::is_in_range(nPos, noteMin, noteMax); + if(isValidPos) { s = mpt::ToWin(sndFile.GetNoteName(static_cast(nPos + 1), m_nInstrument)); s.resize(4); @@ -230,7 +237,7 @@ void CNoteMapWnd::OnPaint() rect.left = rcClient.left + m_cxFont * 2 + 3; rect.right = rcClient.right; s = _T(" .."); - if(pIns && nPos >= 0 && nPos < NOTE_MAX && pIns->Keyboard[nPos]) + if(pIns && isValidPos && pIns->Keyboard[nPos]) { s = mpt::tfmt::right(3, mpt::tfmt::dec(pIns->Keyboard[nPos])); } @@ -755,40 +762,48 @@ bool CNoteMapWnd::HandleNav(WPARAM k) bool redraw = false; //HACK: handle numpad (convert numpad number key to normal number key) - if ((k >= VK_NUMPAD0) && (k <= VK_NUMPAD9)) return HandleChar(k-VK_NUMPAD0+'0'); + if ((k >= VK_NUMPAD0) && (k <= VK_NUMPAD9)) + return HandleChar(k-VK_NUMPAD0+'0'); + + const CSoundFile &sndFile = m_modDoc.GetSoundFile(); + const auto &modSpecs = sndFile.GetModSpecifications(); + UINT noteMin = 0, noteMax = NOTE_MAX - NOTE_MIN; + if(modSpecs.instrumentsMax) + { + noteMin = modSpecs.noteMin - NOTE_MIN; + noteMax = modSpecs.noteMax - NOTE_MIN; + } switch(k) { case VK_RIGHT: - if (!m_bIns) { m_bIns = true; redraw = true; } else - if (m_nNote < NOTE_MAX - NOTE_MIN) { m_nNote++; m_bIns = false; redraw = true; } + if (!m_bIns) { m_bIns = true; redraw = true; } + else if (m_nNote < noteMax) { m_nNote++; m_bIns = false; redraw = true; } break; case VK_LEFT: - if (m_bIns) { m_bIns = false; redraw = true; } else - if (m_nNote) { m_nNote--; m_bIns = true; redraw = true; } + if (m_bIns) { m_bIns = false; redraw = true; } + else if (m_nNote > noteMin) { m_nNote--; m_bIns = true; redraw = true; } break; case VK_UP: - if (m_nNote > 0) { m_nNote--; redraw = true; } + if (m_nNote > noteMin) { m_nNote--; redraw = true; } break; case VK_DOWN: - if (m_nNote < NOTE_MAX - 1) { m_nNote++; redraw = true; } + if (m_nNote < noteMax) { m_nNote++; redraw = true; } break; case VK_PRIOR: - if (m_nNote > 3) { m_nNote -= 3; redraw = true; } else - if (m_nNote > 0) { m_nNote = 0; redraw = true; } + if (m_nNote > noteMin + 3) { m_nNote -= 3; redraw = true; } + else if (m_nNote > noteMin) { m_nNote = noteMin; redraw = true; } break; case VK_NEXT: - if (m_nNote+3 < NOTE_MAX) { m_nNote += 3; redraw = true; } else - if (m_nNote < NOTE_MAX - NOTE_MIN) { m_nNote = NOTE_MAX - NOTE_MIN; redraw = true; } + if (m_nNote + 3 < noteMax) { m_nNote += 3; redraw = true; } + else if (m_nNote < noteMax) { m_nNote = noteMax; redraw = true; } break; case VK_HOME: - if(m_nNote > 0) { m_nNote = 0; redraw = true; } + if(m_nNote > noteMin) { m_nNote = noteMin; redraw = true; } break; case VK_END: - if(m_nNote < NOTE_MAX - NOTE_MIN) { m_nNote = NOTE_MAX - NOTE_MIN; redraw = true; } + if(m_nNote < noteMax) { m_nNote = noteMax; redraw = true; } break; -// case VK_TAB: -// return true; case VK_RETURN: { ModInstrument *pIns = m_modDoc.GetSoundFile().Instruments[m_nInstrument]; diff --git a/soundlib/InstrumentSynth.cpp b/soundlib/InstrumentSynth.cpp index f26bdd1651d..e22860c7258 100644 --- a/soundlib/InstrumentSynth.cpp +++ b/soundlib/InstrumentSynth.cpp @@ -493,7 +493,7 @@ void InstrumentSynth::States::State::ApplyChannelState(ModChannel &chn, int32 &p { uint8 fcNote = static_cast(m_fcPitch >= 0 ? m_fcPitch + chn.nLastNote - NOTE_MIN : m_fcPitch) & 0x7F; static_assert(mpt::array_sizeNoteMap)>::size > 0x7F); - if(m_fcPitch) + if(m_fcPitch && ModCommand::IsNote(chn.nLastNote)) period += (sndFile.GetPeriodFromNote(chn.pModInstrument->NoteMap[fcNote], chn.nFineTune, chn.nC5Speed) - sndFile.GetPeriodFromNote(chn.pModInstrument->NoteMap[chn.nLastNote - NOTE_MIN], chn.nFineTune, chn.nC5Speed)); if(doVibratoFC) diff --git a/soundlib/Load_fc.cpp b/soundlib/Load_fc.cpp index aca53c7d629..30d722b7845 100644 --- a/soundlib/Load_fc.cpp +++ b/soundlib/Load_fc.cpp @@ -423,6 +423,7 @@ bool CSoundFile::ReadFC(FileReader &file, ModLoadingFlags loadFlags) if(!instr) return false; + static_assert(NOTE_MAX - NOTE_MIN + 1 >= 128, "Need at least a note range of 128 for correct emulation of note wrap-around logic in Future Composer"); for(uint8 note = 0; note < static_cast(instr->NoteMap.size()); note++) { if(note < 48) diff --git a/soundlib/Load_it.cpp b/soundlib/Load_it.cpp index 0ce0a47238d..a8654c39218 100644 --- a/soundlib/Load_it.cpp +++ b/soundlib/Load_it.cpp @@ -1098,11 +1098,14 @@ bool CSoundFile::ReadIT(FileReader &file, ModLoadingFlags loadFlags) uint8 note = patternData.ReadUint8(); if(note < 0x80) note += NOTE_MIN; - if(!(GetType() & MOD_TYPE_MPT)) - { - if(note > NOTE_MAX && note < 0xFD) note = NOTE_FADE; - else if(note == 0xFD) note = NOTE_NONE; - } + else if(note == 0xFF) + note = NOTE_KEYOFF; + else if(note == 0xFE) + note = NOTE_NOTECUT; + else if(note == 0xFD && GetType() != MOD_TYPE_MPT) + note = NOTE_NONE; // Note: in MPTM format, NOTE_FADE is written as 0xFD to preserve compatibility with older OpenMPT versions. + else + note = NOTE_FADE; m.note = lastValue[ch].note = note; } if(chnMask[ch] & 2) @@ -1769,14 +1772,23 @@ bool CSoundFile::SaveIT(std::ostream &f, const mpt::PathString &filename, bool c } auto &chnState = chnStates[ch]; - uint8 b = 0; + uint8 b = 1; uint8 vol = 0xFF; uint8 note = m->note; - if (note != NOTE_NONE) b |= 1; - if (m->IsNote()) note -= NOTE_MIN; - if (note == NOTE_FADE && GetType() != MOD_TYPE_MPT) note = 0xF6; - if (m->instr) b |= 2; - if (m->volcmd != VOLCMD_NONE) + if(note >= NOTE_MIN && note <= NOTE_MIN + 119) + note = m->note - NOTE_MIN; + else if(note == NOTE_FADE) + note = (GetType() == MOD_TYPE_MPT) ? 0xFD : 0xF6; + else if(note == NOTE_NOTECUT) + note = 0xFE; + else if(note == NOTE_KEYOFF) + note = 0xFF; + else + b = 0; + + if(m->instr) + b |= 2; + if(m->volcmd != VOLCMD_NONE) { vol = std::min(m->vol, uint8(9)); switch(m->volcmd) @@ -1801,7 +1813,8 @@ bool CSoundFile::SaveIT(std::ostream &f, const mpt::PathString &filename, bool c default: vol = 0xFF; } } - if (vol != 0xFF) b |= 4; + if(vol != 0xFF) + b |= 4; uint8 command = 0, param = 0; if(m->command == CMD_VOLUME && vol == 0xFF) { diff --git a/soundlib/Load_s3m.cpp b/soundlib/Load_s3m.cpp index 409f24e9549..c4737b44012 100644 --- a/soundlib/Load_s3m.cpp +++ b/soundlib/Load_s3m.cpp @@ -969,22 +969,23 @@ bool CSoundFile::SaveS3M(std::ostream &f) const { info |= s3mNotePresent; - if(note == NOTE_NONE) - { - note = s3mNoteNone; - } else if(ModCommand::IsSpecialNote(note)) + if(ModCommand::IsSpecialNote(note)) { // Note Cut note = s3mNoteOff; - } else if(note < 12 + NOTE_MIN) + } else if(note < NOTE_MIN + 12) { // Too low note = 0; - } else if(note <= NOTE_MAX) + } else if(note <= NOTE_MIN + 107) { note -= (12 + NOTE_MIN); note = static_cast((note % 12) + ((note / 12) << 4)); } + else + { + note = s3mNoteNone; + } if(m.instr > 0 && m.instr <= GetNumSamples()) { diff --git a/soundlib/Load_xm.cpp b/soundlib/Load_xm.cpp index 67ac4456be1..e19e3a39e0f 100644 --- a/soundlib/Load_xm.cpp +++ b/soundlib/Load_xm.cpp @@ -1262,9 +1262,12 @@ bool CSoundFile::SaveXM(std::ostream &f, bool compatibilityExport) uint8 note = p->note, command = 0, param = 0; ModSaveCommand(*p, command, param, true, compatibilityExport); - if (note >= NOTE_MIN_SPECIAL) note = 97; else - if ((note <= 12) || (note > 96+12)) note = 0; else - note -= 12; + if(note >= NOTE_MIN_SPECIAL) + note = 97; + else if(note < NOTE_MIN + 12 || note >= NOTE_MIN + 12 + 96) + note = 0; + else + note -= 12; uint8 vol = 0; if (p->volcmd != VOLCMD_NONE) { diff --git a/soundlib/Sndfile.cpp b/soundlib/Sndfile.cpp index efcab78bdf6..63c229cd3be 100644 --- a/soundlib/Sndfile.cpp +++ b/soundlib/Sndfile.cpp @@ -1643,9 +1643,10 @@ mpt::ustring CSoundFile::GetNoteName(const ModCommand::NOTE note, const NoteName return specialNoteNames[note - NOTE_MIN_SPECIAL]; } else if(ModCommand::IsNote(note)) { + const int octave = (note - NOTE_MIN) / 12; return mpt::ustring() .append(noteNames[(note - NOTE_MIN) % 12]) - .append(1, static_cast(UC_('0') + ((note - NOTE_MIN) / 12))) + .append(1, static_cast((octave <= 9 ? UC_('0') : UC_('A') - 10) + octave)) ; // e.g. "C#" + "5" } else if(note == NOTE_NONE) { @@ -2014,7 +2015,7 @@ bool CSoundFile::IsSampleReferencedByInstrument(SAMPLEINDEX sample, INSTRUMENTIN if(targetIns == nullptr) return false; - return mpt::contains(mpt::as_span(targetIns->Keyboard).first(NOTE_MAX), sample); + return mpt::contains(mpt::as_span(targetIns->Keyboard).first(NOTE_MAX - NOTE_MIN + 1), sample); } diff --git a/soundlib/mod_specifications.cpp b/soundlib/mod_specifications.cpp index f002ade75e0..a9edbeefdd0 100644 --- a/soundlib/mod_specifications.cpp +++ b/soundlib/mod_specifications.cpp @@ -29,7 +29,7 @@ constexpr CModSpecifications mptm_ = MOD_TYPE_MPT, // Internal MODTYPE value "mptm", // File extension NOTE_MIN, // Minimum note index - NOTE_MAX, // Maximum note index + NOTE_MIN + 119, // Maximum note index 4000, // Pattern max. 4000, // Order max. MAX_SEQUENCES, // Sequences max diff --git a/soundlib/modcommand.h b/soundlib/modcommand.h index 7ad6038f625..2fcad64fa09 100644 --- a/soundlib/modcommand.h +++ b/soundlib/modcommand.h @@ -23,7 +23,7 @@ enum : uint8 // ModCommand::NOTE { NOTE_NONE = 0, // Empty note cell NOTE_MIN = 1, // Minimum note value - NOTE_MAX = 120, // Maximum note value + NOTE_MAX = 128, // Maximum note value NOTE_MIDDLEC = (5 * 12 + NOTE_MIN), NOTE_KEYOFF = 0xFF, // === (Note Off, releases envelope / fades samples, stops plugin note) NOTE_NOTECUT = 0xFE, // ^^^ (Cuts sample / stops all plugin notes) diff --git a/test/test.cpp b/test/test.cpp index 6059f1fca86..c41c3aefb70 100644 --- a/test/test.cpp +++ b/test/test.cpp @@ -2863,6 +2863,9 @@ static void TestLoadXMFile(const CSoundFile &sndFile) VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(0, 0)->instr, 0); VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(0, 0)->volcmd, VOLCMD_VIBRATOSPEED); VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(0, 0)->vol, 15); + VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(0, 1)->note, NOTE_MIN + 12); + VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(1, 1)->note, NOTE_MIN + 12 + 95); + VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(2, 1)->note, NOTE_KEYOFF); VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(31, 0)->IsEmpty(), true); VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(31, 1)->IsEmpty(), false); VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(31, 1)->IsPcNote(), false); @@ -3128,7 +3131,7 @@ static void TestLoadMPTMFile(const CSoundFile &sndFile) VERIFY_EQUAL_NONCONT(pIns->pluginVelocityHandling, PLUGIN_VELOCITYHANDLING_VOLUME); VERIFY_EQUAL_NONCONT(pIns->pluginVolumeHandling, PLUGIN_VOLUMEHANDLING_MIDI); - for(size_t i = 0; i < NOTE_MAX; i++) + for(size_t i = 0; i < 120; i++) { VERIFY_EQUAL_NONCONT(pIns->Keyboard[i], (i == NOTE_MIDDLEC - 1) ? (ins * 1111) : 1); VERIFY_EQUAL_NONCONT(pIns->NoteMap[i], (i == NOTE_MIDDLEC - 1) ? (i + 13) : (i + 1)); @@ -3211,6 +3214,17 @@ static void TestLoadMPTMFile(const CSoundFile &sndFile) VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(0, 0)->instr, 99); VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(0, 0)->GetValueVolCol(), 1); VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(0, 0)->GetValueEffectCol(), 200); + VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(1, 0)->IsPcNote(), true); + VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(1, 0)->note, NOTE_PCS); + VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(1, 0)->instr, 250); + VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(1, 0)->GetValueVolCol(), 999); + VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(1, 0)->GetValueEffectCol(), 999); + + VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(0, 1)->note, NOTE_MIN); + VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(1, 1)->note, NOTE_MIN + 119); + VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(2, 1)->note, NOTE_KEYOFF); + VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(3, 1)->note, NOTE_NOTECUT); + VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(4, 1)->note, NOTE_FADE); VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(31, 0)->IsEmpty(), true); VERIFY_EQUAL_NONCONT(sndFile.Patterns[1].GetpModCommand(31, 1)->IsEmpty(), false); diff --git a/test/test.mptm b/test/test.mptm index aef62ef43da37868487ad56243016880528e6a37..593041d9b0608c78ad400c8c66bebbe06d25aa51 100644 GIT binary patch delta 431 zcmbOe^CxD48S@Qh--(u8imD0>49Ari7&28E7>(FJyDy?tmen`M{RPjnjiNcpe8MGRR%W( zx5E#$m>HEf>u8BHS@bcSU}5BBn9RWd21Z5}thogx0me4lFgfT4$apSOF^oJ0nCvqxPR<#uGp*6nr>R5=#*l41K7GD&@ob`;7jf@SA2@Q>j8*>#I8SN+cGwx^NV4NJl zq|InDxrWKWI4rfOI5R&_!GzZ_B_%aQAvZHmKR2;LKQF%|RUtnuO`#yMq$IT{PXR1Y zl3H9+ym>VfzbH!(BLnkfK?z}Vr+^?91~vu;20dR8iwDHg^9^BSVAAvT4Paql0SSbK zJF_sbGD$Lqg$MhvFc>iyP0o@~&~OX(+iIRVbzf!JUa?#t13XOgBAOaOn+_oeMYWY+Ys_!~=K< zZ{R_+dr}m<&o6%cXS=bl<^F}0(u%!&yE;2)@v)`xI?v}*K{V;!>6?Ghc|IrGt+@R9 z<)hKc8LR<_#5;pYua>m|(uzo9Fz7vBQ8s+jLuA#C#Uhk><+` z_~29|+#0;(Z#ORe-OEaCmWO?XR18iX8VD?^DQG0NmBEn56kESA7*Ww(AD^CxYZwHn Oy=G4JUWX_i2ZVp}ax?V+