[Scummvm-git-logs] scummvm master -> 162e1bb764e3c24cce1f1c83426d6edc2c0a9e5d

athrxx noreply at scummvm.org
Tue Sep 6 20:02:47 UTC 2022


This automated email contains information about 1 new commit which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
162e1bb764 SCUMM: (DOTT) - fix AdLib transpose/detune


Commit: 162e1bb764e3c24cce1f1c83426d6edc2c0a9e5d
    https://github.com/scummvm/scummvm/commit/162e1bb764e3c24cce1f1c83426d6edc2c0a9e5d
Author: athrxx (athrxx at scummvm.org)
Date: 2022-09-06T22:01:21+02:00

Commit Message:
SCUMM: (DOTT) - fix AdLib transpose/detune

Continued bug fixing for ticket no. 13460 ("Incorrect MIDI
pitch bending"). This time the AdLib part (which applies
to FM-Towns, too, since it is more or less the same driver).
This fixes the sound when the bird is dropping from the
sky in the DOTT intro.

Also separate the GM code for SAMNMAX a bit after
checking the original code.

Changed paths:
    audio/adlib.cpp
    engines/scumm/imuse/drivers/fmtowns.cpp
    engines/scumm/imuse/imuse.cpp
    engines/scumm/imuse/imuse_internal.h
    engines/scumm/imuse/imuse_part.cpp
    engines/scumm/imuse/imuse_player.cpp


diff --git a/audio/adlib.cpp b/audio/adlib.cpp
index 28682298e93..7d9493fb493 100644
--- a/audio/adlib.cpp
+++ b/audio/adlib.cpp
@@ -84,7 +84,7 @@ protected:
 	AdLibVoice *_voice;
 	int16 _pitchBend;
 	byte _pitchBendFactor;
-	//int8 _transposeEff;
+	int8 _transposeEff;
 	byte _volEff;
 	int8 _detuneEff;
 	byte _modWheel;
@@ -110,7 +110,7 @@ public:
 		_voice = nullptr;
 		_pitchBend = 0;
 		_pitchBendFactor = 2;
-		//_transposeEff = 0;
+		_transposeEff = 0;
 		_volEff = 0;
 		_detuneEff = 0;
 		_modWheel = 0;
@@ -148,6 +148,7 @@ public:
 	void panPosition(byte value) override;
 	void pitchBendFactor(byte value) override;
 	void detune(byte value) override;
+	void transpose(int8 value) override;
 	void priority(byte value) override;
 	void sustain(bool value) override;
 	void effectLevel(byte value) override { return; } // Not supported
