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) |