[Scummvm-git-logs] scummvm master -> db130d6dab7cc01df67b1d88ed86496f49bd888c
NMIError
noreply at scummvm.org
Sat Nov 2 17:11:37 UTC 2024
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:
db130d6dab DARKSEED: Improve CD music accuracy
Commit: db130d6dab7cc01df67b1d88ed86496f49bd888c
https://github.com/scummvm/scummvm/commit/db130d6dab7cc01df67b1d88ed86496f49bd888c
Author: Coen Rampen (crampen at gmail.com)
Date: 2024-11-02T18:11:29+01:00
Commit Message:
DARKSEED: Improve CD music accuracy
Changed paths:
audio/adlib_ms.cpp
audio/adlib_ms.h
engines/darkseed/adlib_worx.cpp
engines/darkseed/adlib_worx.h
engines/darkseed/music.cpp
engines/darkseed/sound.cpp
diff --git a/audio/adlib_ms.cpp b/audio/adlib_ms.cpp
index 8ecec4052cc..cd5cac691c5 100644
--- a/audio/adlib_ms.cpp
+++ b/audio/adlib_ms.cpp
@@ -429,6 +429,7 @@ void MidiDriver_ADLIB_Multisource::ActiveNote::init() {
noteCounterValue = 0;
instrumentId = 0;
+ lastWrittenInstrumentId = -1;
instrumentDef = nullptr;
channelAllocated = false;
@@ -740,6 +741,19 @@ void MidiDriver_ADLIB_Multisource::noteOn(uint8 channel, uint8 note, uint8 veloc
// Write out the instrument definition, volume and panning.
writeInstrument(oplChannel, instrument);
}
+ else if (_instrumentWriteMode == INSTRUMENT_WRITE_MODE_FIRST_NOTE_ON) {
+ if (activeNote->lastWrittenInstrumentId != activeNote->instrumentId) {
+ // Write out the instrument definition, volume and panning.
+ writeInstrument(oplChannel, instrument);
+ }
+ else {
+ // Write out volume, if applicable.
+ for (int i = 0; i < activeNote->instrumentDef->getNumberOfOperators(); i++) {
+ if (isVolumeApplicableToOperator(*activeNote->instrumentDef, i))
+ writeVolume(oplChannel, i);
+ }
+ }
+ }
// Calculate and write frequency and block and write key on bit.
writeFrequency(oplChannel, instrument.instrumentDef->rhythmType);
@@ -1624,51 +1638,8 @@ uint8 MidiDriver_ADLIB_Multisource::calculateVolume(uint8 channel, uint8 source,
// Get the volume (level) for this operator from the instrument definition.
uint8 operatorDefVolume = instrumentDef.getOperatorDefinition(operatorNum).level & 0x3F;
- // Determine if volume settings should be applied to this operator. Carrier
- // operators in FM synthesis and all operators in additive synthesis need
- // to have volume settings applied; modulator operators just use the
- // instrument definition volume.
- bool applyVolume = false;
- if (instrumentDef.rhythmType != RHYTHM_TYPE_UNDEFINED) {
- applyVolume = (instrumentDef.rhythmType != RHYTHM_TYPE_BASS_DRUM || operatorNum == 1);
- } else if (instrumentDef.fourOperator) {
- // 4 operator instruments have 4 different operator connections.
- uint8 connection = (instrumentDef.connectionFeedback0 & 0x01) | ((instrumentDef.connectionFeedback1 & 0x01) << 1);
- switch (connection) {
- case 0x00:
- // 4FM
- // Operator 3 is a carrier.
- applyVolume = (operatorNum == 3);
- break;
- case 0x01:
- // 1ADD+3FM
- // Operator 0 is additive and operator 3 is a carrier.
- applyVolume = (operatorNum == 0 || operatorNum == 3);
- break;
- case 0x10:
- // 2FM+2FM
- // Operators 1 and 3 are carriers.
- applyVolume = (operatorNum == 1 || operatorNum == 3);
- break;
- case 0x11:
- // 1ADD+2FM+1ADD
- // Operators 0 and 3 are additive and operator 2 is a carrier.
- applyVolume = (operatorNum == 0 || operatorNum == 2 || operatorNum == 3);
- break;
- default:
- // Should not happen.
- applyVolume = false;
- }
- } else {
- // 2 operator instruments have 2 different operator connections:
- // additive (0x01) or FM (0x00) synthesis. Carrier operators in FM
- // synthesis and all operators in additive synthesis need to have
- // volume settings applied; modulator operators just use the instrument
- // definition volume. In FM synthesis connection, operator 1 is a
- // carrier.
- applyVolume = (instrumentDef.connectionFeedback0 & 0x01) == 0x01 || operatorNum == 1;
- }
- if (!applyVolume)
+ // Determine if volume settings should be applied to this operator.
+ if (!isVolumeApplicableToOperator(instrumentDef, operatorNum))
// No need to apply volume settings; just use the instrument definition
// operator volume.
return operatorDefVolume;
@@ -1733,6 +1704,54 @@ uint8 MidiDriver_ADLIB_Multisource::calculateUnscaledVolume(uint8 channel, uint8
return MIN((uint8)0x3F, unscaledVolume);
}
+bool MidiDriver_ADLIB_Multisource::isVolumeApplicableToOperator(OplInstrumentDefinition &instrumentDef, uint8 operatorNum) {
+ // Determine if volume settings should be applied to this operator. Carrier
+ // operators in FM synthesis and all operators in additive synthesis need
+ // to have volume settings applied; modulator operators just use the
+ // instrument definition volume.
+ bool applyVolume = false;
+ if (instrumentDef.rhythmType != RHYTHM_TYPE_UNDEFINED) {
+ applyVolume = (instrumentDef.rhythmType != RHYTHM_TYPE_BASS_DRUM || operatorNum == 1);
+ } else if (instrumentDef.fourOperator) {
+ // 4 operator instruments have 4 different operator connections.
+ uint8 connection = (instrumentDef.connectionFeedback0 & 0x01) | ((instrumentDef.connectionFeedback1 & 0x01) << 1);
+ switch (connection) {
+ case 0x00:
+ // 4FM
+ // Operator 3 is a carrier.
+ applyVolume = (operatorNum == 3);
+ break;
+ case 0x01:
+ // 1ADD+3FM
+ // Operator 0 is additive and operator 3 is a carrier.
+ applyVolume = (operatorNum == 0 || operatorNum == 3);
+ break;
+ case 0x10:
+ // 2FM+2FM
+ // Operators 1 and 3 are carriers.
+ applyVolume = (operatorNum == 1 || operatorNum == 3);
+ break;
+ case 0x11:
+ // 1ADD+2FM+1ADD
+ // Operators 0 and 3 are additive and operator 2 is a carrier.
+ applyVolume = (operatorNum == 0 || operatorNum == 2 || operatorNum == 3);
+ break;
+ default:
+ // Should not happen.
+ applyVolume = false;
+ }
+ } else {
+ // 2 operator instruments have 2 different operator connections:
+ // additive (0x01) or FM (0x00) synthesis. Carrier operators in FM
+ // synthesis and all operators in additive synthesis need to have
+ // volume settings applied; modulator operators just use the instrument
+ // definition volume. In FM synthesis connection, operator 1 is a
+ // carrier.
+ applyVolume = (instrumentDef.connectionFeedback0 & 0x01) == 0x01 || operatorNum == 1;
+ }
+ return applyVolume;
+}
+
uint8 MidiDriver_ADLIB_Multisource::calculatePanning(uint8 channel, uint8 source) {
if (_oplType != OPL::Config::kOpl3)
return 0;
@@ -1839,6 +1858,7 @@ uint16 MidiDriver_ADLIB_Multisource::determineChannelRegisterOffset(uint8 oplCha
void MidiDriver_ADLIB_Multisource::writeInstrument(uint8 oplChannel, InstrumentInfo instrument) {
ActiveNote *activeNote = (instrument.instrumentDef->rhythmType == RHYTHM_TYPE_UNDEFINED ? &_activeNotes[oplChannel] : &_activeRhythmNotes[instrument.instrumentDef->rhythmType - 1]);
activeNote->instrumentDef = instrument.instrumentDef;
+ activeNote->lastWrittenInstrumentId = instrument.instrumentId;
// Calculate operator volumes and write operator definitions to
// the OPL registers.
diff --git a/audio/adlib_ms.h b/audio/adlib_ms.h
index 115fe87b10b..0c3149643c7 100644
--- a/audio/adlib_ms.h
+++ b/audio/adlib_ms.h
@@ -335,7 +335,12 @@ public:
* write the instrument only once for many notes and allows parameters
* of the instrument to be changed for the following notes.
*/
- INSTRUMENT_WRITE_MODE_PROGRAM_CHANGE
+ INSTRUMENT_WRITE_MODE_PROGRAM_CHANGE,
+ /**
+ * Will write the instrument definition on the first note played with
+ * a new instrument on a channel.
+ */
+ INSTRUMENT_WRITE_MODE_FIRST_NOTE_ON
};
/**
@@ -593,6 +598,11 @@ protected:
* rhythm instruments (@see determineInstrument).
*/
uint8 instrumentId;
+ /**
+ * The ID of the instrument that was last written to the OPL channel,
+ * or -1 if no instrument was written yet.
+ */
+ int16 lastWrittenInstrumentId;
/**
* Pointer to the instrument definition used to play the note.
*/
@@ -1033,6 +1043,16 @@ protected:
* @return The calculated unscaled operator volume (level).
*/
virtual uint8 calculateUnscaledVolume(uint8 channel, uint8 source, uint8 velocity, OplInstrumentDefinition &instrumentDef, uint8 operatorNum);
+ /**
+ * Determines if volume settings should be applied to the operator level.
+ * This depends on the type of the operator (carrier or modulator), which
+ * depends on the type of connection specified in the instrument.
+ *
+ * @param instrumentDef The instrument definition
+ * @param operatorNum The number of the operator (0-1 or 0-3)
+ * @return True if volume should be applied, false otherwise
+ */
+ virtual bool isVolumeApplicableToOperator(OplInstrumentDefinition &instrumentDef, uint8 operatorNum);
/**
* Determines the panning that should be applied to notes played on the
* specified MIDI channel and source.
diff --git a/engines/darkseed/adlib_worx.cpp b/engines/darkseed/adlib_worx.cpp
index d705eabfbd7..92b355887f1 100644
--- a/engines/darkseed/adlib_worx.cpp
+++ b/engines/darkseed/adlib_worx.cpp
@@ -170,6 +170,10 @@ AdLibIbkInstrumentDefinition MidiDriver_Worx_AdLib::WORX_INSTRUMENT_BANK[128] =
{ 0x24, 0x31, 0x54, 0x10, 0x55, 0x50, 0xfd, 0x2d, 0x00, 0x00, 0x0e, 0x00, 0x00, 0x00, 0x00, 0x00 }
};
+const uint16 MidiDriver_Worx_AdLib::OPL_NOTE_FREQUENCIES[12] = {
+ 0x16B, 0x181, 0x198, 0x1B0, 0x1CA, 0x1E5, 0x202, 0x220, 0x241, 0x263, 0x287, 0x2AE
+};
+
MidiDriver_Worx_AdLib::MidiDriver_Worx_AdLib(OPL::Config::OplType oplType, int timerFrequency) :
MidiDriver_ADLIB_Multisource::MidiDriver_ADLIB_Multisource(oplType, timerFrequency) {
@@ -177,10 +181,18 @@ MidiDriver_Worx_AdLib::MidiDriver_Worx_AdLib(OPL::Config::OplType oplType, int t
for (int i = 0; i < 128; i++) {
WORX_INSTRUMENT_BANK[i].toOplInstrumentDefinition(_instrumentBank[i]);
- }
+ // The original code does not add the key scale level bits (bits 6 and 7)
+ // from the instrument definition to the level before it writes the 0x4x
+ // register value, so effectively, KSL is always disabled for operator 1.
+ // This is probably an oversight, but this behavior is implemented here
+ // by clearing the KSL bits of operator 1 in the instrument definition.
+ _instrumentBank[i].operator1.level &= 0x3F;
+ }
+
_defaultChannelVolume = 0x7F;
_channel10Melodic = true;
+ _instrumentWriteMode = INSTRUMENT_WRITE_MODE_FIRST_NOTE_ON;
}
MidiDriver_Worx_AdLib::~MidiDriver_Worx_AdLib() {
@@ -197,32 +209,45 @@ uint8 MidiDriver_Worx_AdLib::allocateOplChannel(uint8 channel, uint8 source, uin
uint8 oplChannel = _melodicChannels[i];
if (_activeNotes[oplChannel].channelAllocated && _activeNotes[oplChannel].source == source && _activeNotes[oplChannel].channel == channel &&
!_activeNotes[oplChannel].noteActive && unusedChannel == 0xFF) {
+ // This OPL channel is already allocated to this source and MIDI
+ // channel, but it is not playing a note. Use this channel.
unusedChannel = oplChannel;
break;
}
if (!_activeNotes[oplChannel].channelAllocated && unallocatedChannel == 0xFF) {
+ // This channel is unallocated. If no unallocated channel has been
+ // found yet, register this channel.
unallocatedChannel = oplChannel;
}
if (_activeNotes[oplChannel].channelAllocated && !(_activeNotes[oplChannel].source == source && _activeNotes[oplChannel].channel == channel) &&
!_activeNotes[oplChannel].noteActive && unusedAllocatedChannel == 0xFF) {
+ // This channel is allocated to a different source and/or MIDI
+ // channel, but it is not playing a note. If a channel of this
+ // type has not yet been found, register it.
unusedAllocatedChannel = oplChannel;
}
}
if (unusedChannel != 0xFF) {
+ // Found an allocated but unused channel.
allocatedChannel = unusedChannel;
}
else if (unallocatedChannel != 0xFF) {
+ // Found an unallocated channel.
allocatedChannel = unallocatedChannel;
}
else if (unusedAllocatedChannel != 0xFF) {
+ // Found an unused channel allocated to a different source / MIDI channel.
allocatedChannel = unusedAllocatedChannel;
}
else {
+ // All channels are playing notes. No channel is freed, so this will
+ // result in the note not being played. Return 0xFF.
_allocationMutex.unlock();
return allocatedChannel;
}
+ // Allocate the OPL channel to the source / MIDI channel.
_activeNotes[allocatedChannel].channelAllocated = true;
_activeNotes[allocatedChannel].source = source;
_activeNotes[allocatedChannel].channel = channel;
@@ -232,4 +257,33 @@ uint8 MidiDriver_Worx_AdLib::allocateOplChannel(uint8 channel, uint8 source, uin
return allocatedChannel;
}
+uint16 MidiDriver_Worx_AdLib::calculateFrequency(uint8 channel, uint8 source, uint8 note) {
+ // Notes for melodic instruments are transposed down by 13 semitones.
+ uint8 transposedNote = MAX(note - 12 - 1, 0);
+
+ // TODO Implement transpose based on transpose controllers
+
+ // Get F-num based on octave note. Note: Worx does not support pitch bend.
+ uint8 octaveNote = transposedNote % 12;
+ uint16 oplFrequency = OPL_NOTE_FREQUENCIES[octaveNote];
+
+ // Get block (octave).
+ uint8 block = transposedNote / 12;
+ block = MIN(block, (uint8) 7);
+
+ // Combine the block and frequency in the OPL Ax and Bx register format.
+ return oplFrequency | (block << 10);
+}
+
+uint8 MidiDriver_Worx_AdLib::calculateUnscaledVolume(uint8 channel, uint8 source, uint8 velocity, OplInstrumentDefinition &instrumentDef, uint8 operatorNum) {
+ // Worx calculates volume by scaling the instrument operator volume by the
+ // channel volume. Note velocity is not used.
+ uint8 operatorVolume = 0x3F - (instrumentDef.getOperatorDefinition(operatorNum).level & 0x3F);
+ uint8 channelVolume = _controlData[source][channel].volume;
+ // Note: the original code loses 1 bit of precision here (bit 0 is always 0).
+ uint8 unscaledVolume = (operatorVolume * channelVolume) >> 7;
+
+ return 0x3F - unscaledVolume;
+}
+
} // namespace Darkseed
diff --git a/engines/darkseed/adlib_worx.h b/engines/darkseed/adlib_worx.h
index 46f581cd2e3..a111f9b705c 100644
--- a/engines/darkseed/adlib_worx.h
+++ b/engines/darkseed/adlib_worx.h
@@ -26,16 +26,38 @@
namespace Darkseed {
+/**
+ * Implementation of the AdLib code from the Worx Toolkit library,
+ * based on version 2.1, as used by Dark Seed (CD version).
+ * Implements the OPL channel allocation algorithm and note frequency
+ * and volume calculation.
+ *
+ * TODO Implementation is incomplete, because Dark Seed does not use
+ * some functionality of the library:
+ * - OPL rhythm mode
+ * - Transpose controllers 0x68 and 0x69 (seems to be buggy)
+ */
class MidiDriver_Worx_AdLib : public MidiDriver_ADLIB_Multisource {
private:
+ /**
+ * The OPL instrument bank of the Worx Toolkit.
+ * This was taken from the Dark Seed executable and might have been
+ * customized for the game.
+ */
static AdLibIbkInstrumentDefinition WORX_INSTRUMENT_BANK[];
+ /**
+ * The OPL frequency (F-num) for each octave note.
+ */
+ static const uint16 OPL_NOTE_FREQUENCIES[];
public:
- MidiDriver_Worx_AdLib(OPL::Config::OplType oplType, int timerFrequency);
+ MidiDriver_Worx_AdLib(OPL::Config::OplType oplType, int timerFrequency = OPL::OPL::kDefaultCallbackFrequency);
~MidiDriver_Worx_AdLib();
protected:
uint8 allocateOplChannel(uint8 channel, uint8 source, uint8 instrumentId) override;
+ uint16 calculateFrequency(uint8 channel, uint8 source, uint8 note) override;
+ uint8 calculateUnscaledVolume(uint8 channel, uint8 source, uint8 velocity, OplInstrumentDefinition &instrumentDef, uint8 operatorNum) override;
};
} // namespace Darkseed
diff --git a/engines/darkseed/music.cpp b/engines/darkseed/music.cpp
index 898ae548add..693896fdf69 100644
--- a/engines/darkseed/music.cpp
+++ b/engines/darkseed/music.cpp
@@ -65,8 +65,7 @@ int MusicPlayer::open() {
} else {
switch (_deviceType) {
case MT_ADLIB:
- // TODO Verify timer frequency
- _driver = new MidiDriver_Worx_AdLib(OPL::Config::kOpl2, 250);
+ _driver = new MidiDriver_Worx_AdLib(OPL::Config::kOpl2);
// Some tracks do not set instruments and expect instrument 0
// to be set on each channel. Make sure this is done every time
// a track starts.
@@ -79,6 +78,7 @@ int MusicPlayer::open() {
break;
}
+ // CD version uses SMF data
_parser = MidiParser::createParser_SMF(0);
}
@@ -87,8 +87,10 @@ int MusicPlayer::open() {
_parser->property(MidiParser::mpDisableAutoStartPlayback, true);
int returnCode = _driver->open();
- if (returnCode != 0)
+ if (returnCode != 0) {
error("MusicPlayer::open - Failed to open MIDI driver - error code %d.", returnCode);
+ return 1;
+ }
syncSoundSettings();
@@ -103,6 +105,7 @@ int MusicPlayer::open() {
void MusicPlayer::onTimer(void *data) {
MusicPlayer *p = (MusicPlayer *)data;
+
Common::StackLock lock(p->_mutex);
if (p->_parser) {
@@ -112,6 +115,7 @@ void MusicPlayer::onTimer(void *data) {
bool MusicPlayer::isPlaying() {
Common::StackLock lock(_mutex);
+
return _parser && _parser->isPlaying();
}
@@ -172,6 +176,8 @@ void MusicPlayer::load(Common::SeekableReadStream *in, int32 size) {
in->seek(startPos);
}
+ if (isPlaying())
+ stop();
_parser->unloadMusic();
if (_musicData) {
delete[] _musicData;
diff --git a/engines/darkseed/sound.cpp b/engines/darkseed/sound.cpp
index ccb44b89653..f8a2953a3b4 100644
--- a/engines/darkseed/sound.cpp
+++ b/engines/darkseed/sound.cpp
@@ -175,7 +175,6 @@ void Sound::killAllSound() {
stopMusic();
}
-
void Sound::syncSoundSettings() {
_musicPlayer->syncSoundSettings();
}
More information about the Scummvm-git-logs
mailing list