@@ -1090,11 +1091,11 @@ void AdLibPart::pitchBend(int16 bend) {
 #ifdef ENABLE_OPL3
 		if (!_owner->_opl3Mode) {
 #endif
-			_owner->adlibNoteOn(voice->_channel, voice->_note/* + _transposeEff*/,
+			_owner->adlibNoteOn(voice->_channel, voice->_note + _transposeEff,
 								  (_pitchBend * _pitchBendFactor >> 6) + _detuneEff);
 #ifdef ENABLE_OPL3
 		} else {
-			_owner->adlibNoteOn(voice->_channel, voice->_note, (_pitchBend * _pitchBendFactor) >> 5);
+			_owner->adlibNoteOn(voice->_channel, voice->_note + _transposeEff, (_pitchBend * _pitchBendFactor) >> 5);
 		}
 #endif
 	}
@@ -1202,11 +1203,11 @@ void AdLibPart::pitchBendFactor(byte value) {
 #ifdef ENABLE_OPL3
 		if (!_owner->_opl3Mode) {
 #endif
-			_owner->adlibNoteOn(voice->_channel, voice->_note /* + _transposeEff*/,
+			_owner->adlibNoteOn(voice->_channel, voice->_note + _transposeEff,
 							  (_pitchBend * _pitchBendFactor >> 6) + _detuneEff);
 #ifdef ENABLE_OPL3
 		} else {
-			_owner->adlibNoteOn(voice->_channel, voice->_note, (_pitchBend * _pitchBendFactor) >> 5);
+			_owner->adlibNoteOn(voice->_channel, voice->_note + _transposeEff, (_pitchBend * _pitchBendFactor) >> 5);
 		}
 #endif
 	}
@@ -1229,11 +1230,19 @@ void AdLibPart::detune(byte value) {
 
 	_detuneEff = value;
 	for (voice = _voice; voice; voice = voice->_next) {
-		_owner->adlibNoteOn(voice->_channel, voice->_note/* + _transposeEff*/,
+		_owner->adlibNoteOn(voice->_channel, voice->_note + _transposeEff,
 							  (_pitchBend * _pitchBendFactor >> 6) + _detuneEff);
 	}
 }
 
+void AdLibPart::transpose(int8 value) {
+	_transposeEff = value;
+	for (AdLibVoice *voice = _voice; voice; voice = voice->_next) {
+		_owner->adlibNoteOn(voice->_channel, voice->_note + _transposeEff,
+			(_pitchBend * _pitchBendFactor >> 6) + _detuneEff);
+	}
+}
+
 void AdLibPart::priority(byte value) {
 	_priEff = value;
 }
@@ -1566,11 +1575,11 @@ void MidiDriver_ADLIB::setPitchBendRange(byte channel, uint range) {
 #ifdef ENABLE_OPL3
 		if (!_opl3Mode) {
 #endif
-			adlibNoteOn(voice->_channel, voice->_note/* + part->_transposeEff*/,
+			adlibNoteOn(voice->_channel, voice->_note + part->_transposeEff,
 						(part->_pitchBend * part->_pitchBendFactor >> 6) + part->_detuneEff);
 #ifdef ENABLE_OPL3
 		} else {
-			adlibNoteOn(voice->_channel, voice->_note, (part->_pitchBend * part->_pitchBendFactor) >> 5);
+			adlibNoteOn(voice->_channel, voice->_note + part->_transposeEff, (part->_pitchBend * part->_pitchBendFactor) >> 5);
 		}
 #endif
 	}
@@ -2076,7 +2085,7 @@ void MidiDriver_ADLIB::mcKeyOn(AdLibVoice *voice, const AdLibInstrument *instr,
 #ifdef ENABLE_OPL3
 	if (!_opl3Mode) {
 #endif
-		adlibNoteOnEx(voice->_channel, /*part->_transposeEff + */note, part->_detuneEff + (part->_pitchBend * part->_pitchBendFactor >> 6));
+		adlibNoteOnEx(voice->_channel, note + part->_transposeEff, part->_detuneEff + (part->_pitchBend * part->_pitchBendFactor >> 6));
 
 		if (instr->flagsA & 0x80) {
 			mcInitStuff(voice, &voice->_s10a, &voice->_s11a, instr->flagsA, &instr->extraA);
@@ -2092,7 +2101,7 @@ void MidiDriver_ADLIB::mcKeyOn(AdLibVoice *voice, const AdLibInstrument *instr,
 #ifdef ENABLE_OPL3
 	} else {
 		adlibSetupChannelSecondary(voice->_channel, second, secVol1, secVol2, pan);
-		adlibNoteOnEx(voice->_channel, note, (part->_pitchBend * part->_pitchBendFactor) >> 5);
+		adlibNoteOnEx(voice->_channel, note + part->_transposeEff, (part->_pitchBend * part->_pitchBendFactor) >> 5);
 	}
 #endif
 }
diff --git a/engines/scumm/imuse/drivers/fmtowns.cpp b/engines/scumm/imuse/drivers/fmtowns.cpp
index 04ccfad9acd..787e787a5cf 100644
--- a/engines/scumm/imuse/drivers/fmtowns.cpp
+++ b/engines/scumm/imuse/drivers/fmtowns.cpp
@@ -132,6 +132,8 @@ public:
 	void pitchBend(int16 bend) override;
 	void controlChange(byte control, byte value) override;
 	void pitchBendFactor(byte value) override;
+	void detune(byte value) override;
+	void transpose(int8 value) override;
 	void priority(byte value) override;
 	void sysEx_customInstrument(uint32 type, const byte *instr) override;
 
@@ -766,6 +768,19 @@ void TownsMidiInputChannel::pitchBendFactor(byte value) {
 		oc->noteOnPitchBend(oc->_note + oc->_in->_transpose, _freqLSB);
 }
 
+void TownsMidiInputChannel::detune(byte value) {
+	_detune = value;
+	_freqLSB = ((_pitchBend * _pitchBendFactor) >> 6) + _detune;
+	for (TownsMidiOutputChannel *oc = _out; oc; oc = oc->_next)
+		oc->noteOnPitchBend(oc->_note + oc->_in->_transpose, _freqLSB);
+}
+
+void TownsMidiInputChannel::transpose(int8 value) {
+	_transpose = value;
+	for (TownsMidiOutputChannel *oc = _out; oc; oc = oc->_next)
+		oc->noteOnPitchBend(oc->_note + _transpose, _freqLSB);
+}
+
 void TownsMidiInputChannel::priority(byte value) {
 	_priority = value;
 }
diff --git a/engines/scumm/imuse/imuse.cpp b/engines/scumm/imuse/imuse.cpp
index 89224ab95fb..3f0599d5914 100644
--- a/engines/scumm/imuse/imuse.cpp
+++ b/engines/scumm/imuse/imuse.cpp
@@ -1632,8 +1632,9 @@ void IMuseInternal::initGM() {
 	// These are the init messages from the DOTT General Midi
 	// driver. This is the major part of the bug fix for bug
 	// no. 13460 ("DOTT: Incorrect MIDI pitch bending").
-	// Might be worthwhile to check if other GM drivers (like
-	// SAM) have the same values...
+	// SAMNMAX has some less of the default settings (since
+	// the driver works a bit different), but it uses the same
+	// value for the pitch bend range.
 	MidiDriver *m = _midi_native;
 	for (int i = 0; i < 16; ++i) {
 		m->send(0x0064B0 | i);
@@ -1641,6 +1642,7 @@ void IMuseInternal::initGM() {
 		m->send(0x1006B0 | i);
 		m->send(0x7F07B0 | i);
 		m->send(0x3F0AB0 | i);
+		m->send(0x0000C0 | i);
 		m->send(0x4000E0 | i);
 		m->send(0x0001B0 | i);
 		m->send(0x0040B0 | i);
diff --git a/engines/scumm/imuse/imuse_internal.h b/engines/scumm/imuse/imuse_internal.h
index f5f46a0c7ee..2f5fa71d108 100644
--- a/engines/scumm/imuse/imuse_internal.h
+++ b/engines/scumm/imuse/imuse_internal.h
@@ -221,6 +221,7 @@ protected:
 	bool _isMIDI;
 	bool _isMT32;
 	bool _isGM;
+	bool _isAdLibOrFMTowns;
 	bool _supportsPercussion;
 
 protected:
@@ -277,6 +278,7 @@ public:
 	bool isMIDI() const { return _isMIDI; }
 	bool isMT32() const { return _isMT32; }
 	bool isGM() const { return _isGM; }
+	bool isAdLibOrFMTowns() const { return _isAdLibOrFMTowns; }
 	bool jump(uint track, uint beat, uint tick);
 	void onTimer();
 	void removePart(Part *part);
diff --git a/engines/scumm/imuse/imuse_part.cpp b/engines/scumm/imuse/imuse_part.cpp
index 32d5893bc8e..6e485dde1cb 100644
--- a/engines/scumm/imuse/imuse_part.cpp
+++ b/engines/scumm/imuse/imuse_part.cpp
@@ -116,6 +116,9 @@ void Part::set_detune(int8 detune) {
 #endif
 	} else {
 		_detune_eff = clamp((_detune = detune) + _player->getDetune(), -128, 127);
+		// Some drivers handle the transpose and the detune in pitchBend()...
+		if (_mc && _player->isAdLibOrFMTowns())
+			_mc->detune(_detune_eff);
 		sendPitchBend();
 	}
 }
@@ -153,7 +156,10 @@ void Part::set_transpose(int8 transpose) {
 		sendTranspose();
 	} else {
 		_transpose_eff = (_transpose == -128) ? 0 : transpose_clamp(_transpose + _player->getTranspose(), -24, 24);
-		sendPitchBend();
+		if (_player->isAdLibOrFMTowns())
+			sendTranspose();
+		else
+			sendPitchBend();
 	}
 }
 
@@ -364,20 +370,35 @@ void Part::sendPitchBend() {
 		return;
 
 	int16 bend = _pitchbend;
-	// RPN-based pitchbend range doesn't work for the MT32,
-	// so we'll do the scaling ourselves.
-	if (_player->_se->isNativeMT32())
-		bend = bend * _pitchbend_factor / 12;
+	int8 transpose = _transpose_eff;
+	int8 detune = _detune_eff;
 
-	// We send the transpose value separately for Amiga (which is more like the original handles this).
-	// Some rhythm instruments depend on this.
-	int8 transpose = _se->_isAmiga ? 0 : _transpose_eff;
+	// For Amiga, AdLib and FM-Towns we send some values separately due to the way the drivers have
+	// been implemented (it must be avoided that the pitchbend factor gets applied on top). So we
+	// neutralize them here for the pitch bend calculation.
+	if (_se->_isAmiga) {
+		transpose = 0;
+	} else if (_player->isAdLibOrFMTowns()) {
+		transpose = detune = 0;
+	} else if (_player->_se->isNativeMT32()) {
+		// RPN-based pitchbend range doesn't work for the MT32, so we'll do the scaling ourselves.
+		bend = bend * _pitchbend_factor / 12;
+	}
 
-	if (_player->isGM())
-		// This is the DOTT formula. Might not be exactly like this for SAM...
-		bend = ((clamp(((bend * _pitchbend_factor) >> 6) + _detune_eff + (transpose << 7), -2048, 2047) + 0x800) << 2) - 0x2000;
-	else
+	if (_player->isGM()) {
+		if (_se->_game_id == GID_SAMNMAX) {
+			if (bend)
+				debug ("PB: _pitchbend %d, _pitchbend_factor %d", bend, _pitchbend_factor);
+			// SAMNMAX formula
+			bend = _pitchbend_factor ? (bend * _pitchbend_factor) >> 5 : bend >> 6;
+			bend = (bend + _detune_eff + (transpose << 8)) << 1;
+		} else {
+			// DOTT formula (from the DOTT GMIDI.IMS driver)
+			bend = clamp(((bend * _pitchbend_factor) >> 6) + _detune_eff + (transpose << 7), -2048, 2047) << 2;
+		}
+	} else {
 		bend = clamp(bend + (_detune_eff * 64 / 12) + (transpose * 8192 / 12), -8192, 8191);
+	}
 
 	_mc->pitchBend(bend);
 }
@@ -386,10 +407,8 @@ void Part::sendTranspose() {
 	if (!_mc)
 		return;
 
-	// See comment above. The transpose function was never implemented into our other drivers,
-	// since this seems to have been handled via pitchBend() instead. The original drivers do have
-	// such functions.
-	if (!_se->_isAmiga)
+	// Some drivers handle the transpose and the detune in pitchBend()...
+	if (!_se->_isAmiga && !_player->isAdLibOrFMTowns())
 		return;
 
 	_mc->transpose(_transpose_eff);
diff --git a/engines/scumm/imuse/imuse_player.cpp b/engines/scumm/imuse/imuse_player.cpp
index 1fb0f842973..2184336ccee 100644
--- a/engines/scumm/imuse/imuse_player.cpp
+++ b/engines/scumm/imuse/imuse_player.cpp
@@ -78,6 +78,7 @@ Player::Player() :
 	_isMT32(false),
 	_isMIDI(false),
 	_isGM(false),
+	_isAdLibOrFMTowns(false),
 	_supportsPercussion(false),
 	_se(nullptr),
 	_vol_chan(0),
@@ -109,6 +110,7 @@ bool Player::startSound(int sound, MidiDriver *midi) {
 	_isMIDI = _se->isMIDI(sound);
 	_supportsPercussion = _se->supportsPercussion(sound);
 	_isGM = (_supportsPercussion && !_isMT32); // Unlike IMuseInternal::isMIDI(), IMuseInternal::supportsPercussion() really filters out all non-MIDI things...
+	_isAdLibOrFMTowns = (_se->_midi_adlib && !_isMIDI);
 
 	_parts = nullptr;
 	_active = true;
@@ -1031,6 +1033,7 @@ void Player::fixAfterLoad() {
 		_isMIDI = _se->isMIDI(_id);
 		_supportsPercussion = _se->supportsPercussion(_id);
 		_isGM = (_supportsPercussion && !_isMT32); // Unlike IMuseInternal::isMIDI(), IMuseInternal::supportsPercussion() really filters out all non-MIDI things...
+		_isAdLibOrFMTowns = (_se->_midi_adlib && !_isMIDI);
 	}
 }
 




More information about the Scummvm-git-logs mailing list