aboutsummaryrefslogtreecommitdiff
path: root/engines/sci
diff options
context:
space:
mode:
authorathrxx2019-07-16 23:18:42 +0200
committerathrxx2019-08-07 16:43:06 +0200
commit47a2f62c151c164b3e53392335b00af14a69f7a9 (patch)
treeb8d09bd39c21a54e071b71e126b5c11e3de1e0f0 /engines/sci
parent0e73472207b18e406f51e8bca9c3450dd908ec38 (diff)
downloadscummvm-rg350-47a2f62c151c164b3e53392335b00af14a69f7a9.tar.gz
scummvm-rg350-47a2f62c151c164b3e53392335b00af14a69f7a9.tar.bz2
scummvm-rg350-47a2f62c151c164b3e53392335b00af14a69f7a9.zip
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...
Diffstat (limited to 'engines/sci')
-rw-r--r--engines/sci/POTFILES1
-rw-r--r--engines/sci/sound/drivers/fb01.cpp240
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<const byte>& 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<byte>(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<byte>(_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<const byte> &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<const byte>& 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