View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0000921 | OpenMPT | Playback Compatibility | public | 2017-02-19 08:04 | 2017-03-31 08:02 |
Reporter | Revenant | Assigned To | Saga Musix | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
Product Version | OpenMPT 1.27.00.* (old testing) | ||||
Target Version | OpenMPT 1.26.09.00 / libopenmpt 0.2-beta22 (upgrade first) | Fixed in Version | OpenMPT 1.26.09.00 / libopenmpt 0.2-beta22 (upgrade first) | ||
Summary | 0000921: A few SoundFX command fixes | ||||
Description | After listening to a lot of SoundFX modules in OpenMPT, I noticed a few issues that this patch addresses:
[also changed 9xx but probably incorrectly, see comment] | ||||
Tags | No tags attached. | ||||
Attached Files | soundfx-fix.patch (3,789 bytes)
diff --git a/OpenMPT/soundlib/Load_sfx.cpp b/OpenMPT/soundlib/Load_sfx.cpp index c83ef78..2c61d1a 100644 --- a/OpenMPT/soundlib/Load_sfx.cpp +++ b/OpenMPT/soundlib/Load_sfx.cpp @@ -10,6 +10,7 @@ #include "stdafx.h" #include "Loaders.h" +#include "Tables.h" OPENMPT_NAMESPACE_BEGIN @@ -73,6 +74,27 @@ struct SFXSampleHeader MPT_BINARY_STRUCT(SFXSampleHeader, 30) +static uint8 ClampSlideParam(uint8 value, uint8 lowNote, uint8 highNote) +//---------------------------------------------------------------------- +{ + uint16 lowPeriod, highPeriod; + + if(lowNote < highNote && + lowNote >= 36 + NOTE_MIN && + highNote >= 36 + NOTE_MIN && + lowNote < CountOf(ProTrackerPeriodTable) + 36 + NOTE_MIN && + highNote < CountOf(ProTrackerPeriodTable) + 36 + NOTE_MIN) + { + lowPeriod = ProTrackerPeriodTable[lowNote - 36 - NOTE_MIN]; + highPeriod = ProTrackerPeriodTable[highNote - 36 - NOTE_MIN]; + + // with a fixed speed of 6 ticks/row, and excluding the first row, + // 1xx/2xx param has a max value of (low-high)/5 to avoid sliding too far + return std::min<uint8>(value, static_cast<uint8>((lowPeriod - highPeriod) / 5)); + } + + return 0; +} bool CSoundFile::ReadSFX(FileReader &file, ModLoadingFlags loadFlags) //------------------------------------------------------------------- @@ -241,11 +263,23 @@ bool CSoundFile::ReadSFX(FileReader &file, ModLoadingFlags loadFlags) break; case 0x3: // enable filter/LED + // give precedence to 7xy/8xy slides + if(slideRate[chn]) + { + m.command = m.param = 0; + break; + } m.command = CMD_MODCMDEX; m.param = 0; break; case 0x4: // disable filter/LED + // give precedence to 7xy/8xy slides + if(slideRate[chn]) + { + m.command = m.param = 0; + break; + } m.command = CMD_MODCMDEX; m.param = 1; break; @@ -255,6 +289,15 @@ bool CSoundFile::ReadSFX(FileReader &file, ModLoadingFlags loadFlags) { m.command = CMD_VOLUME; m.param = std::min(ModCommand::PARAM(0x3F), static_cast<ModCommand::PARAM>((Samples[m.instr].nVolume / 4u) + m.param)); + + // give precedence to 7xy/8xy slides (and move this to the volume column) + if(slideRate[chn]) + { + m.volcmd = VOLCMD_VOLUME; + m.vol = m.param; + m.command = m.param = 0; + break; + } } else { m.command = m.param = 0; @@ -269,6 +312,15 @@ bool CSoundFile::ReadSFX(FileReader &file, ModLoadingFlags loadFlags) m.param = static_cast<ModCommand::PARAM>(Samples[m.instr].nVolume / 4u) - m.param; else m.param = 0; + + // give precedence to 7xy/8xy slides (and move this to the volume column) + if(slideRate[chn]) + { + m.volcmd = VOLCMD_VOLUME; + m.vol = m.param; + m.command = m.param = 0; + break; + } } else { m.command = m.param = 0; @@ -279,14 +331,20 @@ bool CSoundFile::ReadSFX(FileReader &file, ModLoadingFlags loadFlags) slideTo[chn] = lastNote[chn] - (m.param >> 4); m.command = CMD_PORTAMENTODOWN; - m.param = slideRate[chn] = m.param & 0xF; + slideRate[chn] = m.param & 0xF; + m.param = ClampSlideParam(slideRate[chn], slideTo[chn], lastNote[chn]); break; case 0x8: // 8xy: slide up x semitones at speed y slideTo[chn] = lastNote[chn] + (m.param >> 4); m.command = CMD_PORTAMENTOUP; - m.param = slideRate[chn] = m.param & 0xF; + slideRate[chn] = m.param & 0xF; + m.param = ClampSlideParam(slideRate[chn], lastNote[chn], slideTo[chn]); + break; + + case 0x9: // 9xx: set sample offset + m.command = CMD_OFFSET; break; default: | ||||
Has the bug occurred in previous versions? | |||||
Tested code revision (in case you know it) | |||||
ftp://ftp.modland.com/pub/modules/SoundFX/Kerni/youramiga.sfx |
|
Okay, maybe I was wrong about 9xx - flod seems to actually treat it as an automatic vibrato (see https://github.com/photonstorm/Flod/blob/master/Flod%204.0/neoart/flod/soundfx/FXPlayer.as#L237) I'm sure I've said this before, but it'd be nice if I could actually find any official documentation about this tracker... |
|
Should I wait until the 9xx part is fixed before looking at the patch? Or just drop in everything apart from 9xx for now? |
|
Just leave it out for now. I don't think it's used by very many modules, and based on a quick look at Flod's player source it seems to behave fairly differently than the Protracker etc. vibrato commands anyway. |
|
Patch (apart from 9xx import) applied in r7664. |
|
Date Modified | Username | Field | Change |
---|---|---|---|
2017-02-19 08:04 | Revenant | New Issue | |
2017-02-19 08:04 | Revenant | File Added: soundfx-fix.patch | |
2017-02-19 08:04 | Revenant | Note Added: 0002882 | |
2017-02-19 08:20 | manx | Assigned To | => Saga Musix |
2017-02-19 08:20 | manx | Status | new => assigned |
2017-02-19 08:20 | manx | Target Version | => OpenMPT 1.27.01.00 / libopenmpt 0.3.1 (upgrade first) |
2017-02-19 08:34 | Revenant | Note Added: 0002883 | |
2017-02-19 09:37 | Revenant | Description Updated | |
2017-02-19 13:28 | Saga Musix | Note Added: 0002884 | |
2017-02-19 21:56 | Revenant | Note Added: 0002885 | |
2017-02-21 00:20 | Saga Musix | Status | assigned => resolved |
2017-02-21 00:20 | Saga Musix | Resolution | open => fixed |
2017-02-21 00:20 | Saga Musix | Fixed in Version | => OpenMPT 1.27.01.00 / libopenmpt 0.3.1 (upgrade first) |
2017-02-21 00:20 | Saga Musix | Note Added: 0002887 | |
2017-03-19 13:58 | Saga Musix | Fixed in Version | OpenMPT 1.27.01.00 / libopenmpt 0.3.1 (upgrade first) => OpenMPT 1.26.09.00 / libopenmpt 0.2-beta22 (upgrade first) |
2017-03-19 13:58 | Saga Musix | Target Version | OpenMPT 1.27.01.00 / libopenmpt 0.3.1 (upgrade first) => OpenMPT 1.26.09.00 / libopenmpt 0.2-beta22 (upgrade first) |