[Scummvm-git-logs] scummvm master -> f96819149f70a1459218d9fd5b13853f5ce8a32c
bluegr
noreply at scummvm.org
Sun Jun 19 17:46:41 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:
f96819149f SCI: (SCI1+) - improve channel sound flag handling
Commit: f96819149f70a1459218d9fd5b13853f5ce8a32c
https://github.com/scummvm/scummvm/commit/f96819149f70a1459218d9fd5b13853f5ce8a32c
Author: athrxx (athrxx at scummvm.org)
Date: 2022-06-19T20:46:37+03:00
Commit Message:
SCI: (SCI1+) - improve channel sound flag handling
This cleans up some things I noticed when I implemented the mute flag handling. It removes the hack that manually forces flag 2 on all channels with number 9, regardless of the device.
I presume the hack was done for the earliest SCI 1 games, due to improper handling of flag 1.
Changed paths:
engines/sci/resource/resource_audio.cpp
engines/sci/sound/midiparser_sci.cpp
engines/sci/sound/music.cpp
engines/sci/sound/music.h
diff --git a/engines/sci/resource/resource_audio.cpp b/engines/sci/resource/resource_audio.cpp
index 7cfb9d917ea..6b10af29201 100644
--- a/engines/sci/resource/resource_audio.cpp
+++ b/engines/sci/resource/resource_audio.cpp
@@ -927,18 +927,33 @@ SoundResource::SoundResource(uint32 resourceNr, ResourceManager *resMan, SciVers
} else {
channel->flags = channel->number >> 4;
channel->number = channel->number & 0x0F;
-
- // 0x20 is set on rhythm channels to prevent remapping
- // CHECKME: Which SCI versions need that set manually?
- if (channel->number == 9)
- channel->flags |= 2;
- // Note: flag 1: channel start offset is 0 instead of 10
- // (currently: everything 0)
- // also: don't map the channel to device
- // flag 2: don't remap
- // flag 4: start muted
- // QfG2 lacks flags 2 and 4, and uses (flags >= 1) as
- // the condition for starting offset 0, without the "don't map"
+ // Flag 1: Channel start offset is 0 instead of 10 (currently: everything 0)
+ // Also: Don't map the channel to the device at all, but still play it.
+ // It doesn't stop other sounds playing sounds on that channel, it even
+ // allows other sounds to map to this channel (in that case the dontmap
+ // channel has limited access, it can't send control change, program
+ // change and pitch wheel messages.
+ // This is basically a marker for the channel as a "real" channel
+ // (used mostly for rhythm channels on devices that have one). These
+ // channels will also consequently start the parsing at offset 0 instead
+ // of 10: Normal channels would read the parameters of the first couple of
+ // events into the channel structs, but the "real" channels have to
+ // send these to the device right away, since they don't use the stored
+ // data.
+ // Very early games like KQ5 (but including the DOS CD version) and SQ2
+ // have support for this flag, only. It isn't even a flag there, since
+ // all these games do is check for a channel number below 0x10.
+ //
+ // Flag 2: Don't remap the channel. It is placed in the map, but only in the
+ // exact matching slot of the channel number. All the other games except
+ // the very early ones use this flag to mark the rhythm channels. I
+ // haven't seen any usage of flag 1 in any of these games. They all use
+ // flag 2 instead, but still have full support of flag 1 in the code.
+ // Using this flag is really preferable, since there can't be conflicts
+ // with different sounds playing on the channel.
+ //
+ // Flag 4: Start up muted. The channel won't be mapped (and thus, not have any
+ // output), until the mute gets removed.
}
_tracks[trackNr].channelCount++;
channelNr++;
diff --git a/engines/sci/sound/midiparser_sci.cpp b/engines/sci/sound/midiparser_sci.cpp
index a1f900b97a6..7e29c4a5949 100644
--- a/engines/sci/sound/midiparser_sci.cpp
+++ b/engines/sci/sound/midiparser_sci.cpp
@@ -492,8 +492,11 @@ void MidiParser_SCI::sendFromScriptToDriver(uint32 midi) {
}
void MidiParser_SCI::sendToDriver(uint32 midi) {
+ byte midiChannel = midi & 0xf;
+
// State tracking
- trackState(midi);
+ if (!_pSnd->_chan[midiChannel]._dontMap)
+ trackState(midi);
if ((midi & 0xFFF0) == 0x4EB0 && _soundVersion >= SCI_VERSION_1_EARLY) {
// Mute. Handled in trackState()/sendFromScriptToDriver().
@@ -508,10 +511,16 @@ void MidiParser_SCI::sendToDriver(uint32 midi) {
midi = (midi & 0xFFFF) | ((channelVolume & 0xFF) << 16);
}
-
// Channel remapping
- byte midiChannel = midi & 0xf;
+ uint8 msg = (midi & 0xF0);
int16 realChannel = _channelRemap[midiChannel];
+ if (_pSnd->_chan[midiChannel]._dontMap) {
+ // The dontMap channel is supposed to have limited access, if the device channel is already in use.
+ // It probably won't happen, but the original does these checks...
+ if (!_music->isDeviceChannelMapped(midiChannel) || (msg != 0xB0 && msg != 0xC0 && msg != 0xE0))
+ realChannel = midiChannel;
+ }
+
if (realChannel == -1)
return;
diff --git a/engines/sci/sound/music.cpp b/engines/sci/sound/music.cpp
index 5dfbb3b79c4..1ec68ae49fc 100644
--- a/engines/sci/sound/music.cpp
+++ b/engines/sci/sound/music.cpp
@@ -501,20 +501,26 @@ void SciMusic::soundInitSnd(MusicEntry *pSnd) {
assert(chan.number < ARRAYSIZE(pSnd->_chan));
pSnd->_usedChannels[i] = chan.number;
+ // Flag 1 is exclusive towards the other flags. When it is
+ // set the others won't even get evaluated. And it wouldn't
+ // matter, since channels flagged with 1 operate completely
+ // independent of the channel mapping.
+ // For more info on the flags see the comment in
+ // SoundResource::SoundResource().
+ pSnd->_chan[chan.number]._dontMap = (chan.flags & 1);
pSnd->_chan[chan.number]._dontRemap = (chan.flags & 2);
pSnd->_chan[chan.number]._prio = chan.prio;
pSnd->_chan[chan.number]._voices = chan.poly;
pSnd->_chan[chan.number]._mute = (chan.flags & 4) ? 1 : 0;
-
- // CHECKME: Some SCI versions use chan.flags & 1 for this:
- pSnd->_chan[chan.number]._dontMap = false;
-
// FIXME: Most MIDI tracks use the first 10 bytes for
// fixed MIDI commands. SSCI skips those the first iteration,
// but _does_ update channel state (including volume) with
// them. Specifically, prio/voices, patch, volume, pan.
- // This should probably be implemented in
- // MidiParser_SCI::loadMusic.
+ // This should probably be implemented in MidiParser_SCI::loadMusic.
+ //
+ // UPDATE: While we could change how we handle it, we DO
+ // read the commands into the channel data arrays when we call
+ // trackState(). So, I think what we do has the same result...
}
pSnd->pMidiParser->mainThreadBegin();
@@ -1533,6 +1539,10 @@ ChannelRemapping *SciMusic::determineChannelMap() {
return map;
}
+bool SciMusic::isDeviceChannelMapped(int devChannel) const {
+ return _channelMap[devChannel]._song;
+}
+
void SciMusic::resetDeviceChannel(int devChannel, bool mainThread) {
assert(devChannel >= 0 && devChannel <= 0x0F);
diff --git a/engines/sci/sound/music.h b/engines/sci/sound/music.h
index 3e32bc7bad8..8abc6321184 100644
--- a/engines/sci/sound/music.h
+++ b/engines/sci/sound/music.h
@@ -280,6 +280,10 @@ protected:
ChannelRemapping *determineChannelMap();
void resetDeviceChannel(int devChannel, bool mainThread);
+public:
+ // The parsers need to know this for the dontMap channels...
+ bool isDeviceChannelMapped(int devChannel) const;
+
private:
MusicList _playList;
bool _soundOn;
More information about the Scummvm-git-logs
mailing list