From 47a2f62c151c164b3e53392335b00af14a69f7a9 Mon Sep 17 00:00:00 2001 From: athrxx Date: Tue, 16 Jul 2019 23:18:42 +0200 Subject: SCI: (FB01 sound driver) - several fixes Fix SCI0 (LATE/EARLY) variants of the driver which were broken (didn't play at all). This might be of my own doing, since I introduced the initTrack() method into the driver class and the fb01 driver didn't have one so far. SCI1 didn't seem to require much fixing. I modified some things according to my findings in the driver disasms. QFG2 and JONES seem to be fine. I am not too happy with KQ5. It has volume issues, but they might be present in the original, too. I also added an isOpen() check and a mutex to avoid threading issues. When aborting SCI (either quitting ScummVM or returning ot the launcher) while using the fb01 driver I frequently (more often than not) got the assert from backends/midi/windows.cpp, line 95. This fixes that. I've done plenty of checks and experiments with the sound bank initialization. But I found no bugs or possible improvements there. Hard to tell whether the sound is right. That device seems to have a mind of its own... --- engines/sci/POTFILES | 1 + engines/sci/sound/drivers/fb01.cpp | 240 +++++++++++++++++++++++++++++++------ 2 files changed, 203 insertions(+), 38 deletions(-) diff --git a/engines/sci/POTFILES b/engines/sci/POTFILES index 015776c7a1..24576e871f 100644 --- a/engines/sci/POTFILES +++ b/engines/sci/POTFILES @@ -1,4 +1,5 @@ engines/sci/detection.cpp +engines/sci/sound/drivers/fb01.cpp engines/sci/engine/guest_additions.cpp engines/sci/engine/kfile.cpp engines/sci/engine/kgraphics.cpp diff --git a/engines/sci/sound/drivers/fb01.cpp b/engines/sci/sound/drivers/fb01.cpp index 3f3f581ee2..6f104b2ccd 100644 --- a/engines/sci/sound/drivers/fb01.cpp +++ b/engines/sci/sound/drivers/fb01.cpp @@ -29,6 +29,14 @@ #include "common/file.h" #include "common/system.h" #include "common/textconsole.h" +#include "common/mutex.h" + +// The original driver uses the master volume setting of the hardware device by sending sysex messages +// (applies to SCI0 and SCI1). We have a software mastervolume implementation instead. I don't know the +// reason for this. Nor do I know whether our software effective volume calculation matches the device's +// internal volume calculation algorithm. +// I add the original style master volume handling, but make it optional via a #define +#define HARDWARE_MASTERVOLUME namespace Sci { @@ -55,15 +63,22 @@ public: int open(ResourceManager *resMan); void close(); + void initTrack(SciSpan& header); void send(uint32 b); void sysEx(const byte *msg, uint16 length); bool hasRhythmChannel() const { return false; } byte getPlayId() const; - int getPolyphony() const { return kVoices; } // 9 in SCI1? + int getPolyphony() const { return _version <= SCI_VERSION_0_LATE ? 8 : 9; } void setVolume(byte volume); int getVolume(); void playSwitch(bool play); + bool isOpen() const { return _isOpen; } + void lockMutex() { _mutex.lock(); } + void unlockMutex() { _mutex.unlock(); } + + const char *reportMissingFiles() { return _missingFiles; } + private: void noteOn(int channel, int note, int velocity); void noteOff(int channel, int note); @@ -102,18 +117,23 @@ private: struct Voice { int8 channel; // MIDI channel that this voice is assigned to or -1 + uint8 poly; // Number of hardware voices (SCI0); for SCI1 we just set this to 1 int8 note; // Currently playing MIDI note or -1 int bank; // Current bank setting or -1 int patch; // Currently playing patch or -1 - uint8 velocity; // Note velocity - bool isSustained; // Flag indicating a note that is being sustained by the hold pedal + //uint8 velocity; // Note velocity + //bool isSustained; // Flag indicating a note that is being sustained by the hold pedal uint16 age; // Age of the current note - Voice() : channel(-1), note(-1), bank(-1), patch(-1), velocity(0), isSustained(false), age(0) { } + Voice() : channel(-1), note(-1), bank(-1), patch(-1), /*velocity(0), isSustained(false),*/ age(0), poly(1) { } }; bool _playSwitch; int _masterVolume; + int _numParts; + + bool _isOpen; + Common::Mutex _mutex; Channel _channels[16]; Voice _voices[kVoices]; @@ -123,10 +143,14 @@ private: static void midiTimerCallback(void *p); void setTimerCallback(void *timer_param, Common::TimerManager::TimerProc timer_proc); + const char *_missingFiles; + static const char _requiredFiles[2][12]; + byte _sysExBuf[kMaxSysExSize]; }; -MidiPlayer_Fb01::MidiPlayer_Fb01(SciVersion version) : MidiPlayer(version), _playSwitch(true), _masterVolume(15), _timerParam(NULL), _timerProc(NULL) { +MidiPlayer_Fb01::MidiPlayer_Fb01(SciVersion version) : MidiPlayer(version), _playSwitch(true), _masterVolume(15), _timerParam(NULL), _timerProc(NULL), + _numParts(version > SCI_VERSION_0_LATE ? kVoices : 0), _isOpen(false), _missingFiles(0) { MidiDriver::DeviceHandle dev = MidiDriver::detectDevice(MDT_MIDI); _driver = MidiDriver::createMidi(dev); @@ -135,10 +159,23 @@ MidiPlayer_Fb01::MidiPlayer_Fb01(SciVersion version) : MidiPlayer(version), _pla } MidiPlayer_Fb01::~MidiPlayer_Fb01() { + Common::StackLock lock(_mutex); + close(); delete _driver; } void MidiPlayer_Fb01::voiceMapping(int channel, int voices) { + if (_version <= SCI_VERSION_0_LATE) { + // The original SCI0 drivers don't do any software voice mapping. Only inside the device... + for (int i = 0; i < _numParts; ++i) { + if (_voices[i].channel == channel && _voices[i].poly != voices) { + _voices[i].poly = voices; + setVoiceParam(i, 0, voices); + } + } + return; + } + int curVoices = 0; for (int i = 0; i < kVoices; i++) @@ -163,6 +200,8 @@ void MidiPlayer_Fb01::assignVoices(int channel, int voices) { for (int i = 0; i < kVoices; i++) { if (_voices[i].channel == -1) { _voices[i].channel = channel; + if (_voices[i].note != -1) + voiceOff(i); if (--voices == 0) break; } @@ -264,16 +303,23 @@ int MidiPlayer_Fb01::findVoice(int channel) { } void MidiPlayer_Fb01::sendToChannel(byte channel, byte command, byte op1, byte op2) { - for (int i = 0; i < kVoices; i++) { + for (int i = 0; i < _numParts; i++) { // Send command to all voices assigned to this channel + // I case of SCI0 the voice mapping is done inside the device. if (_voices[i].channel == channel) - _driver->send(command | i, op1, op2); + _driver->send(command | (_version <= SCI_VERSION_0_LATE ? channel : i), op1, op2); } } void MidiPlayer_Fb01::setPatch(int channel, int patch) { int bank = 0; + if (_version <= SCI_VERSION_0_LATE && channel == 15) { + // The original driver has some parsing related handling for program 127. + // We can't handle that here. + return; + } + _channels[channel].patch = patch; if (patch >= 48) { @@ -281,13 +327,13 @@ void MidiPlayer_Fb01::setPatch(int channel, int patch) { bank = 1; } - for (int voice = 0; voice < kVoices; voice++) { + for (int voice = 0; voice < _numParts; voice++) { if (_voices[voice].channel == channel) { if (_voices[voice].bank != bank) { _voices[voice].bank = bank; setVoiceParam(voice, 4, bank); } - _driver->send(0xc0 | voice, patch, 0); + _driver->send(0xc0 | (_version <= SCI_VERSION_0_LATE ? channel : voice), patch, 0); } } } @@ -320,12 +366,20 @@ void MidiPlayer_Fb01::noteOn(int channel, int note, int velocity) { return noteOff(channel, note); if (_version > SCI_VERSION_0_LATE) - velocity = volumeTable[velocity >> 1] << 1; + velocity >>= 1; int voice; for (voice = 0; voice < kVoices; voice++) { if ((_voices[voice].channel == channel) && (_voices[voice].note == note)) { voiceOff(voice); + + // Original bug #1: The table is only applied here, but not to the voiceOn() call below. + // Original bug #2: The velocity value also gets another right shift although the table + // size of the volume table is 64 and not 32. + // --> disable this ? It certainly can't do any good. + if (_version > SCI_VERSION_0_LATE) + velocity = volumeTable[velocity >> 1] << 1; + voiceOn(voice, note, velocity); return; } @@ -342,6 +396,12 @@ void MidiPlayer_Fb01::noteOn(int channel, int note, int velocity) { } void MidiPlayer_Fb01::controlChange(int channel, int control, int value) { + // Events for the control channel shouldn't arrive here. + // The original driver handles some specific parsing related midi events + // sent on channel 15, but we (hopefully) do that in the SCI music engine. + if (_version <= SCI_VERSION_0_LATE && channel == 15) + return; + switch (control) { case 0x07: { _channels[channel].volume = value; @@ -349,12 +409,16 @@ void MidiPlayer_Fb01::controlChange(int channel, int control, int value) { if (_version > SCI_VERSION_0_LATE) value = volumeTable[value >> 1] << 1; +#ifdef HARDWARE_MASTERVOLUME + sendToChannel(channel, 0xb0, control, value); +#else byte vol = _masterVolume; if (vol > 0) vol = CLIP(vol + 3, 0, 15); sendToChannel(channel, 0xb0, control, (value * vol / 15) & 0x7f); +#endif break; } case 0x0a: @@ -366,16 +430,19 @@ void MidiPlayer_Fb01::controlChange(int channel, int control, int value) { sendToChannel(channel, 0xb0, control, value); break; case 0x4b: - // In early SCI0, voice count 15 signifies that the channel should be ignored - // for this song. Assuming that there are no embedded voice count commands in - // the MIDI stream, we should be able to get away with simply setting the voice - // count for this channel to 0. - voiceMapping(channel, (value != 15 ? value : 0)); + voiceMapping(channel, value); break; case 0x7b: - for (int i = 0; i < kVoices; i++) - if ((_voices[i].channel == channel) && (_voices[i].note != -1)) - voiceOff(i); + for (int i = 0; i < _numParts; i++) { + if ((_voices[i].channel == channel) && (_voices[i].note != -1)) { + _voices[i].note = -1; + sendToChannel(channel, 0xb0, control, value); + } + } + break; + default: + sendToChannel(channel, 0xb0, control, value); + break; } } @@ -385,6 +452,16 @@ void MidiPlayer_Fb01::send(uint32 b) { byte op1 = (b >> 8) & 0x7f; byte op2 = (b >> 16) & 0x7f; + if (_version <= SCI_VERSION_0_LATE && command != 0xB0 && command != 0xC0) { + // Since the voice mapping takes place inside the hardware, most messages + // are simply passed through. Channel 15 is never assigned to a part but is + // used for certain parsing related events which we cannot handle here. + // Just making sure that no nonsense is sent to the device... + if (channel != 15) + sendToChannel(channel, command, op1, op2); + return; + } + switch (command) { case 0x80: noteOff(channel, op1); @@ -409,9 +486,12 @@ void MidiPlayer_Fb01::send(uint32 b) { void MidiPlayer_Fb01::setVolume(byte volume) { _masterVolume = volume; - +#ifdef HARDWARE_MASTERVOLUME + setSystemParam(0, 0x24, (CLIP(_masterVolume + 3, 0, 15) << 3) + 7); +#else for (uint i = 0; i < MIDI_CHANNELS; i++) controlChange(i, 0x07, _channels[i].volume & 0x7f); +#endif } int MidiPlayer_Fb01::getVolume() { @@ -419,11 +499,20 @@ int MidiPlayer_Fb01::getVolume() { } void MidiPlayer_Fb01::playSwitch(bool play) { + _playSwitch = play; } void MidiPlayer_Fb01::midiTimerCallback(void *p) { + if (!p) + return; + MidiPlayer_Fb01 *m = (MidiPlayer_Fb01 *)p; + m->lockMutex(); + + if (!m->isOpen()) + return; + // Increase the age of the notes for (int i = 0; i < kVoices; i++) { if (m->_voices[i].note != -1) @@ -432,6 +521,8 @@ void MidiPlayer_Fb01::midiTimerCallback(void *p) { if (m->_timerProc) m->_timerProc(m->_timerParam); + + m->unlockMutex(); } void MidiPlayer_Fb01::setTimerCallback(void *timer_param, Common::TimerManager::TimerProc timer_proc) { @@ -456,11 +547,15 @@ void MidiPlayer_Fb01::sendBanks(const SciSpan &data) { } // Send second bank if available - if (data.size() >= 6146 && data.getUint16BEAt(3072) == 0xabcd) { - for (int i = 0; i < 48; i++) { - sendVoiceData(0, data.subspan(3074 + i * 64)); - storeVoiceData(0, 1, i); - } + if (data.size() < 6146) + return; + + if (data.getUint16BEAt(3072) != 0xabcd) + return; + + for (int i = 0; i < 48; i++) { + sendVoiceData(0, data.subspan(3074 + i * 64)); + storeVoiceData(0, 1, i); } } @@ -473,9 +568,8 @@ int MidiPlayer_Fb01::open(ResourceManager *resMan) { return retval; } - // Set system channel to 0. We send this command over all 16 system channels - for (int i = 0; i < 16; i++) - setSystemParam(i, 0x20, 0); + // Set system channel to 0. + setSystemParam(0, 0x20, 0); // Turn off memory protection setSystemParam(0, 0x21, 0); @@ -511,9 +605,19 @@ int MidiPlayer_Fb01::open(ResourceManager *resMan) { if (offset >= buf->size()) error("Failed to locate start of FB-01 sound bank"); + // The newer IMF.DRV versions without the sound bank will still have the 32 byte + // "SIERRA 1" header, but after that they want to send the patch.002 resource. + // These driver version are easy to identify by their small size. + if (buf->subspan(offset).size() < 3072) { + _missingFiles = _requiredFiles[1]; + return MidiDriver::MERR_DEVICE_NOT_AVAILABLE; + } + sendBanks(buf->subspan(offset)); - } else - error("Failed to open IMF.DRV"); + } else { + _missingFiles = _version == SCI_VERSION_0_EARLY ? _requiredFiles[0] : _requiredFiles[1]; + return MidiDriver::MERR_DEVICE_NOT_AVAILABLE; + } } // Set up voices to use MIDI channels 0 - 7 @@ -525,13 +629,65 @@ int MidiPlayer_Fb01::open(ResourceManager *resMan) { // Set master volume setSystemParam(0, 0x24, 0x7f); + _isOpen = true; + return 0; } void MidiPlayer_Fb01::close() { + Common::StackLock lock(_mutex); + _isOpen = false; _driver->close(); } +void MidiPlayer_Fb01::initTrack(SciSpan& header) { + if (!_isOpen || _version > SCI_VERSION_0_LATE) + return; + + uint8 readPos = 0; + uint8 caps = header.getInt8At(readPos++); + if (caps != 0 && (_version == SCI_VERSION_0_EARLY || caps != 2)) + return; + + for (int i = 0; i < 8; ++i) + _voices[i] = Voice(); + + _numParts = 0; + for (int i = 0; i < 16; ++i) { + if (_version == SCI_VERSION_0_LATE) { + uint8 num = header.getInt8At(readPos++) & 0x7F; + uint8 flags = header.getInt8At(readPos++); + + if (flags & 0x02) { + _voices[_numParts].channel = i; + _voices[_numParts].poly = num; + _numParts++; + } + } else { + uint8 val = header.getInt8At(readPos++); + if (val & 0x01) { + if (val & 0x08) { + if (val >> 4) + debugC(9, kDebugLevelSound, "MidiPlayer_Fb01::initTrack(): Unused rhythm channel found: 0x%.02x", i); + } else if ((val >> 4) != 0x0F) { + _voices[_numParts].channel = i; + _voices[_numParts].poly = val >> 4; + _numParts++; + } + } else if (val & 0x08) { + debugC(9, kDebugLevelSound, "MidiPlayer_Fb01::initTrack(): Control channel found: 0x%.02x", i); + } + } + } + + for (int i = 0; i < _numParts; ++i) + setVoiceParam(i, 1, _voices[i].channel); + + initVoices(); + + setVolume(_masterVolume); +} + void MidiPlayer_Fb01::setVoiceParam(byte voice, byte param, byte value) { _sysExBuf[2] = 0x00; _sysExBuf[3] = 0x18 | voice; @@ -592,28 +748,29 @@ void MidiPlayer_Fb01::initVoices() { _sysExBuf[i++] = 0x00; } - // Set up the 8 MIDI channels we will be using - for (int j = 0; j < 8; j++) { + // Set up the MIDI channels we will be using + for (int j = 0; j < _numParts; j++) { + int8 chan = _version > SCI_VERSION_0_LATE ? j : _voices[j].channel; // One voice - _sysExBuf[i++] = 0x70 | j; + _sysExBuf[i++] = 0x70 | chan; _sysExBuf[i++] = 0x00; - _sysExBuf[i++] = 0x01; + _sysExBuf[i++] = _voices[j].poly; // Full range of keys - _sysExBuf[i++] = 0x70 | j; + _sysExBuf[i++] = 0x70 | chan; _sysExBuf[i++] = 0x02; _sysExBuf[i++] = 0x7f; - _sysExBuf[i++] = 0x70 | j; + _sysExBuf[i++] = 0x70 | chan; _sysExBuf[i++] = 0x03; _sysExBuf[i++] = 0x00; // Voice bank 0 - _sysExBuf[i++] = 0x70 | j; + _sysExBuf[i++] = 0x70 | chan; _sysExBuf[i++] = 0x04; _sysExBuf[i++] = 0x00; // Voice 10 - _sysExBuf[i++] = 0x70 | j; + _sysExBuf[i++] = 0x70 | chan; _sysExBuf[i++] = 0x05; _sysExBuf[i++] = 0x0a; } @@ -636,7 +793,7 @@ void MidiPlayer_Fb01::sysEx(const byte *msg, uint16 length) { byte MidiPlayer_Fb01::getPlayId() const { switch (_version) { case SCI_VERSION_0_EARLY: - return 0x01; + return 0x09; case SCI_VERSION_0_LATE: return 0x02; default: @@ -644,8 +801,15 @@ byte MidiPlayer_Fb01::getPlayId() const { } } +const char MidiPlayer_Fb01::_requiredFiles[2][12] = { + "'IMF.DRV'", + "'PATCH.002'" +}; + MidiPlayer *MidiPlayer_Fb01_create(SciVersion version) { return new MidiPlayer_Fb01(version); } } // End of namespace Sci + +#undef HARDWARE_MASTERVOLUME \ No newline at end of file -- cgit v1.2.3